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

From: "Diederik de Haas" <didi.debian@cknow.org>
To: "Piotr Zalewski" <pZ010001011111@proton.me>
Cc: <hjc@rock-chips.com>, <heiko@sntech.de>,
	<andy.yan@rock-chips.com>, <maarten.lankhorst@linux.intel.com>,
	<mripard@kernel.org>, <tzimmermann@suse.de>, <airlied@gmail.com>,
	<simona@ffwll.ch>, "Dang Huynh" <danct12@riseup.net>,
	<dri-devel@lists.freedesktop.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-rockchip@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH drm-misc-next] rockchip/drm: vop2: don't check color_mgmt_changed in atomic_enable
Date: Tue, 10 Jun 2025 13:27:13 +0200	[thread overview]
Message-ID: <DAITFEPGXB6Z.38FAE9O9NSSLX@cknow.org> (raw)
In-Reply-To: <4z0y_F1ZT_xfO0-DwU-D5NwL3t3wHuGmivOB3QFjDsn0hzpz_hOeU63pRVBIgSU1j89jmYPq-TJHPqMCj3fAygwwX6IaPoVsqKqVi2jxmCc=@proton.me>

[-- Attachment #1.1: Type: text/plain, Size: 2743 bytes --]

Hi Piotr,

On Tue Jun 10, 2025 at 12:37 AM CEST, Piotr Zalewski wrote:
> Hi Diederik, sorry for late response

No need to be sorry :-)

(late? Less then 2 days vs my ~4 months before the git bisect ...)

>> 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.

Good to know, thanks :-)

>> > 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 didn't know that, thanks.

> 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
> ```

My first (uneducated) hunch was a timing issue and ``=m`` can reveal
issues which you wouldn't see with ``=y``.
Andy already found an issue "that shouldn't happen" and my latest test
also had an unexpected result. So (eventually) we'll figure it out :-)

>> 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.

I think it would be beneficial if you'd do the tests that Andy asked
'me' to do too, so we can compare results.
FWIW: I have the 4GB RAM version.

Cheers,
  Diederik

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip