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

[Matthew Schwartz]

On 7/17/25 5:18 PM, Melissa Wen wrote:
> 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.

Sure, going to use my Legion Go S w/ Z2 Go for this (DCN 3.1.2). All
of these are from amd-staging-drm-next with AMD_PRIVATE_COLOR=y.

DTN log from Hades II with MangoHud enabled:
https://gist.github.com/matte-schwartz/0c290ffe99bdb4f0d7369ee8817d1145

DTN log from Hades II with MangoHud disabled:
https://gist.github.com/matte-schwartz/3e2867e192ec9742ec545d2c5dd3096a

drm_info from Hades II with MangoHud enabled:
https://gist.github.com/matte-schwartz/456684fc60f7e84173ee2f42de4b774b

drm_info from Hades II with MangoHud disabled:
https://gist.github.com/matte-schwartz/6b635fa584d033234b435667f5d2c153

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

It's uncharted territory for but I do enjoy tinkering, I will take a look
at your branch and see if I can start poking at the missing pieces.

Matt

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