All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ACPI: PM: s2idle: Run both AMD and Microsoft methods if both are supported
@ 2021-09-01 14:21 Mario Limonciello
  2021-09-02 16:16 ` Rafael J. Wysocki
  0 siblings, 1 reply; 2+ messages in thread
From: Mario Limonciello @ 2021-09-01 14:21 UTC (permalink / raw)
  To: Rafael J . Wysocki, linux-acpi
  Cc: Shyam-sundar.S-k, Mario Limonciello, liaoyuan, Maxwell Beck

It was reported that on "HP ENVY x360" that power LED does not come back,
certain keys like brightness controls do not work, and the fan never
spins up, even under load on 5.14 final.

In analysis of the SSDT it's clear that the Microsoft UUID doesn't provide
functional support, but rather the AMD UUID should be supporting this
system.

Because this is a gap in the expected logic, we checked back with internal
team.  The conclusion was that on Windows AMD uPEP *does* run even when
Microsoft UUID present, but most OEM systems have adopted value of "0x3"
for supported functions and hence nothing runs.

Henceforth add support for running both Microsoft and AMD methods.  This
approach will also allow the same logic on Intel systems if desired at a
future time as well by pulling the evaluation of
`lps0_dsm_func_mask_microsoft` out of the `if` block for
`acpi_s2idle_vendor_amd`.

Cc: liaoyuan@gmail.com
Link: https://gitlab.freedesktop.org/drm/amd/uploads/9fbcd7ec3a385cc6949c9bacf45dc41b/acpi-f.20.bin
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1691
Reported-by: Maxwell Beck <max@ryt.one>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/x86/s2idle.c | 67 +++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 3a308461246a..7d1976e5dd8b 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -449,25 +449,30 @@ int acpi_s2idle_prepare_late(void)
 	if (pm_debug_messages_on)
 		lpi_check_constraints();
 
-	if (lps0_dsm_func_mask_microsoft > 0) {
+	/* screen off */
+	if (lps0_dsm_func_mask > 0)
+		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
+					ACPI_LPS0_SCREEN_OFF_AMD :
+					ACPI_LPS0_SCREEN_OFF,
+					lps0_dsm_func_mask, lps0_dsm_guid);
+
+	if (lps0_dsm_func_mask_microsoft > 0)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+
+	/* lps0 entry */
+	if (lps0_dsm_func_mask > 0)
+		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
+					ACPI_LPS0_ENTRY_AMD :
+					ACPI_LPS0_ENTRY,
+					lps0_dsm_func_mask, lps0_dsm_guid);
+	if (lps0_dsm_func_mask_microsoft > 0) {
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-	} else if (acpi_s2idle_vendor_amd()) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
-				lps0_dsm_func_mask, lps0_dsm_guid);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
-				lps0_dsm_func_mask, lps0_dsm_guid);
-	} else {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
-				lps0_dsm_func_mask, lps0_dsm_guid);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
-				lps0_dsm_func_mask, lps0_dsm_guid);
+		/* modern standby entry */
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
+				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
 	}
-
 	return 0;
 }
 
@@ -476,24 +481,30 @@ void acpi_s2idle_restore_early(void)
 	if (!lps0_device_handle || sleep_no_lps0)
 		return;
 
-	if (lps0_dsm_func_mask_microsoft > 0) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+	/* mdoern standby exit */
+	if (lps0_dsm_func_mask_microsoft > 0)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-	} else if (acpi_s2idle_vendor_amd()) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
-				lps0_dsm_func_mask, lps0_dsm_guid);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
-				lps0_dsm_func_mask, lps0_dsm_guid);
-	} else {
+
+	/* lps0 exit */
+	if (lps0_dsm_func_mask > 0)
+		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
+					ACPI_LPS0_EXIT_AMD :
+					ACPI_LPS0_EXIT,
+					lps0_dsm_func_mask, lps0_dsm_guid);
+	if (lps0_dsm_func_mask_microsoft > 0)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
-				lps0_dsm_func_mask, lps0_dsm_guid);
+				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+
+	/* screen on */
+	if (lps0_dsm_func_mask_microsoft > 0)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
-				lps0_dsm_func_mask, lps0_dsm_guid);
-	}
+				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+	if (lps0_dsm_func_mask > 0)
+		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
+					ACPI_LPS0_SCREEN_ON_AMD :
+					ACPI_LPS0_SCREEN_ON,
+					lps0_dsm_func_mask, lps0_dsm_guid);
 }
 
 static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
-- 
2.25.1


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

* Re: [PATCH] ACPI: PM: s2idle: Run both AMD and Microsoft methods if both are supported
  2021-09-01 14:21 [PATCH] ACPI: PM: s2idle: Run both AMD and Microsoft methods if both are supported Mario Limonciello
@ 2021-09-02 16:16 ` Rafael J. Wysocki
  0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2021-09-02 16:16 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Rafael J . Wysocki, ACPI Devel Maling List, Shyam Sundar S K,
	liaoyuan, Maxwell Beck

On Wed, Sep 1, 2021 at 4:21 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> It was reported that on "HP ENVY x360" that power LED does not come back,
> certain keys like brightness controls do not work, and the fan never
> spins up, even under load on 5.14 final.
>
> In analysis of the SSDT it's clear that the Microsoft UUID doesn't provide
> functional support, but rather the AMD UUID should be supporting this
> system.
>
> Because this is a gap in the expected logic, we checked back with internal
> team.  The conclusion was that on Windows AMD uPEP *does* run even when
> Microsoft UUID present, but most OEM systems have adopted value of "0x3"
> for supported functions and hence nothing runs.
>
> Henceforth add support for running both Microsoft and AMD methods.  This
> approach will also allow the same logic on Intel systems if desired at a
> future time as well by pulling the evaluation of
> `lps0_dsm_func_mask_microsoft` out of the `if` block for
> `acpi_s2idle_vendor_amd`.
>
> Cc: liaoyuan@gmail.com
> Link: https://gitlab.freedesktop.org/drm/amd/uploads/9fbcd7ec3a385cc6949c9bacf45dc41b/acpi-f.20.bin
> BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1691
> Reported-by: Maxwell Beck <max@ryt.one>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/acpi/x86/s2idle.c | 67 +++++++++++++++++++++++----------------
>  1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 3a308461246a..7d1976e5dd8b 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -449,25 +449,30 @@ int acpi_s2idle_prepare_late(void)
>         if (pm_debug_messages_on)
>                 lpi_check_constraints();
>
> -       if (lps0_dsm_func_mask_microsoft > 0) {
> +       /* screen off */
> +       if (lps0_dsm_func_mask > 0)
> +               acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +                                       ACPI_LPS0_SCREEN_OFF_AMD :
> +                                       ACPI_LPS0_SCREEN_OFF,
> +                                       lps0_dsm_func_mask, lps0_dsm_guid);
> +
> +       if (lps0_dsm_func_mask_microsoft > 0)
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
>                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
> -                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +
> +       /* lps0 entry */
> +       if (lps0_dsm_func_mask > 0)
> +               acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +                                       ACPI_LPS0_ENTRY_AMD :
> +                                       ACPI_LPS0_ENTRY,
> +                                       lps0_dsm_func_mask, lps0_dsm_guid);
> +       if (lps0_dsm_func_mask_microsoft > 0) {
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
>                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> -       } else if (acpi_s2idle_vendor_amd()) {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> -       } else {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> +               /* modern standby entry */
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
> +                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
>         }
> -
>         return 0;
>  }
>
> @@ -476,24 +481,30 @@ void acpi_s2idle_restore_early(void)
>         if (!lps0_device_handle || sleep_no_lps0)
>                 return;
>
> -       if (lps0_dsm_func_mask_microsoft > 0) {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> -                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +       /* mdoern standby exit */
> +       if (lps0_dsm_func_mask_microsoft > 0)
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
>                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> -                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> -       } else if (acpi_s2idle_vendor_amd()) {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> -       } else {
> +
> +       /* lps0 exit */
> +       if (lps0_dsm_func_mask > 0)
> +               acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +                                       ACPI_LPS0_EXIT_AMD :
> +                                       ACPI_LPS0_EXIT,
> +                                       lps0_dsm_func_mask, lps0_dsm_guid);
> +       if (lps0_dsm_func_mask_microsoft > 0)
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> +                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +
> +       /* screen on */
> +       if (lps0_dsm_func_mask_microsoft > 0)
>                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> -                               lps0_dsm_func_mask, lps0_dsm_guid);
> -       }
> +                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +       if (lps0_dsm_func_mask > 0)
> +               acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +                                       ACPI_LPS0_SCREEN_ON_AMD :
> +                                       ACPI_LPS0_SCREEN_ON,
> +                                       lps0_dsm_func_mask, lps0_dsm_guid);
>  }
>
>  static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
> --

Applied as 5.15-rc1 material with some edits in the new comments added
by this patch (including a typo fix).

Thanks!

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

end of thread, other threads:[~2021-09-02 16:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 14:21 [PATCH] ACPI: PM: s2idle: Run both AMD and Microsoft methods if both are supported Mario Limonciello
2021-09-02 16:16 ` Rafael J. Wysocki

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.