[Freedreno] [PATCH v4 04/19] drm/msm/dpu: drop dpu_mdss_cfg::mdp_count field

[Abhinav Kumar]

On 7/4/2023 12:01 PM, Abhinav Kumar wrote:
> 
> 
> On 7/4/2023 10:28 AM, Dmitry Baryshkov wrote:
>> On Tue, 4 Jul 2023 at 19:10, Abhinav Kumar <quic_abhinavk at quicinc.com> 
>> wrote:
>>>
>>>
>>>
>>> On 7/4/2023 4:52 AM, Dmitry Baryshkov wrote:
>>>> On Tue, 4 Jul 2023 at 13:06, Dmitry Baryshkov
>>>> <dmitry.baryshkov at linaro.org> wrote:
>>>>>
>>>>> On Tue, 4 Jul 2023 at 07:04, Abhinav Kumar 
>>>>> <quic_abhinavk at quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 7/3/2023 7:20 PM, Dmitry Baryshkov wrote:
>>>>>>> On 03/07/2023 05:01, Abhinav Kumar wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 6/19/2023 2:25 PM, Dmitry Baryshkov wrote:
>>>>>>>>> There is always a single MDP TOP block. Drop the mdp_count 
>>>>>>>>> field and
>>>>>>>>> stop declaring dpu_mdp_cfg instances as arrays.
>>>>>>>>>
>>>>>>>>> Tested-by: Marijn Suijten <marijn.suijten at somainline.org>
>>>>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>>>>>>> ---
>>>>>>>>
>>>>>>>> The change drops mdp_count and stops using the array which is 
>>>>>>>> fine and
>>>>>>>> I will support that.
>>>>>>>>
>>>>>>>> But looking at the pattern I saw while using core_revision, both
>>>>>>>> DPU_MDP_VSYNC_SEL and DPU_MDP_AUDIO_SELECT can also be dropped from
>>>>>>>> the catalog in favor of using core_revision.
>>>>>>>>
>>>>>>>> Hence for that, I request you not to stop passing dpu_mdss_cfg to
>>>>>>>> dpu_hw_mdptop_init as that has the necessary information of
>>>>>>>> core_revision.
>>>>>>>
>>>>>>> Sure, I'll restore it. Please note, however, that it might be 
>>>>>>> better to
>>>>>>> pass struct dpu_caps instead of the full struct dpu_mdss_cfg.
>>>>>>>
>>>>>>
>>>>>> Thanks for restoring.
>>>>>>
>>>>>> Can you pls explain this better? dpu_core_rev is part of 
>>>>>> dpu_mdss_cfg,
>>>>>> so dpu_caps wont be enough for this one.
>>>>>
>>>>> Oh, true. For some reason I thought that version is a part of 
>>>>> dpu_caps.
>>>>
>>>> And after additional thought. Maybe it would be better to add a
>>>> separate struct dpu_mdss_version and pass it to the hw block init
>>>> functions?
>>>>
>>>
>>> I would like to see this evolve. Today, we are assuming that only the hw
>>> block init functions are the places we would use those.
>>>
>>>   From what I recall, the DSC over DP series needed the core_revision in
>>> the timing gen code somewhere.
>>
>> I hope you are talking about the DPU driver here, not about the DP
>> driver. For the DP driver please use struct msm_dp_desc.
>>
> 
> Yes DPU driver.
> 
>>>
>>> If we see that pattern is possible once that lands, why not.
>>>
>>> Right now, I would leave it at dpu_mdss_cfg.
>>>

Changed my mind on this due to two reasons:

1) the earlier agreement was to pass dpu_mdss_cfg but passing that will 
be against the design of dpu_hw_*** functions because they have stopped 
passing the index and as dpu_intf_cfg is encapsulated within 
dpu_mdss_cfg, passing both is duplicated information.

2) I have cross-checked that even for DSC over DP, we should be able to 
pass dpu_kms->catalog->mdss_rev with this approach like I have posted now


>>>>>
>>>>>>
>>>>>>>>
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_3_0_msm8998.h   |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_5_0_sm8150.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h   |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_6_3_sm6115.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_6_4_sm6350.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h   |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_6_9_sm6375.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_7_0_sm8350.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h  |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_8_1_sm8450.h    |  7 +---
>>>>>>>>>     .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h    |  7 +---
>>>>>>>>>     .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  1 -
>>>>>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c    | 38 
>>>>>>>>> +++----------------
>>>>>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h    |  8 ++--
>>>>>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  4 +-
>>>>>>>>>     19 files changed, 41 insertions(+), 115 deletions(-)
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> -- 
>>>>> With best wishes
>>>>> Dmitry
>>>>
>>>>
>>>>
>>
>>
>>