Re: [PATCH v2 09/13] drm/msm/dpu: use drmm-managed allocation for dpu_plane

[Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>]

On 7/7/2023 4:12 PM, Dmitry Baryshkov wrote:
> Change struct dpu_plane allocation to use drmm_universal_plane_alloc().
> This removes the need to perform any actions on plane destruction.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

Reviewed-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>

> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 46 +++++------------------
>   1 file changed, 10 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index f114efee1b57..9d9e1cbf0dd7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1170,20 +1170,6 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
>   	}
>   }
>   
> -static void dpu_plane_destroy(struct drm_plane *plane)
> -{
> -	struct dpu_plane *pdpu = plane ? to_dpu_plane(plane) : NULL;
> -
> -	DPU_DEBUG_PLANE(pdpu, "\n");
> -
> -	if (pdpu) {
> -		/* this will destroy the states as well */
> -		drm_plane_cleanup(plane);
> -
> -		kfree(pdpu);
> -	}
> -}
> -
>   static void dpu_plane_destroy_state(struct drm_plane *plane,
>   		struct drm_plane_state *state)
>   {
> @@ -1353,7 +1339,6 @@ static bool dpu_plane_format_mod_supported(struct drm_plane *plane,
>   static const struct drm_plane_funcs dpu_plane_funcs = {
>   		.update_plane = drm_atomic_helper_update_plane,
>   		.disable_plane = drm_atomic_helper_disable_plane,
> -		.destroy = dpu_plane_destroy,
>   		.reset = dpu_plane_reset,
>   		.atomic_duplicate_state = dpu_plane_duplicate_state,
>   		.atomic_destroy_state = dpu_plane_destroy_state,
> @@ -1381,35 +1366,28 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>   	struct dpu_hw_sspp *pipe_hw;
>   	uint32_t num_formats;
>   	uint32_t supported_rotations;
> -	int ret = -EINVAL;
> -
> -	/* create and zero local structure */
> -	pdpu = kzalloc(sizeof(*pdpu), GFP_KERNEL);
> -	if (!pdpu) {
> -		DPU_ERROR("[%u]failed to allocate local plane struct\n", pipe);
> -		ret = -ENOMEM;
> -		return ERR_PTR(ret);
> -	}
> -
> -	/* cache local stuff for later */
> -	plane = &pdpu->base;
> -	pdpu->pipe = pipe;
> +	int ret;
>   
>   	/* initialize underlying h/w driver */
>   	pipe_hw = dpu_rm_get_sspp(&kms->rm, pipe);
>   	if (!pipe_hw || !pipe_hw->cap || !pipe_hw->cap->sblk) {
>   		DPU_ERROR("[%u]SSPP is invalid\n", pipe);
> -		goto clean_plane;
> +		return ERR_PTR(-EINVAL);
>   	}
>   
>   	format_list = pipe_hw->cap->sblk->format_list;
>   	num_formats = pipe_hw->cap->sblk->num_formats;
>   
> -	ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
> +	pdpu = drmm_universal_plane_alloc(dev, struct dpu_plane, base,
> +				0xff, &dpu_plane_funcs,
>   				format_list, num_formats,
>   				supported_format_modifiers, type, NULL);
> -	if (ret)
> -		goto clean_plane;
> +	if (IS_ERR(pdpu))
> +		return ERR_CAST(pdpu);
> +
> +	/* cache local stuff for later */
> +	plane = &pdpu->base;
> +	pdpu->pipe = pipe;
>   
>   	pdpu->catalog = kms->catalog;
>   
> @@ -1439,8 +1417,4 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>   	DPU_DEBUG("%s created for pipe:%u id:%u\n", plane->name,
>   					pipe, plane->base.id);
>   	return plane;
> -
> -clean_plane:
> -	kfree(pdpu);
> -	return ERR_PTR(ret);
>   }
> --
> 2.39.2