Re: [RFC PATCH v1 1/1] ACPI: PM: s2idle: Add lps0_screen_off sysfs interface

[Mario Limonciello <superm1@kernel.org>]

On 12/2/25 3:32 AM, Antheas Kapenekakis wrote:
> On Tue, 2 Dec 2025 at 05:36, Dmitry Osipenko
> <dmitry.osipenko@collabora.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@collabora.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.

Why would you want to expose all those states to userspace?  I feel like 
it is going to be risky to have userspace changing the state machine for 
suspend like that.

Since the _DSM call that is interesting here is focusing specifically on 
screen off I have a slightly different proposal on how this could work.

What about if instead of an explicit userspace calling interface it's an 
inhibition/voting interface:

While in screen on:
* By default no inhibitions are set.
* If no inhibitions are set and all physical displays go into DPMS then 
DRM can do an call (using an exported symbol) to enter screen off.
* If userspace is using a remote display it could set an inhibition.
* When the inhibition is cleared (IE userspace indicates that a remote 
display is no longer in use) then:
   * if all physical displays are already off call screen off.
   * if at least one physical display is on do nothing (turning off 
physical displays would call screen off)

While in screen off
* When a physical display is turned DRM would use exported symbol to 
call screen on.
  * When  an inhibitor is added call screen ON.

By doing it this way userspace still has control, but it's not 
*mandatory* for userspace to be changed.