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

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

[Piotr Zalewski]

Remove color_mgmt_changed check from vop2_crtc_atomic_try_set_gamma to
allow gamma LUT rewrite during modeset when coming out of suspend. Add
a check for color_mgmt_changed directly in vop2_crtc_atomic_flush.

This patch fixes the patch adding gamma LUT support for vop2 [1].

[1] https://lore.kernel.org/linux-rockchip/20241101185545.559090-3-pZ010001011111@proton.me/

Suggested-by: Andy Yan <andy.yan@rock-chips.com>
Signed-off-by: Piotr Zalewski <pZ010001011111@proton.me>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 0c8ec7220fbe..d259f1c6571d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1601,7 +1601,7 @@ static void vop2_crtc_atomic_try_set_gamma(struct vop2 *vop2,
 					   struct drm_crtc *crtc,
 					   struct drm_crtc_state *crtc_state)
 {
-	if (!vop2->lut_regs || !crtc_state->color_mgmt_changed)
+	if (!vop2->lut_regs)
 		return;
 
 	if (!crtc_state->gamma_lut) {
@@ -2669,7 +2669,7 @@ static void vop2_crtc_atomic_flush(struct drm_crtc *crtc,
 	struct vop2 *vop2 = vp->vop2;
 
 	/* In case of modeset, gamma lut update already happened in atomic enable */
-	if (!drm_atomic_crtc_needs_modeset(crtc_state))
+	if (!drm_atomic_crtc_needs_modeset(crtc_state) && crtc_state->color_mgmt_changed)
 		vop2_crtc_atomic_try_set_gamma_locked(vop2, vp, crtc, crtc_state);
 
 	vop2_post_config(crtc);
-- 
2.47.1

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

[Heiko Stuebner]

On Fri, 06 Dec 2024 19:26:10 +0000, Piotr Zalewski wrote:
> Remove color_mgmt_changed check from vop2_crtc_atomic_try_set_gamma to
> allow gamma LUT rewrite during modeset when coming out of suspend. Add
> a check for color_mgmt_changed directly in vop2_crtc_atomic_flush.
> 
> This patch fixes the patch adding gamma LUT support for vop2 [1].
> 
> [1] https://lore.kernel.org/linux-rockchip/20241101185545.559090-3-pZ010001011111@proton.me/
> 
> [...]

Applied, thanks!

[1/1] rockchip/drm: vop2: don't check color_mgmt_changed in atomic_enable
      commit: 9c22b6ece2e5c2308f41ba4bec27cfa158397fa7

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

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

[Diederik de Haas]

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

Hi Piotr,

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.

When I build a (new) 6.14-rc1 kernel with a revert of this commit on
top, then I did not have the above mentioned problem, confirming this
commit was the 'bad' commit.

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.

Cheers,
  Diederik

On Fri Dec 6, 2024 at 8:26 PM CET, Piotr Zalewski wrote:
> Remove color_mgmt_changed check from vop2_crtc_atomic_try_set_gamma to
> allow gamma LUT rewrite during modeset when coming out of suspend. Add
> a check for color_mgmt_changed directly in vop2_crtc_atomic_flush.
>
> This patch fixes the patch adding gamma LUT support for vop2 [1].
>
> [1] https://lore.kernel.org/linux-rockchip/20241101185545.559090-3-pZ010001011111@proton.me/
>
> Suggested-by: Andy Yan <andy.yan@rock-chips.com>
> Signed-off-by: Piotr Zalewski <pZ010001011111@proton.me>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 0c8ec7220fbe..d259f1c6571d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1601,7 +1601,7 @@ static void vop2_crtc_atomic_try_set_gamma(struct vop2 *vop2,
>  					   struct drm_crtc *crtc,
>  					   struct drm_crtc_state *crtc_state)
>  {
> -	if (!vop2->lut_regs || !crtc_state->color_mgmt_changed)
> +	if (!vop2->lut_regs)
>  		return;
>  
>  	if (!crtc_state->gamma_lut) {
> @@ -2669,7 +2669,7 @@ static void vop2_crtc_atomic_flush(struct drm_crtc *crtc,
>  	struct vop2 *vop2 = vp->vop2;
>  
>  	/* In case of modeset, gamma lut update already happened in atomic enable */
> -	if (!drm_atomic_crtc_needs_modeset(crtc_state))
> +	if (!drm_atomic_crtc_needs_modeset(crtc_state) && crtc_state->color_mgmt_changed)
>  		vop2_crtc_atomic_try_set_gamma_locked(vop2, vp, crtc, crtc_state);
>  
>  	vop2_post_config(crtc);


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Piotr Zalewski]

On Thursday, June 5th, 2025 at 10:13 PM, Diederik de Haas <didi.debian@cknow.org> wrote:

> Hi Piotr,
> 
> 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.
> 
> When I build a (new) 6.14-rc1 kernel with a revert of this commit on
> top, then I did not have the above mentioned problem, confirming this
> commit was the 'bad' commit.
> 
> 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.
> 

Hi Diederik,

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
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 
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 will wait with sending a patch because maybe Andy has something to add 
to this.

Best regards, Piotr Zalewski

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

[Diederik de Haas]

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

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.

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Andy Yan]

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

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

[Diederik de Haas]

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

Hi Andy,

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.

Cheers,
  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: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Andy Yan]

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



Hi Diederik,

At 2025-06-08 20:53:37, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>Hi Andy,
>
>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?

Thanks.

>
>Cheers,
>  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: print_lut_0609_1710.patch --]
[-- Type: application/octet-stream, Size: 1223 bytes --]

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index d0f5fea15e21..d90b345b5b53 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -898,6 +898,7 @@ static void vop2_vp_dsp_lut_disable(struct vop2_video_port *vp)
 	u32 dsp_ctrl = vop2_vp_read(vp, RK3568_VP_DSP_CTRL);
 
 	dsp_ctrl &= ~RK3568_VP_DSP_CTRL__DSP_LUT_EN;
+	printk("vop2_vp_dsp_lut_disable dsp_ctrl: 0x%08x\n", dsp_ctrl);
 	vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
 }
 
@@ -1506,7 +1507,7 @@ static void vop2_crtc_atomic_try_set_gamma(struct vop2 *vop2,
 {
 	if (!vop2->lut_regs)
 		return;
-
+	printk("%s  gamma_lut: %px\n", __func__, crtc_state->gamma_lut);
 	if (!crtc_state->gamma_lut) {
 		vop2_vp_dsp_lut_disable(vp);
 		return;
@@ -1643,7 +1644,7 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 	int ret;
 	struct drm_encoder *encoder;
 
-	drm_dbg(vop2->drm, "Update mode to %dx%d%s%d, type: %d for vp%d\n",
+	drm_info(vop2->drm, "Update mode to %dx%d%s%d, type: %d for vp%d\n",
 		hdisplay, vdisplay, mode->flags & DRM_MODE_FLAG_INTERLACE ? "i" : "p",
 		drm_mode_vrefresh(mode), vcstate->output_type, vp->id);
 

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

[Diederik de Haas]

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

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

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: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Piotr Zalewski]

Hi Diederik, sorry for late response

> Interesting that it also happened with drm=y.

I actually checked now and i don't have the issue with drm=y, sorry for 
misinforming you all, hopefully no one's time was wasted.

> 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?

Heh, I'm not that knowledgeable but I will look through it.

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

It is due to DRM modeset which can happens when CRTC (display) config changes 
"significantly". AFAIK modeset happens def. when you go out of suspend or
display timings change. I might have been fooled by serial console last time
as it does not appear to happen twice in short interval when i review the 
journal entries.

> But (IIUC) setting a bit to a value it already has causing issues,
> sounds surprising as well.

Depends on what hardware does, when you write to a register it might cause
many other things to happen and seems like for vop2 it messes something up.

I made a second patch so that the first write is not permitted but all 
subsequent are permitted (regardless of lut en state) - issue disappeared 
too. So it might be that very first write to dsp lut disable happens too 
fast (in relation to something else)?. It is not intuitive because when drm is
a module it happens usually like ~second later.

part of the log with drm=y
```
[    6.543099] rockchip-drm display-subsystem: [drm] GAMMA LUT DISABLE
```

part of the log with drm=m
```
[    7.944120] rockchip-drm display-subsystem: [drm] GAMMA LUT DISABLE
```

> > 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 :-)

cool :)
 
> > 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.

When i have time this week I will check on what version of the kernel i 
tested gamma lut when i sent the patches and test there.

> Thanks a lot!
> 

Thank _you_ for taking your time to do all the bisecting.

Best regards, Piotr Zalewski

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

[Andy Yan]

[-- 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 --]

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

[Diederik de Haas]

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

Hi Andy,

On Tue Jun 10, 2025 at 11:07 AM CEST, Andy Yan wrote:
> At 2025-06-09 20:36:41, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>>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
>>>>>
>>>>> 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.

I renamed it as ``andy-io`` and performed the test:

```sh
root@pt2-scmi:~# echo 'just (re-)booted into my PineTab2; screen is blank'
just (re-)booted into my PineTab2; screen is blank
root@pt2-scmi:~# uname -a
Linux pt2-scmi 6.14.0-rc1-00001-gfbe17d9b77b0 #18 SMP Mon Jun  9 13:17:28 CEST 2025 aarch64 GNU/Linux
root@pt2-scmi:~# ./andy-io -r -4 -l 0x100 0xfe040d00
mmap() failed: Operation not permitted
root@pt2-scmi:~# grep CONFIG_DEVMEM /boot/config-6.14.0-rc1-00001-gfbe17d9b77b0
CONFIG_DEVMEM=y
root@pt2-scmi:~# ./andy-io -w -4 0xfe040d00 0x10000
mmap() failed: Operation not permitted
root@pt2-scmi:~# ./andy-io -w -4 0xfe040000 0x28002
mmap() failed: Operation not permitted
```

I guess this is not what you expected and I don't know why it happens.

Cheers,
  Diederik

> [   73.750524] vop2_crtc_atomic_try_set_gamma  gamma_lut: 0000000000000000
> [   73.750542] vop2_vp_dsp_lut_disable dsp_ctrl: 0x00010000
>>>>>>> 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: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Diederik de Haas]

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

Hi Piotr,

On Tue Jun 10, 2025 at 12:37 AM CEST, Piotr Zalewski wrote:
> Hi Diederik, sorry for late response

No need to be sorry :-)

(late? Less then 2 days vs my ~4 months before the git bisect ...)

>> Interesting that it also happened with drm=y.
>
> I actually checked now and i don't have the issue with drm=y, sorry for 
> misinforming you all, hopefully no one's time was wasted.

Good to know, thanks :-)

>> > 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.
>
> It is due to DRM modeset which can happens when CRTC (display) config changes 
> "significantly". AFAIK modeset happens def. when you go out of suspend or
> display timings change. I might have been fooled by serial console last time
> as it does not appear to happen twice in short interval when i review the 
> journal entries.
>
>> But (IIUC) setting a bit to a value it already has causing issues,
>> sounds surprising as well.
>
> Depends on what hardware does, when you write to a register it might cause
> many other things to happen and seems like for vop2 it messes something up.

I didn't know that, thanks.

> I made a second patch so that the first write is not permitted but all 
> subsequent are permitted (regardless of lut en state) - issue disappeared 
> too. So it might be that very first write to dsp lut disable happens too 
> fast (in relation to something else)?. It is not intuitive because when drm is
> a module it happens usually like ~second later.
>
> part of the log with drm=y
> ```
> [    6.543099] rockchip-drm display-subsystem: [drm] GAMMA LUT DISABLE
> ```
>
> part of the log with drm=m
> ```
> [    7.944120] rockchip-drm display-subsystem: [drm] GAMMA LUT DISABLE
> ```

My first (uneducated) hunch was a timing issue and ``=m`` can reveal
issues which you wouldn't see with ``=y``.
Andy already found an issue "that shouldn't happen" and my latest test
also had an unexpected result. So (eventually) we'll figure it out :-)

>> Sounds like a plan. It could be that this issue surfaced an underlaying
>> issue and if so, fixing that would be even better.
>
> When i have time this week I will check on what version of the kernel i 
> tested gamma lut when i sent the patches and test there.

I think it would be beneficial if you'd do the tests that Andy asked
'me' to do too, so we can compare results.
FWIW: I have the 4GB RAM version.

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]