[Freedreno] [PATCH v2 05/27] drm/msm/dpu: drop EAGAIN check from dpu_format_populate_layout

[Abhinav Kumar]

On 2/3/2023 6:16 AM, Dmitry Baryshkov wrote:
> On 28/01/2023 01:59, Abhinav Kumar wrote:
>>
>>
>> On 1/26/2023 10:05 PM, Dmitry Baryshkov wrote:
>>> On Fri, 27 Jan 2023 at 02:52, Abhinav Kumar 
>>> <quic_abhinavk at quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 12/29/2022 11:18 AM, Dmitry Baryshkov wrote:
>>>>> The pipe's layout is not cached, corresponding data structure is 
>>>>> zeroed
>>>>> out each time in the dpu_plane_sspp_atomic_update(), right before the
>>>>> call to _dpu_plane_set_scanout() -> dpu_format_populate_layout().
>>>>>
>>>>> Drop plane_addr comparison against previous layout and corresponding
>>>>> EAGAIN handling.
>>>>>
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>>
>>>> The change itself LGTM, hence
>>>>
>>>> But, shouldnt we add this EAGAIN validation or in other words fix this
>>>> rather than drop this?
>>>
>>> What for? Does it really save us anything? What's the price of
>>> re-programming the SSPP_SRC0_ADDR registers?
>>>
>> There are 4 Src registers being programmed per sspp.
>>
>> With number of layers going up this will be 4x.
>>
>> So lets say there are 5 layers and only one of their address has 
>> changed, we need to reprogram only 4 regs but now will reprogram 20.
> 
> I think this was the original intention for this change, however the 
> implementation ended up being written in a way when this condition 
> doesn't trigger at all.
> 

Yes, and thats why I wrote that we should fix this rather than drop this.

>>
>> Thats why i thought this is a good optimization.
>>
>> But still, that is a separate change so I am fine if this goes in 
>> first as its just removing dead code anyway.
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>