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@xxxxxxx> wrote:
> > On 1/13/2026 7:36 AM, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > ---
> > >    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.