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

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

Hello，

At 2025-06-08 19:08:50, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>Hi Piotr,
>
>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?

>
>> 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.
>
>> Best regards, Piotr Zalewski
>
>Thanks a lot!
>
>Cheers,
>  Diederik
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip