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

[Melissa Wen]

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"

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.

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