[Public]
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Sent: Friday, February 10, 2023 14:47
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>
> 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@xxxxxxx>
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> 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
