| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1654cfc3-ff8a-4f7f-9929-0442f2b3bb2b@kernel.org>
Date: Mon, 1 Dec 2025 22:43:31 -0600
From: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Robert Beckett <bob.beckett@...labora.com>,
"open list:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>
Cc: 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>,
Antheas Kapenekakis <lkml@...heas.dev>
Subject: Re: [RFC PATCH v1 1/1] ACPI: PM: s2idle: Add lps0_screen_off sysfs
interface
++ dri-devel
On 12/1/2025 10:34 PM, Dmitry Osipenko 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.
As this has to do with actions when displays are off I think you should
also CC dri-devel on the next submission.
>
> 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
> +Date: November 2025
This should be matching the kernel you're targeting this to land, which
is definitely not in the past.
> +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)
I don't really like passing in arbitrary integers, it makes this code
hard to follow.
Could you perhaps use an enum instead?
> +{
> + mutex_lock(&lps0_dsm_screen_off_lock);
> +
I'd use a guard(mutex) here instead.
> + 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;
Based upon how you are using this maybe just use kstrtobool() instead.
> +
> + 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);
You should use sysfs_emit().
> +}
> +
> +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 */
Powered by blists - more mailing lists