| Message ID | 12427214.O9o76ZdvQC@kreacher (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [v1] ACPI: PM: s2idle: Evaluate all Low-Power S0 Idle _DSM functions | expand |
On 4/23/2024 14:19, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Commit 073237281a50 ("ACPI: PM: s2idle: Enable Low-Power S0 Idle MSFT > UUID for non-AMD systems") attempted to avoid evaluating the same Low- > Power S0 Idle _DSM functions for different UUIDs, but that turns out to > be a mistake, because some systems in the field are adversely affected > by it. > > Address this by allowing all Low-Power S0 Idle _DSM functions to be > evaluated, but still print the message regarding duplication of Low- > Power S0 Idle _DSM function sets for different UUIDs. > > Fixes: 073237281a50 ("ACPI: PM: s2idle: Enable Low-Power S0 Idle MSFT UUID for non-AMD systems") > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218750 > Reported-and-tested-by: Mark Pearson <mpearson@lenovo.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Ah glad to hear that my idea worked. FWIW we saw the same type of issue on some AMD systems when we first introduced the MSFT UUID. Suggested-by: Mario Limonciello <mario.limonciello@amd.com> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/acpi/x86/s2idle.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > Index: linux-pm/drivers/acpi/x86/s2idle.c > =================================================================== > --- linux-pm.orig/drivers/acpi/x86/s2idle.c > +++ linux-pm/drivers/acpi/x86/s2idle.c > @@ -492,16 +492,14 @@ static int lps0_device_attach(struct acp > unsigned int func_mask; > > /* > - * Avoid evaluating the same _DSM function for two > - * different UUIDs and prioritize the MSFT one. > + * Log a message if the _DSM function sets for two > + * different UUIDs overlap. > */ > func_mask = lps0_dsm_func_mask & lps0_dsm_func_mask_microsoft; > - if (func_mask) { > + if (func_mask) > acpi_handle_info(adev->handle, > "Duplicate LPS0 _DSM functions (mask: 0x%x)\n", > func_mask); > - lps0_dsm_func_mask &= ~func_mask; > - } > } > } > > > >
On Tue, Apr 23, 2024 at 9:40 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 4/23/2024 14:19, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Commit 073237281a50 ("ACPI: PM: s2idle: Enable Low-Power S0 Idle MSFT > > UUID for non-AMD systems") attempted to avoid evaluating the same Low- > > Power S0 Idle _DSM functions for different UUIDs, but that turns out to > > be a mistake, because some systems in the field are adversely affected > > by it. > > > > Address this by allowing all Low-Power S0 Idle _DSM functions to be > > evaluated, but still print the message regarding duplication of Low- > > Power S0 Idle _DSM function sets for different UUIDs. > > > > Fixes: 073237281a50 ("ACPI: PM: s2idle: Enable Low-Power S0 Idle MSFT UUID for non-AMD systems") > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218750 > > Reported-and-tested-by: Mark Pearson <mpearson@lenovo.com> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Ah glad to hear that my idea worked. We've arrived at the same solution independently, I've only looked at the BZ when the patch was ready to send. > FWIW we saw the same type of issue > on some AMD systems when we first introduced the MSFT UUID. > > Suggested-by: Mario Limonciello <mario.limonciello@amd.com> But sure. > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> Thanks! > > --- > > drivers/acpi/x86/s2idle.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > Index: linux-pm/drivers/acpi/x86/s2idle.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/x86/s2idle.c > > +++ linux-pm/drivers/acpi/x86/s2idle.c > > @@ -492,16 +492,14 @@ static int lps0_device_attach(struct acp > > unsigned int func_mask; > > > > /* > > - * Avoid evaluating the same _DSM function for two > > - * different UUIDs and prioritize the MSFT one. > > + * Log a message if the _DSM function sets for two > > + * different UUIDs overlap. > > */ > > func_mask = lps0_dsm_func_mask & lps0_dsm_func_mask_microsoft; > > - if (func_mask) { > > + if (func_mask) > > acpi_handle_info(adev->handle, > > "Duplicate LPS0 _DSM functions (mask: 0x%x)\n", > > func_mask); > > - lps0_dsm_func_mask &= ~func_mask; > > - } > > } > > } > > > > > > > > >
Index: linux-pm/drivers/acpi/x86/s2idle.c =================================================================== --- linux-pm.orig/drivers/acpi/x86/s2idle.c +++ linux-pm/drivers/acpi/x86/s2idle.c @@ -492,16 +492,14 @@ static int lps0_device_attach(struct acp unsigned int func_mask; /* - * Avoid evaluating the same _DSM function for two - * different UUIDs and prioritize the MSFT one. + * Log a message if the _DSM function sets for two + * different UUIDs overlap. */ func_mask = lps0_dsm_func_mask & lps0_dsm_func_mask_microsoft; - if (func_mask) { + if (func_mask) acpi_handle_info(adev->handle, "Duplicate LPS0 _DSM functions (mask: 0x%x)\n", func_mask); - lps0_dsm_func_mask &= ~func_mask; - } } }