[PATCH v5 29/32] drm/msm/dpu: enable SmartDMA for the rest of the platforms

[Dmitry Baryshkov]

On 14/03/2023 18:35, Abhinav Kumar wrote:
> 
> 
> On 3/14/2023 3:58 AM, Dmitry Baryshkov wrote:
>> On 14/03/2023 07:08, Abhinav Kumar wrote:
>>>
>>>
>>> On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote:
>>>> Enable SmartDMA features for the rest of the platforms where it is
>>>> supposed to work.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>
>>> I am so glad we split this. Without visual validation check we 
>>> wouldnt have caught the issues and would have ended up with a blank 
>>> half screen on our sc7280 CBs on DP.
>>
>> yes, the r_pipe was indeed mea culpa, when I didn't fully validate the 
>> results of a rebase. However this doesn't seem to be an 
>> sc7280-specific question. Are there any platform-specific issues with 
>> SmartDMA/multirect/planes revealed during testing? In other words, 
>> were there any issues which warrant this split?
>>
>>>
>>> For sc7280, I validated the foll cases:
>>
>> Thanks a lot!
>>
>>>
>>> 1) Basic Chrome UI comes up
>>> 2) Validated some browser use-cases and played some youtube videos
>>> 3) Validated multiple plug-in/plug-out cases with DP connected
>>> 4) IGT test cases with DP connected:
>>>      - kms_atomic
>>>      - kms_atomic_transition
>>>      - kms_pipe_basic_crc
>>>
>>> Some notes:
>>>
>>> 1) I wanted to test 4k with this too but my monitor only supports 
>>> 4k at 60 which is not possible with 24bpp with 2 lanes and due to the 
>>> HBR3 black screen issue I could not test that so far. But since I 
>>> have that issue even with 1080P and without these changes, it should 
>>> have no effect due to this series.
>>>
>>> 2) I wanted to test some YUV overlay cases but I still cannot find 
>>> one on chrome. Its always using RGB. Will check with others.
>>
>> Testing YUV would definitely be a must, especially for the SSPP 
>> allocation. Can you possibly check whether contemporaty Xv 
>> implementation employs YUV planes? You can try that using `mpv -xo xv` 
>> or `mplayer -vo xv`
>>
> 
> Let me get some feedback from CrOS folks here instead of just trial and 
> error to find out.
> 
>>>
>>> With these two noted, this change and this series has my
>>>
>>> Tested-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>>>
> 
> There is no guarantee we will end up finding the YUV case, you could 
> have retained the Tested-by for the efforts which were already put in 
> instead of just blatantly removing it.

I can add it back for v6 (or move sc7280 to the previous patch if you'd 
prefer that). Also I think the Tested-by should have included the 
#sc7280 comment. If you don't mind I'll add it.

>>> Only for sc7280 device.
>>>
>>> I still cannot give my R-b on this change till others validate 
>>> visually and ensure things arent broken for them.
>>>
>>> If we don't get enough validation and if only sc7280 remains in this 
>>> change, please re-post it with only that and I will give my R-b too.
>>

-- 
With best wishes
Dmitry