[PATCH] Revert "drm/amd/display: limit clear_update_flags to dcn32 and above"

[Melissa Wen]

On 17-07-2025 19:17, Matthew Schwartz wrote:
> On Thu, Jul 17, 2025 at 12:43 PM Melissa Wen <mwen at igalia.com> wrote:
>>
>>
>>
>> On 17/07/2025 16:28, Matthew Schwartz wrote:
>> > On Thu, Jul 17, 2025 at 12:18 PM Melissa Wen <mwen at igalia.com> wrote:
>> >> On 17-07-2025 17:57, Matthew Schwartz wrote:
>> >>>> On Jul 17, 2025, at 9:58 AM, Limonciello, Mario <Mario.Limonciello at amd.com> wrote:
>> >>>>
>> >>>> On 7/17/25 11:55 AM, Melissa Wen wrote:
>> >>>>> On 17-07-2025 13:39, Limonciello, Mario wrote:
>> >>>>>> On 7/17/25 9:36 AM, Melissa Wen wrote:
>> >>>>>>> This reverts commit e1bd5e0bb4ca0d633ad698abd3658f8265009b81.
>> >>>>>>>
>> >>>>>>> The commit causes a regression in Steam Deck (DCN 3.01), reintroducing a
>> >>>>>>> functional issue reported in [1] that was fixed by calling the
>> >>>>>>> clear_update_flags() from commit 7671f62c10f2a.
>> >>>>>>>
>> >>>>>>> On Steam Deck, we use multiple hw plane color caps and up to two overlay
>> >>>>>>> planes with dynamic pipe split policy. I.e. with 1 primary + 1 overlay,
>> >>>>>>> the driver split planes into two (with 4 pipes), but with 1 primary + 2
>> >>>>>>> overlays, we don't have enough pipe for splitting. Glitches appear in
>> >>>>>>> this pipe-split transition of 1-2 overlay planes, if the driver doesn't
>> >>>>>>> clear update flags.
>> >>>>>>>
>> >>>>>>> Besides that, the issue the commit e1bd5e0bb4ca tries to address [2]
>> >>>>>>> isn't functional.
>> >>>>>>>
>> >>>>>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3441 [1]
>> >>>>>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/4129 [2]
>> >>>>>>> Signed-off-by: Melissa Wen <mwen at igalia.com>
>> >>>>>> Can you please double check the use case that prompted
>> >>>>>> e1bd5e0bb4ca0d633ad698abd3658f8265009b81 in the first place?  IE With
>> >>>>>> this revert you proposed do you see a traceback on unplug of external
>> >>>>>> display?
>> >>>>> Hi Mario,
>> >>>>>
>> >>>>> I don't see either traceback or REG_WAIT timeout when unplugging an
>> >>>>> external HDMI-connected display.
>> >>>>> I tried with the Deck Dock and USB-C to HDMI adapter. Not sure if there
>> >>>>> is a very specific scenario that triggers this that I didn't cover.
>> >>>> No; it's quite a general problem and easy to reproduce.  This makes me
>> >>>> wonder if there should be an exception carved out for Steam Deck instead.
>> >>> I just tested DCN 3.1.2 (Z2 Go) and DCN 3.1.4 (Legion Go/ROG Ally) and
>> >>> I was not able to reproduce the issue so a Steam Deck carveout might make
>> >>> sense here.
>> >> What issue are you trying to reproduce?
>> >> Glitches on Hades II + MangoHud, or traceback/REG_WAIT timeout?
>> > Sorry, should've specified! I was testing for glitches with Hades II + MangoHud.
>> >
>> >> And with what kernel version?
>> > amd-staging-drm-next built from b78bd800, tested on top of SteamOS Main
>> I suspect you don't see the issue because your kernel doesn't expose AMD
>> driver-specific color properties (?)
>> For this, you should compile the kernel with KCFLAGS="-DAMD_PRIVATE_COLOR"
> 
> [missed reply-all, resending this]
> 
> Ah right, thanks for pointing this out. I've gone back and recompiled
> the kernel with
> AMD_PRIVATE_COLOR enabled. However, I'm still unable to reproduce the issue on
> those other platforms. I can see the number of overlay planes change
> in my dtn log,
> as well as the pipeline changes with amdgpu_dm_visual_confirm.
> 
> In the original issue, I was unable to reproduce the Hades II and
> MangoHud glitches
> on my Z1E device at the time so this lack of reproduction on DCN 3.1.2
> and DCN 3.1.4
> might be expected.

[Oh, I realized I only sent my reply to you. Sending the same msg to
everyone now]

Sounds good.

Do you mind to share your DTN log with and without MangoHud enabled and
the drm_info output with MangoHud?
I want to compare those logs with Steam Deck.

> 
>>
>> If no plane color props, you are also not seeing three planes on AMD DTN
>> log and then no transitions in the pipe split.
>>
>> My main concern is that we will just hide the problem with an exception,
>> but it can come back to bite us when the KMS plane color API is merged
>> and gamescope updated to it.
> 
> That makes sense. I've been meaning to circle back to the gamescope
> implementation of that now that Simon is unable to continue since it would
> be good to get some testing started there.

I started working on it here:
https://github.com/melissawen/gamescope/tree/kms_color
But not my area of expertise, so it will be slow and needs some work on
libliftoff.
Let me know if you want to contribute anyway, maybe you can go faster
than me.

Melissa

> 
> Matt
> 
>>
>> Melissa
>>
>> >
>> >> Melissa
>> >>
>> >>> Matt
>> >>>
>> >>>>> The only message on dmesg from this action is:
>> >>>>> amdgpu 0000:04:00.0: amdgpu: pp_od_clk_voltage is not accessible if
>> >>>>> power_dpm_force_performance_level is not in manual mode!
>> >>>>>
>> >>>>> Moreover, if I move to the Desktop mode (KDE), there is no message.
>> >>>>>
>> >>>>> Melissa
>> >>>>>
>> >>>>>>> ---
>> >>>>>>>    drivers/gpu/drm/amd/display/dc/core/dc.c | 5 +++--
>> >>>>>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>> >>>>>>>
>> >>>>>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
>> >>>>>>> index c31f7f8e409f..7c48f72e5917 100644
>> >>>>>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
>> >>>>>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
>> >>>>>>> @@ -5443,7 +5443,8 @@ bool dc_update_planes_and_stream(struct dc *dc,
>> >>>>>>>     else
>> >>>>>>>     ret = update_planes_and_stream_v2(dc, srf_updates,
>> >>>>>>>     surface_count, stream, stream_update);
>> >>>>>>> - if (ret && dc->ctx->dce_version >= DCN_VERSION_3_2)
>> >>>>>>> +
>> >>>>>>> + if (ret)
>> >>>>>>>     clear_update_flags(srf_updates, surface_count, stream);
>> >>>>>>>
>> >>>>>>>     return ret;
>> >>>>>>> @@ -5474,7 +5475,7 @@ void dc_commit_updates_for_stream(struct dc *dc,
>> >>>>>>>     ret = update_planes_and_stream_v1(dc, srf_updates, surface_count, stream,
>> >>>>>>>     stream_update, state);
>> >>>>>>>
>> >>>>>>> - if (ret && dc->ctx->dce_version >= DCN_VERSION_3_2)
>> >>>>>>> + if (ret)
>> >>>>>>>     clear_update_flags(srf_updates, surface_count, stream);
>> >>>>>>>    }
>>