Re: [RFC v1 2/8] acpi/x86: s2idle: Rename LPS0 constants so they mirror their function

[Antheas Kapenekakis]

On Fri, 13 Mar 2026 at 21:13, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> On Fri, Dec 26, 2025 at 11:27 AM Antheas Kapenekakis <lkml@xxxxxxxxxxx> wrote:
> >
> > The LPS0 3/4 constants are part of a firmware notification called
> > "Display on/off", in which the device enters a "Screen Off" state.
> > The LPS0 7/8 constants are part of a firmware notification in which
> > a Windows modern standby computer enters a "sleep" state where the
> > CPU may still be active and maintain the radios.
> >
> > However, currently, the values are named as "Screen on/off"
>
> This is based on the Intel documentation:
> https://uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf

The intel document affirms the rename, the state is referred to as
"Screen Off" with the callbacks being named "Display On/Off".

> > and "MS entry/exit", where Modern Standby is abbreviated as "MS". Therefore,
> > perform a minor rename so that the values match their function.
>
> I'm not sure about the first two at least.

Sleep DSMs are a bit more complicated. They are only defined in the
Microsoft UUID. However, the Windows suspend flow uses both DSM sets
distinctly. LPS0 entry/exit are used the same way in the Microsoft
UUID. The MS_ pair is used for an intermediary sleep state where the
CPU is still active. As the MS_ prefix causes confusion, I would tend
towards keeping this patch.

> > Then, fix the debug message to say that it executes notifications instead of
> > entering states as it is otherwise confusing.
> >
> > Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> > ---
> >  drivers/acpi/x86/s2idle.c | 54 +++++++++++++++++++--------------------
> >  1 file changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> > index 6d4d06236f61..1f13c8b0ef83 100644
> > --- a/drivers/acpi/x86/s2idle.c
> > +++ b/drivers/acpi/x86/s2idle.c
> > @@ -39,19 +39,19 @@ static const struct acpi_device_id lps0_device_ids[] = {
> >  #define ACPI_LPS0_DSM_UUID     "c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
> >
> >  #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS       1
> > -#define ACPI_LPS0_SCREEN_OFF   3
> > -#define ACPI_LPS0_SCREEN_ON    4
> > +#define ACPI_LPS0_DISPLAY_OFF  3
> > +#define ACPI_LPS0_DISPLAY_ON   4
> >  #define ACPI_LPS0_ENTRY                5
> >  #define ACPI_LPS0_EXIT         6
> > -#define ACPI_LPS0_MS_ENTRY      7
> > -#define ACPI_LPS0_MS_EXIT       8
> > +#define ACPI_LPS0_SLEEP_ENTRY  7
> > +#define ACPI_LPS0_SLEEP_EXIT   8
>
> These two renames are fine with me, although I'm not sure if they are
> worth the hassle.

As you mentioned, sleep is reserved, but as this is the canonical name
in internal code (much like display/screen that will not be exposed in
the ABI) it should be appropriate to keep here.

> >
> >  /* AMD */
> >  #define ACPI_LPS0_DSM_UUID_AMD      "e3f32452-febc-43ce-9039-932122d37721"
> >  #define ACPI_LPS0_ENTRY_AMD         2
> >  #define ACPI_LPS0_EXIT_AMD          3
> > -#define ACPI_LPS0_SCREEN_OFF_AMD    4
> > -#define ACPI_LPS0_SCREEN_ON_AMD     5
> > +#define ACPI_LPS0_DISPLAY_OFF_AMD   4
> > +#define ACPI_LPS0_DISPLAY_ON_AMD    5
> >
> >  static acpi_handle lps0_device_handle;
> >  static guid_t lps0_dsm_guid;
> > @@ -340,25 +340,25 @@ static const char *acpi_sleep_dsm_state_to_str(unsigned int state)
> >  {
> >         if (lps0_dsm_func_mask_microsoft || !acpi_s2idle_vendor_amd()) {
> >                 switch (state) {
> > -               case ACPI_LPS0_SCREEN_OFF:
> > -                       return "screen off";
> > -               case ACPI_LPS0_SCREEN_ON:
> > -                       return "screen on";
> > +               case ACPI_LPS0_DISPLAY_OFF:
> > +                       return "display off";
> > +               case ACPI_LPS0_DISPLAY_ON:
> > +                       return "display on";
> >                 case ACPI_LPS0_ENTRY:
> >                         return "lps0 entry";
> >                 case ACPI_LPS0_EXIT:
> >                         return "lps0 exit";
> > -               case ACPI_LPS0_MS_ENTRY:
> > -                       return "lps0 ms entry";
> > -               case ACPI_LPS0_MS_EXIT:
> > -                       return "lps0 ms exit";
> > +               case ACPI_LPS0_SLEEP_ENTRY:
> > +                       return "sleep entry";
> > +               case ACPI_LPS0_SLEEP_EXIT:
> > +                       return "sleep exit";
> >                 }
> >         } else {
> >                 switch (state) {
> > -               case ACPI_LPS0_SCREEN_ON_AMD:
> > -                       return "screen on";
> > -               case ACPI_LPS0_SCREEN_OFF_AMD:
> > -                       return "screen off";
> > +               case ACPI_LPS0_DISPLAY_ON_AMD:
> > +                       return "display on";
> > +               case ACPI_LPS0_DISPLAY_OFF_AMD:
> > +                       return "display off";
> >                 case ACPI_LPS0_ENTRY_AMD:
> >                         return "lps0 entry";
> >                 case ACPI_LPS0_EXIT_AMD:
> > @@ -383,7 +383,7 @@ static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, g
> >         lps0_dsm_state = func;
> >         if (pm_debug_messages_on) {
> >                 acpi_handle_info(lps0_device_handle,
> > -                               "%s transitioned to state %s\n",
> > +                               "%s executed notification %s\n",
> >                                  out_obj ? "Successfully" : "Failed to",
> >                                  acpi_sleep_dsm_state_to_str(lps0_dsm_state));
> >         }
> > @@ -545,12 +545,12 @@ static int acpi_s2idle_prepare_late_lps0(void)
> >         /* Screen off */
> >         if (lps0_dsm_func_mask > 0)
> >                 acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> > -                                       ACPI_LPS0_SCREEN_OFF_AMD :
> > -                                       ACPI_LPS0_SCREEN_OFF,
> > +                                       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_SCREEN_OFF,
> > +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
> >                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> >
> >         /* LPS0 entry */
> > @@ -560,7 +560,7 @@ static int acpi_s2idle_prepare_late_lps0(void)
> >
> >         if (lps0_dsm_func_mask_microsoft > 0) {
> >                 /* Modern Standby entry */
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_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);
> > @@ -613,18 +613,18 @@ static void acpi_s2idle_restore_early_lps0(void)
> >                 acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> >                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> >                 /* Modern Standby exit */
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
> > +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_EXIT,
> >                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> >         }
> >
> >         /* Screen on */
> >         if (lps0_dsm_func_mask_microsoft > 0)
> > -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> > +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_ON,
> >                                 lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> >         if (lps0_dsm_func_mask > 0)
> >                 acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> > -                                       ACPI_LPS0_SCREEN_ON_AMD :
> > -                                       ACPI_LPS0_SCREEN_ON,
> > +                                       ACPI_LPS0_DISPLAY_ON_AMD :
> > +                                       ACPI_LPS0_DISPLAY_ON,
> >                                         lps0_dsm_func_mask, lps0_dsm_guid);
> >  }
> >
> > --
> > 2.52.0
> >
> >
> >
>