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

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

HI Piotr，

在 2025-06-19 19:54:12，"Piotr Zalewski" <pZ010001011111@proton.me> 写道：
>Hi Andy
>
>> 
>> 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),
>> };
>
>Following your suggestion I added vop2 video port registers as not
>volatile and it fixed the issue. I took the values from RK3588 TRM
>Part2 V1.1. See the patch below and confirm if it is correct.
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index d0f5fea15e21..c5c1910fa5ca 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -2596,6 +2596,7 @@ static int vop2_win_init(struct vop2 *vop2)
>  */
> static const struct regmap_range vop2_nonvolatile_range[] = {
> 	regmap_reg_range(0x1000, 0x23ff),
>+	regmap_reg_range(0x0C00, 0x0fff),
> };
> 

For a complete solution, the registers of all four Video Ports should all be
marked as non-volatile.
And they should be arranged in ascending order of address size, so they
should be placed before 0x1000.

Additionally, the comments here should also be updated accordingly.


#define RK3568_VP0_CTRL_BASE                    0x0C00
#define RK3568_VP1_CTRL_BASE                    0x0D00
#define RK3568_VP2_CTRL_BASE                    0x0E00
#define RK3588_VP3_CTRL_BASE                    0x0F00

> static const struct regmap_access_table vop2_volatile_table = {
>
>
>> Actually, there is another question. I still haven't figured out why
>> this problem doesn't occur when compiling rockchipdrm=y .
>
>Couldn't reason out why this only happens with drm=m yet
>unfortunately.

This should eventually lead to an answer. However, I've been quite busy recently
and haven't had time to conduct further analysis yet. I will incorporate it into my TODO list.


>
>Best regards, Piotr Zalewski
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip