Re: [RFC PATCH 1/3] drm/vkms: decouple cursor plane setup from crtc_init

On Sat, Feb 20, 2021 at 11:38:50AM -0300, Melissa Wen wrote:

Initialize CRTC only with primary plane (without cursor) as a preparation to init overlay plane before cursor plane and keep cursor on the top.

Signed-off-by: Melissa Wen melissa.srw@gmail.com

Why can't we first initialize all the planes (primary, cursor, overlay) and then the crtc?

For drivers where there's not really a functional difference between these planes (like vkms, since it's all sw, only difference is z position really) the usual approach is to initialize all planes in a loop. And then call crtc init with the first and last plane for that crtc.

Passing NULL for the cursor for crtc_init and then patching it up afterwards is a bit a hack, so would be good to avoid that. -Daniel

---

drivers/gpu/drm/vkms/vkms_crtc.c | 4 ++-- drivers/gpu/drm/vkms/vkms_drv.h | 2 +- drivers/gpu/drm/vkms/vkms_output.c | 14 +++++++++----- 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 0443b7deeaef..2d4cd7736933 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -270,12 +270,12 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = { };

int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,

•    struct drm_plane *primary, struct drm_plane *cursor)
  

•    struct drm_plane *primary)
  

{ struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc); int ret;

• ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,

• ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, &vkms_crtc_funcs, NULL); if (ret) { DRM_ERROR("Failed to init CRTC\n");

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 35540c7c4416..9ad5ad8b7737 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -110,7 +110,7 @@ struct vkms_device {

/* CRTC */ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,

•    struct drm_plane *primary, struct drm_plane *cursor);
  

•    struct drm_plane *primary);
  
  

int vkms_output_init(struct vkms_device *vkmsdev, int index);

diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index f5f6f15c362c..05d3bb45e6c1 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -47,6 +47,10 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) if (IS_ERR(primary)) return PTR_ERR(primary);

• ret = vkms_crtc_init(dev, crtc, primary);
• if (ret)

• goto err_crtc;
  

• if (vkmsdev->config->cursor) { cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); if (IS_ERR(cursor)) {

@@ -55,9 +59,9 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) } }

• ret = vkms_crtc_init(dev, crtc, primary, cursor);
• if (ret)

• goto err_crtc;
  

• crtc->cursor = cursor;

• if (cursor && !cursor->possible_crtcs)

• cursor->possible_crtcs = drm_crtc_mask(crtc);
  
  

  ret = drm_connector_init(dev, connector, &vkms_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);

@@ -100,11 +104,11 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) err_connector: drm_crtc_cleanup(crtc);

-err_crtc: +err_cursor: if (vkmsdev->config->cursor) drm_plane_cleanup(cursor);

-err_cursor: +err_crtc: drm_plane_cleanup(primary);

return ret;

2.30.0