[Freedreno] [PATCH 13/25] drm/msm/dpu: pass dpu_format to _dpu_hw_sspp_setup_scaler3()

[Dmitry Baryshkov]

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 at 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);
>>   }
>>   /**


-- 
With best wishes
Dmitry