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@xxxxxxxxx> 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@xxxxxxxxxx> 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. -- Dmitry