2022-09-09 18:07:29

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

It was reported that an ASUS Rembrandt laptop has problems with seemingly
unrelated ACPI events after resuming from s2idle. Debugging the issue
proved it's because ASUS has ASL that is only called when using the
Microsoft GUID, not the AMD GUID.

This is a bug from ASUS firmware but this series reworks the s2idle
handling for AMD to allow accounting for this in a quirk.

Additionally as this is a problem that may pop up again on other models
add a module parameter that can be used to try the Microsoft GUID on a
given system.

This module parameter intentionally applies to both Intel and AMD systems
as the same problem could potentially exist on Intel systems that support
both the Intel GUID or the Microsoft GUID.

Mario Limonciello (4):
acpi/x86: s2idle: Move _HID handling for AMD systems into structures
acpi/x86: s2idle: If a new AMD _HID is missing assume Rembrandt
acpi/x86: s2idle: Add module parameter to prefer Microsoft GUID
acpi/x86: s2idle: Add a quirk for ASUS TUF Gaming A17 FA707RE

drivers/acpi/x86/s2idle.c | 97 +++++++++++++++++++++++++++++++--------
1 file changed, 77 insertions(+), 20 deletions(-)

--
2.34.1


2022-09-09 18:07:52

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH 1/4] acpi/x86: s2idle: Move _HID handling for AMD systems into structures

Right now the information about which cases to use for what are in a
comment, but this is error prone. Instead move all information into
a dedicated structure.

Tested-by: [email protected]
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/acpi/x86/s2idle.c | 63 ++++++++++++++++++++++++++++-----------
1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index f9ac12b778e6..a7757551f750 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -363,6 +363,39 @@ static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *d
return ret;
}

+struct amd_lps0_hid_device_data {
+ const unsigned int rev_id;
+ const bool check_off_by_one;
+ const bool prefer_amd_guid;
+};
+
+static const struct amd_lps0_hid_device_data amd_picasso = {
+ .rev_id = 0,
+ .check_off_by_one = true,
+ .prefer_amd_guid = false,
+};
+
+static const struct amd_lps0_hid_device_data amd_cezanne = {
+ .rev_id = 0,
+ .check_off_by_one = false,
+ .prefer_amd_guid = false,
+};
+
+static const struct amd_lps0_hid_device_data amd_rembrandt = {
+ .rev_id = 2,
+ .check_off_by_one = false,
+ .prefer_amd_guid = true,
+};
+
+static const struct acpi_device_id amd_hid_ids[] = {
+ {"AMD0004", (kernel_ulong_t)&amd_picasso, },
+ {"AMD0005", (kernel_ulong_t)&amd_picasso, },
+ {"AMDI0005", (kernel_ulong_t)&amd_picasso, },
+ {"AMDI0006", (kernel_ulong_t)&amd_cezanne, },
+ {"AMDI0007", (kernel_ulong_t)&amd_rembrandt, },
+ {}
+};
+
static int lps0_device_attach(struct acpi_device *adev,
const struct acpi_device_id *not_used)
{
@@ -370,31 +403,27 @@ static int lps0_device_attach(struct acpi_device *adev,
return 0;

if (acpi_s2idle_vendor_amd()) {
- /* AMD0004, AMD0005, AMDI0005:
- * - Should use rev_id 0x0
- * - function mask > 0x3: Should use AMD method, but has off by one bug
- * - function mask = 0x3: Should use Microsoft method
- * AMDI0006:
- * - should use rev_id 0x0
- * - function mask = 0x3: Should use Microsoft method
- * AMDI0007:
- * - Should use rev_id 0x2
- * - Should only use AMD method
- */
- const char *hid = acpi_device_hid(adev);
- rev_id = strcmp(hid, "AMDI0007") ? 0 : 2;
+ static const struct acpi_device_id *dev_id;
+ const struct amd_lps0_hid_device_data *data;
+
+ for (dev_id = &amd_hid_ids[0]; dev_id->id[0]; dev_id++)
+ if (acpi_dev_hid_uid_match(adev, dev_id->id, NULL))
+ break;
+ if (dev_id != NULL)
+ data = (const struct amd_lps0_hid_device_data *) dev_id->driver_data;
+ else
+ return 0;
+ rev_id = data->rev_id;
lps0_dsm_func_mask = validate_dsm(adev->handle,
ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
lps0_dsm_func_mask_microsoft = validate_dsm(adev->handle,
ACPI_LPS0_DSM_UUID_MICROSOFT, 0,
&lps0_dsm_guid_microsoft);
- if (lps0_dsm_func_mask > 0x3 && (!strcmp(hid, "AMD0004") ||
- !strcmp(hid, "AMD0005") ||
- !strcmp(hid, "AMDI0005"))) {
+ if (lps0_dsm_func_mask > 0x3 && data->check_off_by_one) {
lps0_dsm_func_mask = (lps0_dsm_func_mask << 1) | 0x1;
acpi_handle_debug(adev->handle, "_DSM UUID %s: Adjusted function mask: 0x%x\n",
ACPI_LPS0_DSM_UUID_AMD, lps0_dsm_func_mask);
- } else if (lps0_dsm_func_mask_microsoft > 0 &&
+ } else if (lps0_dsm_func_mask_microsoft > 0 && data->prefer_amd_guid &&
(!strcmp(hid, "AMDI0007") ||
!strcmp(hid, "AMDI0008"))) {
lps0_dsm_func_mask_microsoft = -EINVAL;
--
2.34.1

2022-09-09 18:14:58

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH 2/4] acpi/x86: s2idle: If a new AMD _HID is missing assume Rembrandt

A mistake was made that only AMDI0007 was set to rev of "2", but
it should have been also set for AMDI008. If an ID is missing from
the _HID table, then assume it matches Rembrandt behavior.

This implicitly means that if any other behavior changes happen
in the future missing IDs must be added to that table.

Tested-by: [email protected]
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/acpi/x86/s2idle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index a7757551f750..a8256e5a0e8a 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -412,7 +412,7 @@ static int lps0_device_attach(struct acpi_device *adev,
if (dev_id != NULL)
data = (const struct amd_lps0_hid_device_data *) dev_id->driver_data;
else
- return 0;
+ data = &amd_rembrandt;
rev_id = data->rev_id;
lps0_dsm_func_mask = validate_dsm(adev->handle,
ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
--
2.34.1

2022-09-09 18:19:41

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH 4/4] acpi/x86: s2idle: Add a quirk for ASUS TUF Gaming A17 FA707RE

ASUS TUF Gaming A17 FA707RE has problems with ACPI events after
s2idle resume. It's from a missing call to an ASL method in AMD
the s2idle calling path. Force the system to use the Microsoft
Modern Standby calling path instead.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216101
Reported-and-tested-by: [email protected]
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/acpi/x86/s2idle.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index a9b0f2b54a1c..6a2c94fdbeae 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -17,6 +17,7 @@

#include <linux/acpi.h>
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/suspend.h>

#include "../sleep.h"
@@ -400,6 +401,28 @@ static const struct acpi_device_id amd_hid_ids[] = {
{}
};

+static int lps0_prefer_microsoft(const struct dmi_system_id *id)
+{
+ pr_debug("Preferring Microsoft GUID.\n");
+ prefer_microsoft_guid = true;
+ return 0;
+}
+
+static const struct dmi_system_id s2idle_dmi_table[] __initconst = {
+ {
+ /*
+ * ASUS TUF Gaming A17 FA707RE
+ * https://bugzilla.kernel.org/show_bug.cgi?id=216101
+ */
+ .callback = lps0_prefer_microsoft,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming A17"),
+ },
+ },
+ {}
+};
+
static int lps0_device_attach(struct acpi_device *adev,
const struct acpi_device_id *not_used)
{
@@ -568,6 +591,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {

void acpi_s2idle_setup(void)
{
+ dmi_check_system(s2idle_dmi_table);
acpi_scan_add_handler(&lps0_handler);
s2idle_set_ops(&acpi_s2idle_ops_lps0);
}
--
2.34.1

2022-09-09 18:37:17

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH 3/4] acpi/x86: s2idle: Add module parameter to prefer Microsoft GUID

OEMs have made some mistakes in the past for the AMD GUID support
and not populated the method properly. To add an escape hatch for
this problem introduce a module parameter that can force using
the Microsoft GUID.

This is intentionally introduced to both Intel and AMD codepaths
to allow using the parameter as a debugging tactic on either.

Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/acpi/x86/s2idle.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index a8256e5a0e8a..a9b0f2b54a1c 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -27,6 +27,10 @@ static bool sleep_no_lps0 __read_mostly;
module_param(sleep_no_lps0, bool, 0644);
MODULE_PARM_DESC(sleep_no_lps0, "Do not use the special LPS0 device interface");

+static bool prefer_microsoft_guid __read_mostly;
+module_param(prefer_microsoft_guid, bool, 0644);
+MODULE_PARM_DESC(prefer_microsoft_guid, "Prefer selecting Microsoft GUID for LPS0 device");
+
static const struct acpi_device_id lps0_device_ids[] = {
{"PNP0D80", },
{"", },
@@ -402,6 +406,9 @@ static int lps0_device_attach(struct acpi_device *adev,
if (lps0_device_handle)
return 0;

+ lps0_dsm_func_mask_microsoft = validate_dsm(adev->handle,
+ ACPI_LPS0_DSM_UUID_MICROSOFT, 0,
+ &lps0_dsm_guid_microsoft);
if (acpi_s2idle_vendor_amd()) {
static const struct acpi_device_id *dev_id;
const struct amd_lps0_hid_device_data *data;
@@ -416,16 +423,12 @@ static int lps0_device_attach(struct acpi_device *adev,
rev_id = data->rev_id;
lps0_dsm_func_mask = validate_dsm(adev->handle,
ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
- lps0_dsm_func_mask_microsoft = validate_dsm(adev->handle,
- ACPI_LPS0_DSM_UUID_MICROSOFT, 0,
- &lps0_dsm_guid_microsoft);
if (lps0_dsm_func_mask > 0x3 && data->check_off_by_one) {
lps0_dsm_func_mask = (lps0_dsm_func_mask << 1) | 0x1;
acpi_handle_debug(adev->handle, "_DSM UUID %s: Adjusted function mask: 0x%x\n",
ACPI_LPS0_DSM_UUID_AMD, lps0_dsm_func_mask);
} else if (lps0_dsm_func_mask_microsoft > 0 && data->prefer_amd_guid &&
- (!strcmp(hid, "AMDI0007") ||
- !strcmp(hid, "AMDI0008"))) {
+ !prefer_microsoft_guid) {
lps0_dsm_func_mask_microsoft = -EINVAL;
acpi_handle_debug(adev->handle, "_DSM Using AMD method\n");
}
@@ -433,7 +436,8 @@ static int lps0_device_attach(struct acpi_device *adev,
rev_id = 1;
lps0_dsm_func_mask = validate_dsm(adev->handle,
ACPI_LPS0_DSM_UUID, rev_id, &lps0_dsm_guid);
- lps0_dsm_func_mask_microsoft = -EINVAL;
+ if (!prefer_microsoft_guid)
+ lps0_dsm_func_mask_microsoft = -EINVAL;
}

if (lps0_dsm_func_mask < 0 && lps0_dsm_func_mask_microsoft < 0)
--
2.34.1

2022-09-12 15:40:49

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH 4/4] acpi/x86: s2idle: Add a quirk for ASUS TUF Gaming A17 FA707RE

Am Fri, Sep 09, 2022 at 01:05:09PM -0500 schrieb Mario Limonciello:
> ASUS TUF Gaming A17 FA707RE has problems with ACPI events after
> s2idle resume. It's from a missing call to an ASL method in AMD
> the s2idle calling path. Force the system to use the Microsoft
> Modern Standby calling path instead.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216101
> Reported-and-tested-by: [email protected]
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> drivers/acpi/x86/s2idle.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index a9b0f2b54a1c..6a2c94fdbeae 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -17,6 +17,7 @@
>
> #include <linux/acpi.h>
> #include <linux/device.h>
> +#include <linux/dmi.h>
> #include <linux/suspend.h>
>
> #include "../sleep.h"
> @@ -400,6 +401,28 @@ static const struct acpi_device_id amd_hid_ids[] = {
> {}
> };
>
> +static int lps0_prefer_microsoft(const struct dmi_system_id *id)
> +{
> + pr_debug("Preferring Microsoft GUID.\n");
> + prefer_microsoft_guid = true;
> + return 0;
> +}
> +
> +static const struct dmi_system_id s2idle_dmi_table[] __initconst = {

This caused a build warning ...

[...]
> @@ -568,6 +591,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
>
> void acpi_s2idle_setup(void)
> {
> + dmi_check_system(s2idle_dmi_table);

... since it is used from a non-__init function:

WARNING: modpost: vmlinux.o: section mismatch in reference: acpi_s2idle_setup (section: .text) -> s2idle_dmi_table (section: .init.rodata)

regards
Philipp

2022-09-12 15:51:45

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

[Public]



> -----Original Message-----
> From: Philipp Zabel <[email protected]>
> Sent: Monday, September 12, 2022 10:06
> To: Limonciello, Mario <[email protected]>
> Cc: [email protected]; [email protected]; S-k, Shyam-sundar <Shyam-
> [email protected]>; Len Brown <[email protected]>; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop
>
> Am Mon, Sep 12, 2022 at 02:58:51PM +0000 schrieb Limonciello, Mario:
> > [AMD Official Use Only - General]
> >
> >
> >
> > > -----Original Message-----
> > > From: Philipp Zabel <[email protected]>
> > > Sent: Monday, September 12, 2022 09:57
> > > To: Limonciello, Mario <[email protected]>
> > > Cc: [email protected]; [email protected]; S-k, Shyam-sundar <Shyam-
> > > [email protected]>; Len Brown <[email protected]>; linux-
> > > [email protected]; [email protected]
> > > Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop
> > >
> > > Hi Mario,
> > >
> > > Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario Limonciello:
> > > > It was reported that an ASUS Rembrandt laptop has problems with
> > > seemingly
> > > > unrelated ACPI events after resuming from s2idle. Debugging the issue
> > > > proved it's because ASUS has ASL that is only called when using the
> > > > Microsoft GUID, not the AMD GUID.
> > > >
> > > > This is a bug from ASUS firmware but this series reworks the s2idle
> > > > handling for AMD to allow accounting for this in a quirk.
> > > >
> > > > Additionally as this is a problem that may pop up again on other models
> > > > add a module parameter that can be used to try the Microsoft GUID on
> a
> > > > given system.
> > >
> > > thank you, these also helped on an ASUS ROG Zephyrus G14 (2022) with
> > > BIOS version GA402RJ.313. Patches 1-3
> > >
> > > Tested-by: Philipp Zabel <[email protected]> # GA402RJ
> >
> > Did you use acpi.prefer_microsoft_guid=1 for your system then too?
> >
> > If so, I should re-spin this series to add your system's quirk to patch 4.
>
> Yes. I also tested with the following diff applied instead of the module
> parameter:
>
> ----------8<----------
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 6a2c94fdbeae..a247560e31de 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -420,6 +420,14 @@ static const struct dmi_system_id s2idle_dmi_table[]
> __initconst = {
> DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF
> Gaming A17"),
> },
> },
> + {
> + /* ASUS ROG Zephyrus G14 (2022) */
> + .callback = lps0_prefer_microsoft,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK
> COMPUTER INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "ROG
> Zephyrus G14 GA402"),
> + },
> + },
> {}
> };
>
> ---------->8----------
>
> The full DMI Product Name is "ROG Zephyrus G14 GA402RJ_GA402RJ", but
> there is also a near-identical higher spec model GA402RK.
>

Thanks so much! I'll roll it into a v2 after I fix your other comment.

2022-09-12 15:54:59

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

Hi Mario,

Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario Limonciello:
> It was reported that an ASUS Rembrandt laptop has problems with seemingly
> unrelated ACPI events after resuming from s2idle. Debugging the issue
> proved it's because ASUS has ASL that is only called when using the
> Microsoft GUID, not the AMD GUID.
>
> This is a bug from ASUS firmware but this series reworks the s2idle
> handling for AMD to allow accounting for this in a quirk.
>
> Additionally as this is a problem that may pop up again on other models
> add a module parameter that can be used to try the Microsoft GUID on a
> given system.

thank you, these also helped on an ASUS ROG Zephyrus G14 (2022) with
BIOS version GA402RJ.313. Patches 1-3

Tested-by: Philipp Zabel <[email protected]> # GA402RJ

regards
Philipp

2022-09-12 15:57:04

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

Am Mon, Sep 12, 2022 at 02:58:51PM +0000 schrieb Limonciello, Mario:
> [AMD Official Use Only - General]
>
>
>
> > -----Original Message-----
> > From: Philipp Zabel <[email protected]>
> > Sent: Monday, September 12, 2022 09:57
> > To: Limonciello, Mario <[email protected]>
> > Cc: [email protected]; [email protected]; S-k, Shyam-sundar <Shyam-
> > [email protected]>; Len Brown <[email protected]>; linux-
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop
> >
> > Hi Mario,
> >
> > Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario Limonciello:
> > > It was reported that an ASUS Rembrandt laptop has problems with
> > seemingly
> > > unrelated ACPI events after resuming from s2idle. Debugging the issue
> > > proved it's because ASUS has ASL that is only called when using the
> > > Microsoft GUID, not the AMD GUID.
> > >
> > > This is a bug from ASUS firmware but this series reworks the s2idle
> > > handling for AMD to allow accounting for this in a quirk.
> > >
> > > Additionally as this is a problem that may pop up again on other models
> > > add a module parameter that can be used to try the Microsoft GUID on a
> > > given system.
> >
> > thank you, these also helped on an ASUS ROG Zephyrus G14 (2022) with
> > BIOS version GA402RJ.313. Patches 1-3
> >
> > Tested-by: Philipp Zabel <[email protected]> # GA402RJ
>
> Did you use acpi.prefer_microsoft_guid=1 for your system then too?
>
> If so, I should re-spin this series to add your system's quirk to patch 4.

Yes. I also tested with the following diff applied instead of the module
parameter:

----------8<----------
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 6a2c94fdbeae..a247560e31de 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -420,6 +420,14 @@ static const struct dmi_system_id s2idle_dmi_table[] __initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming A17"),
},
},
+ {
+ /* ASUS ROG Zephyrus G14 (2022) */
+ .callback = lps0_prefer_microsoft,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus G14 GA402"),
+ },
+ },
{}
};

---------->8----------

The full DMI Product Name is "ROG Zephyrus G14 GA402RJ_GA402RJ", but
there is also a near-identical higher spec model GA402RK.

regards
Philipp

2022-09-12 16:01:46

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

[AMD Official Use Only - General]



> -----Original Message-----
> From: Philipp Zabel <[email protected]>
> Sent: Monday, September 12, 2022 09:57
> To: Limonciello, Mario <[email protected]>
> Cc: [email protected]; [email protected]; S-k, Shyam-sundar <Shyam-
> [email protected]>; Len Brown <[email protected]>; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop
>
> Hi Mario,
>
> Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario Limonciello:
> > It was reported that an ASUS Rembrandt laptop has problems with
> seemingly
> > unrelated ACPI events after resuming from s2idle. Debugging the issue
> > proved it's because ASUS has ASL that is only called when using the
> > Microsoft GUID, not the AMD GUID.
> >
> > This is a bug from ASUS firmware but this series reworks the s2idle
> > handling for AMD to allow accounting for this in a quirk.
> >
> > Additionally as this is a problem that may pop up again on other models
> > add a module parameter that can be used to try the Microsoft GUID on a
> > given system.
>
> thank you, these also helped on an ASUS ROG Zephyrus G14 (2022) with
> BIOS version GA402RJ.313. Patches 1-3
>
> Tested-by: Philipp Zabel <[email protected]> # GA402RJ

Did you use acpi.prefer_microsoft_guid=1 for your system then too?

If so, I should re-spin this series to add your system's quirk to patch 4.

>
> regards
> Philipp

2022-09-17 08:09:18

by Luke D. Jones

[permalink] [raw]
Subject: Re: [PATCH 4/4] acpi/x86: s2idle: Add a quirk for ASUS TUF Gaming A17 FA707RE

Hi Mario,

On Fri, 2022-09-09 at 13:05 -0500, Mario Limonciello wrote:
> ASUS TUF Gaming A17 FA707RE has problems with ACPI events after
> s2idle resume.  It's from a missing call to an ASL method in AMD
> the s2idle calling path. Force the system to use the Microsoft
> Modern Standby calling path instead.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216101
> Reported-and-tested-by: [email protected]
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
>  drivers/acpi/x86/s2idle.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index a9b0f2b54a1c..6a2c94fdbeae 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -17,6 +17,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/device.h>
> +#include <linux/dmi.h>
>  #include <linux/suspend.h>
>  
>  #include "../sleep.h"
> @@ -400,6 +401,28 @@ static const struct acpi_device_id amd_hid_ids[]
> = {
>         {}
>  };
>  
> +static int lps0_prefer_microsoft(const struct dmi_system_id *id)
> +{
> +       pr_debug("Preferring Microsoft GUID.\n");
> +       prefer_microsoft_guid = true;
> +       return 0;
> +}
> +
> +static const struct dmi_system_id s2idle_dmi_table[] __initconst = {
> +       {
> +               /*
> +                * ASUS TUF Gaming A17 FA707RE
> +                * https://bugzilla.kernel.org/show_bug.cgi?id=216101
> +                */
> +               .callback = lps0_prefer_microsoft,
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
> INC."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming
> A17"),
> +               },
> +       },
> +       {}
> +};
> +
>  static int lps0_device_attach(struct acpi_device *adev,
>                               const struct acpi_device_id *not_used)
>  {
> @@ -568,6 +591,7 @@ static const struct platform_s2idle_ops
> acpi_s2idle_ops_lps0 = {
>  
>  void acpi_s2idle_setup(void)
>  {
> +       dmi_check_system(s2idle_dmi_table);
>         acpi_scan_add_handler(&lps0_handler);
>         s2idle_set_ops(&acpi_s2idle_ops_lps0);
>  }

I'm confirming that this works for another laptop with the same issue -
the GA402R series.

The diff as follows (I'm unsure of how best to submit this as it is
dependant on your work - I don't need attribution for this):


drivers/acpi/x86/s2idle.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index e2b73809ab50..0c8348de5cbc 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -420,6 +420,17 @@ static const struct dmi_system_id
s2idle_dmi_table[] __initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming
A17"),
},
},
+ {
+ /*
+ * ASUS ROG Zephyrus G14 GA402R<variant> series
+ * These laptops have a similar issue to the FA707RE
+ */
+ .callback = lps0_prefer_microsoft,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus G14
GA402R"),
+ },
+ },
{}
};

--
2.37.3

2022-09-17 14:52:05

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 4/4] acpi/x86: s2idle: Add a quirk for ASUS TUF Gaming A17 FA707RE

On 9/17/22 02:35, Luke Jones wrote:
> Hi Mario,
>
> On Fri, 2022-09-09 at 13:05 -0500, Mario Limonciello wrote:
>> ASUS TUF Gaming A17 FA707RE has problems with ACPI events after
>> s2idle resume.  It's from a missing call to an ASL method in AMD
>> the s2idle calling path. Force the system to use the Microsoft
>> Modern Standby calling path instead.
>>
>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216101&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7Cc89466a0fc1b40d2dfd808da987f390b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637989969450224307%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=hFj1HnKnG9HPiQDc0fDFs07flY9N2KiOpY2g61bIdyE%3D&amp;reserved=0
>> Reported-and-tested-by: [email protected]
>> Signed-off-by: Mario Limonciello <[email protected]>
>> ---
>>  drivers/acpi/x86/s2idle.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
>> index a9b0f2b54a1c..6a2c94fdbeae 100644
>> --- a/drivers/acpi/x86/s2idle.c
>> +++ b/drivers/acpi/x86/s2idle.c
>> @@ -17,6 +17,7 @@
>>
>>  #include <linux/acpi.h>
>>  #include <linux/device.h>
>> +#include <linux/dmi.h>
>>  #include <linux/suspend.h>
>>
>>  #include "../sleep.h"
>> @@ -400,6 +401,28 @@ static const struct acpi_device_id amd_hid_ids[]
>> = {
>>         {}
>>  };
>>
>> +static int lps0_prefer_microsoft(const struct dmi_system_id *id)
>> +{
>> +       pr_debug("Preferring Microsoft GUID.\n");
>> +       prefer_microsoft_guid = true;
>> +       return 0;
>> +}
>> +
>> +static const struct dmi_system_id s2idle_dmi_table[] __initconst = {
>> +       {
>> +               /*
>> +                * ASUS TUF Gaming A17 FA707RE
>> +                * https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216101&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7Cc89466a0fc1b40d2dfd808da987f390b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637989969450224307%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=hFj1HnKnG9HPiQDc0fDFs07flY9N2KiOpY2g61bIdyE%3D&amp;reserved=0
>> +                */
>> +               .callback = lps0_prefer_microsoft,
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>> INC."),
>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming
>> A17"),
>> +               },
>> +       },
>> +       {}
>> +};
>> +
>>  static int lps0_device_attach(struct acpi_device *adev,
>>                               const struct acpi_device_id *not_used)
>>  {
>> @@ -568,6 +591,7 @@ static const struct platform_s2idle_ops
>> acpi_s2idle_ops_lps0 = {
>>
>>  void acpi_s2idle_setup(void)
>>  {
>> +       dmi_check_system(s2idle_dmi_table);
>>         acpi_scan_add_handler(&lps0_handler);
>>         s2idle_set_ops(&acpi_s2idle_ops_lps0);
>>  }
>
> I'm confirming that this works for another laptop with the same issue -
> the GA402R series.
>
> The diff as follows (I'm unsure of how best to submit this as it is
> dependant on your work - I don't need attribution for this):
>
>
> drivers/acpi/x86/s2idle.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index e2b73809ab50..0c8348de5cbc 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -420,6 +420,17 @@ static const struct dmi_system_id
> s2idle_dmi_table[] __initconst = {
> DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming
> A17"),
> },
> },
> + {
> + /*
> + * ASUS ROG Zephyrus G14 GA402R<variant> series
> + * These laptops have a similar issue to the FA707RE
> + */
> + .callback = lps0_prefer_microsoft,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
> INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus G14
> GA402R"),
> + },
> + },
> {}
> };
>

Hi Luke,

Thanks!

Your model is actually already rolled into v2 and v3 of the series,
reported by Phillip Zabel.

https://lore.kernel.org/linux-acpi/[email protected]/T/#mc37fd351eb5460a36c9e3da12c408fd3acb9f515

Thanks,

2022-09-18 09:11:31

by Luke D. Jones

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

Hi,

On Mon, 2022-09-12 at 17:06 +0200, Philipp Zabel wrote:
> Am Mon, Sep 12, 2022 at 02:58:51PM +0000 schrieb Limonciello, Mario:
> > [AMD Official Use Only - General]
> >
> >
> >
> > > -----Original Message-----
> > > From: Philipp Zabel <[email protected]>
> > > Sent: Monday, September 12, 2022 09:57
> > > To: Limonciello, Mario <[email protected]>
> > > Cc: [email protected]; [email protected]; S-k, Shyam-sundar
> > > <Shyam-
> > > [email protected]>; Len Brown <[email protected]>; linux-
> > > [email protected]; [email protected]
> > > Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt
> > > laptop
> > >
> > > Hi Mario,
> > >
> > > Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario
> > > Limonciello:
> > > > It was reported that an ASUS Rembrandt laptop has problems with
> > > seemingly
> > > > unrelated ACPI events after resuming from s2idle. Debugging the
> > > > issue
> > > > proved it's because ASUS has ASL that is only called when using
> > > > the
> > > > Microsoft GUID, not the AMD GUID.
> > > >
> > > > This is a bug from ASUS firmware but this series reworks the
> > > > s2idle
> > > > handling for AMD to allow accounting for this in a quirk.
> > > >
> > > > Additionally as this is a problem that may pop up again on
> > > > other models
> > > > add a module parameter that can be used to try the Microsoft
> > > > GUID on a
> > > > given system.
> > >
> > > thank you, these also helped on an ASUS ROG Zephyrus G14 (2022)
> > > with
> > > BIOS version GA402RJ.313. Patches 1-3
> > >
> > > Tested-by: Philipp Zabel <[email protected]> # GA402RJ
> >
> > Did you use acpi.prefer_microsoft_guid=1 for your system then too?
> >
> > If so, I should re-spin this series to add your system's quirk to
> > patch 4.
>
> Yes. I also tested with the following diff applied instead of the
> module
> parameter:
>
> ----------8<----------
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 6a2c94fdbeae..a247560e31de 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -420,6 +420,14 @@ static const struct dmi_system_id
> s2idle_dmi_table[] __initconst = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming
> A17"),
>                 },
>         },
> +       {
> +               /* ASUS ROG Zephyrus G14 (2022) */
> +               .callback = lps0_prefer_microsoft,
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
> INC."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus G14
> GA402"),

Just a note, this needs to be `ROG Zephyrus G14 GA402R` or this will
catch the GA402Q series as well which doesn't require this quirk.

In general the model numbering goes
<Range><Model><Generation><Graphics>, So for my old G14 <GA><402<Q><M>.
Or for example a ROG Strix machine <G><513><Q><Y>

I don't know of any others that may need this quirk.

> +               },
> +       },
>         {}
>  };
>  
> ---------->8----------
>
> The full DMI Product Name is "ROG Zephyrus G14 GA402RJ_GA402RJ", but
> there is also a near-identical higher spec model GA402RK.
>
> regards
> Philipp

2022-09-18 20:02:01

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

On 9/18/22 03:37, Luke Jones wrote:
> Hi,
>
> On Mon, 2022-09-12 at 17:06 +0200, Philipp Zabel wrote:
>> Am Mon, Sep 12, 2022 at 02:58:51PM +0000 schrieb Limonciello, Mario:
>>> [AMD Official Use Only - General]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Philipp Zabel <[email protected]>
>>>> Sent: Monday, September 12, 2022 09:57
>>>> To: Limonciello, Mario <[email protected]>
>>>> Cc: [email protected]; [email protected]; S-k, Shyam-sundar
>>>> <Shyam-
>>>> [email protected]>; Len Brown <[email protected]>; linux-
>>>> [email protected]; [email protected]
>>>> Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt
>>>> laptop
>>>>
>>>> Hi Mario,
>>>>
>>>> Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario
>>>> Limonciello:
>>>>> It was reported that an ASUS Rembrandt laptop has problems with
>>>> seemingly
>>>>> unrelated ACPI events after resuming from s2idle. Debugging the
>>>>> issue
>>>>> proved it's because ASUS has ASL that is only called when using
>>>>> the
>>>>> Microsoft GUID, not the AMD GUID.
>>>>>
>>>>> This is a bug from ASUS firmware but this series reworks the
>>>>> s2idle
>>>>> handling for AMD to allow accounting for this in a quirk.
>>>>>
>>>>> Additionally as this is a problem that may pop up again on
>>>>> other models
>>>>> add a module parameter that can be used to try the Microsoft
>>>>> GUID on a
>>>>> given system.
>>>>
>>>> thank you, these also helped on an ASUS ROG Zephyrus G14 (2022)
>>>> with
>>>> BIOS version GA402RJ.313. Patches 1-3
>>>>
>>>> Tested-by: Philipp Zabel <[email protected]> # GA402RJ
>>>
>>> Did you use acpi.prefer_microsoft_guid=1 for your system then too?
>>>
>>> If so, I should re-spin this series to add your system's quirk to
>>> patch 4.
>>
>> Yes. I also tested with the following diff applied instead of the
>> module
>> parameter:
>>
>> ----------8<----------
>> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
>> index 6a2c94fdbeae..a247560e31de 100644
>> --- a/drivers/acpi/x86/s2idle.c
>> +++ b/drivers/acpi/x86/s2idle.c
>> @@ -420,6 +420,14 @@ static const struct dmi_system_id
>> s2idle_dmi_table[] __initconst = {
>>                         DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF Gaming
>> A17"),
>>                 },
>>         },
>> +       {
>> +               /* ASUS ROG Zephyrus G14 (2022) */
>> +               .callback = lps0_prefer_microsoft,
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>> INC."),
>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus G14
>> GA402"),
>
> Just a note, this needs to be `ROG Zephyrus G14 GA402R` or this will
> catch the GA402Q series as well which doesn't require this quirk.
> In general the model numbering goes
> <Range><Model><Generation><Graphics>, So for my old G14 <GA><402<Q><M>.
> Or for example a ROG Strix machine <G><513><Q><Y>
>

What _HID is used? From your description GA402"Q" is Cezanne generation
right? Can you please share the acpidump for me to confirm what is
happening and if it's expected?

> I don't know of any others that may need this quirk.
>
>> +               },
>> +       },
>>         {}
>>  };
>>
>> ---------->8----------
>>
>> The full DMI Product Name is "ROG Zephyrus G14 GA402RJ_GA402RJ", but
>> there is also a near-identical higher spec model GA402RK.
>>
>> regards
>> Philipp
>

2022-09-18 22:40:58

by Luke D. Jones

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

On Sun, 2022-09-18 at 14:46 -0500, Mario Limonciello wrote:
> On 9/18/22 03:37, Luke Jones wrote:
> > Hi,
> >
> > On Mon, 2022-09-12 at 17:06 +0200, Philipp Zabel wrote:
> > > Am Mon, Sep 12, 2022 at 02:58:51PM +0000 schrieb Limonciello,
> > > Mario:
> > > > [AMD Official Use Only - General]
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Philipp Zabel <[email protected]>
> > > > > Sent: Monday, September 12, 2022 09:57
> > > > > To: Limonciello, Mario <[email protected]>
> > > > > Cc: [email protected]; [email protected]; S-k, Shyam-sundar
> > > > > <Shyam-
> > > > > [email protected]>; Len Brown <[email protected]>; linux-
> > > > > [email protected]; [email protected]
> > > > > Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt
> > > > > laptop
> > > > >
> > > > > Hi Mario,
> > > > >
> > > > > Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario
> > > > > Limonciello:
> > > > > > It was reported that an ASUS Rembrandt laptop has problems
> > > > > > with
> > > > > seemingly
> > > > > > unrelated ACPI events after resuming from s2idle. Debugging
> > > > > > the
> > > > > > issue
> > > > > > proved it's because ASUS has ASL that is only called when
> > > > > > using
> > > > > > the
> > > > > > Microsoft GUID, not the AMD GUID.
> > > > > >
> > > > > > This is a bug from ASUS firmware but this series reworks
> > > > > > the
> > > > > > s2idle
> > > > > > handling for AMD to allow accounting for this in a quirk.
> > > > > >
> > > > > > Additionally as this is a problem that may pop up again on
> > > > > > other models
> > > > > > add a module parameter that can be used to try the
> > > > > > Microsoft
> > > > > > GUID on a
> > > > > > given system.
> > > > >
> > > > > thank you, these also helped on an ASUS ROG Zephyrus G14
> > > > > (2022)
> > > > > with
> > > > > BIOS version GA402RJ.313. Patches 1-3
> > > > >
> > > > > Tested-by: Philipp Zabel <[email protected]> # GA402RJ
> > > >
> > > > Did you use acpi.prefer_microsoft_guid=1 for your system then
> > > > too?
> > > >
> > > > If so, I should re-spin this series to add your system's quirk
> > > > to
> > > > patch 4.
> > >
> > > Yes. I also tested with the following diff applied instead of the
> > > module
> > > parameter:
> > >
> > > ----------8<----------
> > > diff --git a/drivers/acpi/x86/s2idle.c
> > > b/drivers/acpi/x86/s2idle.c
> > > index 6a2c94fdbeae..a247560e31de 100644
> > > --- a/drivers/acpi/x86/s2idle.c
> > > +++ b/drivers/acpi/x86/s2idle.c
> > > @@ -420,6 +420,14 @@ static const struct dmi_system_id
> > > s2idle_dmi_table[] __initconst = {
> > >                          DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF
> > > Gaming
> > > A17"),
> > >                  },
> > >          },
> > > +       {
> > > +               /* ASUS ROG Zephyrus G14 (2022) */
> > > +               .callback = lps0_prefer_microsoft,
> > > +               .matches = {
> > > +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK
> > > COMPUTER
> > > INC."),
> > > +                       DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus
> > > G14
> > > GA402"),
> >
> > Just a note, this needs to be `ROG Zephyrus G14 GA402R` or this
> > will
> > catch the GA402Q series as well which doesn't require this quirk.
> > In general the model numbering goes
> > <Range><Model><Generation><Graphics>, So for my old G14
> > <GA><402<Q><M>.
> > Or for example a ROG Strix machine <G><513><Q><Y>
> >
>
> What _HID is used?  From your description GA402"Q" is Cezanne
> generation
> right? Can you please share the acpidump for me to confirm what is
> happening and if it's expected?

Sure, dumps are here -
https://gitlab.com/asus-linux/reverse-engineering/-/tree/master/ga401qm/408-fw

That repo is a bit haphazard as info tends to get collected
sporadically when issues arise, but it may be of use for you for other
things. I'll try to clean it up some.

>
> > I don't know of any others that may need this quirk.
> >
> > > +               },
> > > +       },
> > >          {}
> > >   };
> > >  
> > > ---------->8----------
> > >
> > > The full DMI Product Name is "ROG Zephyrus G14 GA402RJ_GA402RJ",
> > > but
> > > there is also a near-identical higher spec model GA402RK.
> > >
> > > regards
> > > Philipp
> >
>

2022-09-19 13:33:24

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt laptop

On 9/18/22 17:17, Luke Jones wrote:
> On Sun, 2022-09-18 at 14:46 -0500, Mario Limonciello wrote:
>> On 9/18/22 03:37, Luke Jones wrote:
>>> Hi,
>>>
>>> On Mon, 2022-09-12 at 17:06 +0200, Philipp Zabel wrote:
>>>> Am Mon, Sep 12, 2022 at 02:58:51PM +0000 schrieb Limonciello,
>>>> Mario:
>>>>> [AMD Official Use Only - General]
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Philipp Zabel <[email protected]>
>>>>>> Sent: Monday, September 12, 2022 09:57
>>>>>> To: Limonciello, Mario <[email protected]>
>>>>>> Cc: [email protected]; [email protected]; S-k, Shyam-sundar
>>>>>> <Shyam-
>>>>>> [email protected]>; Len Brown <[email protected]>; linux-
>>>>>> [email protected]; [email protected]
>>>>>> Subject: Re: [PATCH 0/4] Fixups for s2idle on ASUS Rembrandt
>>>>>> laptop
>>>>>>
>>>>>> Hi Mario,
>>>>>>
>>>>>> Am Fri, Sep 09, 2022 at 01:05:05PM -0500 schrieb Mario
>>>>>> Limonciello:
>>>>>>> It was reported that an ASUS Rembrandt laptop has problems
>>>>>>> with
>>>>>> seemingly
>>>>>>> unrelated ACPI events after resuming from s2idle. Debugging
>>>>>>> the
>>>>>>> issue
>>>>>>> proved it's because ASUS has ASL that is only called when
>>>>>>> using
>>>>>>> the
>>>>>>> Microsoft GUID, not the AMD GUID.
>>>>>>>
>>>>>>> This is a bug from ASUS firmware but this series reworks
>>>>>>> the
>>>>>>> s2idle
>>>>>>> handling for AMD to allow accounting for this in a quirk.
>>>>>>>
>>>>>>> Additionally as this is a problem that may pop up again on
>>>>>>> other models
>>>>>>> add a module parameter that can be used to try the
>>>>>>> Microsoft
>>>>>>> GUID on a
>>>>>>> given system.
>>>>>>
>>>>>> thank you, these also helped on an ASUS ROG Zephyrus G14
>>>>>> (2022)
>>>>>> with
>>>>>> BIOS version GA402RJ.313. Patches 1-3
>>>>>>
>>>>>> Tested-by: Philipp Zabel <[email protected]> # GA402RJ
>>>>>
>>>>> Did you use acpi.prefer_microsoft_guid=1 for your system then
>>>>> too?
>>>>>
>>>>> If so, I should re-spin this series to add your system's quirk
>>>>> to
>>>>> patch 4.
>>>>
>>>> Yes. I also tested with the following diff applied instead of the
>>>> module
>>>> parameter:
>>>>
>>>> ----------8<----------
>>>> diff --git a/drivers/acpi/x86/s2idle.c
>>>> b/drivers/acpi/x86/s2idle.c
>>>> index 6a2c94fdbeae..a247560e31de 100644
>>>> --- a/drivers/acpi/x86/s2idle.c
>>>> +++ b/drivers/acpi/x86/s2idle.c
>>>> @@ -420,6 +420,14 @@ static const struct dmi_system_id
>>>> s2idle_dmi_table[] __initconst = {
>>>>                          DMI_MATCH(DMI_PRODUCT_NAME, "ASUS TUF
>>>> Gaming
>>>> A17"),
>>>>                  },
>>>>          },
>>>> +       {
>>>> +               /* ASUS ROG Zephyrus G14 (2022) */
>>>> +               .callback = lps0_prefer_microsoft,
>>>> +               .matches = {
>>>> +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK
>>>> COMPUTER
>>>> INC."),
>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "ROG Zephyrus
>>>> G14
>>>> GA402"),
>>>
>>> Just a note, this needs to be `ROG Zephyrus G14 GA402R` or this
>>> will
>>> catch the GA402Q series as well which doesn't require this quirk.
>>> In general the model numbering goes
>>> <Range><Model><Generation><Graphics>, So for my old G14
>>> <GA><402<Q><M>.
>>> Or for example a ROG Strix machine <G><513><Q><Y>
>>>
>>
>> What _HID is used?  From your description GA402"Q" is Cezanne
>> generation
>> right? Can you please share the acpidump for me to confirm what is
>> happening and if it's expected?
>
> Sure, dumps are here -
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fasus-linux%2Freverse-engineering%2F-%2Ftree%2Fmaster%2Fga401qm%2F408-fw&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7Cf7ff7320f72f483145c108da99c39de1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637991362709739412%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=S8%2Bqp%2BtwFh6j1ZikKLpwRodmrhhSnthVDwuJZ8Ncs0A%3D&amp;reserved=0
>
> That repo is a bit haphazard as info tends to get collected
> sporadically when issues arise, but it may be of use for you for other
> things. I'll try to clean it up some.
>

Thanks for sharing it.

So this system uses "AMDI0005", so it matches "amd_picasso" behavior in
the patch series. That is "data->prefer_amd_guid" is false, so the
quirk and parameter shouldn't actually do anything in this system.

>>
>>> I don't know of any others that may need this quirk.
>>>
>>>> +               },
>>>> +       },
>>>>          {}
>>>>   };
>>>>
>>>> ---------->8----------
>>>>
>>>> The full DMI Product Name is "ROG Zephyrus G14 GA402RJ_GA402RJ",
>>>> but
>>>> there is also a near-identical higher spec model GA402RK.
>>>>
>>>> regards
>>>> Philipp
>>>
>>
>