[PATCH v4 01/42] drm/msm/dpu: use CTL_SC7280_MASK for sm8450's ctl_0

[Abhinav Kumar]

On 4/4/2023 5:33 PM, Dmitry Baryshkov wrote:
> On 05/04/2023 01:12, Abhinav Kumar wrote:
>>
>>
>> On 4/4/2023 6:05 AM, Dmitry Baryshkov wrote:
>>> On sm8450 platform the CTL_0 doesn't differ from the rest of CTL blocks,
>>> so switch it to CTL_SC7280_MASK too.
>>>
>>> Some background: original commit 100d7ef6995d ("drm/msm/dpu: add support
>>> for SM8450") had all (relevant at that time) bit spelled individually.
>>> Then commit 0e91bcbb0016 ("drm/msm/dpu: Add SM8350 to hw catalog"),
>>> despite being a mismerge, correctly changed all other CTL entries to use
>>> CTL_SC7280_MASK, except CTL_0.
>>>
>>
>> I think having it spelled individually is better. If we start using 
>> one chipset's mask for another, we are again going down the same path 
>> of this becoming one confused file.
>>
>> So, even though I agree that 0e91bcbb0016 ("drm/msm/dpu: Add SM8350 to 
>> hw catalog") corrected the mask to re-use sc7280, with the individual 
>> catalog file, its better to have it separate and spelled individually.
>>
>> This change is not heading in the direction of the rest of the series.
> 
> I didn't create duplicates of all the defines. This is done well in the 
> style of patch37. I'm not going to add all per-SoC feature masks.
> 

Yes, I was actually going to comment even on patch 37.

We are again trying to generalize a CTL's caps based on DPU version, the 
same mistake which led us down this path.

So today you have CTL_DPU_0_MASK , CTL_DPU_5_MASK , CTL_DPU_7_MASK  and 
CTL_DPU_9_MASK and this builds on an assumption that you can get 5 by 
ORing ACTIVE_CFG with 0.

+#define CTL_DPU_5_MASK (CTL_DPU_0_MASK | \
+			BIT(DPU_CTL_ACTIVE_CFG))
+

This is again moving towards that problematic pattern.

Why dont we stick to CTL features individually spelling it then work 
towards generalizing as we discussed.

>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> index 6840b22a4159..83f8f83e2b29 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> @@ -975,7 +975,7 @@ static const struct dpu_ctl_cfg sm8450_ctl[] = {
>>>       {
>>>       .name = "ctl_0", .id = CTL_0,
>>>       .base = 0x15000, .len = 0x204,
>>> -    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_SPLIT_DISPLAY) 
>>> | BIT(DPU_CTL_FETCH_ACTIVE),
>>> +    .features = BIT(DPU_CTL_SPLIT_DISPLAY) | CTL_SC7280_MASK,
>>>       .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
>>>       },
>>>       {
>