Re: [PATCH v3 1/4] drm/vkms: init plane using drmm_universal_plane_alloc

On Tue, Apr 13, 2021 at 04:50:08AM -0300, Melissa Wen wrote:

By using drmm_universal_plane_alloc instead of drm_universal_plane_init, we let the DRM infrastructure handles resource allocation and cleanup. We can also get rid of some code repetitions for plane cleanup, improving code maintainability in vkms.

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

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

---

drivers/gpu/drm/vkms/vkms_drv.h | 8 ++++++-- drivers/gpu/drm/vkms/vkms_output.c | 19 +++++-------------- drivers/gpu/drm/vkms/vkms_plane.c | 29 +++++++++++------------------ 3 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 35540c7c4416..70fb79621617 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -37,6 +37,10 @@ struct vkms_plane_state { struct vkms_composer *composer; };

+struct vkms_plane {

• struct drm_plane base;

+};

/**

• vkms_crtc_state - Driver specific CRTC state
• @base: base CRTC state

@@ -114,8 +118,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,

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

-struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,

• 		  enum drm_plane_type type, int index);
  

+struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,

• 		   enum drm_plane_type type, int index);
  
  

/* CRC Support */ const char *const *vkms_get_crc_sources(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index f5f6f15c362c..6979fbc7f821 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -39,7 +39,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) struct drm_connector *connector = &output->connector; struct drm_encoder *encoder = &output->encoder; struct drm_crtc *crtc = &output->crtc;

• struct drm_plane *primary, *cursor = NULL;

• struct vkms_plane *primary, *cursor = NULL; int ret; int writeback;

@@ -49,15 +49,13 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)

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

• if (IS_ERR(cursor)) {
  

• 	ret = PTR_ERR(cursor);
  

• 	goto err_cursor;
  

• }
  

• if (IS_ERR(cursor))
  

• 	return PTR_ERR(cursor);
  

  }

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

• ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base); if (ret)

• goto err_crtc;
  

• return ret;
  
  

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

@@ -100,12 +98,5 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) err_connector: drm_crtc_cleanup(crtc);

-err_crtc:

• if (vkmsdev->config->cursor)

• drm_plane_cleanup(cursor);
  

-err_cursor:

• drm_plane_cleanup(primary);
• return ret;

} diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 6d310d31b75d..135140f8e87a 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -86,7 +86,6 @@ static void vkms_plane_reset(struct drm_plane *plane) static const struct drm_plane_funcs vkms_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane,

• .destroy = drm_plane_cleanup, .reset = vkms_plane_reset, .atomic_duplicate_state = vkms_plane_duplicate_state, .atomic_destroy_state = vkms_plane_destroy_state,

@@ -191,18 +190,14 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { .cleanup_fb = vkms_cleanup_fb, };

-struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,

• 		  enum drm_plane_type type, int index)
  

+struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,

• 		   enum drm_plane_type type, int index)
  

{ struct drm_device *dev = &vkmsdev->drm; const struct drm_plane_helper_funcs *funcs;

• struct drm_plane *plane;

• struct vkms_plane *plane; const u32 *formats;

• int ret, nformats;
• plane = kzalloc(sizeof(*plane), GFP_KERNEL);
• if (!plane)

• return ERR_PTR(-ENOMEM);
  

• int nformats;

  if (type == DRM_PLANE_TYPE_CURSOR) { formats = vkms_cursor_formats;

@@ -214,16 +209,14 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, funcs = &vkms_primary_helper_funcs; }

• ret = drm_universal_plane_init(dev, plane, 1 << index,

• 		       &vkms_plane_funcs,
  

• 		       formats, nformats,
  

• 		       NULL, type, NULL);
  

• if (ret) {

• kfree(plane);
  

• return ERR_PTR(ret);
  

• }

• plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index,

• 			   &vkms_plane_funcs,
  

• 			   formats, nformats,
  

• 			   NULL, type, NULL);
  

• if (IS_ERR(plane))

• return plane;
  
  

• drm_plane_helper_add(plane, funcs);

• drm_plane_helper_add(&plane->base, funcs);

  return plane;

}

2.30.2