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@xxxxxxxxx>
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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
