[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1b0d2349-dbf7-47aa-95c9-1974e63d111a@kernel.org>
Date: Fri, 27 Jun 2025 09:06:10 -0500
From: Mario Limonciello <superm1@...nel.org>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Hans de Goede <hansg@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Mika Westerberg <westeri@...nel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
<brgl@...ev.pl>, "open list:GPIO ACPI SUPPORT" <linux-gpio@...r.kernel.org>,
"open list:GPIO ACPI SUPPORT" <linux-acpi@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)..."
<linux-input@...r.kernel.org>, Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH v3 4/4] Input: Don't send fake button presses to wake
system
On 6/26/2025 11:56 PM, Dmitry Torokhov wrote:
> On Thu, Jun 26, 2025 at 05:21:35PM -0500, Mario Limonciello wrote:
>> 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@...il.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@...nel.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.
>
> For the latter you may want to raise an issue with systemd, and for the
> former I guess it is being too clever and does not activate the
> workaround if suspend was not initiated by it? I think Gnome is being
> too careful.
>
> Thanks.
>
Sure I could file bugs with both the projects.
But before I do if all userspace needs to account for this with a series
of workarounds at resume time, you still think that is that really the
best way forward?
Hans, you have a lot of experience in the GNOME community. Your thoughts?
Powered by blists - more mailing lists