Re: [PATCH] drm/amd: Allow s0ix without BIOS support

["Rafael Ávila de Espíndola" <rafael@espindo.la>]

Mario Limonciello <mario.limonciello@amd.com> writes:

> 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@espindo.la>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2364
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Rafael Ávila de Espíndola <rafael@espindo.la>

I have tested this patch on a Gigabyte B550I AORUS PRO AX with a 4350G
and can confirm that the errors are gone and the power consumption during
suspend is down to 7.7 W.

Thanks,
Rafael


> ---
>  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;
> -	}
>  
>  #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