[vlc-devel] [PATCH v4] linux: kernel mode setting (KMS) vout plugin
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Thu Jun 28 14:31:59 CEST 2018
On 26.06.2018 11:30, Steve Lhomme wrote:
> On 6/26/2018 10:06 AM, Steve Lhomme wrote:
>> On 6/11/2018 2:34 PM, Juha-Pekka Heikkila wrote:
>>> 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 overide DRM requested framebuffer type.
>>>
>>> v4: rebase
>>>
>>> v3: Increased HW frame amount to 3 to be able to reduce stall
>>> from kernel. HW frame amount can be now changed by changing
>>> defined MAXHWBUF. General cleanup.
>>> Rémi Denis-Courmont: I kept module description in beginning
>>> of kms.c, otherwise my plugin look different from all other
>>> plugins in this. I changed desciption for my commandline
>>
>> Not all plugins. We now try to avoid this. It's even easier to find a
>> module definition by scrolling down at the bottom of the file.
ack. I'll move it to the end of file.
>>
>>> parameters in hope to underline their different function.
>>> Destruction of HW buffers is now moved to picture destroy
>>> callback.
>>>
>>> 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 | 829
>>> +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 844 insertions(+)
>>> create mode 100644 modules/video_output/kms.c
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index dfb8ecd..564c32b 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -3392,6 +3392,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 5d18a36..2be8758 100644
>>> --- a/modules/video_output/Makefile.am
>>> +++ b/modules/video_output/Makefile.am
>>> @@ -422,6 +422,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..e26c38b
>>> --- /dev/null
>>> +++ b/modules/video_output/kms.c
>>> @@ -0,0 +1,829 @@
>>> +/*****************************************************************************
>>>
>>> + * 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 <sys/mman.h>
>>> +
>>> +#include <xf86drm.h>
>>> +#include <xf86drmMode.h>
>>> +#include <drm_fourcc.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 request to VLC for
>>> output format")
>>> +
>>> +#define DRM_CHROMA_TEXT N_("Image format used by DRM")
>>> +#define DRM_CHROMA_LONGTEXT N_("Chroma fourcc override for DRM
>>> framebuffer format selection")
>>> +
>>> +static int Open (vlc_object_t *);
>>> +static void Close(vlc_object_t *);
>>> +
>>> +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)
>>> +
>>> + 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)
>>> + set_description(N_("Linux kernel mode setting video output"))
>>> + set_capability("vout display", 30)
>>> + set_callbacks(Open, Close)
>>> +vlc_module_end ()
>>> +
>>> +/*****************************************************************************
>>>
>>> + * Local prototypes
>>> +
>>> *****************************************************************************/
>>>
>>> +
>>> +/*
>>> + * how many hw buffers are allocated for page flipping. I think
>>> + * 3 is enough so we shouldn't get unexpected stall from kernel.
>>> + */
>>> +#define MAXHWBUF 3
>>> +
>>> +typedef enum { drvSuccess, drvTryNext, drvFail } deviceRval;
>>> +
>>> +struct vout_display_sys_t {
>>> +/*
>>> + * buffer information
>>> + */
>>> + uint32_t width;
>>> + uint32_t height;
>>> + uint32_t stride;
>>> + uint32_t size;
>>> + uint32_t offsets[PICTURE_PLANE_MAX];
>>> +
>>> + uint32_t handle[MAXHWBUF];
>>> + uint8_t *map[MAXHWBUF];
>>> +
>>> + uint32_t fb[MAXHWBUF];
>>> + 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
>>> + */
>>> + uint32_t crtc;
>>> + uint32_t plane_id;
>>> +
>>> +/*
>>> + * other generic stuff
>>> + */
>>> + int drm_fd;
>>> +};
>>> +
>>> +typedef struct {
>>> + vout_display_sys_t *p_voutsys;
>>> +} picture_sys_t;
>>> +
>>> +
>>> +static void DestroyFB(vout_display_sys_t const *sys, uint32_t const
>>> buf)
>>> +{
>>> + struct drm_mode_destroy_dumb destroy_req = { .handle =
>>> sys->handle[buf] };
>>> +
>>> + munmap(sys->map[buf], sys->size);
>>> + 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 deviceRval CreateFB(vout_display_t *vd, const 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 = {};
>>> + unsigned int tile_width = 512, tile_height = 16;
>>> + deviceRval ret;
>>> + int i;
>>> + uint32_t offsets[] = {0,0,0,0}, handles[] = {0,0,0,0},
>>> + pitches[] = {0,0,0,0};
>>> +
>>> + 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);
>>
>> Shouldn't it be 2*ALIGN(sys->height, tile_height)? You may have
>> misaligned buffers if the alignment doesn't add the same value.
Good catch, the problem here is not about misaligned buffers really but
to get enough memory for two planes. I didn't spot this earlier but knew
something was wrong so there is actually reserved bit more memory than
really needed. This is problem for those biplanar formats in my code,
Pxxx and NV12. I'll fix.
>>> + 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);
>>
>> So 4*ALIGN(sys->width, tile_width)?
No, this is correct in my code. This is about getting stride length
right and matching to tile width. Here is made the assumption at maximum
is 32bpp formats available (which you can verify from DRM headers,
nobody is supporting bigger bpp and I don't expect this to change for
some time)
>>> + break;
>>> + }
>>> +
>>> + ret = drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_CREATE_DUMB,
>>> &create_req);
>>> + if (ret < 0) {
>>> + msg_Err(vd, "Cannot create dumb buffer");
>>> + return drvFail;
>>> + }
>>> +
>>> + sys->size = create_req.size;
>>> + sys->handle[buf] = create_req.handle;
>>> +
>>> + /*
>>> + * create framebuffer object for the dumb-buffer
>>> + * index 0 has to be filled in any case.
>>> + */
>>> + for (i = 0; i < 4 && (sys->offsets[i] || i < 1); i++) {
>>> + handles[i] = create_req.handle;
>>
>> instead of 4 use ARRAY_SIZE(handles)
>>
>>
>>
>>> + pitches[i] = sys->stride;
>>> + offsets[i] = sys->offsets[i];
>>> + }
>>> +
>>> + ret = drmModeAddFB2(sys->drm_fd, sys->width, sys->height,
>>> sys->drm_fourcc,
>>> + handles, pitches, offsets, &sys->fb[buf],
>>> 1<<1);
>>
>> If 1<<1 is a flag maybe it has a pretty name ?
I think this can live without fb modifiers, I'll just put zero in that
place. It will not make any difference for the performance. At some
point I will come back to this when start to make v2 of this with tiled
memory usage.
>>
>>> +
>>> + if (ret) {
>>> + msg_Err(vd, "Cannot create frame buffer");
>>> + ret = drvFail;
>>> + 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");
>>> + ret = drvFail;
>>> + 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");
>>> + ret = drvFail;
>>> + goto err_fb;
>>> + }
>>> + return drvSuccess;
>>> +
>>> +err_fb:
>>> + drmModeRmFB(sys->drm_fd, sys->fb[buf]);
>>> + sys->fb[buf] = 0;
>>> +
>>> +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 deviceRval FindCRTC(vout_display_t *vd, drmModeRes const *res,
>>> + drmModeConnector const *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, "Got CRTC %d from current encoder",
>>> enc->crtc_id);
>>> +
>>> + sys->crtc = enc->crtc_id;
>>> + drmModeFreeEncoder(enc);
>>> + return drvSuccess;
>>> + }
>>> + drmModeFreeEncoder(enc);
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * Iterate all available encoders to find CRTC
>>> + */
>>> + for (i = 0; i < conn->count_encoders; i++) {
>>> + enc = drmModeGetEncoder(sys->drm_fd, conn->encoders[i]);
>>> +
>>> + for (j = 0; enc && j < res->count_crtcs; ++j) {
>>> + if (ffs(enc->possible_crtcs) == j && res->crtcs[j]) {
>>> + msg_Dbg(vd, "Got CRTC %d", res->crtcs[j]);
>>> + sys->crtc = res->crtcs[j];
>>> + drmModeFreeEncoder(enc);
>>> + return drvSuccess;
>>> + }
>>> + }
>>> + drmModeFreeEncoder(enc);
>>> + }
>>> +
>>> + msg_Err(vd , "Cannot find CRTC for connector %d",
>>> conn->connector_id);
>>> + return drvTryNext;
>>> +}
>>> +
>>> +
>>> +static deviceRval SetupDevice(vout_display_t *vd, drmModeRes const
>>> *res,
>>> + drmModeConnector const *conn)
>>> +{
>>> + vout_display_sys_t *sys = vd->sys;
>>> + deviceRval ret;
>>> + int c, c2;
>>> +
>>> + if (conn->connection != DRM_MODE_CONNECTED || conn->count_modes
>>> == 0)
>>> + return drvTryNext;
>>> +
>>> + sys->width = conn->modes[0].hdisplay;
>>> + sys->height = conn->modes[0].vdisplay;
>>> + msg_Dbg(vd, "Mode resolution for connector %u is %ux%u",
>>> + conn->connector_id, sys->width, sys->height);
>>> +
>>> + ret = FindCRTC(vd, res, conn);
>>> + if (ret != drvSuccess) {
>>> + msg_Dbg(vd , "No valid CRTC for connector %d",
>>> conn->connector_id);
>>> + return ret;
>>> + }
>>> +
>>> + for (c = 0; c < MAXHWBUF; c++) {
>>> + ret = CreateFB(vd, c);
>>> + if (ret != drvSuccess) {
>>> + msg_Err(vd, "Cannot create framebuffer %d for connector
>>> %d", c,
>>> + conn->connector_id);
>>> + for (c2 = 0; c2 < c; c2++)
>>> + DestroyFB(sys, c2);
>>> +
>>> + return ret;
>>> + } else {
>>> + msg_Dbg(vd, "Created HW framebuffer %d/%d", c+1, MAXHWBUF);
>>> + }
>>> + }
>>> + return drvSuccess;
>>> +}
>>> +
>>> +
>>> +/** 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 },
>>> +};
>>> +
>>> +
>>> +static void CheckFourCCList(uint32_t drmfourcc, uint32_t plane_id)
>>> +{
>>> + unsigned i;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(fourccmatching); 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;
>>> + bool YUVFormat;
>>> +
>>> + /*
>>> + * 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 pipe %c
>>> format %2d: %.4s",
>>> + plane->plane_id, types[planetype],
>>> + ('@'+ffs(plane->possible_crtcs)), 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 < ARRAY_SIZE(fourccmatching); 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;
>>> + }
>>> +
>>> + /*
>>> + * favor yuv format according to YUVFormat flag.
>>> + * check for exact match first, then YUVFormat and then !YUVFormat
>>> + */
>>> + YUVFormat = vlc_fourcc_IsYUV(sys->vlc_fourcc);
>>
>> You may read this value right before you need it. In this first loop
>> it's not used.
>>> + for (c = i = 0; c < ARRAY_SIZE(fourccmatching); 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 < ARRAY_SIZE(fourccmatching); c++) {
>>> + if (fourccmatching[c].isYUV == YUVFormat
>>> + && 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 < ARRAY_SIZE(fourccmatching); c++) {
>>> + if (!fourccmatching[c].isYUV != YUVFormat
>>> + && 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;
>>> + int c;
>>> + bool found_connector = false;
>>> +
>>> + /*
>>> + * Open framebuffer device
>>> + */
>>> + psz_device = var_InheritString(vd, KMS_VAR);
>>> + if (psz_device == NULL) {
>>> + msg_Err(vd, "Don't know which DRM device to open");
>>> + return VLC_EGENERIC;
>>> + }
>>> +
>>> + sys->drm_fd = vlc_open(psz_device, O_RDWR);
>>> + if (sys->drm_fd == -1) {
>>> + msg_Err(vd, "cannot open %s", psz_device);
>>> + free(psz_device);
>>> + return VLC_EGENERIC;
>>> + }
>>> +
>>> + ret = drmSetMaster(sys->drm_fd);
>>> + if (ret < 0) {
>>> + msg_Err(vd, "Cannot become DRM master on %s", psz_device);
>>> +
>>> + free(psz_device);
>>> + vlc_close(sys->drm_fd);
>>> + sys->drm_fd = 0;
>>> + return VLC_EGENERIC;
>>> + }
>>> + free(psz_device);
>>> +
>>> + drmSetClientCap(sys->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
>>> +
>>> + if (!ChromaNegotiation(vd)) {
>>
>> No error message here ?
Error message was printed in ChromaNegotiation about why it failed. The
last 'return false' in the end of ChromaNegotiation function is such the
execution will never reach there but it will just silence compiler warning.
>> Maybe use a goto as the code is similar to the following error handling.
>>
>>> + drmDropMaster(sys->drm_fd);
>>> + 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);
>>> + drmDropMaster(sys->drm_fd);
>>> + vlc_close(sys->drm_fd);
>>> + sys->drm_fd = 0;
>>> + return VLC_EGENERIC;
>>> + }
>>> +
>>> + modeRes = drmModeGetResources(sys->drm_fd);
>>> + if (modeRes == NULL) {
>>> + msg_Err(vd, "Didn't get DRM resources");
>>> + vlc_close(sys->drm_fd);
>>> + drmDropMaster(sys->drm_fd);
>>> + sys->drm_fd = 0;
>>> + return VLC_EGENERIC;
>>> + }
>>> +
>>> + for (c = 0; c < modeRes->count_connectors && sys->crtc == 0; c++) {
>>> +
>>> + conn = drmModeGetConnector(sys->drm_fd,
>>> modeRes->connectors[c]);
>>> + if (conn == NULL)
>>> + continue;
>>> +
>>> + found_connector = true;
>>> +
>>> + ret = SetupDevice(vd, modeRes, conn);
>>> + if (ret != drvSuccess) {
>>> + if (ret != drvTryNext) {
>>> + msg_Err(vd, "Cannot do setup for connector %u:%u", c,
>>> + modeRes->connectors[c]);
>>> +
>>> + drmModeFreeConnector(conn);
>>> + drmModeFreeResources(modeRes);
>>> + drmDropMaster(sys->drm_fd);
>>> + vlc_close(sys->drm_fd);
>>> + sys->drm_fd = 0;
>>> + return VLC_EGENERIC;
>>> + }
>>> + drmModeFreeConnector(conn);
>>> + found_connector = false;
>>> + continue;
>>> + }
>>> + drmModeFreeConnector(conn);
>>> + }
>>> + drmModeFreeResources(modeRes);
>>> + if (!found_connector) {
>>> + drmModeFreeResources(modeRes);
>>> + drmDropMaster(sys->drm_fd);
>>> + vlc_close(sys->drm_fd);
>>> + sys->drm_fd = 0;
>>> + return VLC_EGENERIC;
>>> + }
>>> + return VLC_SUCCESS;
>>> +}
>>> +
>>> +
>>> +static int Control(vout_display_t *vd, int query, va_list args)
>>> +{
>>> + VLC_UNUSED(vd);
>>> + VLC_UNUSED(query);
>>> + VLC_UNUSED(args);
>>> + return VLC_EGENERIC;
>>> +}
>>> +
>>> +
>>> +static void CustomDestroyPicture(picture_t *p_picture)
>>> +{
>>> + picture_sys_t *psys = (picture_sys_t*)p_picture->p_sys;
>>> + vout_display_sys_t *sys = (vout_display_sys_t *)psys->p_voutsys;
>>> + int c;
>>> +
>>> + for (c = 0; c < MAXHWBUF; c++)
>>> + DestroyFB(sys, c);
>>
>> Given the buffers/resources are not allocated when the pool is
>> created, they should not be released when the pool is released. In
>> this case when the module is released.
This was written after review comment from Remi. He mentioned some
codecs are slow to die and may be writing to picture until the picture
element itself is destroyed. This is where I knew for certain my picture
is thrown away.
>>
>>> +
>>> + drmSetClientCap(sys->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
>>> + drmDropMaster(sys->drm_fd);
>>> + vlc_close(sys->drm_fd);
>>> + sys->drm_fd = 0;
>>> + free(p_picture->p_sys);
>>> + free(p_picture);
>>> +}
>>> +
>>> +
>>> +static picture_pool_t *Pool(vout_display_t *vd, unsigned count)
>>> +{
>>> + VLC_UNUSED(count);
>>> + vout_display_sys_t *sys = vd->sys;
>>> + picture_sys_t *psys;
>>> + picture_resource_t rsc;
>>> + int i;
>>> +
>>> + if (!sys->pool && !sys->picture) {
>>> + memset(&rsc, 0, sizeof(rsc));
>>> +
>>> + for (i = 0; i < PICTURE_PLANE_MAX; i++) {
>>> + 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;
>>> + }
>>> +
>>> + psys = calloc(1, sizeof(*psys));
>>> + if (psys == NULL)
>>> + return NULL;
>>> +
>>> + psys->p_voutsys = sys;
>>> + rsc.p_sys = psys;
>>> + rsc.pf_destroy = CustomDestroyPicture;
>>> +
>>> + sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
>>> +
>>> + if (!sys->picture) {
>>> + free((void*)psys);
>>> + return NULL;
>>> + }
>>> +
>>> + sys->pool = picture_pool_New(1, &sys->picture);
>>> + if (!sys->pool)
>>> + picture_Release(sys->picture);
>>> + }
>>> +
>>> + return sys->pool;
>>> +}
>>> +
>>> +
>>> +static void Display(vout_display_t *vd, picture_t *picture,
>>> + subpicture_t *subpicture)
>>> +{
>>> + VLC_UNUSED(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",
>>> + sys->plane_id,
>>> + sys->fb[sys->front_buf]);
>>> + } else {
>>> + sys->front_buf++;
>>> + sys->front_buf %= MAXHWBUF;
>>> +
>>> + for (i = 0; i < PICTURE_PLANE_MAX; i++)
>>> + sys->picture->p[i].p_pixels =
>>> + sys->map[sys->front_buf]+sys->offsets[i];
>>
>> Are you switching buffers in the single pool picture here ? IMO it
>> would be cleaner to use MAXHWBUF pictures in the pool and let the
>> picture pool do its job. You won't have to manage the front_buf yourself.
>
> While you're at it you might dump MAXHWBUF if the resources are usually
> not contrained. And allocate the amount of buffers requested in the
> Pool() call. This way you may benefit from direct rendering which is
> faster.
I tried this earlier but I didn't get it working with two buffers. I was
getting only my first picture used with visible tearing and second
buffer stayed black. On irc someone suggested I will release only that
buffer where I want my next frame to be rendered but this resulted in
deadlock which I couldn't go around.
For this reason I had on my first patch two pools both containing their
own picture but I knew that is not the correct way and Remi said its one
pool and one picture thus I copied the way fb plugin has done it (btw,
currently it seems to me doublebuffering in fb plugin is broken)
Is there some vout plugin where the pooling is used and it work? I tried
to look for but I didn't spot such.
>
> If working in the DRM memory (is it GPU memory ?) is slow then you
> should set vd->info.is_slow = true. In this case you won't get direct
> rendering but you won't slow down decoding.
On desktops display memory is generally on the same area as gpu memory
but this is not mandated anywhere, I'd expect there are devices where
gpu is a device of its own totally separate from display. I don't see
display memory being so slow there would be need for special
arrangements. On some ways you could think display VRAM is the fastest
memory of the device because missed read will make physical display look
black to the user.
>
> Note that the core requests more buffers than it actually use most of
> the time. So if the amount of memory you can allocate is too small you
> should just allocate as much as you can and the core will allocate a
> separate memory pool for the decoder. But if that's CPU memory you're
> using it will just allocate more memory in the same place...
>
>>> + }
>>> + picture_Release(picture);
>>> +}
>>> +
>>> +
>>> +/**
>>> + * 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));
>>
>> Use vlc_obj_calloc().
>>
>>> + if (!sys)
>>> + return VLC_ENOMEM;
>>> +
>>> + sys->pool = NULL;
>>> + sys->drm_fourcc = 0;
>>> + sys->vlc_fourcc = 0;
>>
>> calloc already sets everything to 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)) {
>>
>> != VLC_SUCCESS is more readable
>>
>>> + 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_window_ReportSize(vd->cfg->window, sys->width, sys->height);
>>> + return VLC_SUCCESS;
>>> +}
>>> +
>>> +
>>> +static void CloseDisplay(vout_display_t *vd)
>>> +{
>>> + vout_display_sys_t *sys = vd->sys;
>>> +
>>> + if (sys->pool)
>>> + picture_pool_Release(sys->pool);
>>> +
>>> + if (sys->drm_fd)
>>> + drmDropMaster(sys->drm_fd);
>>> +
>>> + free(sys);
>>> +}
>>> +
>>> +
>>> +/**
>>> + * Terminate an output method created by Open
>>> + */
>>> +static void Close(vlc_object_t *object)
>>> +{
>>> + vout_display_t *vd = (vout_display_t *)object;
>>> +
>>> + CloseDisplay(vd);
>>> +}
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
Thanks for the review comments. I'll make the changes and submit patch v5.
If you have in mind some working example where the picture pooling is in
use in vout module it would be awesome. I feel my current version is
still fighting against the system but it's because I didn't get the
picture pooling working.
/Juha-Pekka
More information about the vlc-devel
mailing list