[PATCH] drm/amd: apply s0ix codepath to more cases

[Limonciello, Mario]

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Tuesday, January 11, 2022 12:54
> To: Limonciello, Mario <Mario.Limonciello at amd.com>; amd-
> gfx at lists.freedesktop.org
> Cc: Bjoren Dasse <bjoern.daase at gmail.com>
> Subject: Re: [PATCH] drm/amd: apply s0ix codepath to more cases
> 
> 
> 
> On 1/11/2022 10:59 PM, Mario Limonciello wrote:
> > On some OEM setups users can configure the BIOS for S3 or S2idle.
> > When configured to S3 users can still choose 's2idle' in the kernel
> > by using `/sys/power/mem_sleep`.  When configured this way, the system
> > will use more power but should still work for suspend and resume.
> >
> > As such, remove the checks that put the GPU driver into the S3 codepaths
> > when the user has selected s2idle.
> >
> > Reported-by: Bjoren Dasse <bjoern.daase at gmail.com>
> > BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
> > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=215387
> > Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 15 +++++++++------
> >   1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > index 4811b0faafd9..a46bac8f79af 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > @@ -1040,11 +1040,14 @@ void amdgpu_acpi_detect(void)
> >    */
> >   bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
> >   {
> > -#if IS_ENABLED(CONFIG_AMD_PMC) && IS_ENABLED(CONFIG_SUSPEND)
> > -	if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
> > -		if (adev->flags & AMD_IS_APU)
> > -			return pm_suspend_target_state ==
> PM_SUSPEND_TO_IDLE;
> > -	}
> > +	if (!(adev->flags & AMD_IS_APU))
> > +		return false;
> > +	if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
> > +		dev_warn_once(adev->dev,
> > +			      "BIOS is not configured for suspend-to-idle, power
> consumption will be higher\n");
> > +#if !IS_ENABLED(CONFIG_AMD_PMC)
> > +	dev_warn_once(adev->dev,
> > +		      "amd-pmc is not enabled in the kernel, power consumption
> will be higher\n");
> >   #endif
> 
> It makes sense to emit these warnings only when pm_suspend_target_state
> == PM_SUSPEND_TO_IDLE.
> 

That's true - thanks.  The other open question ends up being whether this even
will work since the SMU never powers down the GPU without the rest of the
firmware support.

The possibilities I see are:
1) Just try it (what is happening now).
2) Reset GPU after coming back for safety.
3) Don't power down GPU without firmware support.
4) Fail the suspend without firmware support.

> Thanks,
> Lijo
> 
> > -	return false;
> > +	return pm_suspend_target_state == PM_SUSPEND_TO_IDLE;
> >   }
> >