RE: [PATCH v4] drm/amd: Warn users about potential s0ix problems

["Liang, Prike" <Prike.Liang@amd.com>]

The S2idle suspend/resume process seems also depends on the CONFIG_SUSPEND. Moreover, why this check function still return true even when BIOS/AMDPMC not configured correctly? You know we still looking into some S0ix abort issue and system will run into such problem when mark those misconfigured case also as s0ix.   

Thanks,
Prike
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, January 21, 2022 3:59 AM
> To: amd-gfx@lists.freedesktop.org; Lazar, Lijo <Lijo.Lazar@amd.com>; Liang,
> Prike <Prike.Liang@amd.com>
> Cc: Bjoren Dasse <bjoern.daase@gmail.com>
> Subject: RE: [PATCH v4] drm/amd: Warn users about potential s0ix problems
> 
> [Public]
> 
> Add back on Lijo and Prike, my mistake they got dropped from CC.
> 
> > -----Original Message-----
> > From: Limonciello, Mario <Mario.Limonciello@amd.com>
> > Sent: Tuesday, January 18, 2022 21:41
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Limonciello, Mario <Mario.Limonciello@amd.com>; Bjoren Dasse
> > <bjoern.daase@gmail.com>
> > Subject: [PATCH v4] drm/amd: Warn users about potential s0ix problems
> >
> > 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`.  Before commit 6dc8265f9803
> ("drm/amdgpu:
> > always reset the asic in suspend (v2)"), the GPU would crash.  Now
> > when configured this way, the system should resume but will use more
> power.
> >
> > As such, adjust the `amdpu_acpi_is_s0ix function` to warn users about
> > potential power consumption issues during their first attempt at
> > suspending.
> >
> > Reported-by: Bjoren Dasse <bjoern.daase@gmail.com>
> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> > v3->v4:
> >  * Add back in CONFIG_SUSPEND check
> > v2->v3:
> >  * Better direct users how to recover in the bad cases
> > v1->v2:
> >  * Only show messages in s2idle cases
> >
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 21 +++++++++++++++-----
> -
> >  1 file changed, 15 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..2531da6cbec3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > @@ -1040,11 +1040,20 @@ 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;
> > -	}
> > -#endif
> > +#if IS_ENABLED(CONFIG_SUSPEND)
> > +	if (!(adev->flags & AMD_IS_APU) ||
> > +	    pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
> > +		return false;
> > +#else
> >  	return false;
> > +#endif
> > +	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");
> > +#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
> > +	return true;
> >  }
> > --
> > 2.25.1