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

Limonciello, Mario Mario.Limonciello at amd.com
Thu Jun 1 15:33:35 UTC 2023


[AMD Official Use Only - General]

> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Wednesday, May 31, 2023 10:22 PM
> To: Limonciello, Mario <Mario.Limonciello at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; Rafael Ávila de Espíndola
> <rafael at espindo.la>
> Subject: Re: [PATCH v2 1/2] drm/amd: Disallow s0ix without BIOS support
> again
>
> On Wed, May 31, 2023 at 9:26 AM Alex Deucher <alexdeucher at gmail.com>
> wrote:
> >
> > On Tue, May 30, 2023 at 6:34 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.
> > >
> > > 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 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>
> >
> > Patch 1 is:
> > Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> > 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.
> >
>
> You'll need to revert d2a197a45daacd ("drm/amd: Only run s3 or s0ix if
> system is configured properly") as well, otherwise, we'll break
> runtime pm.
>

Can you elaborate more on your thought process?  d2a197a45daacd was added in 5.18
and cf488dcd0ab7 was added in 6.3.  I can't imagine runtime PM is broken the whole time
on dGPUs.

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


More information about the amd-gfx mailing list