Re: [PATCH v3 4/4] Input: Don't send fake button presses to wake system

[Mario Limonciello <superm1@kernel.org>]

On 6/26/2025 2:40 PM, Dmitry Torokhov wrote:
> On Thu, Jun 26, 2025 at 09:31:12PM +0200, Rafael J. Wysocki wrote:
>> On Thu, Jun 26, 2025 at 9:28 PM Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>>
>>> On Thu, Jun 26, 2025 at 09:18:56PM +0200, Rafael J. Wysocki wrote:
>>>> On Thu, Jun 26, 2025 at 9:16 PM Hans de Goede <hansg@kernel.org> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 26-Jun-25 21:14, Dmitry Torokhov wrote:
>>>>>> On Thu, Jun 26, 2025 at 08:57:30PM +0200, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 26-Jun-25 20:48, Dmitry Torokhov wrote:
>>>>>>>> On Thu, Jun 26, 2025 at 01:20:54PM -0500, Mario Limonciello wrote:
>>> [...]
>>>>>>>>> I want to note this driver works quite differently than how ACPI power
>>>>>>>>> button does.
>>>>>>>>>
>>>>>>>>> You can see in acpi_button_notify() that the "keypress" is only forwarded
>>>>>>>>> when not suspended [1].  Otherwise it's just wakeup event (which is what my
>>>>>>>>> patch was modeling).
>>>>>>>>>
>>>>>>>>> https://github.com/torvalds/linux/blob/v6.16-rc3/drivers/acpi/button.c#L461
>>>>>>>>> [1]
>>>>>>>>
>>>>>>>> If you check acpi_button_resume() you will see that the events are sent
>>>>>>>> from there. Except that for some reason they chose to use KEY_WAKEUP and
>>>>>>>> not KEY_POWER, oh well. Unlike acpi button driver gpio_keys is used on
>>>>>>>> multiple other platforms.
>>>>>>>
>>>>>>> Interesting, but the ACPI button code presumably only does this on resume
>>>>>>> for a normal press while the system is awake it does use KEY_POWER, right ?
>>>>>>
>>>>>> Yes. It is unclear to me why they chose to mangle the event on wakeup,
>>>>>> it does not seem to be captured in the email discussions or in the patch
>>>>>> description.
>>>>>
>>>>> I assume they did this to avoid the immediate re-suspend on wakeup by
>>>>> power-button issue. GNOME has a workaround for this, but I assume that
>>>>> some userspace desktop environments are still going to have a problem
>>>>> with this.
>>>>
>>>> It was done for this reason IIRC, but it should have been documented
>>>> more thoroughly.
>>>
>>> I assert that it should not have been done and instead dealt with in
>>> userspace. There are numerous drivers in the kernel emitting
>>> KEY_POWER. Let userspace decide how to handle this, what keys to ignore,
>>> what keys to process and when.
>>
>> Please see my last message in this thread (just sent) and see the
>> changelog of commit 16f70feaabe9 ("ACPI: button: trigger wakeup key
>> events").
>>
>> This appears to be about cases when no event would be signaled to user
>> space at all (power button wakeup from ACPI S3).
> 
> Ahh, in S3 we do not know if we've been woken up with Sleep or Power
> button, right? So we can not send the "right" event code and use
> "neutral" KEY_WAKEUP for both. Is this right?
> 
> Thanks.
> 

I did some more experiments with this affected system that started this 
thread (which uses s2idle).

I only applied patch 3 in this series to help the debounce behavior and 
figure out impacts from patch 4 with existing Linux userspace.

If suspended using systemd in GNOME (click the GUI button) on Ubuntu 
24.04 the GNOME workaround mitigates this problem and no visible impact.

If I suspend by hand using the kernel interface and then press power 
button to wake:

# echo mem | sudo tee /sys/power/state:

* When GNOME is running:
I get the shutdown popup and it eventually shuts down.

* When GNOME isn't running (just on a VT):
System shuts down.