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

[Mario Limonciello <superm1@kernel.org>]

On 12/3/25 4:12 AM, Antheas Kapenekakis wrote:
> On Wed, 3 Dec 2025 at 07:47, Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
>>
>> On 12/3/25 05:12, Mario Limonciello (AMD) (kernel.org) wrote:
>>>
>>>
>>> On 12/2/2025 4:35 PM, Dmitry Osipenko wrote:
>>>> On 12/3/25 00:25, Mario Limonciello (AMD) (kernel.org) wrote:
>>>>>> An inhibitor process in logind can handle this gracefully very simply.
>>>>>> Involving the DRM subsystem just adds a lot of complexity and it is
>>>>>> not clear what the benefit would be. There are no known devices that
>>>>>> hook DRM components into that DSM.
>>>>>>
>>>>>>> By doing it this way userspace still has control, but it's not
>>>>>>> mandatory for userspace to be changed.
>>>>>>
>>>>>> On that note, the screen off calls/userspace implementations are
>>>>>> optional under both patch series. If userspace is not aware of them,
>>>>>> they are still called by the kernel when suspending.
>>>>>
>>>>> With the proposal I mentioned you can get the LPS0 _DSM called on a
>>>>> handheld when the screen gets called without changing userspace.
>>>>>
>>>>>>
>>>>>> Current userland also duplicates the functionality of the screen off
>>>>>> call, which is primarily turning off the keyboard backlight. So along
>>>>>> implementing this call, userspace software like powerdevil/upower
>>>>>> needs to be tweaked to avoid doing that if the screen off state is
>>>>>> available.
>>>>>
>>>>> Sure Any hooking for turning off LEDs manually based off the screen off
>>>>> _DSM is totally feasible.
>>>>
>>>> It's not that trivial to add screen on/off hooks to DRM, there is no one
>>>> central place for that from what I can tell. I'm seeing variant with DRM
>>>> hooks as unnecessary complexity that doesn't solve any practical problem.
>>>
>>> Is it really that hard?  I figured that any time
>>> connector->dpms != mode from drm_atomic_connector_commit_dpms() could
>>> walk through all the connectors and make a judgement call whether to
>>> notify the potentially exported symbol.
>>
>> - drm_atomic_connector_commit_dpms() is used only for atomic ioctl path
>> - there is another legacy kms path
>> - AFAICT, DRM takes a different path when display is enabled initially
>> by kernel
>>
>> Here we have 3 places where to plug the hook. Gives a strong feeling of
>> a red flag, IMO.

It could easily be a symbol that all 3 places call which enumerates 
connectors and decides whether to call the LPS0 symbol.

But yeah; you might be right it's too complex.

>>
>>>> A week ago in a private conversation, Daniel Stone gave an example of
>>>> laptop's lid-close handling that is done purely in userspace.
>>>> Technically, kernel could have DRM hooks for that too, but it doesn't.
>>>
>>> All the way into hardware sleep?  There are certain requirements needed
>>> for hardware sleep that kernel drivers are normally used to put devices
>>> into the right state.  I guess PCIe devices you can hack around with
>>> userspace PCI config space writes but you're going to confuse the kernel
>>> pretty badly.
>>
>> - Userspace gets notification for a changed lid state
>> - Userspace takes action of turning display on/off
>> - Kernel DRM doesn't know and doesn't care about lid state,
>> force-disabling display on machine suspension
>>
>> Don't see how this is different for the case of the LPS0 notifications.
>> Maybe I'm not getting your point well, in that case please clarify more.

I thought you were talking about total sleep handling, but you're just 
talking about other userspace events related to a lid.  This is much 
different.

>>
>>>> Userspace would need to be taught about new power modes in any case.
>>>> Addition of DRM hooks should require a well-defined justification, which
>>>> is currently absent.
>>>>
>>>
>>> Why does userspace need to know about them?  Besides the inhibitor can't
>>> this be invisible to userspace?  I thought this mostly is for the
>>> firmware to flash some LEDs and maybe change some power limits.
>>
>> What I was saying is that LPS0 inhibitors would represent the power mode
>> controls by themselves. Userspace would have to know how to drive them.
>>
>> Userspace power managers are already driving displays DPMS. Combining
>> this with knowledge about the LPS0 inhibitors gives userspace ability to
>> support the new device power states. Hence, there is no practical need
>> to bother kernel DRM with the LPS0 burden.
> 
> _Technically_ DPMS is driven by the compositor, not by any power manager.
> 
> However, I think for Gnome and KDE they link to logind's inactivity
> hook so practically you are correct for inactivity.
> 
> Moreover, traditionally, compositors do not fire DPMS for suspend, the
> kernel does. I think KDE fixed that recently though. This is why the
> display used to wake up twice during hibernation, and why you get a
> frozen display when suspending/resuming. This also complicates
> suspend-then-hibernate checks because the display wakes up. i.e., they
> should fire DPMS for suspend but a lot for them don't
> 
> With compositors such as gamescope that do not have a dbus API it gets
> more hairy. And it also means that the power manager/kernel cannot
> control DPMS without the compositor's consent. If they do that, the
> compositor will crash due to rejected commits. We have a suspend hook
> for gamescope on Bazzite though, it improves suspend appearance.

It's OT and tangential to this thread; but Gamescope should probably 
grow native DPMS control for this particular case.

> 
> Just throwing in this for context, although it builds more of a case
> of not involving DRM.
> 

I don't feel it's appropriate to build new API in the kernel to work 
around shortcomings of an individual compositor.