* [PATCH v4] drm/amd: Warn users about potential s0ix problems
@ 2022-01-19 3:41 Mario Limonciello
2022-01-20 19:58 ` Limonciello, Mario
0 siblings, 1 reply; 4+ messages in thread
From: Mario Limonciello @ 2022-01-19 3:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Bjoren Dasse, Mario Limonciello
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v4] drm/amd: Warn users about potential s0ix problems
2022-01-19 3:41 [PATCH v4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
@ 2022-01-20 19:58 ` Limonciello, Mario
2022-01-21 10:20 ` Liang, Prike
0 siblings, 1 reply; 4+ messages in thread
From: Limonciello, Mario @ 2022-01-20 19:58 UTC (permalink / raw)
To: amd-gfx, Lazar, Lijo, Liang, Prike; +Cc: Bjoren Dasse
[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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v4] drm/amd: Warn users about potential s0ix problems
2022-01-20 19:58 ` Limonciello, Mario
@ 2022-01-21 10:20 ` Liang, Prike
2022-01-21 17:46 ` Limonciello, Mario
0 siblings, 1 reply; 4+ messages in thread
From: Liang, Prike @ 2022-01-21 10:20 UTC (permalink / raw)
To: Limonciello, Mario, amd-gfx, Lazar, Lijo; +Cc: Bjoren Dasse
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v4] drm/amd: Warn users about potential s0ix problems
2022-01-21 10:20 ` Liang, Prike
@ 2022-01-21 17:46 ` Limonciello, Mario
0 siblings, 0 replies; 4+ messages in thread
From: Limonciello, Mario @ 2022-01-21 17:46 UTC (permalink / raw)
To: Liang, Prike, amd-gfx, Lazar, Lijo; +Cc: Bjoren Dasse
[Public]
> 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.
If return false then the driver expects to go the "S3 path" which "may" work now
after the patches that reset GPU on resume landed.
If you think this is wrong and it should be returning false in this case - then I think
we need another function that checks if s3 is actually a valid target. Then in
suspend we test if we're doing s3, and we test if we're (safely) doing s0ix.
If we can't do either then the GPU stays fully powered up across the suspend
cycle and you see these warnings.
Thoughts on that?
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-21 17:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-19 3:41 [PATCH v4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
2022-01-20 19:58 ` Limonciello, Mario
2022-01-21 10:20 ` Liang, Prike
2022-01-21 17:46 ` Limonciello, Mario
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.