| Message ID | 20230730011920.354575-6-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/dpu: use managed memory allocations | expand |
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@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. 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 >
Hi Jessica, On Tue, 15 Aug 2023 at 23:17, Jessica Zhang <quic_jesszhan@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@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. > > 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 > >
On 8/16/2023 12:27 AM, Dmitry Baryshkov wrote: > Hi Jessica, > > On Tue, 15 Aug 2023 at 23:17, Jessica Zhang <quic_jesszhan@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@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@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
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> + #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)) {
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@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(-)