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

[Jessica Zhang]

On 8/16/2023 12:27 AM, Dmitry Baryshkov wrote:
> Hi Jessica,
> 
> On Tue, 15 Aug 2023 at 23:17, Jessica Zhang <quic_jesszhan at quicinc.com> wrote:
>>
>>
>>
>> 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 at linaro.org>
>>> ---
>>>    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.
> 
> I personally do not like putting unused includes into common headers.
> Each file should contain includes that are used by the particular file
> only. Header should include only the files required to process
> definitions in this header.

Acked. In that case, the rest of this LGTM:

Reviewed-by: Jessica Zhang <quic_jesszhan at quicinc.com>

Thanks,

Jessica Zhang

> 
>>
>> 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
>>>
> 
> 
> 
> -- 
> With best wishes
> Dmitry