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

[Piotr Zalewski]

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),
 };
 
 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.

Best regards, Piotr Zalewski