On Tue, May 30, 2023 at 6:34 PM Mario Limonciello
<mario.limonciello@xxxxxxx> 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.
>
> The original commit also fixed a problem during resume from suspend to idle
> without hardware support, but this is no longer necessary with commit
> ca4751866397 ("drm/amd: Don't allow s0ix on APUs older than Raven")
>
> Revert commit cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support")
> to make it match the expected behavior again.
>
> Cc: Rafael Ávila de Espíndola <rafael@xxxxxxxxxx>
> 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@xxxxxxx>
Patch 1 is:
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
Patch 2 seems a bit much, but I could be convinced if you think it
will actually help more than a warn would. Users already assume warn
is a kernel crash. I'm not sure the average user makes a distinction
between warn and err.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index aeeec211861c..e1b01554e323 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1092,16 +1092,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
>
