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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/16/2023 12:27 AM, Dmitry Baryshkov wrote:
Hi Jessica,

On Tue, 15 Aug 2023 at 23:17, Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> 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@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.

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@xxxxxxxxxxx>

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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux