Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Drop unused members from HW structs

[Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>]

On 4/20/2023 7:33 AM, Dmitry Baryshkov wrote:
> On 18/04/2023 02:14, Marijn Suijten wrote:
> > Some of these members were initialized while never read, while others
> > were not even assigned any value at all.  Drop them to save some space,
> > and above all confusion when looking at these members.
> >
> > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> > Fixes: 84a33d0fd921 ("drm/msm/dpu: add dpu_hw_wb abstraction for 
> > writeback blocks")
>
> The fixes headers are slightly questionable, as unused fields are not a 
> bug. Nevertheless:

Yes, I would also not treat this as a "fix" but just cleanup.

I think if this series is slightly re-ordered to keep the second patch 
before this one, I can pick that one up for -fixes and then this one and 
the third can be left for 6.5

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> > Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 1 -
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 -
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 1 -
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 --
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 1 -
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   | 5 -----
> >   6 files changed, 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > index 84ee2efa9c66..a9c90249a6ac 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > @@ -374,7 +374,6 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf 
> > idx,
> >        */
> >       c->idx = idx;
> >       c->cap = cfg;
> > -    c->mdss = m;
> >       _setup_intf_ops(&c->ops, c->cap->features);
> >       return c;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > index 643dd10bc030..e07b2e33af3e 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> > @@ -90,7 +90,6 @@ struct dpu_hw_intf {
> >       /* intf */
> >       enum dpu_intf idx;
> >       const struct dpu_intf_cfg *cap;
> > -    const struct dpu_mdss_cfg *mdss;
> >       /* ops */
> >       struct dpu_hw_intf_ops ops;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > index cf70a9bd1034..bb3ddec5c7d7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -812,7 +812,6 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(enum dpu_sspp 
> > idx,
> >       }
> >       /* Assign ops */
> > -    hw_pipe->catalog = catalog;
> >       hw_pipe->ubwc = catalog->ubwc;
> >       hw_pipe->idx = idx;
> >       hw_pipe->cap = cfg;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > index 74b98b6b3bc3..5004a02fd61e 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > @@ -341,7 +341,6 @@ struct dpu_hw_sspp_ops {
> >    * struct dpu_hw_sspp - pipe description
> >    * @base: hardware block base structure
> >    * @hw: block hardware details
> > - * @catalog: back pointer to catalog
> >    * @ubwc: ubwc configuration data
> >    * @idx: pipe index
> >    * @cap: pointer to layer_cfg
> > @@ -350,7 +349,6 @@ struct dpu_hw_sspp_ops {
> >   struct dpu_hw_sspp {
> >       struct dpu_hw_blk base;
> >       struct dpu_hw_blk_reg_map hw;
> > -    const struct dpu_mdss_cfg *catalog;
> >       const struct dpu_ubwc_cfg *ubwc;
> >       /* Pipe */
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> > index 2d28afdf860e..db5e6040017f 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> > @@ -262,7 +262,6 @@ struct dpu_hw_wb *dpu_hw_wb_init(enum dpu_wb idx,
> >       }
> >       /* Assign ops */
> > -    c->mdp = &m->mdp[0];
> >       c->idx = idx;
> >       c->caps = cfg;
> >       _setup_wb_ops(&c->ops, c->caps->features);
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> > index 3ff5a48541e2..b91923f879f1 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> > @@ -76,15 +76,12 @@ struct dpu_hw_wb_ops {
> >   /**
> >    * struct dpu_hw_wb : WB driver object
> >    * @hw: block hardware details
> > - * @mdp: pointer to associated mdp portion of the catalog
> >    * @idx: hardware index number within type
> >    * @wb_hw_caps: hardware capabilities
> >    * @ops: function pointers
> > - * @hw_mdp: MDP top level hardware block
> >    */
> >   struct dpu_hw_wb {
> >       struct dpu_hw_blk_reg_map hw;
> > -    const struct dpu_mdp_cfg *mdp;
> >       /* wb path */
> >       int idx;
> > @@ -92,8 +89,6 @@ struct dpu_hw_wb {
> >       /* ops */
> >       struct dpu_hw_wb_ops ops;
> > -
> > -    struct dpu_hw_mdp *hw_mdp;
> >   };
> >   /**