[PATCH drm-misc-next] rockchip/drm: vop2: don't check color_mgmt_changed in atomic_enable
Andy Yan
andyshrk at 163.com
Wed Jun 11 12:49:38 UTC 2025
Hi Deiderik and Piotr,
At 2025-06-11 20:26:38, "Diederik de Haas" <didi.debian at cknow.org> wrote:
>Hi,
>
>On Wed Jun 11, 2025 at 2:15 PM CEST, Andy Yan wrote:
>> At 2025-06-11 18:56:31, "Diederik de Haas" <didi.debian at cknow.org> wrote:
>>>On Wed Jun 11, 2025 at 9:41 AM CEST, Andy Yan wrote:
>>>> At 2025-06-10 19:02:11, "Diederik de Haas" <didi.debian at cknow.org> wrote:
>>>>>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 at 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 at 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 at 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 at 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 at 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:
>>>>>
>>>> 7543:# CONFIG_IO_STRICT_DEVMEM is not set
>>>>
>>>> CONFIG_IO_STRICT_DEVMEM should not be set to y if you want to access an IO address from usersapce.
>>>
>>>That last one seems to be the culprit:
>>>
>>>My kernel config is based upon Debian's and in commit
>>>ef7e196951aa ("[arm*,powerpc*,s390x,x86] Enable IO_STRICT_DEVMEM")
>>>
>>>I found "can be reverted using the kernel parameter: iomem=relaxed", so
>>>I added that parameter and rebooted:
>>>
>>>```sh
>>>root at pt2-scmi:~# echo 'just (re-)booted into my PineTab2; screen is blank'
>>>just (re-)booted into my PineTab2; screen is blank
>>>root at 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 at pt2-scmi:~# cat /proc/cmdline
>>>root=UUID=42bbb627-189b-49e3-ae42-699815dc2cbb ignore_loglevel ro rootwait earlycon console=tty0 console=ttyS2,1500000n8 fw_devlink=off iomem=relaxed
>>>root at pt2-scmi:~# ./andy-io -r -4 -l 0x100 0xfe040d00
>>>fe040d00: 0000000f 00000000 00000000 00000000
>>>fe040d10: 00000010 00000000 00000000 00000000
>>>fe040d20: 00000000 00000000 00000000 00000000
>>>fe040d30: 01b70010 00500370 00100510 10001000
>>>fe040d40: 00000000 00000000 03b00010 00500370
>>>fe040d50: 05120004 00100510 00000000 00000000
>>>fe040d60: 00000000 00000000 00000000 00000000
>>>fe040d70: 00000000 00000000 00000000 00000000
>>>fe040d80: 15110903 00030911 1a150b04 00040b15
>>>fe040d90: 15110903 00030911 00000000 00000000
>>>fe040da0: 00000000 00000000 00000000 00000000
>>>fe040db0: 00000000 00000000 00000000 00000000
>>>fe040dc0: 00000000 00000000 00000000 00000000
>>>fe040dd0: 00000000 00000000 00000000 00000000
>>>fe040de0: 00000000 00000000 00000000 00000000
>>>fe040df0: 00000000 00000000 00000000 00000000
>>>root at pt2-scmi:~# ./andy-io -w -4 0xfe040d00 0x10000
>>>root at pt2-scmi:~# echo 'screen just turned on \o/'
>>>screen just turned on \o/
>>
>> Do you mean the screen is turned on(that you see the display on the screen) here ?
>
>Yep. Before the ``-w`` command the screen was blank/black and after the
>``-w`` command the (sway based) login screen was visible.
The root case for the problem is now clear。
Most of the registers in VOP need to write the cfd_done register(call vop2_cfg_done)
after you have configured the registers. Then, they will take effect only when the VSYNC event occurs(It doesn't take effect immediately after you finish writing.).
This also includes all the VP_DSP_CTRL registers.
see the code:
vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);
-->
static void vop2_vp_dsp_lut_disable(struct vop2_video_port *vp)
{
u32 dsp_ctrl = vop2_vp_read(vp, RK3568_VP_DSP_CTRL);
When we read this register, we are reading the actual effective value,
not the one(dsp_ctrl) that was just written in before (it has not yet taken effect)
So when we continue to write about this register here, we overwrite the actual
value we originally intended to put in.
dsp_ctrl &= ~RK3568_VP_DSP_CTRL__DSP_LUT_EN;
vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
}
I think the correct solution should be similar to the Windows-related registry settings.
All the registers related to Video Ports should be set as non-volatile, see:
/*
* The window registers are only updated when config done is written.
* Until that they read back the old value. As we read-modify-write
* these registers mark them as non-volatile. This makes sure we read
* the new values from the regmap register cache.
*/
static const struct regmap_range vop2_nonvolatile_range[] = {
regmap_reg_range(0x1000, 0x23ff),
};
static const struct regmap_access_table vop2_volatile_table = {
.no_ranges = vop2_nonvolatile_range,
.n_no_ranges = ARRAY_SIZE(vop2_nonvolatile_range),
};
Actually, there is another question. I still haven't figured out why
this problem doesn't occur when compiling rockchipdrm=y .
>
>>> root at pt2-scmi:~# ./andy-io -r -4 -l 0x100 0xfe040d00
>>>fe040d00: 00010000 00000000 00000000 00000000
>>>fe040d10: 00000010 00000000 00000000 00000000
>>>fe040d20: 00000000 00000000 00000000 00000000
>>>fe040d30: 01b70010 00500370 00100510 10001000
>>>fe040d40: 00000000 00000000 03b00010 00500370
>>>fe040d50: 05120004 00100510 00000000 00000000
>>>fe040d60: 00000000 00000000 00000000 00000000
>>>fe040d70: 00000000 00000000 00000000 00000000
>>>fe040d80: 15110903 00030911 1a150b04 00040b15
>>>fe040d90: 15110903 00030911 00000000 00000000
>>>fe040da0: 00000000 00000000 00000000 00000000
>>>fe040db0: 00000000 00000000 00000000 00000000
>>>fe040dc0: 00000000 00000000 00000000 00000000
>>>fe040dd0: 00000000 00000000 00000000 00000000
>>>fe040de0: 00000000 00000000 00000000 00000000
>>>fe040df0: 00000000 00000000 00000000 00000000
>>>root at pt2-scmi:~# ./andy-io -w -4 0xfe040000 0x28002
>>>root at pt2-scmi:~# echo "screen is still on ... don't see any changes on screen"
>>>screen is still on ... don't see any changes on screen
>>>root at pt2-scmi:~# ./andy-io -r -4 -l 0x100 0xfe040d00
>>>fe040d00: 00010000 00000000 00000000 00000000
>>>fe040d10: 00000010 00000000 00000000 00000000
>>>fe040d20: 00000000 00000000 00000000 00000000
>>>fe040d30: 01b70010 00500370 00100510 10001000
>>>fe040d40: 00000000 00000000 03b00010 00500370
>>>fe040d50: 05120004 00100510 00000000 00000000
>>>fe040d60: 00000000 00000000 00000000 00000000
>>>fe040d70: 00000000 00000000 00000000 00000000
>>>fe040d80: 15110903 00030911 1a150b04 00040b15
>>>fe040d90: 15110903 00030911 00000000 00000000
>>>fe040da0: 00000000 00000000 00000000 00000000
>>>fe040db0: 00000000 00000000 00000000 00000000
>>>fe040dc0: 00000000 00000000 00000000 00000000
>>>fe040dd0: 00000000 00000000 00000000 00000000
>>>fe040de0: 00000000 00000000 00000000 00000000
>>>fe040df0: 00000000 00000000 00000000 00000000
>>>```
>>>
>>>For completeness, I then closed the lid and opened it again:
>>>
>>>```sh
>>>root at pt2-scmi:~# dmesg | grep "vop2_"
>>>[ 5.128785] rockchip-drm display-subsystem: bound fe040000.vop (ops vop2_crtc_atomic_try_set_gamma.part.0 [rockchipdrm])
>>>[ 5.138031] rockchip-drm display-subsystem: bound fe0a0000.hdmi (ops vop2_crtc_atomic_try_set_gamma.part.0 [rockchipdrm])
>>>[ 5.139641] rockchip-drm display-subsystem: bound fe060000.dsi (ops vop2_crtc_atomic_try_set_gamma.part.0 [rockchipdrm])
>>>[ 5.160937] vop2_crtc_atomic_try_set_gamma gamma_lut: 0000000000000000
>>>[ 5.160950] vop2_vp_dsp_lut_disable dsp_ctrl: 0x0000000f
>>>[ 1931.879232] vop2_crtc_atomic_try_set_gamma gamma_lut: 0000000000000000
>>>[ 1931.879245] vop2_vp_dsp_lut_disable dsp_ctrl: 0x00010000
>>>```
>>>
>>>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.
>>>
>
More information about the dri-devel
mailing list