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

[Mario Limonciello <mario.limonciello@amd.com>]

On 10/24/2025 10:21 AM, Antheas Kapenekakis wrote:
> From: Alex Deucher <alexander.deucher@amd.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@amd.com>
> Tested-by: Antheas Kapenekakis <lkml@antheas.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.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@amd.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?

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