[PATCH v3 18/27] drm/msm/dpu: populate SmartDMA features in hw catalog

[Abhinav Kumar]

On 2/3/2023 8:10 PM, Dmitry Baryshkov wrote:
> On 04/02/2023 04:43, Abhinav Kumar wrote:
>>
>>
>> On 2/3/2023 6:29 PM, Dmitry Baryshkov wrote:
>>> On 04/02/2023 01:35, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
>>>>> Downstream driver uses dpu->caps->smart_dma_rev to update
>>>>> sspp->cap->features with the bit corresponding to the supported 
>>>>> SmartDMA
>>>>> version. Upstream driver does not do this, resulting in SSPP subdriver
>>>>> not enbaling setup_multirect callback. Add corresponding SmartDMA SSPP
>>>>> feature bits to dpu hw catalog.
>>>>>
>>>>
>>>> While reviewing this patch, I had a first hand experience of how we 
>>>> are reusing SSPP bitmasks for so many chipsets but I think overall 
>>>> you got them right here :)
>>>>
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>>> ---
>>>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 +++++++---
>>>>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>>>>
>>>>> 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 cf053e8f081e..fc818b0273e7 100644
>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>>>> @@ -21,13 +21,16 @@
>>>>>       (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3))
>>>>>   #define VIG_SDM845_MASK \
>>>>> -    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))
>>>>> +    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | 
>>>>> BIT(DPU_SSPP_SCALER_QSEED3) |\
>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2))
>>>>>   #define VIG_SC7180_MASK \
>>>>> -    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
>>>>> +    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | 
>>>>> BIT(DPU_SSPP_SCALER_QSEED4) |\
>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2))
>>>>>   #define VIG_SM8250_MASK \
>>>>> -    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | 
>>>>> BIT(DPU_SSPP_SCALER_QSEED3LITE))
>>>>> +    (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | 
>>>>> BIT(DPU_SSPP_SCALER_QSEED3LITE) |\
>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2))
>>>>>   #define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
>>>>> @@ -42,6 +45,7 @@
>>>>>   #define DMA_SDM845_MASK \
>>>>>       (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | 
>>>>> BIT(DPU_SSPP_QOS_8LVL) |\
>>>>>       BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\
>>>>> +    BIT(DPU_SSPP_SMART_DMA_V2) |\
>>>>>       BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT))
>>>>>   #define DMA_CURSOR_SDM845_MASK \
>>>>
>>>> VIG_SDM845_MASK and DMA_SDM845_MASK are used for many other chipsets 
>>>> like 8250, 8450, 8550.
>>>>
>>>> At the moment, for visual validation of this series, I only have 
>>>> sc7180/sc7280. We are leaving the rest for CI.
>>>>
>>>> Was that an intentional approach?
>>>>
>>>> If so, we will need tested-by tags from folks having 
>>>> 8350/8450/8550/sc8280x,qcm2290?
>>>>
>>>> I am only owning the visual validation on sc7280 atm.
>>>
>>> I'm not quite sure what is your intent here. Are there any SoCs after 
>>> 845 that do not have SmartDMA 2.5? Or do you propose to enable 
>>> SmartDMA only for the chipsets that we can visually test? That sounds 
>>> strange.
>>>
>>
>> Yes I was thinking to enable smartDMA at the moment on chipsets which 
>> we can validate visually that display comes up. But I am not sure if 
>> thats entirely practical.
>>
>> But the intent was I just want to make sure basic display does come up 
>> with smartDMA enabled if we are enabling it for all chipsets.
> 
> I don't think it is practical or logical. We don't require validating 
> other changes on all possible chipsets, so what is so different with 
> this one?
> 

Thats because with smartDMA if the programming of stages goes wrong we 
could potentially just see a blank screen. Its not about other changes, 
this change in particular controls enabling a feature.

But thats just my thought. I am not going to request to ensure this or 
block this for this.

You can still have my

Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>

But think of the validations that have to be done before we merge it.