amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/amd: Allow s0ix without BIOS support
@ 2023-01-25 18:33 Mario Limonciello
  2023-01-25 18:35 ` Alex Deucher
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Mario Limonciello @ 2023-01-25 18:33 UTC (permalink / raw)
  To: amd-gfx; +Cc: Mario Limonciello, Rafael Ávila de Espíndola

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amd: Allow s0ix without BIOS support
  2023-01-25 18:33 [PATCH] drm/amd: Allow s0ix without BIOS support Mario Limonciello
@ 2023-01-25 18:35 ` Alex Deucher
  2023-01-26 10:46 ` Rafael Ávila de Espíndola
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2023-01-25 18:35 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: Rafael Ávila de Espíndola, amd-gfx

On Wed, Jan 25, 2023 at 1:33 PM Mario Limonciello
<mario.limonciello@amd.com> 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@espindo.la>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2364
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amd: Allow s0ix without BIOS support
  2023-01-25 18:33 [PATCH] drm/amd: Allow s0ix without BIOS support Mario Limonciello
  2023-01-25 18:35 ` Alex Deucher
@ 2023-01-26 10:46 ` Rafael Ávila de Espíndola
  2023-01-30 14:08 ` Rafael Ávila de Espíndola
  2023-02-02  3:49 ` Alex Deucher
  3 siblings, 0 replies; 7+ messages in thread
From: Rafael Ávila de Espíndola @ 2023-01-26 10:46 UTC (permalink / raw)
  To: Mario Limonciello, amd-gfx; +Cc: Mario Limonciello

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amd: Allow s0ix without BIOS support
  2023-01-25 18:33 [PATCH] drm/amd: Allow s0ix without BIOS support Mario Limonciello
  2023-01-25 18:35 ` Alex Deucher
  2023-01-26 10:46 ` Rafael Ávila de Espíndola
@ 2023-01-30 14:08 ` Rafael Ávila de Espíndola
  2023-01-30 14:36   ` Limonciello, Mario
  2023-02-02  3:49 ` Alex Deucher
  3 siblings, 1 reply; 7+ messages in thread
From: Rafael Ávila de Espíndola @ 2023-01-30 14:08 UTC (permalink / raw)
  To: Mario Limonciello, amd-gfx; +Cc: Mario Limonciello

BTW, to which git repo this gets added first? I took a look at
git://anongit.freedesktop.org/drm-tip, but it is not there.

Thanks,
Rafael

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] drm/amd: Allow s0ix without BIOS support
  2023-01-30 14:08 ` Rafael Ávila de Espíndola
@ 2023-01-30 14:36   ` Limonciello, Mario
  0 siblings, 0 replies; 7+ messages in thread
From: Limonciello, Mario @ 2023-01-30 14:36 UTC (permalink / raw)
  To: Rafael Ávila de Espíndola, amd-gfx

[Public]



> -----Original Message-----
> From: Rafael Ávila de Espíndola <rafael@espindo.la>
> Sent: Monday, January 30, 2023 08:08
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>
> Subject: Re: [PATCH] drm/amd: Allow s0ix without BIOS support
> 
> BTW, to which git repo this gets added first? I took a look at
> git://anongit.freedesktop.org/drm-tip, but it is not there.
> 
> Thanks,
> Rafael

Hi,

It will first show up in amd-staging-drm-next here:

https://gitlab.freedesktop.org/agd5f/linux/-/commits/amd-staging-drm-next

It hasn't been refreshed for things accepted this last week yet, but it should
show up this week some time.

Thanks,

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/amd: Allow s0ix without BIOS support
  2023-01-25 18:33 [PATCH] drm/amd: Allow s0ix without BIOS support Mario Limonciello
                   ` (2 preceding siblings ...)
  2023-01-30 14:08 ` Rafael Ávila de Espíndola
@ 2023-02-02  3:49 ` Alex Deucher
  2023-02-02 16:49   ` Limonciello, Mario
  3 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2023-02-02  3:49 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: Rafael Ávila de Espíndola, amd-gfx

On Wed, Jan 25, 2023 at 1:33 PM Mario Limonciello
<mario.limonciello@amd.com> 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@espindo.la>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2364
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  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.

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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] drm/amd: Allow s0ix without BIOS support
  2023-02-02  3:49 ` Alex Deucher
@ 2023-02-02 16:49   ` Limonciello, Mario
  0 siblings, 0 replies; 7+ messages in thread
From: Limonciello, Mario @ 2023-02-02 16:49 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Rafael Ávila de Espíndola, amd-gfx

[AMD Official Use Only - General]



> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Wednesday, February 1, 2023 21:49
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Rafael Ávila de Espíndola
> <rafael@espindo.la>
> Subject: Re: [PATCH] drm/amd: Allow s0ix without BIOS support
> 
> On Wed, Jan 25, 2023 at 1:33 PM Mario Limonciello
> <mario.limonciello@amd.com> 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@espindo.la>
> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2364
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >  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
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-02-02 16:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 18:33 [PATCH] drm/amd: Allow s0ix without BIOS support Mario Limonciello
2023-01-25 18:35 ` Alex Deucher
2023-01-26 10:46 ` Rafael Ávila de Espíndola
2023-01-30 14:08 ` Rafael Ávila de Espíndola
2023-01-30 14:36   ` Limonciello, Mario
2023-02-02  3:49 ` Alex Deucher
2023-02-02 16:49   ` Limonciello, Mario

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).