Message-ID: 
 <CAGwozwG=F1BC8UF6Xkest5pwZG6iRjNjk5zpjmSV8Yh-0S2tGA@mail.gmail.com>
Date: Tue, 2 Dec 2025 10:32:27 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
 Mario Limonciello <superm1@...nel.org>,
	Robert Beckett <bob.beckett@...labora.com>, linux-acpi@...r.kernel.org,
	kernel@...labora.com, linux-kernel@...r.kernel.org,
	Sebastian Reichel <sebastian.reichel@...labora.com>,
 Xaver Hugl <xaver.hugl@...il.com>,
	Richard Hughes <richard@...hsie.com>, William Jon McCann <mccann@....edu>,
	"Jaap A . Haitsma" <jaap@...tsma.org>, Benjamin Canou <bookeldor@...il.com>,
	Bastien Nocera <hadess@...ess.net>, systemd-devel@...ts.freedesktop.org,
	Lennart Poettering <lennart@...ttering.net>
Subject: Re: [RFC PATCH v1 1/1] ACPI: PM: s2idle: Add lps0_screen_off sysfs
 interface

On Tue, 2 Dec 2025 at 05:36, Dmitry Osipenko
<dmitry.osipenko@...labora.com> wrote:
>
> Add `/sys/power/lps0_screen_off` interface to allow userspace to control
> Display OFF/ON DSM notifications at runtime. Writing "1" to this file
> triggers the OFF notification, and "0" triggers the ON notification.
>
> Userspace should write "1" after turning off all physical and remote
> displays. It should write "0" before turning on any of displays.
>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>
> ---
>  Documentation/ABI/testing/sysfs-power |  13 +++
>  drivers/acpi/x86/s2idle.c             | 149 +++++++++++++++++++++++---
>  2 files changed, 145 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power
> index d38da077905a..af7c81ae517c 100644
> --- a/Documentation/ABI/testing/sysfs-power
> +++ b/Documentation/ABI/testing/sysfs-power
> @@ -470,3 +470,16 @@ Description:
>
>                  Minimum value: 1
>                  Default value: 3
> +
> +What:          /sys/power/lps0_screen_off

Hi,
thanks for having a second stab at this. My initial series for this
was kind of complicated, I would need to rewrite it anyway [1].

I will second Mario on the integer values. The main.c file provides
the capabilities used in other power sysfs values and an ABI for doing
string options.

For me, I have a bit of a problem with the ABI. I kind of prefer the
one in [1]. There are three sleep states in Modern Standby: Screen
Off, Sleep, and LPS0/DRIPS (and a fake resume one I added). The only
one the kernel is suspended in is LPS0.

So the ABI should ideally be able to cover all three, even if at first
you only do screen off. This means the name kind of becomes a problem.
lps0_screen_off implies lps0 (is not the state, is also an ACPI x86
specific term) and is limited to screen_off (cannot add sleep).

I used /sys/power/standby in my series, which I think was fine because
you'd be able to add hooks to it for general drivers in the future.
This way, it would not be limited to ACPI devices and the name implies
that.

Two other notes. At this point we tested pretty much devices from all
manufacturers with my series. These notifications are used to control,
for sleep: thermal envelope, fan, power button light, for screen off:
keyboard backlight, device RGB, for lenovo power light as well. Yes,
DRI should be cc'd, but no-one has used these notifications to do GPU
specific stuff yet. You can call this ABI with a screen on just fine
on all known devices.

Handheld manufacturers typically tie their controllers to them as
well, as xinput does not implement the new suspend features in Windows
and blocks restricted modern standby, so they have to be turned off
beforehand. The exception to that is the Xbox Ally devices. This is
because with the Ally X, Asus switched to the Xbox GIP protocol which
does support these suspend features but still kept powering off the
controller. For the Xbox Allies, they went a step further and no
longer power off the controller.

Another difference between those two states and LPS0/DRIPS, is that
the LPS0/DRIPS specification binds the state to the power state of
certain onboard devices specified by ACPI (ie when the GPU, XYZ
components suspend, you enter this state). With Screen Off/Sleep,
there is no such requirement. For Screen Off, the general idea of a
screen is used, but sleep is completely arbitrary and in Windows is
defined by which software inhibitors lapse. This makes more sense
because even for LPS0/DRIPS in Windows, the way it enters it is
programmatic now as well (after all software inhibitors lapse). To
that end, there are three types of inhibitions in Windows, one for
screen on (such as video), screen off (such as compiling a kernel,
writing a CD), and sleep (periodic system processes; email
notifications; low CPU%).

Antheas

[1] https://lore.kernel.org/all/20241121172239.119590-1-lkml@antheas.dev/

> +Date:          November 2025
> +Contact:       Dmitrii Osipenko <dmitry.osipenko@...labora.com>
> +Description:
> +               This file is available if the ACPI/OSPM system supports
> +               Display Off/On DSM notifications. It controls state of the
> +               notification.
> +
> +               Writing a "1" to this file invokes Display Off Notification.
> +               Writing a "0" to this file invokes Display On Notification.
> +
> +               Notifications are only triggered on state transitions.
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 6d4d06236f61..d5cb5e22431d 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -18,7 +18,10 @@
>  #include <linux/acpi.h>
>  #include <linux/device.h>
>  #include <linux/dmi.h>
> +#include <linux/kobject.h>
> +#include <linux/mutex.h>
>  #include <linux/suspend.h>
> +#include <linux/sysfs.h>
>
>  #include "../sleep.h"
>
> @@ -61,6 +64,11 @@ static guid_t lps0_dsm_guid_microsoft;
>  static int lps0_dsm_func_mask_microsoft;
>  static int lps0_dsm_state;
>
> +static DEFINE_MUTEX(lps0_dsm_screen_off_lock);
> +static bool lps0_dsm_screen_state_off;
> +static bool lps0_screen_off_suspended;
> +static bool lps0_screen_off_sysfs_inhibit;
> +
>  /* Device constraint entry structure */
>  struct lpi_device_info {
>         char *name;
> @@ -513,6 +521,76 @@ static struct acpi_scan_handler lps0_handler = {
>         .attach = lps0_device_attach,
>  };
>
> +static bool lps0_has_screen_off_dsm(void)
> +{
> +       int id = acpi_s2idle_vendor_amd() ?
> +                ACPI_LPS0_SCREEN_ON_AMD : ACPI_LPS0_SCREEN_OFF;
> +
> +       if (lps0_dsm_func_mask_microsoft > 0 &&
> +           (lps0_dsm_func_mask & BIT(ACPI_LPS0_SCREEN_OFF)))
> +               return true;
> +
> +       if (lps0_dsm_func_mask > 0 && (lps0_dsm_func_mask & BIT(id)))
> +               return true;
> +
> +       return false;
> +}
> +
> +static void lps0_dsm_screen_off(void)
> +{
> +       if (lps0_dsm_screen_state_off)
> +               return;
> +
> +       if (lps0_dsm_func_mask > 0)
> +               acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
> +                                       ACPI_LPS0_SCREEN_OFF_AMD :
> +                                       ACPI_LPS0_SCREEN_OFF,
> +                                       lps0_dsm_func_mask, lps0_dsm_guid);
> +
> +       if (lps0_dsm_func_mask_microsoft > 0)
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> +                                       lps0_dsm_func_mask_microsoft,
> +                                       lps0_dsm_guid_microsoft);
> +
> +       lps0_dsm_screen_state_off = true;
> +}
> +
> +static void lps0_dsm_screen_on(void)
> +{
> +       if (!lps0_dsm_screen_state_off)
> +               return;
> +
> +       if (lps0_dsm_func_mask_microsoft > 0)
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_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,
> +                                       lps0_dsm_func_mask, lps0_dsm_guid);
> +
> +       lps0_dsm_screen_state_off = false;
> +}
> +
> +static void lps0_dsm_screen_off_set(int sysfs_off, int suspended)
> +{
> +       mutex_lock(&lps0_dsm_screen_off_lock);
> +
> +       if (sysfs_off > -1)
> +               lps0_screen_off_sysfs_inhibit = sysfs_off;
> +       if (suspended > -1)
> +               lps0_screen_off_suspended = suspended;
> +
> +       if (lps0_screen_off_suspended || lps0_screen_off_sysfs_inhibit)
> +               lps0_dsm_screen_off();
> +       else
> +               lps0_dsm_screen_on();
> +
> +       mutex_unlock(&lps0_dsm_screen_off_lock);
> +}
> +
>  static int acpi_s2idle_begin_lps0(void)
>  {
>         if (pm_debug_messages_on && !lpi_constraints_table) {
> @@ -543,15 +621,7 @@ static int acpi_s2idle_prepare_late_lps0(void)
>                 lpi_check_constraints();
>
>         /* 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,
> -                                       lps0_dsm_func_mask, lps0_dsm_guid);
> -
> -       if (lps0_dsm_func_mask_microsoft > 0)
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> -                               lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +       lps0_dsm_screen_off_set(-1, true);
>
>         /* LPS0 entry */
>         if (lps0_dsm_func_mask > 0 && acpi_s2idle_vendor_amd())
> @@ -618,14 +688,7 @@ static void acpi_s2idle_restore_early_lps0(void)
>         }
>
>         /* Screen on */
> -       if (lps0_dsm_func_mask_microsoft > 0)
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_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,
> -                                       lps0_dsm_func_mask, lps0_dsm_guid);
> +       lps0_dsm_screen_off_set(-1, false);
>  }
>
>  static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
> @@ -673,4 +736,56 @@ void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg)
>  }
>  EXPORT_SYMBOL_GPL(acpi_unregister_lps0_dev);
>
> +static ssize_t lps0_screen_off_store(struct kobject *kobj,
> +                                    struct kobj_attribute *attr,
> +                                    const char *buf, size_t count)
> +{
> +       unsigned long val;
> +
> +       if (kstrtoul(buf, 10, &val))
> +               return -EINVAL;
> +
> +       if (val > 1)
> +               return -EINVAL;
> +
> +       lps0_dsm_screen_off_set(val, -1);
> +
> +       return count;
> +}
> +
> +static ssize_t lps0_screen_off_show(struct kobject *kobj,
> +                                   struct kobj_attribute *attr,
> +                                   char *buf)
> +{
> +       return sprintf(buf, "%d\n", lps0_screen_off_sysfs_inhibit);
> +}
> +
> +static struct kobj_attribute lps0_screen_off_attr =
> +       __ATTR(lps0_screen_off, 0644,
> +              lps0_screen_off_show, lps0_screen_off_store);
> +
> +static struct attribute *lps0_screen_off_attrs[] = {
> +       &lps0_screen_off_attr.attr,
> +       NULL,
> +};
> +
> +static struct attribute_group lps0_screen_off_attr_group = {
> +       .attrs = lps0_screen_off_attrs,
> +};
> +
> +static int lps0_dsm_screen_off_init(void)
> +{
> +       int ret;
> +
> +       if (!lps0_has_screen_off_dsm())
> +               return 0;
> +
> +       ret = sysfs_create_group(power_kobj, &lps0_screen_off_attr_group);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +late_initcall(lps0_dsm_screen_off_init);
> +
>  #endif /* CONFIG_SUSPEND */
> --
> 2.51.1
>
>

Powered by blists - more mailing lists