A number of laptops have been showing up where lots of EC controlled
features weren't working after resume. They've varied from KBD
backlight, to fans, brightness control and lots more.
In kernel 6.1 we introduced a module parameter through
commit a0bc002393d4 ("ACPI: x86: s2idle: Add module parameter to
prefer Microsoft GUID") and a series of quirks in follow up commits
for systems that people reported the problems.
3 more systems recently reported issues; and so rather than increasing
the list /again/ to add these new systems we took a hard look at the
"why".
The AMD GUID/_REV 2 path was introduced for vendors to be able to
differentiate from the Microsoft path. Vendors could populate this
with unique code for their designs. Conceptually this was supposed
to help the ecosystem, however in practice we've found that there
are more machines that don't populate it than do.
The only models that have populated this with unique code for avoiding
a bug specific to their design is the HP Elitebook 835, 845, and 865 G9
systems.
To avoid growing the list further this series rips out the module
parameter support, all the quirks and sets the default policy to follow
the Microsoft GUID path for AMD Rembrandt or later. We validated this
on OEM systems and we found this fixes them.
To avoid regressing the HP systems that use the AMD GUID/_REV 2
path, let them keep taking it. The reason they take it is believed to
be a bug with WLAN firmware. If this is fixed in the future, we may
consider dropping the HP systems as well and having no quirks.
Mario Limonciello (2):
ACPI: x86: s2idle: Force AMD GUID/_REV 2 on HP Elitebook 865
ACPI: x86: s2idle: Stop using AMD specific codepath for Rembrandt+
drivers/acpi/x86/s2idle.c | 87 ++++++---------------------------------
1 file changed, 13 insertions(+), 74 deletions(-)
--
2.34.1
After we introduced a module parameter and quirk infrastructure for
picking the Microsoft GUID over the SOC vendor GUID we discovered
that lots and lots of systems are getting this wrong.
The table continues to grow, and is becoming unwieldy.
We don't really have any benefit to forcing vendors to populate the
AMD GUID. This is just extra work, and more and more vendors seem
to mess it up. As the Microsoft GUID is used by Windows as well,
it's very likely that it won't be messed up like this.
So drop all the quirks forcing it and the Rembrandt behavior. This
means that Cezanne or later effectively only run the Microsoft GUID
codepath with the exception of HP Elitebook 8*5 G9.
Fixes: fd894f05cf30 ("ACPI: x86: s2idle: If a new AMD _HID is missing assume Rembrandt")
Cc: [email protected] # 6.1
Reported-by: Benjamin Cheng <[email protected]>
Reported-by: [email protected]
Reported-by: Paul <[email protected]>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2292
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216768
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/acpi/x86/s2idle.c | 87 ++-------------------------------------
1 file changed, 3 insertions(+), 84 deletions(-)
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 422415cb14f4..c7afce465a07 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -28,10 +28,6 @@ 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_dsm_guid __read_mostly;
-module_param(prefer_microsoft_dsm_guid, bool, 0644);
-MODULE_PARM_DESC(prefer_microsoft_dsm_guid, "Prefer using Microsoft GUID in LPS0 device _DSM evaluation");
-
static const struct acpi_device_id lps0_device_ids[] = {
{"PNP0D80", },
{"", },
@@ -369,27 +365,15 @@ static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *d
}
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[] = {
@@ -397,7 +381,6 @@ static const struct acpi_device_id amd_hid_ids[] = {
{"AMD0005", (kernel_ulong_t)&amd_picasso, },
{"AMDI0005", (kernel_ulong_t)&amd_picasso, },
{"AMDI0006", (kernel_ulong_t)&amd_cezanne, },
- {"AMDI0007", (kernel_ulong_t)&amd_rembrandt, },
{}
};
@@ -407,68 +390,7 @@ static int lps0_prefer_amd(const struct dmi_system_id *id)
rev_id = 2;
return 0;
}
-
-static int lps0_prefer_microsoft(const struct dmi_system_id *id)
-{
- pr_debug("Preferring Microsoft GUID.\n");
- prefer_microsoft_dsm_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"),
- },
- },
- {
- /* 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"),
- },
- },
- {
- /*
- * Lenovo Yoga Slim 7 Pro X 14ARH7
- * https://bugzilla.kernel.org/show_bug.cgi?id=216473 : 82V2
- * https://bugzilla.kernel.org/show_bug.cgi?id=216438 : 82TL
- */
- .callback = lps0_prefer_microsoft,
- .matches = {
- DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
- DMI_MATCH(DMI_PRODUCT_NAME, "82"),
- },
- },
- {
- /*
- * ASUSTeK COMPUTER INC. ROG Flow X13 GV301RE_GV301RE
- * https://gitlab.freedesktop.org/drm/amd/-/issues/2148
- */
- .callback = lps0_prefer_microsoft,
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
- DMI_MATCH(DMI_PRODUCT_NAME, "ROG Flow X13 GV301"),
- },
- },
- {
- /*
- * ASUSTeK COMPUTER INC. ROG Flow X16 GV601RW_GV601RW
- * https://gitlab.freedesktop.org/drm/amd/-/issues/2148
- */
- .callback = lps0_prefer_microsoft,
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
- DMI_MATCH(DMI_PRODUCT_NAME, "ROG Flow X16 GV601"),
- },
- },
{
/*
* AMD Rembrandt based HP EliteBook 835/845/865 G9
@@ -504,16 +426,14 @@ static int lps0_device_attach(struct acpi_device *adev,
if (dev_id->id[0])
data = (const struct amd_lps0_hid_device_data *) dev_id->driver_data;
else
- data = &amd_rembrandt;
- rev_id = data->rev_id;
+ data = &amd_cezanne;
lps0_dsm_func_mask = validate_dsm(adev->handle,
ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
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 &&
- !prefer_microsoft_dsm_guid) {
+ } else if (lps0_dsm_func_mask_microsoft > 0 && rev_id) {
lps0_dsm_func_mask_microsoft = -EINVAL;
acpi_handle_debug(adev->handle, "_DSM Using AMD method\n");
}
@@ -521,8 +441,7 @@ 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);
- if (!prefer_microsoft_dsm_guid)
- lps0_dsm_func_mask_microsoft = -EINVAL;
+ lps0_dsm_func_mask_microsoft = -EINVAL;
}
if (lps0_dsm_func_mask < 0 && lps0_dsm_func_mask_microsoft < 0)
--
2.34.1
Am Thu, Dec 15, 2022 at 01:16:16PM -0600 schrieb Mario Limonciello:
> After we introduced a module parameter and quirk infrastructure for
> picking the Microsoft GUID over the SOC vendor GUID we discovered
> that lots and lots of systems are getting this wrong.
>
> The table continues to grow, and is becoming unwieldy.
>
> We don't really have any benefit to forcing vendors to populate the
> AMD GUID. This is just extra work, and more and more vendors seem
> to mess it up. As the Microsoft GUID is used by Windows as well,
> it's very likely that it won't be messed up like this.
>
> So drop all the quirks forcing it and the Rembrandt behavior. This
> means that Cezanne or later effectively only run the Microsoft GUID
> codepath with the exception of HP Elitebook 8*5 G9.
>
> Fixes: fd894f05cf30 ("ACPI: x86: s2idle: If a new AMD _HID is missing assume Rembrandt")
> Cc: [email protected] # 6.1
> Reported-by: Benjamin Cheng <[email protected]>
> Reported-by: [email protected]
> Reported-by: Paul <[email protected]>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2292
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216768
> Signed-off-by: Mario Limonciello <[email protected]>
Reviewed-by: Philipp Zabel <[email protected]>
Tested-by: Philipp Zabel <[email protected]>
regards
Philipp
Am Thu, Dec 15, 2022 at 01:16:16PM -0600 schrieb Mario Limonciello:
> After we introduced a module parameter and quirk infrastructure for > picking the Microsoft GUID over the SOC vendor GUID we discovered >
that lots and lots of systems are getting this wrong. > > The table
continues to grow, and is becoming unwieldy. > > We don't really have
any benefit to forcing vendors to populate the > AMD GUID. This is just
extra work, and more and more vendors seem > to mess it up. As the
Microsoft GUID is used by Windows as well, > it's very likely that it
won't be messed up like this. > > So drop all the quirks forcing it and
the Rembrandt behavior. This > means that Cezanne or later effectively
only run the Microsoft GUID > codepath with the exception of HP
Elitebook 8*5 G9. > > Fixes: fd894f05cf30 ("ACPI: x86: s2idle: If a new
AMD _HID is missing assume Rembrandt") > Cc: [email protected] #
6.1 > Reported-by: Benjamin Cheng <[email protected]> > Reported-by:
[email protected] > Reported-by: Paul <[email protected]> > Link:
https://gitlab.freedesktop.org/drm/amd/-/issues/2292 > Link:
https://bugzilla.kernel.org/show_bug.cgi?id=216768 > Signed-off-by:
Mario Limonciello <[email protected]> Tested-by: François ARMAND <[email protected]>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2307
Regards - and thanks for the help Mario!
François
On Thu, Dec 15, 2022 at 8:16 PM Mario Limonciello
<[email protected]> wrote:
>
> A number of laptops have been showing up where lots of EC controlled
> features weren't working after resume. They've varied from KBD
> backlight, to fans, brightness control and lots more.
> In kernel 6.1 we introduced a module parameter through
> commit a0bc002393d4 ("ACPI: x86: s2idle: Add module parameter to
> prefer Microsoft GUID") and a series of quirks in follow up commits
> for systems that people reported the problems.
>
> 3 more systems recently reported issues; and so rather than increasing
> the list /again/ to add these new systems we took a hard look at the
> "why".
>
> The AMD GUID/_REV 2 path was introduced for vendors to be able to
> differentiate from the Microsoft path. Vendors could populate this
> with unique code for their designs. Conceptually this was supposed
> to help the ecosystem, however in practice we've found that there
> are more machines that don't populate it than do.
>
> The only models that have populated this with unique code for avoiding
> a bug specific to their design is the HP Elitebook 835, 845, and 865 G9
> systems.
>
> To avoid growing the list further this series rips out the module
> parameter support, all the quirks and sets the default policy to follow
> the Microsoft GUID path for AMD Rembrandt or later. We validated this
> on OEM systems and we found this fixes them.
>
> To avoid regressing the HP systems that use the AMD GUID/_REV 2
> path, let them keep taking it. The reason they take it is believed to
> be a bug with WLAN firmware. If this is fixed in the future, we may
> consider dropping the HP systems as well and having no quirks.
>
> Mario Limonciello (2):
> ACPI: x86: s2idle: Force AMD GUID/_REV 2 on HP Elitebook 865
> ACPI: x86: s2idle: Stop using AMD specific codepath for Rembrandt+
>
> drivers/acpi/x86/s2idle.c | 87 ++++++---------------------------------
> 1 file changed, 13 insertions(+), 74 deletions(-)
>
> --
Both patches applied as 6.2-rc material, thanks!