Message-ID: <d6f72b8f-1961-406a-aed8-5f4570ce98d1@amd.com>
Date: Fri, 24 Oct 2025 11:52:19 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Antheas Kapenekakis <lkml@...heas.dev>
Cc: Alex Deucher <alexander.deucher@....com>,
 Shyam Sundar S K <Shyam-sundar.S-k@....com>, Perry Yuan
 <perry.yuan@....com>, amd-gfx@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v1 3/3] drm/amdgpu: only send the SMU RLC notification on
 S3



On 10/24/2025 11:45 AM, Antheas Kapenekakis wrote:
> On Fri, 24 Oct 2025 at 18:24, Antheas Kapenekakis <lkml@...heas.dev> wrote:
>>
>> On Fri, 24 Oct 2025 at 18:20, Mario Limonciello
>> <mario.limonciello@....com> wrote:
>>>
>>>
>>>
>>> On 10/24/2025 10:54 AM, Mario Limonciello wrote:
>>>>
>>>>
>>>> On 10/24/2025 10:21 AM, Antheas Kapenekakis wrote:
>>>>> From: Alex Deucher <alexander.deucher@....com>
>>>>>
>>>>> For S0ix, the RLC is not powered down. Rework the Van Gogh logic to
>>>>> skip powering it down and skip part of post-init.
>>>>>
>>>>> Fixes: 8c4e9105b2a8 ("drm/amdgpu: optimize RLC powerdown notification
>>>>> on Vangogh")
>>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4659
>>>>> Signed-off-by: Alex Deucher <alexander.deucher@....com>
>>>>> Tested-by: Antheas Kapenekakis <lkml@...heas.dev>
>>>>> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c       | 8 +++++---
>>>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c        | 6 ++++++
>>>>>    drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 3 +++
>>>>>    3 files changed, 14 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/
>>>>> drm/amd/amdgpu/amdgpu_device.c
>>>>> index 3d032c4e2dce..220b12d59795 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> @@ -5243,9 +5243,11 @@ int amdgpu_device_suspend(struct drm_device
>>>>> *dev, bool notify_clients)
>>>>>        if (amdgpu_sriov_vf(adev))
>>>>>            amdgpu_virt_release_full_gpu(adev, false);
>>>>> -    r = amdgpu_dpm_notify_rlc_state(adev, false);
>>>>> -    if (r)
>>>>> -        return r;
>>>>> +    if (!adev->in_s0ix) {
>>>>> +        r = amdgpu_dpm_notify_rlc_state(adev, false);
>>>>> +        if (r)
>>>>> +            return r;
>>>>> +    }
>>>>
>>>> Just FYI this is going to clash with my unwind failed suspend series [1].
>>>>
>>>> This is fine, just whichever "lands" first the other will need to rework
>>>> a little bit and I wanted to mention it.
>>>>
>>>> Link: https://lore.kernel.org/amd-gfx/20251023165243.317153-2-
>>>> mario.limonciello@....com/ [1]
>>>>
>>>> This does have me wondering though why amdgpu_dpm_notify_rlc_state() is
>>>> even in amdgpu_device_suspend()?  This is only used on Van Gogh.
>>>> Should we be pushing this deeper into amdgpu_device_ip_suspend_phase2()?
>>>>
>>>> Or should we maybe overhaul this to move the RLC notification into
>>>> a .set_mp1_state callback instead so it's more similar to all the other
>>>> ASICs?
>>>>
>>>
>>> My proposal as such is here:
>>>
>>> https://lore.kernel.org/amd-gfx/20251024161216.345691-1-mario.limonciello@amd.com/
>>>
>>> It would need some testing though to make sure it didn't break Steam
>>> Deck or Steam Deck OLED.
>>
>> I will give it a quick go on my OLED.
> 
> Horribly broken. Did not enter S3 and when waking up fan maxed out and
> it bootlooped. Journalctl stops on suspend entry. It works on the Xbox
> ally though
> 
> My series works on both

OK.

Can you try Alex's idea instead?  Just comment out the RLC notification 
code in amdgpu_device_suspend().

It's supposed to already be called in smu_disable_dpms() anyway.

> 
> 
>>
>>>>>        return 0;
>>>>>    }
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/
>>>>> drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> index fb8086859857..244b8c364d45 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> @@ -2040,6 +2040,12 @@ static int smu_disable_dpms(struct smu_context
>>>>> *smu)
>>>>>            smu->is_apu && (amdgpu_in_reset(adev) || adev->in_s0ix))
>>>>>            return 0;
>>>>> +    /* vangogh s0ix */
>>>>> +    if ((amdgpu_ip_version(adev, MP1_HWIP, 0) == IP_VERSION(11, 5, 0) ||
>>>>> +         amdgpu_ip_version(adev, MP1_HWIP, 0) == IP_VERSION(11, 5,
>>>>> 2)) &&
>>>>> +        adev->in_s0ix)
>>>>> +        return 0;
>>>>> +
>>>>
>>>> How about for GPU reset, does PMFW handle this too?
>>>>
>>>>>        /*
>>>>>         * For gpu reset, runpm and hibernation through BACO,
>>>>>         * BACO feature has to be kept enabled.
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c b/
>>>>> drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> index 2c9869feba61..0708d0f0938b 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> @@ -2217,6 +2217,9 @@ static int vangogh_post_smu_init(struct
>>>>> smu_context *smu)
>>>>>        uint32_t total_cu = adev->gfx.config.max_cu_per_sh *
>>>>>            adev->gfx.config.max_sh_per_se * adev-
>>>>>> gfx.config.max_shader_engines;
>>>>> +    if (adev->in_s0ix)
>>>>> +        return 0;
>>>>> +
>>>>>        /* allow message will be sent after enable message on Vangogh*/
>>>>>        if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_GFXCLK_BIT) &&
>>>>>                (adev->pg_flags & AMD_PG_SUPPORT_GFX_PG)) {
>>>>
>>>
>>>
> 

Powered by blists - more mailing lists