[PATCH] drm/amd: Don't allow s0ix on APUs older than Raven

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello at amd.com>
> Sent: Friday, February 10, 2023 14:47
> To: amd-gfx at lists.freedesktop.org
> Cc: Limonciello, Mario <Mario.Limonciello at amd.com>; Deucher, Alexander
> <Alexander.Deucher at amd.com>
> Subject: [PATCH] drm/amd: Don't allow s0ix on APUs older than Raven
> 
> APUs before Raven didn't support s0ix.  As we just relieved some
> of the safety checks for s0ix to improve power consumption on
> APUs that support it but that are missing BIOS support a new
> blind spot was introduced that a user could "try" to run s0ix.
> 
> Plug this hole so that if users try to run s0ix on anything older
> than Raven it will just skip suspend of the GPU.
> 
> Fixes: cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support")
> Suggested-by: Alexander Deucher <Alexander.Deucher at amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 5 ++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index fa7375b97fd47..25e902077caf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1073,6 +1073,9 @@ bool amdgpu_acpi_is_s0ix_active(struct
> amdgpu_device *adev)
>  	    (pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
>  		return false;
> 
> +	if (adev->asic_type < CHIP_RAVEN)
> +		return false;
> +
>  	/*
>  	 * If ACPI_FADT_LOW_POWER_S0 is not set in the FADT, it is
> generally
>  	 * risky to do any special firmware-related preparations for entering
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6c2fe50b528e0..98f8d9873cd84 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2414,7 +2414,7 @@ static int amdgpu_pmops_suspend(struct device
> *dev)
> 
>  	if (amdgpu_acpi_is_s0ix_active(adev))
>  		adev->in_s0ix = true;
> -	else
> +	else if (amdgpu_acpi_is_s3_active(adev))
>  		adev->in_s3 = true;

Relooking at this, I wonder if it actually needs this too:


 	if (!adev->in_s0ix && !adev->in_s3)
 		return 0;


>  	return amdgpu_device_suspend(drm_dev, true);
>  }
> @@ -2436,6 +2436,9 @@ static int amdgpu_pmops_resume(struct device
> *dev)
>  	struct amdgpu_device *adev = drm_to_adev(drm_dev);
>  	int r;
> 
> +	if (!adev->in_s0ix && !adev->in_s3)
> +		return 0;
> +
>  	/* Avoids registers access if device is physically gone */
>  	if (!pci_device_is_present(adev->pdev))
>  		adev->no_hw_access = true;
> --
> 2.25.1