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
>
> >
> > 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
> > >
