| Message ID | 20220209172520.3719906-14-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/dpu: wide planes support | expand |
On 2/9/2022 9:25 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@linaro.org> > --- > 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 d8120168f974..7194c14f87bc 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_pipe *ctx, > } > > static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe *ctx, > - struct dpu_hw_pipe_cfg *sspp, > - void *scaler_cfg) This change does two things: 1) pass fmt and stop passing dpu_hw_pipe_cfg 2) change the scaler_cfg from void to struct dpu_hw_scaler3_cfg So it seems like we had this void casting to allow different versions of the scaler to be passed and based on catalog bits the appropriate structs can be used (scaler2/scaler3) In the current DPU we have only scaler3. For that reason this is fine. I do not know what versions of scaler we will support in DPU. Do you think we can retain the void casting in this change and just change passing the format? > + 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) > 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_pipe *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 74171fb4e585..eee8501ea80d 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h > @@ -334,13 +334,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 > + * @format: pixel format parameters > */ > void (*setup_scaler)(struct dpu_hw_pipe *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 3ce7dcc285e2..e9421fa2fb2e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -646,8 +646,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); > } > > /**
On 10/05/2022 01:30, Abhinav Kumar wrote: > > > On 2/9/2022 9:25 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@linaro.org> >> --- >> 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 d8120168f974..7194c14f87bc 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_pipe *ctx, >> } >> static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe *ctx, >> - struct dpu_hw_pipe_cfg *sspp, >> - void *scaler_cfg) > > This change does two things: > > 1) pass fmt and stop passing dpu_hw_pipe_cfg > 2) change the scaler_cfg from void to struct dpu_hw_scaler3_cfg > > So it seems like we had this void casting to allow different versions of > the scaler to be passed and based on catalog bits the appropriate > structs can be used (scaler2/scaler3) > > In the current DPU we have only scaler3. For that reason this is fine. > > I do not know what versions of scaler we will support in DPU. > > Do you think we can retain the void casting in this change and just > change passing the format? Generally passing a void pointer for the known structure is a bad practice. You loose type safety for no added benefit. I can take a look on the QSEED2 requirements. >> + 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) >> 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_pipe *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 74171fb4e585..eee8501ea80d 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h >> @@ -334,13 +334,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 >> + * @format: pixel format parameters >> */ >> void (*setup_scaler)(struct dpu_hw_pipe *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 3ce7dcc285e2..e9421fa2fb2e 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> @@ -646,8 +646,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); >> } >> /**
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 d8120168f974..7194c14f87bc 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_pipe *ctx, } static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe *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) 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_pipe *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 74171fb4e585..eee8501ea80d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -334,13 +334,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 + * @format: pixel format parameters */ void (*setup_scaler)(struct dpu_hw_pipe *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 3ce7dcc285e2..e9421fa2fb2e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -646,8 +646,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); } /**
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@linaro.org> --- 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(-)