[vlc-devel] [PATCH v2] linux: kernel mode setting (KMS) vout plugin

Juha-Pekka Heikkila juhapekka.heikkila at gmail.com
Tue Jun 5 14:23:04 CEST 2018


On 04.06.2018 20:48, Rémi Denis-Courmont wrote:
> Le maanantaina 4. kesäkuuta 2018, 17.09.47 EEST Juha-Pekka Heikkila a écrit :
>> Add new kernel mode setting video output plugin.
>> By default it tries to match vlc suggested video mode to anything
>> kms api offers. There are command line parameters to control
>> which video format to use for both vlc and for drm.
>>
>> kms-vlc-chroma parameter to force VLC to use FourCC as output.
>> kms-drm-chroma parameter to give DRM FourCC request for framebuffer type.
>>
>> v2, Rémi Denis-Courmont: Multiple picture pools are gone as well as
>> TTY code which I found out I don't need.
>> vlc_fourcc_GetYUVFallback/vlc_fourcc_GetRGBFallback I didn't find
>> easy way to do back'n'forth mapping of what kernel offers and what
>> vlc offers thus I have simple table of most common formats which
>> my plugin uses to find 'best' match.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> ---
>>   configure.ac                     |   5 +
>>   modules/video_output/Makefile.am |  10 +
>>   modules/video_output/kms.c       | 855
>> +++++++++++++++++++++++++++++++++++++++ 3 files changed, 870 insertions(+)
>>   create mode 100644 modules/video_output/kms.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index 960e245..622516a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -3338,6 +3338,11 @@ AC_CHECK_HEADER([linux/fb.h], [
>>   ])
>>
>>   dnl
>> +dnl  Linux kernel mode setting module
>> +dnl
>> +PKG_ENABLE_MODULES_VLC([KMS], [], [libdrm >= 2.4.83], [Linux kernel mode
>> setting output], [auto]) +
>> +dnl
>>   dnl  libcaca plugin
>>   dnl
>>   PKG_ENABLE_MODULES_VLC([CACA], [], [caca >= 0.99.beta14], [libcaca
>> output],[auto]) diff --git a/modules/video_output/Makefile.am
>> b/modules/video_output/Makefile.am index 17967b5..a864c1a 100644
>> --- a/modules/video_output/Makefile.am
>> +++ b/modules/video_output/Makefile.am
>> @@ -403,6 +403,16 @@ EXTRA_LTLIBRARIES += libfb_plugin.la
>>   vout_LTLIBRARIES += $(LTLIBfb)
>>
>>
>> +### Kernel Mode Setting ###
>> +
>> +libkms_plugin_la_SOURCES = video_output/kms.c
>> +libkms_plugin_la_CFLAGS = $(AM_CFLAGS) $(KMS_CFLAGS)
>> +libkms_plugin_la_LIBADD = $(KMS_LIBS)
>> +libkms_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(voutdir)'
>> +EXTRA_LTLIBRARIES += libkms_plugin.la
>> +vout_LTLIBRARIES += $(LTLIBkms)
>> +
>> +
>>   ### Coloured ASCII art ###
>>   libcaca_plugin_la_SOURCES = video_output/caca.c
>>   libcaca_plugin_la_CFLAGS = $(AM_CFLAGS) $(CACA_CFLAGS)
>> diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
>> new file mode 100644
>> index 0000000..e9d23a8
>> --- /dev/null
>> +++ b/modules/video_output/kms.c
>> @@ -0,0 +1,855 @@
>> +/**************************************************************************
>> *** + * kms.c : kernel mode setting plugin for vlc
>> +
>> ***************************************************************************
>> ** + * Copyright © 2018 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"), + * to deal in the Software without restriction, including
>> without limitation + * the rights to use, copy, modify, merge, publish,
>> distribute, sublicense, + * and/or sell copies of the Software, and to
>> permit persons to whom the + * Software is furnished to do so, subject to
>> the following conditions: + *
>> + * The above copyright notice and this permission notice (including the
>> next + * paragraph) shall be included in all copies or substantial portions
>> of the + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
>> IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
>> CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
>> TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE
>> SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE.
>> +
>> ***************************************************************************
>> **/ +
>> +/**************************************************************************
>> *** + * Preamble
>> +
>> ***************************************************************************
>> **/ +
>> +#ifdef HAVE_CONFIG_H
>> +# include "config.h"
>> +#endif
>> +
>> +#include <fcntl.h>
>> +#include <errno.h>
>> +#include <sys/mman.h>
>> +
>> +#include <xf86drm.h>
>> +#include <xf86drmMode.h>
>> +#include <drm_fourcc.h>
>> +#include <drm.h>
>> +
>> +#include <vlc_common.h>
>> +#include <vlc_plugin.h>
>> +#include <vlc_vout_display.h>
>> +#include <vlc_picture_pool.h>
>> +#include <vlc_fs.h>
>> +
>> +/**************************************************************************
>> *** + * Module descriptor
>> +
>> ***************************************************************************
>> **/ +#define KMS_VAR "kms"
>> +
>> +#define DEVICE_TEXT N_("Framebuffer device")
>> +#define DEVICE_LONGTEXT N_(\
>> +    "Framebuffer device to use for rendering (usually /dev/dri/card0).")
>> +
>> +#define VLC_CHROMA_TEXT N_("Image format used by VLC")
>> +#define VLC_CHROMA_LONGTEXT N_("Chroma fourcc told to VLC about
>> framebuffer.") +
>> +#define DRM_CHROMA_TEXT N_("Image format used by DRM")
>> +#define DRM_CHROMA_LONGTEXT N_("Chroma fourcc for requested DRM
>> framebuffer.") +
>> +static int  Open (vlc_object_t *);
>> +static void Close(vlc_object_t *);
> 
> Reorder to avoid forward decls.
> 

These are needed for module definition just below here as module 
examples show and said module begins with. All the modules begin by 
having these two declarations for this reason, otherwise 
vlc_module_begin will have to move to end of file.

>> +
>> +vlc_module_begin ()
>> +    set_shortname("kms")
>> +    set_category(CAT_VIDEO)
>> +    set_subcategory(SUBCAT_VIDEO_VOUT)
>> +    add_loadfile(KMS_VAR, "/dev/dri/card0", DEVICE_TEXT, DEVICE_LONGTEXT,
>> +                 false)
>> +
>> +    add_string( "kms-vlc-chroma", NULL, VLC_CHROMA_TEXT,
>> VLC_CHROMA_LONGTEXT,
>> +                true)
>> +    add_string( "kms-drm-chroma", NULL, DRM_CHROMA_TEXT,
>> DRM_CHROMA_LONGTEXT,
>> +                true)
> 
> Two options seems a bit redundant.
> 

They do different things, this is what I mentioned earlier. VLC chroma 
will ask VLC to produce certain type chroma. DRM chroma will override 
request for DRM plane format selection. It is possible there is in 
hardware better format available than where my automatic selection goes.

Agreed DRM chroma selection is more of a developer type option but 
removing it will cause I'll need to patch my own VLC always. Place where 
I currently use this is when VLC is able to produce P010 format but I'm 
wanting to see P016 for example..these two formats are compatible.


>> +    set_description(N_("Linux kernel mode setting video output"))
>> +    set_capability("vout display", 30)
>> +    set_callbacks(Open, Close)
>> +vlc_module_end ()
>> +
>> +/**************************************************************************
>> *** + * Local prototypes
>> +
>> ***************************************************************************
>> **/ +
>> +#define HWBUF1 0
>> +#define HWBUF2 1
> 
> I'm not a huge fan of #define ONE 1 things.
> 

I made these just to underline where hwbuffers are talked about. I'll 
change them to enums so I get also automatic definition of maximum HW 
planes. I did prefer some name for these instead of magic one or zero.

>> +
>> +struct vout_display_sys_t {
>> +/*
>> + * buffer information
>> + */
>> +    uint32_t        width;
>> +    uint32_t        height;
>> +    uint32_t        stride;
>> +    uint32_t        size;
>> +    uint32_t        offsets[5];
>> +
>> +    uint32_t        handle[2];
>> +    uint8_t         *map[2];
>> +
>> +    uint32_t        fb[2];
>> +    picture_t       *picture;
>> +    picture_pool_t  *pool;
>> +
>> +    unsigned int    front_buf;
>> +
>> +    bool            forced_drm_fourcc;
>> +    uint32_t        drm_fourcc;
>> +    vlc_fourcc_t    vlc_fourcc;
>> +
>> +/*
>> + * modeset information
>> + */
>> +    drmModeModeInfo mode;
>> +    uint32_t        crtc;
>> +    uint32_t        plane_id;
>> +
>> +/*
>> + * other generic stuff
>> + */
>> +    int             drm_fd;
>> +};
>> +
>> +
>> +static void DestroyFB(vout_display_t *vd, int buf)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    struct drm_mode_destroy_dumb destroy_req = { .handle = sys->handle[buf]
>> }; +
>> +    munmap(sys->map[buf], sys->size);
> 
> Memory mapped in a picture buffer should be unmapped in the picture destroy
> callback. Due to the subtle workings of some video decoders, a picture buffer
> can outlive the video output that created it.
> 

I'll check this and fix it into next version, thanks.

>> +    drmModeRmFB(sys->drm_fd, sys->fb[buf]);
>> +    drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_req);
>> +}
>> +
>> +
>> +#define ALIGN(v, a) (((v) + (a)-1) & ~((a)-1))
>> +
>> +static int CreateFB(vout_display_t *vd, int buf)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    struct drm_mode_create_dumb create_req = { .width = sys->width*2,
>> +                                               .height = sys->height,
>> +                                               .bpp = 16 };
>> +    struct drm_mode_destroy_dumb destroy_req;
>> +    struct drm_mode_map_dumb modify_req = {};
>> +    struct drm_mode_fb_cmd2 fb_cmd = {};
>> +    unsigned int tile_width = 512, tile_height = 16;
>> +    int ret, i;
>> +
>> +    switch(sys->drm_fourcc) {
>> +#ifdef DRM_FORMAT_P010
>> +    case DRM_FORMAT_P010:
>> +#endif
>> +#ifdef DRM_FORMAT_P012
>> +    case DRM_FORMAT_P012:
>> +#endif
>> +#ifdef DRM_FORMAT_P016
>> +    case DRM_FORMAT_P016:
>> +#endif
>> +#if defined(DRM_FORMAT_P010) || defined(DRM_FORMAT_P012) ||
>> defined(DRM_FORMAT_P016) +        sys->stride = ALIGN(sys->width*2,
>> tile_width);
>> +        sys->offsets[1] = sys->stride*ALIGN(sys->height, tile_height);
>> +        create_req.height = ALIGN(sys->height*2, tile_height);
>> +		break;
>> +#endif
>> +    case DRM_FORMAT_NV12:
>> +        sys->stride = ALIGN(sys->width, tile_width);
>> +        sys->offsets[1] = sys->stride*ALIGN(sys->height, tile_height);
>> +        create_req.height = ALIGN(sys->height*2, tile_height);
>> +        break;
>> +    default:
>> +        create_req.height = ALIGN(sys->height*2, tile_height);
>> +
>> +        /*
>> +         * width *4 so there's enough space for anything.
>> +         */
>> +        sys->stride = ALIGN(sys->width*4, tile_width);
>> +        break;
> 
> TBH, I doubt that really works. Some pixel formats definitely exceed 32-bits
> per pixel.
> 
> Maybe add bpp to the table below and reuse it here.
> 

There exist pixel formats which exceed 32bpp but what are looking into 
here is display HW buffers, not those fancy things like GPU provided 
buffers or such. DRM Fourccs list all available formats and none of them 
need more than 32bpp for stride length. IMHO, without real knowledge of 
the future, this layer is not going to see more complex formats than 
what we have today because nobody want to make display HW for this part 
more complex.

>> +    }
>> +
>> +    ret = drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_CREATE_DUMB, &create_req);
>> +    if (ret < 0) {
>> +        msg_Err(vd, "cannot create buffer (%s)", vlc_strerror_c(errno));
>> +        return -errno;
> 
> Does libdrm guarantee error values via errno?
> 

Good catch, error is in ret.

>> +    }
>> +
>> +    sys->size = create_req.size;
>> +    sys->handle[buf] = create_req.handle;
>> +
>> +    /*
>> +     * create framebuffer object for the dumb-buffer
>> +     */
>> +    fb_cmd.width  = sys->width;
>> +    fb_cmd.height = sys->height;
>> +    fb_cmd.pixel_format = sys->drm_fourcc;
>> +    fb_cmd.flags = (1<<1);
>> +    fb_cmd.handles[0] = create_req.handle;
>> +    fb_cmd.pitches[0] = sys->stride;
>> +    fb_cmd.modifier[0] = 0;
>> +
>> +    for (i = 1; i < 4 && sys->offsets[i]; i++) {
>> +        fb_cmd.handles[i] = create_req.handle;
>> +        fb_cmd.pitches[i] = sys->stride;
>> +        fb_cmd.modifier[i] = 0;
>> +        fb_cmd.offsets[i] = sys->offsets[i];
>> +    }
>> +
>> +    ret = drmModeAddFB2(sys->drm_fd, sys->width, sys->height,
>> sys->drm_fourcc,
>> +                        (uint32_t*)&fb_cmd.handles,
>> +                        (uint32_t*)&fb_cmd.pitches,
>> +                        (uint32_t*)&fb_cmd.offsets,
>> +                        (uint32_t*)&sys->fb[buf], 1<<1);
> 
> Dubious casts.
> 
>> +
>> +    if (ret) {
>> +        msg_Err(vd, "cannot create framebuffer (%s)",
>> +                vlc_strerror_c(errno));
>> +        ret = -errno;
>> +        goto err_destroy;
>> +    }
>> +
>> +    modify_req.handle = sys->handle[buf];
>> +    ret = drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_MAP_DUMB, &modify_req);
>> +    if (ret) {
>> +        msg_Err(vd, "cannot map dumb buffer (%s)", vlc_strerror_c(errno));
>> +        ret = -errno;
>> +        goto err_fb;
>> +    }
>> +
>> +    sys->map[buf] = mmap(0, sys->size, PROT_READ | PROT_WRITE, MAP_SHARED,
>> +                         sys->drm_fd, modify_req.offset);
>> +
>> +    if (sys->map[buf] == MAP_FAILED) {
>> +        msg_Err(vd, "cannot mmap dumb buffer (%s)", vlc_strerror_c(errno));
>> +        ret = -errno;
>> +        goto err_fb;
>> +    }
>> +    return 0;
>> +err_fb:
>> +    drmModeRmFB(sys->drm_fd, sys->fb[buf]);
>> +err_destroy:
>> +    memset(&destroy_req, 0, sizeof(destroy_req));
>> +    destroy_req.handle = sys->handle[buf];
>> +    drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_req);
>> +    return ret;
>> +}
>> +
>> +
>> +static int FindCRTC(vout_display_t *vd, drmModeRes *res,
>> +                             drmModeConnector *conn)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    drmModeEncoder *enc;
>> +    int i, j;
>> +
>> +    /*
>> +     * Try current encoder and crtc combo
>> +     */
>> +    if (conn->encoder_id) {
>> +        enc = drmModeGetEncoder(sys->drm_fd, conn->encoder_id);
>> +        if (enc) {
>> +            if (enc->crtc_id) {
>> +                msg_Dbg(vd, "found crtc %d from current encoder!",
>> +                        enc->crtc_id);
> 
> Capitalize CRTC.
> 
>> +                sys->crtc = enc->crtc_id;
>> +                drmModeFreeEncoder(enc);
>> +                return 0;
>> +            }
>> +            drmModeFreeEncoder(enc);
>> +        }
>> +    }
>> +
>> +    /*
>> +     * Iterate all other available encoders to find crtc
>> +     */
>> +    for(i = 0; i < conn->count_encoders; i++) {
>> +        enc = drmModeGetEncoder(sys->drm_fd, conn->encoders[i]);
>> +        if (!enc) {
>> +            msg_Info(vd, "cannot retrieve encoder %u:%u (%s)", i,
>> +                     conn->encoders[i], vlc_strerror_c(errno));
>> +            continue;
>> +        }
>> +
>> +        for (j = 0; j < res->count_crtcs; ++j) {
>> +            if (!(enc->possible_crtcs & (1 << j)))
>> +                continue;
>> +
>> +            if (res->crtcs[j]) {
>> +                msg_Dbg(vd, "found crtc %d", res->crtcs[j]);
>> +                sys->crtc = res->crtcs[j];
>> +                drmModeFreeEncoder(enc);
>> +                return 0;
>> +            }
>> +        }
>> +        drmModeFreeEncoder(enc);
>> +    }
>> +
>> +    msg_Err(vd , "cannot find crtc for connector %d", conn->connector_id);
>> +    return -ENOENT;
>> +}
>> +
>> +
>> +static int SetupDevice(vout_display_t *vd, drmModeRes *res,
>> +                             drmModeConnector *conn)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    int ret;
>> +
>> +    if (conn->connection != DRM_MODE_CONNECTED)
>> +        return -ENOENT;
>> +
>> +    /*
>> +     * check if there is at least one valid mode
>> +     */
>> +    if (conn->count_modes == 0) {
>> +        msg_Err(vd, "no valid mode for connector %d", conn->connector_id);
>> +        return -EFAULT;
> 
> Seemingly insane error code.
> 

Agree, I shouldn't have used these internally in my own code.

>> +    }
>> +
>> +    memcpy(&sys->mode, &conn->modes[0], sizeof(sys->mode));
>> +    sys->width = conn->modes[0].hdisplay;
>> +    sys->height = conn->modes[0].vdisplay;
>> +    msg_Dbg(vd, "mode for connector %u is %ux%u", conn->connector_id,
>> +            sys->width, sys->height);
>> +
>> +    ret = FindCRTC(vd, res, conn);
>> +    if (ret) {
>> +        msg_Dbg(vd , "no valid crtc for connector %d", conn->connector_id);
>> +        return ret;
>> +    }
>> +
>> +    /*
>> +     * create framebuffer #1 for this CRTC
>> +     */
>> +    ret = CreateFB(vd, HWBUF1);
>> +    if (ret) {
>> +        msg_Err(vd, "cannot create framebuffer 0 for connector %d",
>> +                conn->connector_id);
>> +
>> +        return ret;
>> +    }
>> +
>> +    /*
>> +     * create framebuffer #2 for this CRTC
>> +     */
>> +    ret = CreateFB(vd, HWBUF2);
>> +    if (ret) {
>> +        msg_Err(vd, "cannot create framebuffer 1 for connector %u",
>> +                conn->connector_id);
>> +
>> +        DestroyFB(vd, HWBUF1);
>> +        return ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +/** fourccmatching, matching drm to vlc fourccs and see if it was present
>> + * in HW. Here really is two lists, one in RGB and second in YUV. They're
>> + * listed in order of preference.
>> + *
>> + * fourccmatching::drm DRM fourcc code from drm_fourcc.h
>> + * fourccmatching::vlc VLC fourcc code from vlc_fourcc.h under title
>> 'Chromas' + * fourccmatching::plane_id from which plane this drm fourcc was
>> found + * fourccmatching::present if this mode was available in HW
>> + * fourccmatching::isYUV as name suggest..
>> + */
>> +static struct
>> +{
>> +    uint32_t     drm;
>> +    vlc_fourcc_t vlc;
>> +    uint32_t     plane_id;
>> +    bool         present;
>> +    bool         isYUV;
>> +} fourccmatching[] = {
>> +    { .drm = DRM_FORMAT_XRGB8888, .vlc = VLC_CODEC_RGB32, .isYUV = false },
>> +    { .drm = DRM_FORMAT_RGB565, .vlc = VLC_CODEC_RGB16, .isYUV = false },
>> +#if defined DRM_FORMAT_P010
>> +    { .drm = DRM_FORMAT_P010, .vlc = VLC_CODEC_P010, .isYUV = true },
>> +#endif
>> +    { .drm = DRM_FORMAT_NV12, .vlc = VLC_CODEC_NV12, .isYUV = true },
>> +    { .drm = DRM_FORMAT_YUYV, .vlc = VLC_CODEC_YUYV, .isYUV = true },
>> +    { .drm = DRM_FORMAT_YVYU, .vlc = VLC_CODEC_YVYU, .isYUV = true },
>> +    { .drm = DRM_FORMAT_UYVY, .vlc = VLC_CODEC_UYVY, .isYUV = true },
>> +    { .drm = DRM_FORMAT_VYUY, .vlc = VLC_CODEC_VYUY, .isYUV = true },
>> +};
>> +
>> +#define fccllen sizeof(fourccmatching)/sizeof(fourccmatching[0])
> 
> VLC has the usual ARRAY_SIZE().
> 
>> +static void CheckFourCCList(uint32_t drmfourcc, uint32_t plane_id)
>> +{
>> +    unsigned i;
>> +
>> +    for( i = 0; i < fccllen; i++ ) {
>> +        if(fourccmatching[i].drm == drmfourcc) {
>> +            if(fourccmatching[i].present)
>> +                /* this drmfourcc already has a plane_id found where it
>> +                 * could be used, we'll stay with earlier findings.
>> +                 */
>> +                break;
>> +
>> +            fourccmatching[i].present = true;
>> +            fourccmatching[i].plane_id = plane_id;
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>> +static bool ChromaNegotiation(vout_display_t *vd)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    unsigned i, c, propi;
>> +    uint32_t planetype;
>> +    const char types[][16] = { "OVERLAY", "PRIMARY", "CURSOR", "UNKNOWN" };
>> +    drmModePlaneRes *plane_res = NULL;
>> +    drmModePlane *plane;
>> +    drmModeObjectProperties *props;
>> +    drmModePropertyPtr pp;
>> +
>> +    /*
>> +     * For convenience print out in debug prints all supported
>> +     * drm modes so they can be seen if use verbose mode.
>> +     */
>> +    plane_res = drmModeGetPlaneResources(sys->drm_fd);
>> +    sys->plane_id = 0;
>> +
>> +    if( plane_res != NULL && plane_res->count_planes > 0 ) {
>> +        msg_Dbg(vd, "List of DRM supported modes on this machine:");
>> +        for(c = 0; c < plane_res->count_planes; c++) {
>> +
>> +            plane = drmModeGetPlane(sys->drm_fd, plane_res->planes[c]);
>> +            if(plane != NULL && plane->count_formats > 0) {
>> +                props = drmModeObjectGetProperties(sys->drm_fd,
>> +                                                   plane->plane_id,
>> +                                                   DRM_MODE_OBJECT_PLANE);
>> +
>> +                planetype = 3;
>> +                pp = NULL;
>> +                for(propi = 0; propi < props->count_props; propi++) {
>> +                    pp = drmModeGetProperty(sys->drm_fd,
>> props->props[propi]); +                    if(strcmp(pp->name, "type") ==
>> 0) {
>> +                        break;
>> +                    }
>> +                    drmModeFreeProperty(pp);
>> +                }
>> +
>> +                if(pp != NULL) {
>> +                    drmModeFreeProperty(pp);
>> +                    planetype = props->prop_values[propi];
>> +                }
>> +
>> +                for( i = 0; i < plane->count_formats; i++) {
>> +                    CheckFourCCList(plane->formats[i], plane->plane_id);
>> +
>> +                    if(sys->forced_drm_fourcc && sys->plane_id == 0 &&
>> +                            plane->formats[i] == sys->drm_fourcc) {
>> +                        sys->plane_id = plane->plane_id;
>> +                    }
>> +
>> +                    /*
>> +                     * we don't advertise about cursor plane because
>> +                     * of its special limitations.
>> +                     */
>> +                    if(planetype != DRM_PLANE_TYPE_CURSOR)
>> +                        msg_Dbg(vd, "plane id %d type %s format %2d: %.4s",
>> +                                plane->plane_id, types[planetype], i, +
>>                              (char*)&plane->formats[i]);
>> +                }
>> +                drmModeFreePlane(plane);
>> +            } else {
>> +                msg_Err(vd, "Couldn't get list of DRM formats");
>> +                drmModeFreePlaneResources(plane_res);
>> +                return false;
>> +            }
>> +        }
>> +        drmModeFreePlaneResources(plane_res);
>> +    }
>> +
>> +    if(sys->forced_drm_fourcc) {
>> +        for( c = i = 0; c < fccllen; c++ ) {
>> +            if(fourccmatching[c].drm == sys->drm_fourcc) {
>> +                sys->vlc_fourcc = fourccmatching[c].vlc;
>> +                break;
>> +            }
>> +        }
>> +        if(sys->plane_id == 0) {
>> +            msg_Err(vd, "Forced DRM fourcc (%.4s) not available in
>> kernel.", +                    (char*)&sys->drm_fourcc);
>> +            return false;
>> +        }
>> +        return true;
>> +    }
>> +
>> +    if(vlc_fourcc_IsYUV(sys->vlc_fourcc)) {
>> +        /*
>> +         * favor yuv formats
>> +         * check for exact match first, then yuv and then rgb
>> +         */
>> +
>> +        for( c = i = 0; c < fccllen; c++ ) {
>> +            if(fourccmatching[c].vlc == sys->vlc_fourcc) {
>> +                if(!sys->forced_drm_fourcc && fourccmatching[c].present) {
>> +                    sys->drm_fourcc = fourccmatching[c].drm;
>> +                    sys->plane_id = fourccmatching[c].plane_id;
>> +                 }
>> +
>> +                if(!sys->drm_fourcc) {
>> +                    msg_Err(vd, "Forced VLC fourcc (%.4s) not matching
>> anything available in kernel, please set manually", +
>>       (char*)&sys->vlc_fourcc);
>> +                    return false;
>> +                }
>> +                return true;
>> +            }
>> +        }
>> +        for( c = i = 0; c < fccllen; c++ ) {
>> +            if(fourccmatching[c].isYUV && fourccmatching[c].present) {
>> +                if(!sys->forced_drm_fourcc) {
>> +                    sys->drm_fourcc = fourccmatching[c].drm;
>> +                    sys->plane_id = fourccmatching[c].plane_id;
>> +                 }
>> +                sys->vlc_fourcc = fourccmatching[c].vlc;
>> +                return true;
>> +            }
>> +        }
>> +        for( i = 0; c < fccllen; c++ ) {
>> +            if(!fourccmatching[c].isYUV && fourccmatching[c].present) {
>> +                if(!sys->forced_drm_fourcc) {
>> +                    sys->drm_fourcc = fourccmatching[c].drm;
>> +                    sys->plane_id = fourccmatching[c].plane_id;
>> +                 }
>> +                sys->vlc_fourcc = fourccmatching[c].vlc;
>> +                return true;
>> +            }
>> +        }
>> +    } else {
>> +        /*
>> +         * favor rgb formats
>> +         * check for exact match first, then rgb and then yuv
>> +         */
>> +        for( c = i = 0; c < fccllen; c++ ) {
>> +            if(fourccmatching[c].vlc == sys->vlc_fourcc) {
>> +                if(!sys->forced_drm_fourcc && fourccmatching[c].present) {
>> +                    sys->drm_fourcc = fourccmatching[c].drm;
>> +                    sys->plane_id = fourccmatching[c].plane_id;
>> +                 }
>> +
>> +                if(!sys->drm_fourcc) {
>> +                    msg_Err(vd, "Forced VLC fourcc (%.4s) not matching
>> anything available in kernel, please set manually", +
>>       (char*)&sys->vlc_fourcc);
>> +                    return false;
>> +                }
>> +                return true;
>> +            }
>> +        }
>> +        for( c = i = 0; c < fccllen; c++ ) {
>> +            if(fourccmatching[c].isYUV && fourccmatching[c].present) {
>> +                if(!sys->forced_drm_fourcc) {
>> +                    sys->drm_fourcc = fourccmatching[c].drm;
>> +                    sys->plane_id = fourccmatching[c].plane_id;
>> +                 }
>> +                sys->vlc_fourcc = fourccmatching[c].vlc;
>> +                return true;
>> +            }
>> +        }
>> +        for( i = 0; c < fccllen; c++ ) {
>> +            if(!fourccmatching[c].isYUV && fourccmatching[c].present) {
>> +                if(!sys->forced_drm_fourcc) {
>> +                    sys->drm_fourcc = fourccmatching[c].drm;
>> +                    sys->plane_id = fourccmatching[c].plane_id;
>> +                 }
>> +                sys->vlc_fourcc = fourccmatching[c].vlc;
>> +                return true;
>> +            }
>> +        }
>> +    }
>> +
>> +    return false;
>> +}
>> +
>> +
>> +static int OpenDisplay(vout_display_t *vd)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    char *psz_device;
>> +    int ret;
>> +    uint64_t dumbRet;
>> +    drmModeConnector *conn;
>> +    drmModeRes *modeRes;
>> +    unsigned int c;
>> +    bool found_connector = false;
>> +
>> +    /*
>> +     * Open framebuffer device
>> +     */
>> +    if (!(psz_device = var_InheritString(vd, KMS_VAR))) {
> 
> Please avoid assignment as truth value.
> 
>> +        msg_Err(vd, "don't know which drm device to open");
> 
> Capitalize DRM.
> 
>> +        return VLC_EGENERIC;
>> +    }
>> +
>> +    sys->drm_fd = vlc_open(psz_device, O_RDWR);
>> +    if (sys->drm_fd == -1) {
>> +        msg_Err(vd, "cannot open %s (%s)", psz_device,
>> vlc_strerror_c(errno)); +        free(psz_device);
>> +        return VLC_EGENERIC;
>> +    }
> 
> In my understanding, this should probably eventually be split to a separate
> vout window plugin to support EGL/GBM over KMS too.
> 

It can be done but I don't know what is the benefit of doing this? As is 
this plugin is really light weight, core VLC is written in nicely robust 
way which made basic glueing of KMS in here quite simple. Mixing in EGL 
or GBM will add processing and memory overhead as well as the dependecies.

This plugin is basic bare bones for doing KMS. I have idea after we 
reach to the level of maturity it could be committed upstream I'll do 
bit of rework for adding KMS features which may take bit more time for 
testing. Most interesting of these is scaling of planes which can reduce 
traffic and overhead quite much if output screen resolution is quite high.

Another insterest for this is to switch to use atomic commits, place 
where this will help is on buffer switching in HW. Currently if VLC 
output video rate is higher than screen refresh my plugin will stall 
until next vblank, using atomic ioctls this can be avoided.

>> +
>> +    ret = drmSetMaster(sys->drm_fd);
>> +    if (ret < 0) {
>> +        msg_Err(vd, "Cannot become drm master on %s (%s)", psz_device,
>> +                vlc_strerror_c(-ret));
>> +
>> +        free(psz_device);
>> +        vlc_close(sys->drm_fd);
>> +        sys->drm_fd = 0;
> 
> Zero is _not_ an error value for FD.
> 

No but zero here just return it to original state as *sys was coming 
from calloc. Though, now that you pointed it out I noticed I have bug in 
close function on this. In any case I'm not expecting to talk to 
terminal with this fd so zero was ok for me on this.

>> +        return VLC_EGENERIC;
>> +    }
>> +    free(psz_device);
>> +
>> +    drmSetClientCap(sys->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
>> +
>> +    if(!ChromaNegotiation(vd)) {
>> +        vlc_close(sys->drm_fd);
>> +        sys->drm_fd = 0;
>> +        return VLC_EGENERIC;
>> +    }
>> +
>> +    msg_Dbg(vd, "Using vlc chroma '%.4s', drm chroma '%.4s'",
>> +            (char*)&sys->vlc_fourcc, (char*)&sys->drm_fourcc);
>> +
>> +
>> +    ret = drmGetCap(sys->drm_fd, DRM_CAP_DUMB_BUFFER, &dumbRet);
>> +    if (ret < 0 || !dumbRet) {
>> +        msg_Err(vd, "Device '%s' does not support dumb buffers",
>> psz_device); +        return VLC_EGENERIC;
>> +    }
>> +
>> +    modeRes = drmModeGetResources(sys->drm_fd);
>> +    if (modeRes == NULL) {
>> +        msg_Err(vd, "Didn't get DRM resources. (%s)",
>> vlc_strerror_c(errno)); +        return VLC_EGENERIC;
>> +    }
>> +
>> +    for (c = 0; c < (unsigned)modeRes->count_connectors && sys->crtc == 0;
>> +         c++) {
>> +
>> +        conn = drmModeGetConnector(sys->drm_fd, modeRes->connectors[c]);
>> +        if (conn == NULL) {
>> +            msg_Dbg(vd, "Couldn't retrieve DRM connector %d:%d (%s)", c,
>> +                    modeRes->connectors[c], vlc_strerror_c(errno));
>> +            continue;
>> +        }
>> +
>> +        found_connector = true;
>> +
>> +        ret = SetupDevice(vd, modeRes, conn);
>> +        if (ret) {
>> +            if (ret != -ENOENT) {
>> +                errno = -ret;
>> +                msg_Err(vd, "cannot setup device for connector %u:%u (%s)",
>> +                    c, modeRes->connectors[c], vlc_strerror_c(errno)); +
>>           }
>> +            drmModeFreeConnector(conn);
>> +            found_connector = false;
>> +            continue;
>> +        }
>> +        drmModeFreeConnector(conn);
>> +    }
>> +    drmModeFreeResources(modeRes);
>> +    if(!found_connector) {
>> +        return VLC_EGENERIC;
>> +    }
>> +    return VLC_SUCCESS;
>> +}
>> +
>> +static int Control(vout_display_t *vd, int query, va_list args)
>> +{
>> +    (void) vd; (void) query; (void) args;
>> +    return VLC_EGENERIC;
>> +}
>> +
>> +
>> +static picture_pool_t *Pool(vout_display_t *vd, unsigned count)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    picture_resource_t rsc;
>> +    int i;
>> +
>> +    if (!sys->pool && !sys->picture) {
>> +        memset(&rsc, 0, sizeof(rsc));
>> +
>> +        for(i = 0; i < 5; i++ ) {
> 
> Did you mean PICTURE_PLANE_MAX ?
> 

Ah yes. Need to fix it in few places, thanks.

>> +            rsc.p[i].p_pixels = sys->map[0]+sys->offsets[i];
>> +            rsc.p[i].i_lines  = sys->height;
>> +            rsc.p[i].i_pitch  = sys->stride;
>> +        }
>> +        sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
>> +
>> +        if (!sys->picture) {
>> +            return NULL;
>> +        }
>> +
>> +        sys->pool = picture_pool_New(1, &sys->picture);
>> +        if(!sys->pool)
>> +            picture_Release(sys->picture);
>> +    }
>> +
>> +    return sys->pool;
>> +    VLC_UNUSED(count);
> 
> Some compilers might not like code after return.
> 
>> +}
>> +
>> +
>> +static void Display(vout_display_t *vd, picture_t *picture,
>> +                    subpicture_t *subpicture)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    int i;
>> +
>> +    if (drmModeSetPlane(sys->drm_fd, sys->plane_id, sys->crtc,
>> +                         sys->fb[sys->front_buf], 1<<1,
>> +                         0, 0, sys->width, sys->height,
>> +                         0, 0, sys->width << 16, sys->height << 16))
>> +    {
>> +        msg_Err(vd, "Cannot do set plane for plane id %u, fb %x (%s)",
>> +                sys->plane_id,
>> +                sys->fb[sys->front_buf],
>> +                vlc_strerror_c(errno));
>> +    } else {
>> +        sys->front_buf ^= 1;
>> +
>> +        for(i = 0; i < 5; i++ ) {
>> +            sys->picture->p[i].p_pixels =
>> sys->map[sys->front_buf]+sys->offsets[i]; +        }
>> +    }
>> +
>> +    picture_Release(picture);
>> +    VLC_UNUSED(subpicture);
>> +}
>> +
>> +
>> +
>> +/**
>> + * This function allocates and initializes a KMS vout method.
>> + */
>> +static int Open(vlc_object_t *object)
>> +{
>> +    vout_display_t *vd = (vout_display_t *)object;
>> +    vout_display_sys_t *sys;
>> +    vlc_fourcc_t local_vlc_chroma;
>> +    uint32_t local_drm_chroma;
>> +    video_format_t fmt = {};
>> +    char *chroma;
>> +
>> +    if (vout_display_IsWindowed(vd))
>> +        return VLC_EGENERIC;
>> +
>> +    /*
>> +     * Allocate instance and initialize some members
>> +     */
>> +    vd->sys = sys = calloc(1, sizeof(*sys));
>> +    if (!sys)
>> +        return VLC_ENOMEM;
>> +
>> +    sys->pool = NULL;
>> +    sys->map[HWBUF1] = MAP_FAILED;
>> +    sys->map[HWBUF2] = MAP_FAILED;
>> +    sys->drm_fourcc = 0;
>> +    sys->vlc_fourcc = 0;
>> +
>> +    chroma = var_InheritString(vd, "kms-vlc-chroma");
>> +    if (chroma) {
>> +        local_vlc_chroma = vlc_fourcc_GetCodecFromString(VIDEO_ES, chroma);
>> +
>> +        if (local_vlc_chroma) {
>> +            sys->vlc_fourcc = local_vlc_chroma;
>> +            msg_Dbg(vd, "forcing VLC to use chroma '%4s'", chroma);
>> +         } else {
>> +            sys->vlc_fourcc = vd->fmt.i_chroma;
>> +            msg_Dbg(vd, "chroma %4s invalid, using default", chroma);
>> +         }
>> +
>> +        free(chroma);
>> +        chroma = NULL;
>> +    } else {
>> +        sys->vlc_fourcc = vd->fmt.i_chroma;
>> +        msg_Dbg(vd, "chroma %4s invalid, using default", chroma);
>> +    }
>> +
>> +    chroma = var_InheritString(vd, "kms-drm-chroma");
>> +    if (chroma) {
>> +        local_drm_chroma = VLC_FOURCC(chroma[0], chroma[1], chroma[2],
>> +                                      chroma[3]);
>> +
>> +        if (local_drm_chroma) {
>> +            sys->forced_drm_fourcc = true;
>> +            sys->drm_fourcc = local_drm_chroma;
>> +            msg_Dbg(vd, "Setting DRM chroma to '%4s'", chroma);
>> +        }
>> +        else
>> +            msg_Dbg(vd, "chroma %4s invalid, using default", chroma);
>> +
>> +        free(chroma);
>> +        chroma = NULL;
>> +    }
>> +
>> +    if (OpenDisplay(vd)) {
>> +        Close(VLC_OBJECT(vd));
>> +        return VLC_EGENERIC;
>> +    }
>> +
>> +    video_format_ApplyRotation(&fmt, &vd->fmt);
>> +
>> +    fmt.i_width = fmt.i_visible_width  = sys->width;
>> +    fmt.i_height = fmt.i_visible_height = sys->height;
>> +    fmt.i_chroma = sys->vlc_fourcc;
>> +
>> +    vd->fmt     = fmt;
>> +    vd->pool    = Pool;
>> +    vd->prepare = NULL;
>> +    vd->display = Display;
>> +    vd->control = Control;
>> +
>> +    vout_display_SendEventDisplaySize(vd, fmt.i_visible_width,
>> +                                      fmt.i_visible_height);
>> +    return VLC_SUCCESS;
>> +}
>> +
>> +static void CloseDisplay(vout_display_t *vd)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +
>> +    if (sys->map[HWBUF1] != MAP_FAILED) {
>> +        DestroyFB(vd, HWBUF1);
>> +        sys->map[HWBUF1] = MAP_FAILED;
> 
> Seems useless?
> 
>> +    }
>> +
>> +    if (sys->map[HWBUF2] != MAP_FAILED) {
>> +        DestroyFB(vd, HWBUF2);
>> +        sys->map[HWBUF2] = MAP_FAILED;
>> +    }
>> +
>> +    if (sys->drm_fd >= 0) {
>> +        drmSetClientCap(sys->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
>> +        drmDropMaster(sys->drm_fd);
>> +        vlc_close(sys->drm_fd);
>> +        sys->drm_fd = 0;
>> +    }
>> +}
>> +
>> +/**
>> + * Terminate an output method created by Open
>> + */
>> +static void Close(vlc_object_t *object)
>> +{
>> +    vout_display_t *vd = (vout_display_t *)object;
>> +    CloseDisplay(vd);
>> +}
> 
> 



More information about the vlc-devel mailing list