[vlc-devel] [PATCH 1/2] linux: Fix CRTC code in KMS plugin.

Rémi Denis-Courmont remi at remlab.net
Sun Jan 13 11:07:30 CET 2019


Le torstaina 10. tammikuuta 2019, 14.21.53 EET Juha-Pekka Heikkila a écrit :
> Add missed drmModeSetCrtc(..) call. This does two things. Firstly
> if requested chroma is not on default CRTC we'll get plane showing
> correctly. Secondly this allow exiting gracefully in situtation
> where Xserver is running in the background but has released
> DRM master for VLC to use.
> 
> Added printing out in debug information available connectors.
> 
> Small detail fixes.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  modules/video_output/kms.c | 94
> ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 79
> insertions(+), 15 deletions(-)
> 
> diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
> index bd65ee8..789df96 100644
> --- a/modules/video_output/kms.c
> +++ b/modules/video_output/kms.c
> @@ -53,9 +53,11 @@
>  #define DEVICE_LONGTEXT \
>      "Framebuffer device to use for rendering (usually /dev/dri/card0)."
> 
> +#define VLC_CHROMA_PARAM "kms-vlc-chroma"
>  #define VLC_CHROMA_TEXT "Image format used by VLC"
>  #define VLC_CHROMA_LONGTEXT "Chroma fourcc request to VLC for output
> format"
> 
> +#define DRM_CHROMA_PARAM "kms-drm-chroma"
>  #define DRM_CHROMA_TEXT "Image format used by DRM"
>  #define DRM_CHROMA_LONGTEXT "Chroma fourcc override for DRM framebuffer
> format selection"
> 
> @@ -93,6 +95,10 @@ struct vout_display_sys_t {
>  /*
>   * modeset information
>   */
> +    uint32_t        connector_id;
> +    drmModeModeInfo used_mode;
> +    uint32_t        old_crtc_id;
> +    drmModeCrtc     *old_crtc;
>      uint32_t        crtc;
>      uint32_t        plane_id;
> 
> @@ -122,7 +128,7 @@ static void DestroyFB(vout_display_sys_t const *sys,
> uint32_t const buf) 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,
> +    struct drm_mode_create_dumb create_req = { .width = (sys->width|511)+1,
> .height = sys->height, .bpp = 32 };

Uh? Are you sure you want 1..512 bytes of padding? I would rather expect 
0..511 bytes.

>      struct drm_mode_destroy_dumb destroy_req;
> @@ -252,9 +258,8 @@ static deviceRval FindCRTC(vout_display_t *vd,
> drmModeRes const *res, 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]);
> +        for (j = 0; enc && j < res->count_crtcs; j++) {
> +            if (enc->possible_crtcs & (1 << j)) {
>                  sys->crtc = res->crtcs[j];
>                  drmModeFreeEncoder(enc);
>                  return drvSuccess;
> @@ -278,10 +283,10 @@ static deviceRval SetupDevice(vout_display_t *vd,
> drmModeRes const *res, 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);
> +    memcpy(&sys->used_mode, &conn->modes[0], sizeof(sys->used_mode));
> +    sys->width = sys->used_mode.hdisplay;
> +    sys->height = sys->used_mode.vdisplay;
> +    msg_Dbg(vd, "Mode resolution is %ux%u", sys->width, sys->height);
> 
>      ret = FindCRTC(vd, res, conn);
>      if (ret != drvSuccess) {
> @@ -302,6 +307,7 @@ static deviceRval SetupDevice(vout_display_t *vd,
> drmModeRes const *res, msg_Dbg(vd, "Created HW framebuffer %d/%d", c+1,
> MAXHWBUF); }
>      }
> +    sys->connector_id = conn->connector_id;
>      return drvSuccess;
>  }
> 
> @@ -505,8 +511,34 @@ static int OpenDisplay(vout_display_t *vd)
>      drmModeConnector *conn;
>      drmModeRes *modeRes;
>      int c;
> +    uint32_t i;

Keep automatic variables as local as possible.

>      bool found_connector = false;
> 
> +    static const struct
> +    {
> +        uint32_t    type;
> +        const char* name;
> +    } connectors[] = {
> +        { .type = DRM_MODE_CONNECTOR_VGA, .name = "vga"},
> +        { .type = DRM_MODE_CONNECTOR_DVII, .name = "DVII"},
> +        { .type = DRM_MODE_CONNECTOR_DVID, .name = "DVID"},
> +        { .type = DRM_MODE_CONNECTOR_DVIA, .name = "DVIA"},
> +        { .type = DRM_MODE_CONNECTOR_Composite, .name = "Composite"},
> +        { .type = DRM_MODE_CONNECTOR_SVIDEO, .name = "SVIDEO"},
> +        { .type = DRM_MODE_CONNECTOR_LVDS, .name = "LVDS"},
> +        { .type = DRM_MODE_CONNECTOR_Component, .name = ""},
> +        { .type = DRM_MODE_CONNECTOR_9PinDIN, .name = "Component"},
> +        { .type = DRM_MODE_CONNECTOR_DisplayPort, .name = "DisplayPort"},
> +        { .type = DRM_MODE_CONNECTOR_HDMIA, .name = "HDMIA"},
> +        { .type = DRM_MODE_CONNECTOR_HDMIB, .name = "HDMIB"},
> +        { .type = DRM_MODE_CONNECTOR_TV, .name = "TV"},
> +        { .type = DRM_MODE_CONNECTOR_eDP, .name = "embedded DisplayPort"},
> +        { .type = DRM_MODE_CONNECTOR_VIRTUAL, .name = "VIRTUAL"},
> +        { .type = DRM_MODE_CONNECTOR_DSI, .name = "DSI"},
> +        { .type = DRM_MODE_CONNECTOR_DPI, .name = "DPI"},
> +        { .type = DRM_MODE_CONNECTOR_Unknown, .name = "Unknown"}
> +    };
> +
>      /*
>       * Open framebuffer device
>       */
> @@ -544,12 +576,22 @@ static int OpenDisplay(vout_display_t *vd)
>          goto err_out;
>      }
> 
> -    for (c = 0; c < modeRes->count_connectors && sys->crtc == 0; c++) {
> -
> +    for (c = 0; c < modeRes->count_connectors; c++) {
>          conn = drmModeGetConnector(sys->drm_fd, modeRes->connectors[c]);
>          if (conn == NULL)
>              continue;
> 
> +        for (i = 0; i < ARRAY_SIZE(connectors)-1; i++) {
> +            if (conn->connector_type == connectors[i].type)
> +                break;
> +        }
> +        msg_Dbg(vd, "** Connector #%d type %s, connected [%c]", c,
> +                connectors[i].name,
> +                (conn->connection==DRM_MODE_CONNECTED)?'X':' ');
> +
> +        if (sys->crtc != 0)
> +            continue;
> +
>          found_connector = true;
> 
>          ret = SetupDevice(vd, modeRes, conn);
> @@ -655,6 +697,16 @@ static void Display(vout_display_t *vd, picture_t
> *picture) vout_display_sys_t *sys = vd->sys;
>      int i;
> 
> +    if (sys->old_crtc == NULL) {
> +        sys->old_crtc = drmModeGetCrtc(sys->drm_fd, sys->crtc);
> +        if(drmModeSetCrtc(sys->drm_fd, sys->crtc,sys->fb[0],
> +                          0, 0,
> +                          &sys->connector_id, 1,
> +                          &sys->used_mode)) {
> +            msg_Err(vd, "Cannot do modesetting");
> +        }
> +    }
> +
>      if (drmModeSetPlane(sys->drm_fd, sys->plane_id, sys->crtc,
>                           sys->fb[sys->front_buf], 0,
>                           0, 0, sys->width, sys->height,
> @@ -680,8 +732,20 @@ static void Close(vout_display_t *vd)
>  {
>      vout_display_sys_t *sys = vd->sys;
> 
> -    if (sys->pool)
> +    if (sys->pool) {
> +        if(drmModeSetCrtc(sys->drm_fd,
> +                   sys->old_crtc->crtc_id,
> +                   sys->old_crtc->buffer_id,
> +                   sys->old_crtc->x,
> +                   sys->old_crtc->y,
> +                   &sys->connector_id,
> +                   1,
> +                   &sys->old_crtc->mode))
> +            msg_Err(vd, "error modeset xit");

Does this really make sense? I can imagine that this works properly if DRM 
users are started and stopped in strictly stacked order. But otherwise?

> +        drmModeFreeCrtc(sys->old_crtc);
> +
>          picture_pool_Release(sys->pool);
> +    }
> 
>      if (sys->drm_fd)
>          drmDropMaster(sys->drm_fd);
> @@ -709,7 +773,7 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg, if (!sys)
>          return VLC_ENOMEM;
> 
> -    chroma = var_InheritString(vd, "kms-vlc-chroma");
> +    chroma = var_InheritString(vd, VLC_CHROMA_PARAM);
>      if (chroma) {
>          local_vlc_chroma = vlc_fourcc_GetCodecFromString(VIDEO_ES, chroma);
> 
> @@ -728,7 +792,7 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg, msg_Dbg(vd, "Chroma %4s invalid, using default",
> chroma); }
> 
> -    chroma = var_InheritString(vd, "kms-drm-chroma");
> +    chroma = var_InheritString(vd, DRM_CHROMA_PARAM);
>      if (chroma) {
>          local_drm_chroma = VLC_FOURCC(chroma[0], chroma[1], chroma[2],
>                                        chroma[3]);
> @@ -777,9 +841,9 @@ vlc_module_begin ()
>      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, +    add_string(VLC_CHROMA_PARAM, NULL,
> VLC_CHROMA_TEXT, VLC_CHROMA_LONGTEXT, true)
> -    add_string( "kms-drm-chroma", NULL, DRM_CHROMA_TEXT,
> DRM_CHROMA_LONGTEXT, +    add_string(DRM_CHROMA_PARAM, NULL,
> DRM_CHROMA_TEXT, DRM_CHROMA_LONGTEXT, true)
>      set_description("Linux kernel mode setting video output")
>      set_capability("vout display", 30)


-- 
Реми Дёни-Курмон
http://www.remlab.net/





More information about the vlc-devel mailing list