Re: [PATCH v3 05/13] drm/msm/dpu: use devres-managed allocation for MDP TOP

[Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>]

On 7/29/2023 6:19 PM, Dmitry Baryshkov wrote:
> Use devm_kzalloc to create MDP TOP structure. This allows us to remove
> corresponding kfree and drop dpu_hw_mdp_destroy() function.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 17 +++++++----------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h |  8 +++++---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c    |  5 ++---
>   3 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> index cff48763ce25..481b373d9ccb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> @@ -2,6 +2,8 @@
>   /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>    */
>   
> +#include <drm/drm_managed.h>

Hi Dmitry,

Is it possible to put this #include in a common header? Since it seems 
that this is a common change for a lot of patches in this series.

Thanks,

Jessica Zhang

> +
>   #include "dpu_hwio.h"
>   #include "dpu_hw_catalog.h"
>   #include "dpu_hw_top.h"
> @@ -268,16 +270,17 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
>   		ops->intf_audio_select = dpu_hw_intf_audio_select;
>   }
>   
> -struct dpu_hw_mdp *dpu_hw_mdptop_init(const struct dpu_mdp_cfg *cfg,
> -		void __iomem *addr,
> -		const struct dpu_mdss_cfg *m)
> +struct dpu_hw_mdp *dpu_hw_mdptop_init(struct drm_device *dev,
> +				      const struct dpu_mdp_cfg *cfg,
> +				      void __iomem *addr,
> +				      const struct dpu_mdss_cfg *m)
>   {
>   	struct dpu_hw_mdp *mdp;
>   
>   	if (!addr)
>   		return ERR_PTR(-EINVAL);
>   
> -	mdp = kzalloc(sizeof(*mdp), GFP_KERNEL);
> +	mdp = drmm_kzalloc(dev, sizeof(*mdp), GFP_KERNEL);
>   	if (!mdp)
>   		return ERR_PTR(-ENOMEM);
>   
> @@ -292,9 +295,3 @@ struct dpu_hw_mdp *dpu_hw_mdptop_init(const struct dpu_mdp_cfg *cfg,
>   
>   	return mdp;
>   }
> -
> -void dpu_hw_mdp_destroy(struct dpu_hw_mdp *mdp)
> -{
> -	kfree(mdp);
> -}
> -
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> index 8b1463d2b2f0..6f3dc98087df 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
> @@ -145,13 +145,15 @@ struct dpu_hw_mdp {
>   
>   /**
>    * dpu_hw_mdptop_init - initializes the top driver for the passed config
> + * @dev:  Corresponding device for devres management
>    * @cfg:  MDP TOP configuration from catalog
>    * @addr: Mapped register io address of MDP
>    * @m:    Pointer to mdss catalog data
>    */
> -struct dpu_hw_mdp *dpu_hw_mdptop_init(const struct dpu_mdp_cfg *cfg,
> -		void __iomem *addr,
> -		const struct dpu_mdss_cfg *m);
> +struct dpu_hw_mdp *dpu_hw_mdptop_init(struct drm_device *dev,
> +				      const struct dpu_mdp_cfg *cfg,
> +				      void __iomem *addr,
> +				      const struct dpu_mdss_cfg *m);
>   
>   void dpu_hw_mdp_destroy(struct dpu_hw_mdp *mdp);
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 6e0643ea4868..d4f4cb402663 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -820,8 +820,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
>   
>   	dpu_kms->catalog = NULL;
>   
> -	if (dpu_kms->hw_mdp)
> -		dpu_hw_mdp_destroy(dpu_kms->hw_mdp);
>   	dpu_kms->hw_mdp = NULL;
>   }
>   
> @@ -1091,7 +1089,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>   
>   	dpu_kms->rm_init = true;
>   
> -	dpu_kms->hw_mdp = dpu_hw_mdptop_init(dpu_kms->catalog->mdp,
> +	dpu_kms->hw_mdp = dpu_hw_mdptop_init(dev,
> +					     dpu_kms->catalog->mdp,
>   					     dpu_kms->mmio,
>   					     dpu_kms->catalog);
>   	if (IS_ERR(dpu_kms->hw_mdp)) {
> --
> 2.39.2