Re:Re: [PATCH drm-misc-next] rockchip/drm: vop2: don't check color_mgmt_changed in atomic_enable

["Andy Yan" <andyshrk@163.com>]

[-- Attachment #1: Type: text/plain, Size: 6607 bytes --]



Hi Diederik，
At 2025-06-09 20:36:41, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>Hi Andy,
>
>On Mon Jun 9, 2025 at 11:15 AM CEST, Andy Yan wrote:
>> At 2025-06-08 20:53:37, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>>>On Sun Jun 8, 2025 at 2:10 PM CEST, Andy Yan wrote:
>>>> At 2025-06-08 19:08:50, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>>>>>On Sat Jun 7, 2025 at 5:32 PM CEST, Piotr Zalewski wrote:
>>>>>> On Thursday, June 5th, 2025 at 10:13 PM, Diederik de Haas <didi.debian@cknow.org> wrote:
>>>>>>> Since kernel 6.14-rc1 I have the problem that visual output is no longer
>>>>>>> shown on my PineTab2 and a `git bisect` pointed to this patch/commit
>>>>>>> as the culprit. What is important to note is that `CONFIG_DRM=m` seems
>>>>>>> to be required as the problem does not occur with `CONFIG_DRM=y`.
>>>>>>> 
>>>>>>> Near the end of my bisect session, something interesting occurred.
>>>>>>> I was booted into a 'bad' kernel (ie no visual output) and when I
>>>>>>> started to build my final kernel, I closed the lid of the PineTab2 which
>>>>>>> made it go into suspend. When my final kernel was built, I opened the
>>>>>>> lid again, which made it resume, to transfer my final kernel to it.
>>>>>>> And much to my surprise, I then did have visual output.
>>>>>>> When I read the (below) commit message of the 'offending' commit, it may
>>>>>>> not be such a surprise after all.
>>>>>>> 
>>>>>>> I did try it on a Quartz64-B (also rk3566) and it did not have any issue
>>>>>>> (output via HDMI).
>>>>>>> I don't know what the cause for this issue is, hopefully you do.
>>>>>>
>>>>>> I tested and confirmed that this happens with drm=m but also in my case 
>>>>>> it happened when drm=y. After some testing I found out that at boot modeset
>>>>>
>>>>>Interesting that it also happened with drm=y.
>>>>>As you're more knowledgeable then I am with this, maybe look through
>>>>>https://lists.sr.ht/~diederik/pine64-discuss/<D9AM2OOLREO0.2JMAI42J06TW0@cknow.org>
>>>>>
>>>>>to see if you may spot something relevant?
>>>>>
>>>>>> happened twice and at short interval and since this patch allows for gamma 
>>>>>> LUT update regardless of color_mgmt_changed state this makes DSP CTRL GAMMA 
>>>>>> LUT EN bit to be unset twice too. It seems that VOP does not like it. I 
>>>>>
>>>>>Happy to see you found the cause :-)
>>>>>Do you happen to know why it was unset twice? That sounds suboptimal.
>>>>>But (IIUC) setting a bit to a value it already has causing issues,
>>>>>sounds surprising as well.
>>>>
>>>> I have conducted tests on both rk3566-box-demo (with drm set to y) and rk3568-lubancat-2 (with drm set to m), 
>>>> but I was unable to reproduce this issue. Could you two please share your kernel defconfig and the corresponding kernel startup logs? 
>>>> Additionally, both of my two boards tested with HDMI output. What kind of display interface does your board use for output?
>>>
>>>I wasn't able to reproduce this issue on my PINE64 Quartz-B (rk3566) 
>>>with HDMI output either, but the problem is present on a PineTab2 [1]
>>>(also rk3566) which uses a MIPI DSI connection to the display panel.
>>>
>>>Kernel config:
>>>https://paste.sr.ht/~diederik/aa747ed170aa01cc759fbe1ffd9cebe8c887b10b
>>>
>>>dmesg kernel 6.14-rc1:
>>>https://paste.sr.ht/~diederik/733fbf8bb7f6aee8b68cf5a652157d445462c24a
>>>
>>>dmesg kernel 6.14-rc1 with Piotr's patch:
>>>https://paste.sr.ht/~diederik/db1af672cfb611acbfbdf35adb6f170e5c38febc
>>>
>>>Both dmesg outputs contain a suspend-resume cycle.
>>>I'm using a USB Wi-Fi adapter for the wireless connection.
>>>
>>>[1] https://wiki.pine64.org/wiki/PineTab2
>>>
>>>Happy to provide more info and/or do some tests.
>>
>> Can you apply the patch in the attachment, reproduce this issue(without Piotr's patch), 
>> and then provide me with a copy of the kernel log?
>
>Same test as above, but added ``dmesg | grep "vop2_"`` at the end as well
>
>dmesg kernel 6.14-rc1 with Andy's print_lut_0609_1710 patch:
>https://paste.sr.ht/~diederik/ac356ee8b0f7e772c7310293d99d95644f59a4ee


root@pt2-scmi:~# dmesg | grep "vop2_"
[    4.996281] rockchip-drm display-subsystem: bound fe040000.vop (ops vop2_crtc_atomic_try_set_gamma.part.0 [rockchipdrm])
[    5.005207] rockchip-drm display-subsystem: bound fe0a0000.hdmi (ops vop2_crtc_atomic_try_set_gamma.part.0 [rockchipdrm])
[    5.006798] rockchip-drm display-subsystem: bound fe060000.dsi (ops vop2_crtc_atomic_try_set_gamma.part.0 [rockchipdrm])
[    5.021204] vop2_crtc_atomic_try_set_gamma  gamma_lut: 0000000000000000
[    5.021219] vop2_vp_dsp_lut_disable dsp_ctrl: 0x0000000f

It seems that dsp_ctrl: 0x0000000f , this value is not what we expected.

The expected is 0x00010000.

Could you please do an experiment for me? When there is no display on your screen, 
execute the following command and see if the screen can resume displaying:

./data/io  -w -4 0xfe040d00 0x10000; io -w -4 0xfe040000 0x28002 

I have placed the io tool in the attachment.

You can use command like bellow to read back to confirm if what you write has taken effect:
io -r -4 -l 0x100  0xfe040d00 

you may need to make CONFIG_DEVMEM=y so that you can write the register by io command.



[   73.750524] vop2_crtc_atomic_try_set_gamma  gamma_lut: 0000000000000000
[   73.750542] vop2_vp_dsp_lut_disable dsp_ctrl: 0x00010000
>
>Thanks!
>
>Diederik
>
>>>>>> patched vop2_vp_dsp_lut_disable function so that dsp_ctrl is set only if 
>>>>>> GAMMA LUT EN bit is set. I checked that this also does not break the gamma 
>>>>>> lut functionality with emphasis on out-of/into suspend behavior.
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>>>> index d0f5fea15e21..7ddf311b38c6 100644
>>>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>>>> @@ -897,6 +897,9 @@ static void vop2_vp_dsp_lut_disable(struct vop2_video_port *vp)
>>>>>>  {
>>>>>>  	u32 dsp_ctrl = vop2_vp_read(vp, RK3568_VP_DSP_CTRL);
>>>>>>  
>>>>>> +	if ((dsp_ctrl & RK3568_VP_DSP_CTRL__DSP_LUT_EN) == 0)
>>>>>> +		return;
>>>>>> +
>>>>>>  	dsp_ctrl &= ~RK3568_VP_DSP_CTRL__DSP_LUT_EN;
>>>>>>  	vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
>>>>>>  }
>>>>>
>>>>>I built a kernel with 6.14-rc1 + this patch and can confirm the screen
>>>>>has output again :-)
>>>>>
>>>>>> I will wait with sending a patch because maybe Andy has something to add 
>>>>>> to this.
>>>>>
>>>>>Sounds like a plan. It could be that this issue surfaced an underlaying
>>>>>issue and if so, fixing that would be even better.
>

[-- Attachment #2: io --]
[-- Type: application/octet-stream, Size: 593608 bytes --]

[-- Attachment #3: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip