Re: [RFC v2 02/10] acpi/x86: s2idle: Move Modern Standby calls to s2idle begin/end

[Antheas Kapenekakis]

On Fri, 8 May 2026 at 21:25, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> On Sat, Apr 25, 2026 at 11:57 PM Antheas Kapenekakis <lkml@xxxxxxxxxxx> wrote:
> >
> > In Windows, the modern standby calls for sleep entry/exit and display
> > on/off happen while the kernel device subsystems are active and the
> > device is asleep. Currently, in the Linux kernel they happen in
> > prepare_late, after e.g. the USB subsystem has turned off. This
> > disimilarity causes obscure issues in certain devices that use these
> > calls to turn off peripherals that should not be active during modern
> > standby, e.g. handheld controllers, and RGB.
> >
> > Therefore, move these calls to _begin(), and _end() to match Windows.
> > Particularly for _end(), introduce a acpi_s2idle_end_lps0() function to
> > wrap acpi_s2idle_end(), matching the structure introduced with
> > acpi_s2idle_begin_lps0().
> >
> > Of note is that unlike the ACPI ABI of LPS0, there is no device power
> > state requirement before entering sleep/screen off, therefore it is
> > appropriate to move these calls to _begin(), before s2idle suspend.
> >
> > Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states
> > Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> > ---
> >  drivers/acpi/x86/s2idle.c | 63 +++++++++++++++++++++++----------------
> >  1 file changed, 37 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> > index 61a044b59776..f5aefba8b191 100644
> > --- a/drivers/acpi/x86/s2idle.c
> > +++ b/drivers/acpi/x86/s2idle.c
> > @@ -517,8 +517,10 @@ static struct acpi_scan_handler lps0_handler = {
> >
> >  static int acpi_s2idle_begin_lps0(void)
> >  {
> > -       if (lps0_device_handle && !sleep_no_lps0 && check_lps0_constraints &&
> > -           !lpi_constraints_table) {
> > +       if (!lps0_device_handle || sleep_no_lps0)
> > +               return acpi_s2idle_begin();
> > +
> > +       if (check_lps0_constraints && !lpi_constraints_table) {
> >                 if (acpi_s2idle_vendor_amd())
> >                         lpi_device_get_constraints_amd();
> >                 else
> > @@ -532,6 +534,24 @@ static int acpi_s2idle_begin_lps0(void)
> >                         lpi_constraints_table = ERR_PTR(-ENODATA);
> >         }
> >
> > +       /* Display off */
> > +       if (lps0_dsm_func_mask > 0)
> > +               acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> > +                                       ACPI_LPS0_DISPLAY_OFF_AMD :
> > +                                       ACPI_LPS0_DISPLAY_OFF,
> > +                                       lps0_dsm_func_mask, lps0_dsm_guid);
> > +
> > +       if (lps0_dsm_func_mask_microsoft > 0)
> > +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
> > +                                       lps0_dsm_func_mask_microsoft,
> > +                                       lps0_dsm_guid_microsoft);
> > +
> > +       /* Modern Standby entry */
> > +       if (lps0_dsm_func_mask_microsoft > 0)
> > +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_ENTRY,
> > +                                       lps0_dsm_func_mask_microsoft,
> > +                                       lps0_dsm_guid_microsoft);
>
> This is almost certainly too early at least for some systems out there.
>
> While I can agree with putting ACPI_LPS0_DISPLAY_OFF here, I don't
> quite agree with the other one.

Can you expand? I am not aware of any such systems and the
specification does not require that any subsystem be powered off prior
to the notification. In the contrary, since some systems use the call
to power off their USB gamepad, this suggests that the USB system is
active during the call in Windows, where in Linux it has already been
put to D3 under current kernels

We deployed a prior variant of this series to around ~60k devices and
found no issue.

Best,
Antheas

> > +
> >         return acpi_s2idle_begin();
> >  }
> >
> > @@ -545,36 +565,17 @@ static int acpi_s2idle_prepare_late_lps0(void)
> >         if (check_lps0_constraints)
> >                 lpi_check_constraints();
> >
> > -       /* Display off */
> > +       /* LPS0 entry */
> >         if (lps0_dsm_func_mask > 0)
> >                 acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> > -                                       ACPI_LPS0_DISPLAY_OFF_AMD :
> > -                                       ACPI_LPS0_DISPLAY_OFF,
> > +                                               ACPI_LPS0_ENTRY_AMD :
> > +                                               ACPI_LPS0_ENTRY,
> >                                         lps0_dsm_func_mask, lps0_dsm_guid);
> >
> >         if (lps0_dsm_func_mask_microsoft > 0)
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
> > -                                       lps0_dsm_func_mask_microsoft,
> > -                                       lps0_dsm_guid_microsoft);
> > -
> > -       /* LPS0 entry */
> > -       if (lps0_dsm_func_mask > 0 && acpi_s2idle_vendor_amd())
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
> > -                                       lps0_dsm_func_mask, lps0_dsm_guid);
> > -
> > -       if (lps0_dsm_func_mask_microsoft > 0) {
> > -               /* Modern Standby entry */
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_ENTRY,
> > -                                       lps0_dsm_func_mask_microsoft,
> > -                                       lps0_dsm_guid_microsoft);
> >                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
> >                                         lps0_dsm_func_mask_microsoft,
> >                                         lps0_dsm_guid_microsoft);
> > -       }
> > -
> > -       if (lps0_dsm_func_mask > 0 && !acpi_s2idle_vendor_amd())
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
> > -                                       lps0_dsm_func_mask, lps0_dsm_guid);
> >
> >         list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
> >                 if (handler->prepare)
> > @@ -615,9 +616,19 @@ static void acpi_s2idle_restore_early_lps0(void)
> >                                         ACPI_LPS0_EXIT,
> >                                         lps0_dsm_func_mask, lps0_dsm_guid);
> >
> > -       if (lps0_dsm_func_mask_microsoft > 0) {
> > +       if (lps0_dsm_func_mask_microsoft > 0)
> >                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> >                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> > +}
> > +
> > +static void acpi_s2idle_end_lps0(void)
> > +{
> > +       acpi_s2idle_end();
> > +
> > +       if (!lps0_device_handle || sleep_no_lps0)
> > +               return;
> > +
> > +       if (lps0_dsm_func_mask_microsoft > 0) {
> >                 /* Intent to turn on display */
> >                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_TURN_ON_DISPLAY,
> >                                         lps0_dsm_func_mask_microsoft,
> > @@ -648,7 +659,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
> >         .wake = acpi_s2idle_wake,
> >         .restore_early = acpi_s2idle_restore_early_lps0,
> >         .restore = acpi_s2idle_restore,
> > -       .end = acpi_s2idle_end,
> > +       .end = acpi_s2idle_end_lps0,
> >  };
> >
> >  void __init acpi_s2idle_setup(void)
> > --
>