Re: [PATCH v3 7/9] drm/msm/dpu: drop _dpu_crtc_check_and_setup_lm_bounds from atomic_begin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/18/2024 8:26 PM, Dmitry Baryshkov wrote:
On Wed, 19 Jun 2024 at 01:56, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
On 6/13/2024 4:20 PM, Abhinav Kumar wrote:
On 6/13/2024 3:36 PM, Dmitry Baryshkov wrote:
The dpu_crtc_atomic_check() already calls the function
_dpu_crtc_check_and_setup_lm_bounds().  There is no need to call it
again from dpu_crtc_atomic_begin().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 --
   1 file changed, 2 deletions(-)

Reviewed-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>

This change is causing a small regression on sc7280 chromebook.

I have tested and concluded that this is causing the chrome boot
animation to disappear.

I have tested a couple of times and without this change it works fine.

If this change was meant as an optimization, can we drop this one and
investigate later why this is causing one? I have not spent time
investigating why it happened. Rest of the series works well and I dont
see any dependency as such. Let me know if that works for you. Otherwise
I will have to spend a little more time on this patch and why chrome
compositor does not like this for the animation screen.
Oh, my. Thank you for the test!
I think I know what's happening. The cstate->num_mixers gets set only
in dpu_encoder_virt_atomic_mode_set(). So during
dpu_crtc_atomic_check() we don't have cstate->num_mixers is stale (and
if it is 0, the check is skipped).

Yes, it is a possible explanation for this.

I guess I'll have to move cstate->mixers[] and cstate->num_mixers
assignment to the dpu_encoder_virt_atomic_check(). And maybe we should
start thinking about my old idea of moving resource allocation to the
CRTC code.

I wonder if thats the right fix though because it seems correct to me 
that num_mixers is set in mode_set after the atomic_check phase.
Perhaps the right way would be to breakup check_and_set() to check() and 
set() respectively and call only the check() part in atomic_check() and 
keep the set() part in atomic_begin to avoid duplication.
Either way, I think we should re-visit this as this patch by itself is 
an optimization and I am totally fine if you want to merge the rest of 
this series just dropping this one for now.


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux