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

["Limonciello, Mario" <Mario.Limonciello@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 5788 bytes --]

[Public]

That is what I was hoping to convey with the warning messages.  I'll take a stab at rewording them and send out an updated revision.

From: Liang, Prike <Prike.Liang@amd.com>
Sent: Tuesday, January 18, 2022 02:28
To: Limonciello, Mario <Mario.Limonciello@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Bjoren Dasse <bjoern.daase@gmail.com>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems

If the flag ACPI_FADT_LOW_POWER_S0 not set or AMDPMC driver not build, then that seems will mess up the suspend entry and unable to enter either S3 nor S2idle properly. In this S2idle configuration issue case, how about add some message to notify end user how to configure S2idle correctly?

Thanks,
Prike
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> On Behalf Of Limonciello, Mario
Sent: Tuesday, January 18, 2022 1:26 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Bjoren Dasse <bjoern.daase@gmail.com<mailto:bjoern.daase@gmail.com>>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems


[Public]

Yes, that's part of why I want to make sure there are explicit warnings to users about using this flow.
When not enabled in ACPI then also the LPS0 device is not exported and AMD_PMC won't load or be used.

I think from amdgpu perspective it should behave relatively similar to an aborted suspend.

From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Monday, January 17, 2022 11:20
To: Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Bjoren Dasse <bjoern.daase@gmail.com<mailto:bjoern.daase@gmail.com>>
Subject: Re: [PATCH v2] drm/amd: Warn users about potential s0ix problems

Any problem with PMFW sequence in the way Linux handles s2idle when it's not enabled in ACPI?

Thanks,
Lijo
________________________________
From: Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@amd.com>>
Sent: Monday, January 17, 2022 10:45:44 PM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>; Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Cc: Bjoren Dasse <bjoern.daase@gmail.com<mailto:bjoern.daase@gmail.com>>
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@amd.com<mailto:Mario.Limonciello@amd.com>>
> Sent: Tuesday, January 11, 2022 14:00
> To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> Cc: Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@amd.com>>; Bjoren Dasse
> <bjoern.daase@gmail.com<mailto:bjoern.daase@gmail.com>>
> 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@gmail.com<mailto:bjoern.daase@gmail.com>>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1824&data=04%7C01%7CPrike.Liang%40amd.com%7C925d02acbde94f61cb2b08d9d9de74bc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637780371747154944%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=fO8dU9u204YesBNjoI99%2BAkngggcEgkQhet2mnkCRls%3D&reserved=0>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
> ---
> 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

[-- Attachment #2: Type: text/html, Size: 11992 bytes --]