Any problem with PMFW sequence in the way Linux handles s2idle when it's not enabled in ACPI?
Lijo
From: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
Sent: Monday, January 17, 2022 10:45:44 PM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Cc: Bjoren Dasse <bjoern.daase@xxxxxxxxx>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems
Sent: Monday, January 17, 2022 10:45:44 PM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Cc: Bjoren Dasse <bjoern.daase@xxxxxxxxx>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems
[Public]
This has been sitting a week or so.
Bump on review for this patch.
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Sent: Tuesday, January 11, 2022 14:00
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Bjoren Dasse
> <bjoern.daase@xxxxxxxxx>
> Subject: [PATCH v2] 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@xxxxxxxxx>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> v1->v2:
> * Only show messages in s2idle cases
> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 16 ++++++++++------
> 1 file changed, 10 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..1295de6d6c30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1040,11 +1040,15 @@ 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) ||
> + pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
> + 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
> - return false;
> + return true;
> }
> --
> 2.25.1
This has been sitting a week or so.
Bump on review for this patch.
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Sent: Tuesday, January 11, 2022 14:00
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Bjoren Dasse
> <bjoern.daase@xxxxxxxxx>
> Subject: [PATCH v2] 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@xxxxxxxxx>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> v1->v2:
> * Only show messages in s2idle cases
> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 16 ++++++++++------
> 1 file changed, 10 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..1295de6d6c30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1040,11 +1040,15 @@ 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) ||
> + pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
> + 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
> - return false;
> + return true;
> }
> --
> 2.25.1