Message-Id: <DAI0A1Y753FJ.B0NMT8L5VPEH@cknow.org>
Date: Mon, 09 Jun 2025 14:36:41 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Andy Yan" <andyshrk@....com>
Cc: "Piotr Zalewski" <pZ010001011111@...ton.me>, <hjc@...k-chips.com>,
 <heiko@...ech.de>, <andy.yan@...k-chips.com>,
 <maarten.lankhorst@...ux.intel.com>, <mripard@...nel.org>,
 <tzimmermann@...e.de>, <airlied@...il.com>, <simona@...ll.ch>, "Dang Huynh"
 <danct12@...eup.net>, <dri-devel@...ts.freedesktop.org>,
 <linux-arm-kernel@...ts.infradead.org>,
 <linux-rockchip@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH drm-misc-next] rockchip/drm: vop2: don't check
 color_mgmt_changed in atomic_enable

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@...ow.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@...ow.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@...ow.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@...ow.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

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.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists