[PATCH 1/2] drm/amd: Disallow s0ix without BIOS support again

[Alex Deucher]

On Tue, May 30, 2023 at 1:53 PM Mario Limonciello
<mario.limonciello at amd.com> wrote:
>
> commit cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support") showed
> improvements to power consumption over suspend when s0ix wasn't enabled in
> BIOS and the system didn't support S3.
>
> This patch however was misguided because the reason the system didn't
> support S3 was because SMT was disabled in OEM BIOS setup.
> This prevented the BIOS from allowing S3.
>
> Also allowing GPUs to use the s2idle path actually causes problems if
> they're invoked on systems that may not support s2idle in the platform
> firmware. `systemd` has a tendency to try to use `s2idle` if `deep` fails
> for any reason, which could lead to unexpected flows.
>
> To make this the behavior discoverable and expected, revert commit
> cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support") and offer
> a message if SMT appears to be disabled.
>
> Cc: Rafael Ávila de Espíndola <rafael at espindo.la>
> Link: https://github.com/torvalds/linux/blob/v6.1/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c#L1060
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2599
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 3a6b2e2089f6..a3523d03d769 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -28,6 +28,7 @@
>  #include <linux/xarray.h>
>  #include <linux/power_supply.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/sched/smt.h>
>  #include <linux/suspend.h>
>  #include <acpi/video.h>
>  #include <acpi/actbl.h>
> @@ -1473,6 +1474,13 @@ void amdgpu_acpi_release(void)
>   */
>  bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
>  {
> +#ifdef CONFIG_X86
> +       if (!sched_smt_active()) {
> +               dev_warn_once(adev->dev,
> +                             "SMT is disabled by the BIOS.\n"
> +                             "To use suspend-to-ram enable SMT in BIOS setup.\n");
> +       }
> +#endif

Will this generate a spurious warning on platforms that are natively non-SMT?

Alex

>         return !(adev->flags & AMD_IS_APU) ||
>                 (pm_suspend_target_state == PM_SUSPEND_MEM);
>  }
> @@ -1499,16 +1507,20 @@ bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
>          * S0ix even though the system is suspending to idle, so return false
>          * in that case.
>          */
> -       if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
> +       if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
>                 dev_warn_once(adev->dev,
>                               "Power consumption will be higher as BIOS has not been configured for suspend-to-idle.\n"
>                               "To use suspend-to-idle change the sleep mode in BIOS setup.\n");
> +               return false;
> +       }
>
>  #if !IS_ENABLED(CONFIG_AMD_PMC)
>         dev_warn_once(adev->dev,
>                       "Power consumption will be higher as the kernel has not been compiled with CONFIG_AMD_PMC.\n");
> -#endif /* CONFIG_AMD_PMC */
> +       return false;
> +#else
>         return true;
> +#endif /* CONFIG_AMD_PMC */
>  }
>
>  #endif /* CONFIG_SUSPEND */
> --
> 2.34.1
>