On 5/30/2023 4:34 PM, Alex Deucher wrote:
On Tue, May 30, 2023 at 2:19 PM Limonciello, Mario
<Mario.Limonciello@xxxxxxx> wrote:
[AMD Official Use Only - General]
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Tuesday, May 30, 2023 1:16 PM
To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Rafael Ávila de Espíndola
<rafael@xxxxxxxxxx>
Subject: Re: [PATCH 1/2] drm/amd: Disallow s0ix without BIOS support again
On Tue, May 30, 2023 at 1:53 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.
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@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>
---
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?
Yeah; it could. I'm not sure how we can reliably detect this. I thought about looking for
the 'ht' flag, but that probably wouldn't work for this case.
Are there AMD Zen CPUs or APUs that are non-SMT? Could gate the sched_smt_active()
check to only run when it's an AMD x86 Zen SoC.
Some of the more budget conscient Athlon parts don't have SMT IIRC.
Alex
In that case, I think the best solution is to just revert cf488dcd0ab7.
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
