[PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
in lps0_device_attach() because pm_debug_messages_on might never be set
(and that is the case on production systems most of the time), but it
turns out that LPS0 _DSM Function 1 is generally problematic on some
platforms and causes suspend issues to occur when pm_debug_messages_on
is set now.

In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
in the cases when the system does not reach the deepest platform idle
state during suspend-to-idle for some reason.  If such diagnostics is
not necessary, evaluating it is a loss of time, so using it along with
the other pm_debug_messages_on diagnostics is questionable because the
latter is expected to be suitable for collecting debug information even
during production use of system suspend.

For this reason, add a module parameter called check_lps0_constraints
to control whether or not the list of LPS0 constraints will be checked
in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().

Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/x86/s2idle.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -28,6 +28,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 check_lps0_constraints __read_mostly;
+module_param(check_lps0_constraints, bool, 0644);
+MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");
+
 static const struct acpi_device_id lps0_device_ids[] = {
 	{"PNP0D80", },
 	{"", },
@@ -515,7 +519,7 @@ static struct acpi_scan_handler lps0_han
 
 static int acpi_s2idle_begin_lps0(void)
 {
-	if (lps0_device_handle && !sleep_no_lps0 && pm_debug_messages_on &&
+	if (lps0_device_handle && !sleep_no_lps0 && check_lps0_constraints &&
 	    !lpi_constraints_table) {
 		if (acpi_s2idle_vendor_amd())
 			lpi_device_get_constraints_amd();
@@ -540,7 +544,7 @@ static int acpi_s2idle_prepare_late_lps0
 	if (!lps0_device_handle || sleep_no_lps0)
 		return 0;
 
-	if (pm_debug_messages_on)
+	if (check_lps0_constraints)
 		lpi_check_constraints();
 
 	/* Screen off */

Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[Mario Limonciello]

On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
> needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
> in lps0_device_attach() because pm_debug_messages_on might never be set
> (and that is the case on production systems most of the time), but it
> turns out that LPS0 _DSM Function 1 is generally problematic on some
> platforms and causes suspend issues to occur when pm_debug_messages_on
> is set now.

Any ideas why it's causing problems?  AML doing something it shouldn't?

> 
> In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
> in the cases when the system does not reach the deepest platform idle
> state during suspend-to-idle for some reason.  If such diagnostics is
> not necessary, evaluating it is a loss of time, so using it along with
> the other pm_debug_messages_on diagnostics is questionable because the
> latter is expected to be suitable for collecting debug information even
> during production use of system suspend.
> 
> For this reason, add a module parameter called check_lps0_constraints
> to control whether or not the list of LPS0 constraints will be checked
> in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
> LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().
> 
> Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/x86/s2idle.c |    8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -28,6 +28,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 check_lps0_constraints __read_mostly;
> +module_param(check_lps0_constraints, bool, 0644);
> +MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");

I'm personally not really a fan of another module parameter for 
debugging.  I know some other subsystem maintainers are very anti-module 
parameters too.

I did like having /sys/power/pm_debug_messages able to be a one stop 
shop for debugging messages at runtime.

So I had another idea I wanted to throw around.  What if instead we had 
a file /sys/kernel/debug/x86/lps0_constraints?

If the file is never accessed never evaluate constraints.  If you read 
it once then you can get a dump of all the current constraints and any 
future suspends during that boot will also include constraints in the 
logs (IE call lpi_check_constraints()).

> +
>   static const struct acpi_device_id lps0_device_ids[] = {
>   	{"PNP0D80", },
>   	{"", },
> @@ -515,7 +519,7 @@ static struct acpi_scan_handler lps0_han
>   
>   static int acpi_s2idle_begin_lps0(void)
>   {
> -	if (lps0_device_handle && !sleep_no_lps0 && pm_debug_messages_on &&
> +	if (lps0_device_handle && !sleep_no_lps0 && check_lps0_constraints &&
>   	    !lpi_constraints_table) {
>   		if (acpi_s2idle_vendor_amd())
>   			lpi_device_get_constraints_amd();
> @@ -540,7 +544,7 @@ static int acpi_s2idle_prepare_late_lps0
>   	if (!lps0_device_handle || sleep_no_lps0)
>   		return 0;
>   
> -	if (pm_debug_messages_on)
> +	if (check_lps0_constraints)
>   		lpi_check_constraints();
>   
>   	/* Screen off */
> 
> 
> 

Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[Rafael J. Wysocki]

On Tue, Jan 13, 2026 at 10:48 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
> On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
> > needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
> > in lps0_device_attach() because pm_debug_messages_on might never be set
> > (and that is the case on production systems most of the time), but it
> > turns out that LPS0 _DSM Function 1 is generally problematic on some
> > platforms and causes suspend issues to occur when pm_debug_messages_on
> > is set now.
>
> Any ideas why it's causing problems?  AML doing something it shouldn't?

It's not a clear AML bug AFAICS.  Rather, it seems to have
dependencies on something that is not ready when it is evaluated, so
an ordering issue or similar.

> >
> > In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
> > in the cases when the system does not reach the deepest platform idle
> > state during suspend-to-idle for some reason.  If such diagnostics is
> > not necessary, evaluating it is a loss of time, so using it along with
> > the other pm_debug_messages_on diagnostics is questionable because the
> > latter is expected to be suitable for collecting debug information even
> > during production use of system suspend.
> >
> > For this reason, add a module parameter called check_lps0_constraints
> > to control whether or not the list of LPS0 constraints will be checked
> > in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
> > LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().
> >
> > Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   drivers/acpi/x86/s2idle.c |    8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > --- a/drivers/acpi/x86/s2idle.c
> > +++ b/drivers/acpi/x86/s2idle.c
> > @@ -28,6 +28,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 check_lps0_constraints __read_mostly;
> > +module_param(check_lps0_constraints, bool, 0644);
> > +MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");
>
> I'm personally not really a fan of another module parameter for
> debugging.  I know some other subsystem maintainers are very anti-module
> parameters too.
>
> I did like having /sys/power/pm_debug_messages able to be a one stop
> shop for debugging messages at runtime.

Well, this is not just debug messages, rather a whole debug facility
enabled via pm_debug_messages, which is kind of confusing.

> So I had another idea I wanted to throw around.  What if instead we had
> a file /sys/kernel/debug/x86/lps0_constraints?

Then you cannot use this without debugfs.

> If the file is never accessed never evaluate constraints.  If you read
> it once then you can get a dump of all the current constraints and any
> future suspends during that boot will also include constraints in the
> logs (IE call lpi_check_constraints()).

So if it is not in debugfs, it would need to be in sysfs and then I
don't see much difference between it and a module param, honestly.

I actually prefer the latter because it uses an existing infra.

Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[Mario Limonciello]

On 1/13/2026 3:57 PM, Rafael J. Wysocki wrote:
> On Tue, Jan 13, 2026 at 10:48 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>> On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
>>> needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
>>> in lps0_device_attach() because pm_debug_messages_on might never be set
>>> (and that is the case on production systems most of the time), but it
>>> turns out that LPS0 _DSM Function 1 is generally problematic on some
>>> platforms and causes suspend issues to occur when pm_debug_messages_on
>>> is set now.
>>
>> Any ideas why it's causing problems?  AML doing something it shouldn't?
> 
> It's not a clear AML bug AFAICS.  Rather, it seems to have
> dependencies on something that is not ready when it is evaluated, so
> an ordering issue or similar.

Ah I see.

> 
>>>
>>> In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
>>> in the cases when the system does not reach the deepest platform idle
>>> state during suspend-to-idle for some reason.  If such diagnostics is
>>> not necessary, evaluating it is a loss of time, so using it along with
>>> the other pm_debug_messages_on diagnostics is questionable because the
>>> latter is expected to be suitable for collecting debug information even
>>> during production use of system suspend.
>>>
>>> For this reason, add a module parameter called check_lps0_constraints
>>> to control whether or not the list of LPS0 constraints will be checked
>>> in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
>>> LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().
>>>
>>> Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>>    drivers/acpi/x86/s2idle.c |    8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> --- a/drivers/acpi/x86/s2idle.c
>>> +++ b/drivers/acpi/x86/s2idle.c
>>> @@ -28,6 +28,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 check_lps0_constraints __read_mostly;
>>> +module_param(check_lps0_constraints, bool, 0644);
>>> +MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");
>>
>> I'm personally not really a fan of another module parameter for
>> debugging.  I know some other subsystem maintainers are very anti-module
>> parameters too.
>>
>> I did like having /sys/power/pm_debug_messages able to be a one stop
>> shop for debugging messages at runtime.
> 
> Well, this is not just debug messages, rather a whole debug facility
> enabled via pm_debug_messages, which is kind of confusing.

Good point.

> 
>> So I had another idea I wanted to throw around.  What if instead we had
>> a file /sys/kernel/debug/x86/lps0_constraints?
> 
> Then you cannot use this without debugfs.

I mean; yeah.  But it really is debugging and most distros have debugfs 
enabled by default these days don't they?

> 
>> If the file is never accessed never evaluate constraints.  If you read
>> it once then you can get a dump of all the current constraints and any
>> future suspends during that boot will also include constraints in the
>> logs (IE call lpi_check_constraints()).
> 
> So if it is not in debugfs, it would need to be in sysfs and then I
> don't see much difference between it and a module param, honestly.
> 
> I actually prefer the latter because it uses an existing infra.

I agree if debugfs is decided to be out sysfs and module parameter are 
equal footing and then this patch makes sense.

Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[Rafael J. Wysocki]

On Tue, Jan 13, 2026 at 11:00 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
> On 1/13/2026 3:57 PM, Rafael J. Wysocki wrote:
> > On Tue, Jan 13, 2026 at 10:48 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >> On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>
> >>> Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
> >>> needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
> >>> in lps0_device_attach() because pm_debug_messages_on might never be set
> >>> (and that is the case on production systems most of the time), but it
> >>> turns out that LPS0 _DSM Function 1 is generally problematic on some
> >>> platforms and causes suspend issues to occur when pm_debug_messages_on
> >>> is set now.
> >>
> >> Any ideas why it's causing problems?  AML doing something it shouldn't?
> >
> > It's not a clear AML bug AFAICS.  Rather, it seems to have
> > dependencies on something that is not ready when it is evaluated, so
> > an ordering issue or similar.
>
> Ah I see.
>
> >
> >>>
> >>> In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
> >>> in the cases when the system does not reach the deepest platform idle
> >>> state during suspend-to-idle for some reason.  If such diagnostics is
> >>> not necessary, evaluating it is a loss of time, so using it along with
> >>> the other pm_debug_messages_on diagnostics is questionable because the
> >>> latter is expected to be suitable for collecting debug information even
> >>> during production use of system suspend.
> >>>
> >>> For this reason, add a module parameter called check_lps0_constraints
> >>> to control whether or not the list of LPS0 constraints will be checked
> >>> in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
> >>> LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().
> >>>
> >>> Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>> ---
> >>>    drivers/acpi/x86/s2idle.c |    8 ++++++--
> >>>    1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> --- a/drivers/acpi/x86/s2idle.c
> >>> +++ b/drivers/acpi/x86/s2idle.c
> >>> @@ -28,6 +28,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 check_lps0_constraints __read_mostly;
> >>> +module_param(check_lps0_constraints, bool, 0644);
> >>> +MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");
> >>
> >> I'm personally not really a fan of another module parameter for
> >> debugging.  I know some other subsystem maintainers are very anti-module
> >> parameters too.
> >>
> >> I did like having /sys/power/pm_debug_messages able to be a one stop
> >> shop for debugging messages at runtime.
> >
> > Well, this is not just debug messages, rather a whole debug facility
> > enabled via pm_debug_messages, which is kind of confusing.
>
> Good point.
>
> >
> >> So I had another idea I wanted to throw around.  What if instead we had
> >> a file /sys/kernel/debug/x86/lps0_constraints?
> >
> > Then you cannot use this without debugfs.
>
> I mean; yeah.  But it really is debugging and most distros have debugfs
> enabled by default these days don't they?

Some of them don't.

That's one of the reasons why tracing has its own pseudo-filesystem
now, so it is a real obstacle.

> >
> >> If the file is never accessed never evaluate constraints.  If you read
> >> it once then you can get a dump of all the current constraints and any
> >> future suspends during that boot will also include constraints in the
> >> logs (IE call lpi_check_constraints()).
> >
> > So if it is not in debugfs, it would need to be in sysfs and then I
> > don't see much difference between it and a module param, honestly.
> >
> > I actually prefer the latter because it uses an existing infra.
>
> I agree if debugfs is decided to be out sysfs and module parameter are
> equal footing and then this patch makes sense.

I think that debugfs cannot be relied on to be always present.

Re: [PATCH v1] ACPI: PM: s2idle: Add module parameter for LPS0 constraints checking

[Mario Limonciello (AMD) (kernel.org)]

On 1/13/2026 4:05 PM, Rafael J. Wysocki wrote:
> On Tue, Jan 13, 2026 at 11:00 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>> On 1/13/2026 3:57 PM, Rafael J. Wysocki wrote:
>>> On Tue, Jan 13, 2026 at 10:48 PM Mario Limonciello
>>> <mario.limonciello@amd.com> wrote:
>>>> On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
>>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>>
>>>>> Commit 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when
>>>>> needed") attempted to avoid useless evaluation of LPS0 _DSM Function 1
>>>>> in lps0_device_attach() because pm_debug_messages_on might never be set
>>>>> (and that is the case on production systems most of the time), but it
>>>>> turns out that LPS0 _DSM Function 1 is generally problematic on some
>>>>> platforms and causes suspend issues to occur when pm_debug_messages_on
>>>>> is set now.
>>>>
>>>> Any ideas why it's causing problems?  AML doing something it shouldn't?
>>>
>>> It's not a clear AML bug AFAICS.  Rather, it seems to have
>>> dependencies on something that is not ready when it is evaluated, so
>>> an ordering issue or similar.
>>
>> Ah I see.
>>
>>>
>>>>>
>>>>> In Linux, LPS0 _DSM Function 1 is only useful for diagnostics and only
>>>>> in the cases when the system does not reach the deepest platform idle
>>>>> state during suspend-to-idle for some reason.  If such diagnostics is
>>>>> not necessary, evaluating it is a loss of time, so using it along with
>>>>> the other pm_debug_messages_on diagnostics is questionable because the
>>>>> latter is expected to be suitable for collecting debug information even
>>>>> during production use of system suspend.
>>>>>
>>>>> For this reason, add a module parameter called check_lps0_constraints
>>>>> to control whether or not the list of LPS0 constraints will be checked
>>>>> in acpi_s2idle_prepare_late_lps0() and so whether or not to evaluate
>>>>> LPS0 _DSM Function 1 (once) in acpi_s2idle_begin_lps0().
>>>>>
>>>>> Fixes: 32ece31db4df ("ACPI: PM: s2idle: Only retrieve constraints when needed")
>>>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>> ---
>>>>>     drivers/acpi/x86/s2idle.c |    8 ++++++--
>>>>>     1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> --- a/drivers/acpi/x86/s2idle.c
>>>>> +++ b/drivers/acpi/x86/s2idle.c
>>>>> @@ -28,6 +28,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 check_lps0_constraints __read_mostly;
>>>>> +module_param(check_lps0_constraints, bool, 0644);
>>>>> +MODULE_PARM_DESC(check_lps0_constraints, "Check LPS0 device constraints");
>>>>
>>>> I'm personally not really a fan of another module parameter for
>>>> debugging.  I know some other subsystem maintainers are very anti-module
>>>> parameters too.
>>>>
>>>> I did like having /sys/power/pm_debug_messages able to be a one stop
>>>> shop for debugging messages at runtime.
>>>
>>> Well, this is not just debug messages, rather a whole debug facility
>>> enabled via pm_debug_messages, which is kind of confusing.
>>
>> Good point.
>>
>>>
>>>> So I had another idea I wanted to throw around.  What if instead we had
>>>> a file /sys/kernel/debug/x86/lps0_constraints?
>>>
>>> Then you cannot use this without debugfs.
>>
>> I mean; yeah.  But it really is debugging and most distros have debugfs
>> enabled by default these days don't they?
> 
> Some of them don't.
> 
> That's one of the reasons why tracing has its own pseudo-filesystem
> now, so it is a real obstacle.
> 
>>>
>>>> If the file is never accessed never evaluate constraints.  If you read
>>>> it once then you can get a dump of all the current constraints and any
>>>> future suspends during that boot will also include constraints in the
>>>> logs (IE call lpi_check_constraints()).
>>>
>>> So if it is not in debugfs, it would need to be in sysfs and then I
>>> don't see much difference between it and a module param, honestly.
>>>
>>> I actually prefer the latter because it uses an existing infra.
>>
>> I agree if debugfs is decided to be out sysfs and module parameter are
>> equal footing and then this patch makes sense.
> 
> I think that debugfs cannot be relied on to be always present.

OK, I think the patch is fine as is then.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>