Hi Danilo,
thanks for submitting this patch.
On Wed, Jul 20, 2022 at 04:02:13PM +0200, Danilo Krummrich wrote:
> Use drm managed resource allocation (drmm_universal_plane_alloc()) in
> order to cleanup/simplify drm plane .destroy callback.
>
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> ---
> drivers/gpu/drm/virtio/virtgpu_plane.c | 30 +++++++-------------------
> 1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 6d3cc9e238a4..3008551d6a05 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -67,16 +67,10 @@ uint32_t virtio_gpu_translate_format(uint32_t drm_fourcc)
> return format;
> }
>
> -static void virtio_gpu_plane_destroy(struct drm_plane *plane)
> -{
> - drm_plane_cleanup(plane);
> - kfree(plane);
> -}
> -
> static const struct drm_plane_funcs virtio_gpu_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .destroy = virtio_gpu_plane_destroy,
> + .destroy = drm_plane_cleanup,
>From the documentation of drmm_universal_plane_alloc:
The @drm_plane_funcs.destroy hook must be NULL.
So the above assignment looks wrong.
The rest of this patch looks OK.
Sam
> .reset = drm_atomic_helper_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> @@ -379,11 +373,7 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
> const struct drm_plane_helper_funcs *funcs;
> struct drm_plane *plane;
> const uint32_t *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 = virtio_gpu_cursor_formats;
> @@ -394,17 +384,13 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
> nformats = ARRAY_SIZE(virtio_gpu_formats);
> funcs = &virtio_gpu_primary_helper_funcs;
> }
> - ret = drm_universal_plane_init(dev, plane, 1 << index,
> - &virtio_gpu_plane_funcs,
> - formats, nformats,
> - NULL, type, NULL);
> - if (ret)
> - goto err_plane_init;
> +
> + plane = drmm_universal_plane_alloc(dev, struct drm_plane, dev,
> + 1 << index, &virtio_gpu_plane_funcs,
> + formats, nformats, NULL, type, NULL);
> + if (IS_ERR(plane))
> + return plane;
>
> drm_plane_helper_add(plane, funcs);
> return plane;
> -
> -err_plane_init:
> - kfree(plane);
> - return ERR_PTR(ret);
> }
> --
> 2.36.1
