[AMD Official Use Only - General] > -----Original Message----- > From: Alex Deucher <alexdeucher@xxxxxxxxx> > Sent: Wednesday, February 1, 2023 21:49 > To: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Rafael Ávila de Espíndola > <rafael@xxxxxxxxxx> > Subject: Re: [PATCH] drm/amd: Allow s0ix without BIOS support > > On Wed, Jan 25, 2023 at 1:33 PM Mario Limonciello > <mario.limonciello@xxxxxxx> wrote: > > > > We guard the suspend entry code from running unless we have proper > > BIOS support for either S3 mode or s0ix mode. > > > > If a user's system doesn't support either of these modes the kernel > > still does offer s2idle in `/sys/power/mem_sleep` so there is an > > expectation from users that it works even if the power consumption > > remains very high. > > > > Rafael Ávila de Espíndola reports that a system of his has a > > non-functional graphics stack after resuming. That system doesn't > > support S3 and the FADT doesn't indicate support for low power idle. > > > > Through some experimentation it was concluded that even without the > > hardware s0i3 support provided by the amd_pmc driver the power > > consumption over suspend is decreased by running amdgpu's s0ix > > suspend routine. > > > > The numbers over suspend showed: > > * No patch: 9.2W > > * Skip amdgpu suspend entirely: 10.5W > > * Run amdgpu s0ix routine: 7.7W > > > > As this does improve the power, remove some of the guard rails in > > `amdgpu_acpi.c` for only running s0ix suspend routines in the right > > circumstances. > > > > However if this turns out to cause regressions for anyone, we should > > revert this change and instead opt for skipping suspend/resume routines > > entirely or try to fix the underlying behavior that makes graphics fail > > after resume without underlying platform support. > > > > Reported-by: Rafael Ávila de Espíndola <rafael@xxxxxxxxxx> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2364 > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > index 57b5e11446c65..fa7375b97fd47 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > > @@ -1079,20 +1079,16 @@ 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; > > Thinking about this more, I think we may need to check the asic type > here. Pre-Raven APUs didn't support S0ix at all so this may break > them if they have any checks that use amdgpu_acpi_is_s0ix_active() in > their code paths. For them what should we be doing when they try to do s2idle though? S3 path? Or nothing? > > Alex > > > > - } > > > > #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"); > > - return false; > > -#else > > - return true; > > #endif /* CONFIG_AMD_PMC */ > > + return true; > > } > > > > #endif /* CONFIG_SUSPEND */ > > -- > > 2.25.1 > >
