Re: [PATCH v2 10/27] drm/msm/dpu: pass dpu_format to _dpu_hw_sspp_setup_scaler3()

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

On 31/01/2023 07:13, Abhinav Kumar wrote:
> On 12/29/2022 11:18 AM, Dmitry Baryshkov wrote:
> > There is no need to pass full dpu_hw_pipe_cfg instance to
> > _dpu_hw_sspp_setup_scaler3, pass just struct dpu_format pointer.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 9 ++++-----
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 7 +++----
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 4 ++--
> >   3 files changed, 9 insertions(+), 11 deletions(-)
> >
> > 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 f7f81ab08fa2..176cd6dc9a69 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -415,19 +415,18 @@ static void dpu_hw_sspp_setup_pe_config(struct 
> > dpu_hw_sspp *ctx,
> >   }
> >   static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_sspp *ctx,
> > -        struct dpu_hw_pipe_cfg *sspp,
> > -        void *scaler_cfg)
> > +        struct dpu_hw_scaler3_cfg *scaler3_cfg,
> > +        const struct dpu_format *format)
> >   {
> >       u32 idx;
> > -    struct dpu_hw_scaler3_cfg *scaler3_cfg = scaler_cfg;
> > -    if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx) || !sspp
> > +    if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx)
> >           || !scaler3_cfg)
>
> Do we need to check for !format ?

We don't have since the calling sequence ensures that it is not NULL, 
but let's check for it since it's a different module.

> >           return;
> >       dpu_hw_setup_scaler3(&ctx->hw, scaler3_cfg, idx,
> >               ctx->cap->sblk->scaler_blk.version,
> > -            sspp->layout.format);
> > +            format);
> >   }
> >   static u32 _dpu_hw_sspp_get_scaler3_ver(struct dpu_hw_sspp *ctx)
> > 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 f5aae563741a..c713343378aa 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > @@ -317,13 +317,12 @@ struct dpu_hw_sspp_ops {
> >       /**
> >        * setup_scaler - setup scaler
> > -     * @ctx: Pointer to pipe context
> > -     * @pipe_cfg: Pointer to pipe configuration
> >        * @scaler_cfg: Pointer to scaler configuration
>
> This doc needs to be fixed from scaler_cfg to scaler3_cfg
>
> > +     * @format: pixel format parameters
> >        */
> >       void (*setup_scaler)(struct dpu_hw_sspp *ctx,
> > -        struct dpu_hw_pipe_cfg *pipe_cfg,
> > -        void *scaler_cfg);
> > +        struct dpu_hw_scaler3_cfg *scaler3_cfg,
> > +        const struct dpu_format *format);
> >       /**
> >        * get_scaler_ver - get scaler h/w version
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 172a2c012917..cbff4dea8662 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -677,8 +677,8 @@ static void _dpu_plane_setup_scaler(struct 
> > dpu_sw_pipe *pipe,
> >       if (pipe_hw->ops.setup_scaler &&
> >               pipe->multirect_index != DPU_SSPP_RECT_1)
> >           pipe_hw->ops.setup_scaler(pipe_hw,
> > -                pipe_cfg,
> > -                &scaler3_cfg);
> > +                &scaler3_cfg,
> > +                fmt);
> >   }
> >   /**

--
With best wishes
Dmitry