HI Piotr，
在 2025-06-19 19:54:12，"Piotr Zalewski" <pz010001011...@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
