linux-kernel - Re: [PATCH v1 3/3] drm/amdgpu: only send the SMU RLC notification on S3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <27439123-98aa-4096-a4e4-3c8eecb3aaca@amd.com>
Date: Fri, 24 Oct 2025 11:20:32 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Antheas Kapenekakis <lkml@...heas.dev>,
 Alex Deucher <alexander.deucher@....com>,
 Shyam Sundar S K <Shyam-sundar.S-k@....com>, Perry Yuan <perry.yuan@....com>
Cc: 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 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.

>>       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)) {
>