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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4z0y_F1ZT_xfO0-DwU-D5NwL3t3wHuGmivOB3QFjDsn0hzpz_hOeU63pRVBIgSU1j89jmYPq-TJHPqMCj3fAygwwX6IaPoVsqKqVi2jxmCc=@proton.me>
Date: Mon, 09 Jun 2025 22:37:13 +0000
From: Piotr Zalewski <pZ010001011111@...ton.me>
To: Diederik de Haas <didi.debian@...ow.org>
Cc: hjc@...k-chips.com, heiko@...ech.de, andy.yan@...k-chips.com, maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch, Dang Huynh <danct12@...eup.net>, dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH drm-misc-next] rockchip/drm: vop2: don't check color_mgmt_changed in atomic_enable

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