[RFC v2 00/10] acpi/x86: s2idle: Introduce and implement hint class ABI and idle hint for s2idle

[RFC v2 00/10] acpi/x86: s2idle: Introduce and implement hint class ABI and idle hint for s2idle

[Antheas Kapenekakis]

This is a two-part RFC series that in the first part cleans up the Modern
Standby firmware notification order and in the second part exposes them to
userspace using a new hint class ABI that can be extended in the future to
add more userspace hints to devices.

For more information, see the V1 description.

In the first part, we begin by renaming the s2idle.c constants to match
their spec from Windows. Following, we place all MS firmware notifications
to begin() and end() to match Windows, ensuring that the USB subsystems and
EC are active when they fire, eliminating a class of bugs due to the
mismatch. Finally, we add a small delay quirk to begin that is required by
ROG Ally devices, which is bound to asus-wmi through the handler interface
exposed by s2idle.

Since these bugs completely fix the sleep behavior for all models and
firmware versions of Ally devices, including those introduced with the
previous suspend fixes when mcu_powersave is off (which is part of the
reason the driver forced it to on; not just standby battery drain), a
cleanup follows that removes the duplicate handling of CSEE (the underlying
function of the Display On/Off _DSMs and which malfunctioned because the
USB subsystem was suspended during its calls) and the now unnecessary
firmware version checks and error logging.

In the second part, a new /sys/class/hint interface is introduced with an
initial hint, "idle", which allows modifying the idle state of the device.
I chose the name "idle" instead of "activity" to leave "activity" free to
use in the future for e.g. "gaming", "video", which might be useful
especially for embedded devices. "idle" might alias with "s2idle" so the
name is not set in stone.

Each driver that wants to implement hints can register a hint class and add
hints to it, which are then exposed in /sys/class/hint/<class>/idle
allowing them to receive updates as well. Current WMI drivers should not,
as their behavior is already handled by the initial s2idle idle hint.
However, non-MS devices can e.g. the ChromeOS EC or even the CPU scheduler.

The two parts are designed to be independent of each other, so that we can
merge the re-order in an initial kernel version and the hint interface in a
later kernel version, if needed. As such, after the first part is cleaned
up it will be resent as non-RFC and the Asus/hid/px86 maintainers will be
cc'd. Then, we will iterate on the hint interface.

To test this series, it is recommended to enable logs with:

sudo bash -c "cat > /sys/kernel/debug/dynamic_debug/control" << EOF
file drivers/acpi/x86/s2idle.c +p
file kernel/power/suspend.c +p
file drivers/base/hint.c +p
EOF
echo 1 | sudo tee /sys/power/pm_debug_messages

Here are some commands afterwards:

echo "active" | sudo tee /sys/class/hint/s2idle/idle; cat /sys/class/hint/s2idle/idle
echo "inactive" | sudo tee /sys/class/hint/s2idle/idle; cat /sys/class/hint/s2idle/idle
echo "snooze" | sudo tee /sys/class/hint/s2idle/idle; cat /sys/class/hint/s2idle/idle

I have not tested this series on Ally devices yet, focusing on the Z13.
A previous version of the series performed correctly and is what was used
in e.g., Bazzite, with around 5-10k Ally users.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---

Changes since v1:
  - Rebase to v7.0
  - Other than the first patch, the rest are re-written.
  - The hint class is based on a simplified version of platform-profiles
  - Transition handling is now done in s2idle.c exclusively and is not
    part of the main ABI. Drivers receive idle hints updates instead.
  - The rename patch now also renames upstream ACPI_MS_TURN_ON_DISPLAY to
    ACPI_LPS0_TURN_ON_DISPLAY for consistency. It is also fired
    unconditionally now by the transition handler in patch n-1.

V1: https://lore.kernel.org/all/20251226102656.6296-1-lkml@antheas.dev/

Antheas Kapenekakis (10):
  acpi/x86: s2idle: Rename LPS0 constants so they mirror their function
  acpi/x86: s2idle: Move Modern Standby calls to s2idle begin/end
  acpi/x86: s2idle: Add support for adding a delay after begin MS calls
  platform/x86: asus-wmi: add s2idle begin delay for Ally devices
  HID: asus: remove quirk handling for Ally devices
  platform/x86: asus-wmi: Remove Ally s2idle resume fixes
  Documentation: Add documentation for the new sysfs hints class
  hint: Add hint class ABI for devices to receive updates on host
    activity
  acpi/x86: s2idle: Listen to idle hints to perform MS transitions
  acpi/x86: s2idle: Subtract delay from last DSM fire in begin delay

 Documentation/ABI/testing/sysfs-class-hint |  78 ++++++
 MAINTAINERS                                |   8 +
 drivers/acpi/Kconfig                       |   1 +
 drivers/acpi/x86/s2idle.c                  | 309 ++++++++++++++++-----
 drivers/base/Kconfig                       |   3 +
 drivers/base/Makefile                      |   1 +
 drivers/base/hint.c                        | 283 +++++++++++++++++++
 drivers/hid/hid-asus.c                     | 113 +-------
 drivers/platform/x86/asus-nb-wmi.c         |  40 +++
 drivers/platform/x86/asus-wmi.c            | 132 +--------
 drivers/platform/x86/asus-wmi.h            |  14 +
 include/linux/acpi.h                       |   1 +
 include/linux/hint.h                       |  38 +++
 include/linux/platform_data/x86/asus-wmi.h |   8 -
 14 files changed, 727 insertions(+), 302 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-hint
 create mode 100644 drivers/base/hint.c
 create mode 100644 include/linux/hint.h


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.53.0

[RFC v2 01/10] acpi/x86: s2idle: Rename LPS0 constants so they mirror their function

[Antheas Kapenekakis]

The LPS0 3/4 constants are part of a firmware notification called
"Display on/off", in which the device enters a "Screen Off" state.
The LPS0 7/8 constants are part of a firmware notification in which
a Windows modern standby computer enters a "sleep" state where the
CPU may still be active and maintain the radios.

However, currently, the values are named as "Screen on/off" and "MS
entry/exit", where Modern Standby is abbreviated as "MS". Therefore,
perform a minor rename so that the values match their function. Then,
fix the debug message to say that it executes notifications instead of
entering states as it is otherwise confusing.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/acpi/x86/s2idle.c | 84 +++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index b6b1dd76a06b..61a044b59776 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -43,20 +43,20 @@ static const struct acpi_device_id lps0_device_ids[] = {
 #define ACPI_LPS0_DSM_UUID	"c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
 
 #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS	1
-#define ACPI_LPS0_SCREEN_OFF	3
-#define ACPI_LPS0_SCREEN_ON	4
+#define ACPI_LPS0_DISPLAY_OFF	3
+#define ACPI_LPS0_DISPLAY_ON	4
 #define ACPI_LPS0_ENTRY		5
 #define ACPI_LPS0_EXIT		6
-#define ACPI_LPS0_MS_ENTRY      7
-#define ACPI_LPS0_MS_EXIT       8
-#define ACPI_MS_TURN_ON_DISPLAY 9
+#define ACPI_LPS0_SLEEP_ENTRY		7
+#define ACPI_LPS0_SLEEP_EXIT		8
+#define ACPI_LPS0_TURN_ON_DISPLAY	9
 
 /* AMD */
 #define ACPI_LPS0_DSM_UUID_AMD      "e3f32452-febc-43ce-9039-932122d37721"
 #define ACPI_LPS0_ENTRY_AMD         2
 #define ACPI_LPS0_EXIT_AMD          3
-#define ACPI_LPS0_SCREEN_OFF_AMD    4
-#define ACPI_LPS0_SCREEN_ON_AMD     5
+#define ACPI_LPS0_DISPLAY_OFF_AMD   4
+#define ACPI_LPS0_DISPLAY_ON_AMD    5
 
 static acpi_handle lps0_device_handle;
 static guid_t lps0_dsm_guid;
@@ -343,27 +343,27 @@ static const char *acpi_sleep_dsm_state_to_str(unsigned int state)
 {
 	if (lps0_dsm_func_mask_microsoft || !acpi_s2idle_vendor_amd()) {
 		switch (state) {
-		case ACPI_LPS0_SCREEN_OFF:
-			return "screen off";
-		case ACPI_LPS0_SCREEN_ON:
-			return "screen on";
+		case ACPI_LPS0_DISPLAY_OFF:
+			return "display off";
+		case ACPI_LPS0_DISPLAY_ON:
+			return "display on";
 		case ACPI_LPS0_ENTRY:
 			return "lps0 entry";
 		case ACPI_LPS0_EXIT:
 			return "lps0 exit";
-		case ACPI_LPS0_MS_ENTRY:
-			return "lps0 ms entry";
-		case ACPI_LPS0_MS_EXIT:
-			return "lps0 ms exit";
-		case ACPI_MS_TURN_ON_DISPLAY:
-			return "lps0 ms turn on display";
+		case ACPI_LPS0_SLEEP_ENTRY:
+			return "sleep entry";
+		case ACPI_LPS0_SLEEP_EXIT:
+			return "sleep exit";
+		case ACPI_LPS0_TURN_ON_DISPLAY:
+			return "turn on display";
 		}
 	} else {
 		switch (state) {
-		case ACPI_LPS0_SCREEN_ON_AMD:
-			return "screen on";
-		case ACPI_LPS0_SCREEN_OFF_AMD:
-			return "screen off";
+		case ACPI_LPS0_DISPLAY_ON_AMD:
+			return "display on";
+		case ACPI_LPS0_DISPLAY_OFF_AMD:
+			return "display off";
 		case ACPI_LPS0_ENTRY_AMD:
 			return "lps0 entry";
 		case ACPI_LPS0_EXIT_AMD:
@@ -388,7 +388,7 @@ static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, g
 	lps0_dsm_state = func;
 	if (pm_debug_messages_on) {
 		acpi_handle_info(lps0_device_handle,
-				"%s transitioned to state %s\n",
+				"%s executed notification %s\n",
 				 out_obj ? "Successfully" : "Failed to",
 				 acpi_sleep_dsm_state_to_str(lps0_dsm_state));
 	}
@@ -545,16 +545,17 @@ static int acpi_s2idle_prepare_late_lps0(void)
 	if (check_lps0_constraints)
 		lpi_check_constraints();
 
-	/* Screen off */
+	/* Display off */
 	if (lps0_dsm_func_mask > 0)
 		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
-					ACPI_LPS0_SCREEN_OFF_AMD :
-					ACPI_LPS0_SCREEN_OFF,
+					ACPI_LPS0_DISPLAY_OFF_AMD :
+					ACPI_LPS0_DISPLAY_OFF,
 					lps0_dsm_func_mask, lps0_dsm_guid);
 
 	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
 
 	/* LPS0 entry */
 	if (lps0_dsm_func_mask > 0 && acpi_s2idle_vendor_amd())
@@ -563,10 +564,12 @@ static int acpi_s2idle_prepare_late_lps0(void)
 
 	if (lps0_dsm_func_mask_microsoft > 0) {
 		/* Modern Standby entry */
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_ENTRY,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
 	}
 
 	if (lps0_dsm_func_mask > 0 && !acpi_s2idle_vendor_amd())
@@ -616,21 +619,24 @@ static void acpi_s2idle_restore_early_lps0(void)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
 		/* Intent to turn on display */
-		acpi_sleep_run_lps0_dsm(ACPI_MS_TURN_ON_DISPLAY,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_TURN_ON_DISPLAY,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
 		/* Modern Standby exit */
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_EXIT,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
 	}
 
-	/* Screen on */
+	/* Display on */
 	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_ON,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
 	if (lps0_dsm_func_mask > 0)
 		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
-					ACPI_LPS0_SCREEN_ON_AMD :
-					ACPI_LPS0_SCREEN_ON,
+					ACPI_LPS0_DISPLAY_ON_AMD :
+					ACPI_LPS0_DISPLAY_ON,
 					lps0_dsm_func_mask, lps0_dsm_guid);
 }
 
-- 
2.53.0

Re: [RFC v2 01/10] acpi/x86: s2idle: Rename LPS0 constants so they mirror their function

[Rafael J. Wysocki]

On Sat, Apr 25, 2026 at 11:57 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> The LPS0 3/4 constants are part of a firmware notification called
> "Display on/off", in which the device enters a "Screen Off" state.
> The LPS0 7/8 constants are part of a firmware notification in which
> a Windows modern standby computer enters a "sleep" state where the
> CPU may still be active and maintain the radios.
>
> However, currently, the values are named as "Screen on/off" and "MS
> entry/exit", where Modern Standby is abbreviated as "MS". Therefore,
> perform a minor rename so that the values match their function. Then,
> fix the debug message to say that it executes notifications instead of
> entering states as it is otherwise confusing.
>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/acpi/x86/s2idle.c | 84 +++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index b6b1dd76a06b..61a044b59776 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -43,20 +43,20 @@ static const struct acpi_device_id lps0_device_ids[] = {
>  #define ACPI_LPS0_DSM_UUID     "c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
>
>  #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS       1
> -#define ACPI_LPS0_SCREEN_OFF   3
> -#define ACPI_LPS0_SCREEN_ON    4
> +#define ACPI_LPS0_DISPLAY_OFF  3
> +#define ACPI_LPS0_DISPLAY_ON   4

You've clearly ignored my feedback on this.

Thanks!

Re: [RFC v2 01/10] acpi/x86: s2idle: Rename LPS0 constants so they mirror their function

[Antheas Kapenekakis]

On Sun, 26 Apr 2026 at 16:15, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, Apr 25, 2026 at 11:57 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > The LPS0 3/4 constants are part of a firmware notification called
> > "Display on/off", in which the device enters a "Screen Off" state.
> > The LPS0 7/8 constants are part of a firmware notification in which
> > a Windows modern standby computer enters a "sleep" state where the
> > CPU may still be active and maintain the radios.
> >
> > However, currently, the values are named as "Screen on/off" and "MS
> > entry/exit", where Modern Standby is abbreviated as "MS". Therefore,
> > perform a minor rename so that the values match their function. Then,
> > fix the debug message to say that it executes notifications instead of
> > entering states as it is otherwise confusing.
> >
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >  drivers/acpi/x86/s2idle.c | 84 +++++++++++++++++++++------------------
> >  1 file changed, 45 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> > index b6b1dd76a06b..61a044b59776 100644
> > --- a/drivers/acpi/x86/s2idle.c
> > +++ b/drivers/acpi/x86/s2idle.c
> > @@ -43,20 +43,20 @@ static const struct acpi_device_id lps0_device_ids[] = {
> >  #define ACPI_LPS0_DSM_UUID     "c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
> >
> >  #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS       1
> > -#define ACPI_LPS0_SCREEN_OFF   3
> > -#define ACPI_LPS0_SCREEN_ON    4
> > +#define ACPI_LPS0_DISPLAY_OFF  3
> > +#define ACPI_LPS0_DISPLAY_ON   4
>
> You've clearly ignored my feedback on this.

Hi,

did https://lore.kernel.org/all/CAGwozwE_QbiNk53+kTjwELtWhK31LCrBm5fgx2b46sZa_5wjaA@mail.gmail.com/
not address your feedback?

The notifications are named display off / on in the document you linked.

I did not receive a response so I considered it resolved.

Antheas

> Thanks!
>

Re: [RFC v2 01/10] acpi/x86: s2idle: Rename LPS0 constants so they mirror their function

[Rafael J. Wysocki]

On Mon, Apr 27, 2026 at 4:08 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Sun, 26 Apr 2026 at 16:15, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Sat, Apr 25, 2026 at 11:57 PM Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > >
> > > The LPS0 3/4 constants are part of a firmware notification called
> > > "Display on/off", in which the device enters a "Screen Off" state.
> > > The LPS0 7/8 constants are part of a firmware notification in which
> > > a Windows modern standby computer enters a "sleep" state where the
> > > CPU may still be active and maintain the radios.
> > >
> > > However, currently, the values are named as "Screen on/off" and "MS
> > > entry/exit", where Modern Standby is abbreviated as "MS". Therefore,
> > > perform a minor rename so that the values match their function. Then,
> > > fix the debug message to say that it executes notifications instead of
> > > entering states as it is otherwise confusing.
> > >
> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > >  drivers/acpi/x86/s2idle.c | 84 +++++++++++++++++++++------------------
> > >  1 file changed, 45 insertions(+), 39 deletions(-)
> > >
> > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> > > index b6b1dd76a06b..61a044b59776 100644
> > > --- a/drivers/acpi/x86/s2idle.c
> > > +++ b/drivers/acpi/x86/s2idle.c
> > > @@ -43,20 +43,20 @@ static const struct acpi_device_id lps0_device_ids[] = {
> > >  #define ACPI_LPS0_DSM_UUID     "c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
> > >
> > >  #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS       1
> > > -#define ACPI_LPS0_SCREEN_OFF   3
> > > -#define ACPI_LPS0_SCREEN_ON    4
> > > +#define ACPI_LPS0_DISPLAY_OFF  3
> > > +#define ACPI_LPS0_DISPLAY_ON   4
> >
> > You've clearly ignored my feedback on this.
>
> Hi,
>
> did https://lore.kernel.org/all/CAGwozwE_QbiNk53+kTjwELtWhK31LCrBm5fgx2b46sZa_5wjaA@mail.gmail.com/
> not address your feedback?

I missed that one, sorry.

> The notifications are named display off / on in the document you linked.
>
> I did not receive a response so I considered it resolved.

OK, fair enough.

I'll review the whole series later this week if all goes well.

[RFC v2 02/10] acpi/x86: s2idle: Move Modern Standby calls to s2idle begin/end

[Antheas Kapenekakis]

In Windows, the modern standby calls for sleep entry/exit and display
on/off happen while the kernel device subsystems are active and the
device is asleep. Currently, in the Linux kernel they happen in
prepare_late, after e.g. the USB subsystem has turned off. This
disimilarity causes obscure issues in certain devices that use these
calls to turn off peripherals that should not be active during modern
standby, e.g. handheld controllers, and RGB.

Therefore, move these calls to _begin(), and _end() to match Windows.
Particularly for _end(), introduce a acpi_s2idle_end_lps0() function to
wrap acpi_s2idle_end(), matching the structure introduced with
acpi_s2idle_begin_lps0().

Of note is that unlike the ACPI ABI of LPS0, there is no device power
state requirement before entering sleep/screen off, therefore it is
appropriate to move these calls to _begin(), before s2idle suspend.

Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/acpi/x86/s2idle.c | 63 +++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 61a044b59776..f5aefba8b191 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -517,8 +517,10 @@ static struct acpi_scan_handler lps0_handler = {
 
 static int acpi_s2idle_begin_lps0(void)
 {
-	if (lps0_device_handle && !sleep_no_lps0 && check_lps0_constraints &&
-	    !lpi_constraints_table) {
+	if (!lps0_device_handle || sleep_no_lps0)
+		return acpi_s2idle_begin();
+
+	if (check_lps0_constraints && !lpi_constraints_table) {
 		if (acpi_s2idle_vendor_amd())
 			lpi_device_get_constraints_amd();
 		else
@@ -532,6 +534,24 @@ static int acpi_s2idle_begin_lps0(void)
 			lpi_constraints_table = ERR_PTR(-ENODATA);
 	}
 
+	/* Display off */
+	if (lps0_dsm_func_mask > 0)
+		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
+					ACPI_LPS0_DISPLAY_OFF_AMD :
+					ACPI_LPS0_DISPLAY_OFF,
+					lps0_dsm_func_mask, lps0_dsm_guid);
+
+	if (lps0_dsm_func_mask_microsoft > 0)
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
+
+	/* Modern Standby entry */
+	if (lps0_dsm_func_mask_microsoft > 0)
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_ENTRY,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
+
 	return acpi_s2idle_begin();
 }
 
@@ -545,36 +565,17 @@ static int acpi_s2idle_prepare_late_lps0(void)
 	if (check_lps0_constraints)
 		lpi_check_constraints();
 
-	/* Display off */
+	/* LPS0 entry */
 	if (lps0_dsm_func_mask > 0)
 		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
-					ACPI_LPS0_DISPLAY_OFF_AMD :
-					ACPI_LPS0_DISPLAY_OFF,
+						ACPI_LPS0_ENTRY_AMD :
+						ACPI_LPS0_ENTRY,
 					lps0_dsm_func_mask, lps0_dsm_guid);
 
 	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
-
-	/* LPS0 entry */
-	if (lps0_dsm_func_mask > 0 && acpi_s2idle_vendor_amd())
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
-					lps0_dsm_func_mask, lps0_dsm_guid);
-
-	if (lps0_dsm_func_mask_microsoft > 0) {
-		/* Modern Standby entry */
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_ENTRY,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
 					lps0_dsm_func_mask_microsoft,
 					lps0_dsm_guid_microsoft);
-	}
-
-	if (lps0_dsm_func_mask > 0 && !acpi_s2idle_vendor_amd())
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
-					lps0_dsm_func_mask, lps0_dsm_guid);
 
 	list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
 		if (handler->prepare)
@@ -615,9 +616,19 @@ static void acpi_s2idle_restore_early_lps0(void)
 					ACPI_LPS0_EXIT,
 					lps0_dsm_func_mask, lps0_dsm_guid);
 
-	if (lps0_dsm_func_mask_microsoft > 0) {
+	if (lps0_dsm_func_mask_microsoft > 0)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+}
+
+static void acpi_s2idle_end_lps0(void)
+{
+	acpi_s2idle_end();
+
+	if (!lps0_device_handle || sleep_no_lps0)
+		return;
+
+	if (lps0_dsm_func_mask_microsoft > 0) {
 		/* Intent to turn on display */
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_TURN_ON_DISPLAY,
 					lps0_dsm_func_mask_microsoft,
@@ -648,7 +659,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
 	.wake = acpi_s2idle_wake,
 	.restore_early = acpi_s2idle_restore_early_lps0,
 	.restore = acpi_s2idle_restore,
-	.end = acpi_s2idle_end,
+	.end = acpi_s2idle_end_lps0,
 };
 
 void __init acpi_s2idle_setup(void)
-- 
2.53.0

[RFC v2 03/10] acpi/x86: s2idle: Add support for adding a delay after begin MS calls

[Antheas Kapenekakis]

Certain platform/USB devices interact with Modern Standby firmware
notifications. This is particularly true with Asus, where the keyboards
are wired up to turn off their backlight during the Display Off
notification using a predetermined delay. While for Asus Keyboards this
does not cause an issue, it does manifest in ROG Ally devices, where the
controller waits for the animation to complete before saving its state.

In Windows, this is not a problem, because there is an ample delay after
these calls and before LPS0, typically seconds to minutes.

Therefore, introduce a delay quirk after these calls, to ensure affected
devices have time to uninitialize, and attach it to acpi_s2idle_dev_ops
so it can be consumed by device drivers.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/acpi/x86/s2idle.c | 11 +++++++++++
 include/linux/acpi.h      |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index f5aefba8b191..8b48f999e0e9 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/suspend.h>
@@ -517,6 +518,9 @@ static struct acpi_scan_handler lps0_handler = {
 
 static int acpi_s2idle_begin_lps0(void)
 {
+	struct acpi_s2idle_dev_ops *handler;
+	int delay = 0;
+
 	if (!lps0_device_handle || sleep_no_lps0)
 		return acpi_s2idle_begin();
 
@@ -552,6 +556,13 @@ static int acpi_s2idle_begin_lps0(void)
 					lps0_dsm_func_mask_microsoft,
 					lps0_dsm_guid_microsoft);
 
+	list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
+		if (handler->begin_delay && handler->begin_delay > delay)
+			delay = handler->begin_delay;
+	}
+	if (delay > 0)
+		msleep(delay);
+
 	return acpi_s2idle_begin();
 }
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4d2f0bed7a06..a416e5c5798a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1154,6 +1154,7 @@ struct acpi_s2idle_dev_ops {
 	void (*prepare)(void);
 	void (*check)(void);
 	void (*restore)(void);
+	int begin_delay;
 };
 #if defined(CONFIG_SUSPEND) && defined(CONFIG_X86)
 int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg);
-- 
2.53.0

Re: [RFC v2 03/10] acpi/x86: s2idle: Add support for adding a delay after begin MS calls

[Mario Limonciello]

On 4/25/26 16:57, Antheas Kapenekakis wrote:
> Certain platform/USB devices interact with Modern Standby firmware
> notifications. This is particularly true with Asus, where the keyboards
> are wired up to turn off their backlight during the Display Off
> notification using a predetermined delay. While for Asus Keyboards this
> does not cause an issue, it does manifest in ROG Ally devices, where the
> controller waits for the animation to complete before saving its state.
> 
> In Windows, this is not a problem, because there is an ample delay after
> these calls and before LPS0, typically seconds to minutes.
> 
> Therefore, introduce a delay quirk after these calls, to ensure affected
> devices have time to uninitialize, and attach it to acpi_s2idle_dev_ops
> so it can be consumed by device drivers.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/acpi/x86/s2idle.c | 11 +++++++++++
>   include/linux/acpi.h      |  1 +
>   2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index f5aefba8b191..8b48f999e0e9 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -16,6 +16,7 @@
>    */
>   
>   #include <linux/acpi.h>
> +#include <linux/delay.h>
>   #include <linux/device.h>
>   #include <linux/dmi.h>
>   #include <linux/suspend.h>
> @@ -517,6 +518,9 @@ static struct acpi_scan_handler lps0_handler = {
>   
>   static int acpi_s2idle_begin_lps0(void)
>   {
> +	struct acpi_s2idle_dev_ops *handler;
> +	int delay = 0;
> +
>   	if (!lps0_device_handle || sleep_no_lps0)
>   		return acpi_s2idle_begin();
>   
> @@ -552,6 +556,13 @@ static int acpi_s2idle_begin_lps0(void)
>   					lps0_dsm_func_mask_microsoft,
>   					lps0_dsm_guid_microsoft);
>   
> +	list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
> +		if (handler->begin_delay && handler->begin_delay > delay)
> +			delay = handler->begin_delay;
> +	}
> +	if (delay > 0)
> +		msleep(delay);

Is this the correct location?  You wouldn't want it at the check callback?

> +
>   	return acpi_s2idle_begin();
>   }
>   
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4d2f0bed7a06..a416e5c5798a 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1154,6 +1154,7 @@ struct acpi_s2idle_dev_ops {
>   	void (*prepare)(void);
>   	void (*check)(void);
>   	void (*restore)(void);
> +	int begin_delay;
>   };
>   #if defined(CONFIG_SUSPEND) && defined(CONFIG_X86)
>   int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg);

Re: [RFC v2 03/10] acpi/x86: s2idle: Add support for adding a delay after begin MS calls

[Antheas Kapenekakis]

On Tue, 28 Apr 2026 at 03:57, Mario Limonciello <superm1@kernel.org> wrote:
>
> On 4/25/26 16:57, Antheas Kapenekakis wrote:
> > Certain platform/USB devices interact with Modern Standby firmware
> > notifications. This is particularly true with Asus, where the keyboards
> > are wired up to turn off their backlight during the Display Off
> > notification using a predetermined delay. While for Asus Keyboards this
> > does not cause an issue, it does manifest in ROG Ally devices, where the
> > controller waits for the animation to complete before saving its state.
> >
> > In Windows, this is not a problem, because there is an ample delay after
> > these calls and before LPS0, typically seconds to minutes.
> >
> > Therefore, introduce a delay quirk after these calls, to ensure affected
> > devices have time to uninitialize, and attach it to acpi_s2idle_dev_ops
> > so it can be consumed by device drivers.
> >
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >   drivers/acpi/x86/s2idle.c | 11 +++++++++++
> >   include/linux/acpi.h      |  1 +
> >   2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> > index f5aefba8b191..8b48f999e0e9 100644
> > --- a/drivers/acpi/x86/s2idle.c
> > +++ b/drivers/acpi/x86/s2idle.c
> > @@ -16,6 +16,7 @@
> >    */
> >
> >   #include <linux/acpi.h>
> > +#include <linux/delay.h>
> >   #include <linux/device.h>
> >   #include <linux/dmi.h>
> >   #include <linux/suspend.h>
> > @@ -517,6 +518,9 @@ static struct acpi_scan_handler lps0_handler = {
> >
> >   static int acpi_s2idle_begin_lps0(void)
> >   {
> > +     struct acpi_s2idle_dev_ops *handler;
> > +     int delay = 0;
> > +
> >       if (!lps0_device_handle || sleep_no_lps0)
> >               return acpi_s2idle_begin();
> >
> > @@ -552,6 +556,13 @@ static int acpi_s2idle_begin_lps0(void)
> >                                       lps0_dsm_func_mask_microsoft,
> >                                       lps0_dsm_guid_microsoft);
> >
> > +     list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
> > +             if (handler->begin_delay && handler->begin_delay > delay)
> > +                     delay = handler->begin_delay;
> > +     }
> > +     if (delay > 0)
> > +             msleep(delay);
>
> Is this the correct location?  You wouldn't want it at the check callback?

This delay is meant to be added between Display Off/Sleep Entry and
suspend to allow devices that need it to settle before they are put on
D3, so it needs to go here. check() happens after suspend (they are
already on D3). So e.g. check() cannot replace it. We would need
something like a begin_check()

I will reply to the other patch with more context.

> > +
> >       return acpi_s2idle_begin();
> >   }
> >
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 4d2f0bed7a06..a416e5c5798a 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1154,6 +1154,7 @@ struct acpi_s2idle_dev_ops {
> >       void (*prepare)(void);
> >       void (*check)(void);
> >       void (*restore)(void);
> > +     int begin_delay;
> >   };
> >   #if defined(CONFIG_SUSPEND) && defined(CONFIG_X86)
> >   int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg);
>
>

[RFC v2 04/10] platform/x86: asus-wmi: add s2idle begin delay for Ally devices

[Antheas Kapenekakis]

Asus ROG Ally devices have a small fade animation during their suspend
sequence. This animation is triggered by the Display Off firmware
notification. After it, they power off, disconnect, and save their
state. Without a delay after that notification, if the device is placed
into D3, it causes it to reboot during resume, or particularly for
original Allies, also leave the xpad device connected, causing spurious
wakeups when a gamepad button is pressed and higher standby drain.

Therefore, introduce a small delay quirk and bind it to an s2idle dev
ops device. Place this quirk in the asus-wmi driver, as the USB gamepad
disconnects, making it hard to place in hid-asus, and to allow for
proper functionality when the hid-asus driver is blacklisted.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/asus-nb-wmi.c | 40 ++++++++++++++++++++++++++++++
 drivers/platform/x86/asus-wmi.c    | 12 +++++++++
 drivers/platform/x86/asus-wmi.h    | 14 +++++++++++
 3 files changed, 66 insertions(+)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b4677c5bba5b..da963f3a17b1 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -155,6 +155,10 @@ static struct quirk_entry quirk_asus_z13 = {
 	.tablet_switch_mode = asus_wmi_kbd_dock_devid,
 };
 
+static struct quirk_entry quirk_ally = {
+	.lps0_begin_delay = 500,
+};
+
 static int dmi_matched(const struct dmi_system_id *dmi)
 {
 	pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -553,6 +557,42 @@ static const struct dmi_system_id asus_quirks[] = {
 		},
 		.driver_data = &quirk_asus_z13,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUS ROG Ally",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
+		},
+		.driver_data = (void *)&quirk_ally,
+	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUS ROG Ally X",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_BOARD_NAME, "RC72L"),
+		},
+		.driver_data = (void *)&quirk_ally,
+	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUS ROG Xbox Ally",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_BOARD_NAME, "RC73Y"),
+		},
+		.driver_data = (void *)&quirk_ally,
+	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUS ROG Xbox Ally X",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_BOARD_NAME, "RC73X"),
+		},
+		.driver_data = (void *)&quirk_ally,
+	},
 	{},
 };
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 7c0915e097ba..3316415abbdf 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -337,6 +337,8 @@ struct asus_wmi {
 	struct asus_wmi_debug debug;
 
 	struct asus_wmi_driver *driver;
+
+	struct acpi_s2idle_dev_ops s2idle_dev_ops;
 };
 
 /* Global to allow setting externally without requiring driver data */
@@ -5030,6 +5032,12 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_platform;
 
+	if (asus->driver->quirks->lps0_begin_delay) {
+		asus->s2idle_dev_ops.begin_delay =
+			asus->driver->quirks->lps0_begin_delay;
+		acpi_register_lps0_dev(&asus->s2idle_dev_ops);
+	}
+
 	if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_INIT) {
 		if (acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
 					&& dmi_check_system(asus_rog_ally_device))
@@ -5184,6 +5192,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_custom_fan_curve:
 fail_platform_profile_setup:
 fail_fan_boost_mode:
+	if (asus->driver->quirks->lps0_begin_delay)
+		acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
 fail_platform:
 	kfree(asus);
 	return err;
@@ -5207,6 +5217,8 @@ static void asus_wmi_remove(struct platform_device *device)
 	asus_fan_set_auto(asus);
 	throttle_thermal_policy_set_default(asus);
 	asus_wmi_battery_exit(asus);
+	if (asus->driver->quirks->lps0_begin_delay)
+		acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
 
 	kfree(asus);
 }
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 5cd4392b964e..2a8c51d39bf8 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -52,6 +52,20 @@ struct quirk_entry {
 	 */
 	int no_display_toggle;
 	u32 xusb2pr;
+	/*
+	 * Ally devices uninitialize after the display off DSM of modern
+	 * stanby, after a predetermined fade animation on their RGB.
+	 * If the USB subsystem puts the controller into D3 before that,
+	 * it loses its state and (i) for original allies, it leaves the
+	 * xpad device connected, causing spurious wake-ups and higher
+	 * power draw, (ii) for newer allies using the adaptive protocol
+	 * causes the controller to reboot on resume if mcu_powersave is
+	 * false. Therefore, allow adding a delay for the affected devices.
+	 * (if MCU powersave is true the controller always reboots, but
+	 * this also causes an unwelcome 5-7s delay on resume, this issue
+	 * is present on all firmwares)
+	 */
+	int lps0_begin_delay;
 };
 
 struct asus_wmi_driver {
-- 
2.53.0

Re: [RFC v2 04/10] platform/x86: asus-wmi: add s2idle begin delay for Ally devices

[Mario Limonciello]

On 4/25/26 16:57, Antheas Kapenekakis wrote:
> Asus ROG Ally devices have a small fade animation during their suspend
> sequence. This animation is triggered by the Display Off firmware
> notification. After it, they power off, disconnect, and save their
> state. Without a delay after that notification, if the device is placed
> into D3, it causes it to reboot during resume, or particularly for
> original Allies, also leave the xpad device connected, causing spurious
> wakeups when a gamepad button is pressed and higher standby drain.
> 
> Therefore, introduce a small delay quirk and bind it to an s2idle dev
> ops device. Place this quirk in the asus-wmi driver, as the USB gamepad
> disconnects, making it hard to place in hid-asus, and to allow for
> proper functionality when the hid-asus driver is blacklisted.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/platform/x86/asus-nb-wmi.c | 40 ++++++++++++++++++++++++++++++
>   drivers/platform/x86/asus-wmi.c    | 12 +++++++++
>   drivers/platform/x86/asus-wmi.h    | 14 +++++++++++
>   3 files changed, 66 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index b4677c5bba5b..da963f3a17b1 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -155,6 +155,10 @@ static struct quirk_entry quirk_asus_z13 = {
>   	.tablet_switch_mode = asus_wmi_kbd_dock_devid,
>   };
>   
> +static struct quirk_entry quirk_ally = {
> +	.lps0_begin_delay = 500,
> +};
> +
>   static int dmi_matched(const struct dmi_system_id *dmi)
>   {
>   	pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -553,6 +557,42 @@ static const struct dmi_system_id asus_quirks[] = {
>   		},
>   		.driver_data = &quirk_asus_z13,
>   	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Ally",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Ally X",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC72L"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Xbox Ally",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC73Y"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Xbox Ally X",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC73X"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
>   	{},
>   };
>   
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 7c0915e097ba..3316415abbdf 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -337,6 +337,8 @@ struct asus_wmi {
>   	struct asus_wmi_debug debug;
>   
>   	struct asus_wmi_driver *driver;
> +
> +	struct acpi_s2idle_dev_ops s2idle_dev_ops;
>   };
>   
>   /* Global to allow setting externally without requiring driver data */
> @@ -5030,6 +5032,12 @@ static int asus_wmi_add(struct platform_device *pdev)
>   	if (err)
>   		goto fail_platform;
>   
> +	if (asus->driver->quirks->lps0_begin_delay) {
> +		asus->s2idle_dev_ops.begin_delay =
> +			asus->driver->quirks->lps0_begin_delay;
> +		acpi_register_lps0_dev(&asus->s2idle_dev_ops);
> +	}
> +
>   	if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_INIT) {
>   		if (acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
>   					&& dmi_check_system(asus_rog_ally_device))
> @@ -5184,6 +5192,8 @@ static int asus_wmi_add(struct platform_device *pdev)
>   fail_custom_fan_curve:
>   fail_platform_profile_setup:
>   fail_fan_boost_mode:
> +	if (asus->driver->quirks->lps0_begin_delay)
> +		acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
>   fail_platform:
>   	kfree(asus);
>   	return err;
> @@ -5207,6 +5217,8 @@ static void asus_wmi_remove(struct platform_device *device)
>   	asus_fan_set_auto(asus);
>   	throttle_thermal_policy_set_default(asus);
>   	asus_wmi_battery_exit(asus);
> +	if (asus->driver->quirks->lps0_begin_delay)
> +		acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
>   
>   	kfree(asus);
>   }
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 5cd4392b964e..2a8c51d39bf8 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -52,6 +52,20 @@ struct quirk_entry {
>   	 */
>   	int no_display_toggle;
>   	u32 xusb2pr;
> +	/*
> +	 * Ally devices uninitialize after the display off DSM of modern
> +	 * stanby, after a predetermined fade animation on their RGB.
standby
> +	 * If the USB subsystem puts the controller into D3 before that,
> +	 * it loses its state and (i) for original allies, it leaves the
> +	 * xpad device connected, causing spurious wake-ups and higher
> +	 * power draw, (ii) for newer allies using the adaptive protocol
> +	 * causes the controller to reboot on resume if mcu_powersave is
> +	 * false. Therefore, allow adding a delay for the affected devices.
> +	 * (if MCU powersave is true the controller always reboots, but
> +	 * this also causes an unwelcome 5-7s delay on resume, this issue
> +	 * is present on all firmwares)
> +	 */
> +	int lps0_begin_delay;
>   };
>   
>   struct asus_wmi_driver {

Re: [systemd-devel] [RFC v2 04/10] platform/x86: asus-wmi: add s2idle begin delay for Ally devices

[Paul Menzel]

Dear Antheas,


Thank you for your patch.

Am 25.04.26 um 23:57 schrieb Antheas Kapenekakis:
> Asus ROG Ally devices have a small fade animation during their suspend
> sequence. This animation is triggered by the Display Off firmware
> notification. After it, they power off, disconnect, and save their

Does *they* mean the system firmware?

> state. Without a delay after that notification, if the device is placed
> into D3, it causes it to reboot during resume, or particularly for
> original Allies, also leave the xpad device connected, causing spurious
> wakeups when a gamepad button is pressed and higher standby drain.
> 
> Therefore, introduce a small delay quirk and bind it to an s2idle dev
> ops device. Place this quirk in the asus-wmi driver, as the USB gamepad
> disconnects, making it hard to place in hid-asus, and to allow for
> proper functionality when the hid-asus driver is blacklisted.

Reading the commit message, it’s not clear to me, if it’s a firmware bug 
or not. I’d say, it is. Could Asus fix *additionally* fix it?

Also please list the devices added to the quirk list to the commit message.

> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/platform/x86/asus-nb-wmi.c | 40 ++++++++++++++++++++++++++++++
>   drivers/platform/x86/asus-wmi.c    | 12 +++++++++
>   drivers/platform/x86/asus-wmi.h    | 14 +++++++++++
>   3 files changed, 66 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index b4677c5bba5b..da963f3a17b1 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -155,6 +155,10 @@ static struct quirk_entry quirk_asus_z13 = {
>   	.tablet_switch_mode = asus_wmi_kbd_dock_devid,
>   };
>   
> +static struct quirk_entry quirk_ally = {
> +	.lps0_begin_delay = 500,

I’d append the unit to the variable name. (But I also saw that 
`s2idle_dev_ops.begin_delay` does not follow this schema.)

> +};
> +
>   static int dmi_matched(const struct dmi_system_id *dmi)
>   {
>   	pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -553,6 +557,42 @@ static const struct dmi_system_id asus_quirks[] = {
>   		},
>   		.driver_data = &quirk_asus_z13,
>   	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Ally",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Ally X",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC72L"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Xbox Ally",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC73Y"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ROG Xbox Ally X",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "RC73X"),
> +		},
> +		.driver_data = (void *)&quirk_ally,
> +	},
>   	{},
>   };
>   
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 7c0915e097ba..3316415abbdf 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -337,6 +337,8 @@ struct asus_wmi {
>   	struct asus_wmi_debug debug;
>   
>   	struct asus_wmi_driver *driver;
> +
> +	struct acpi_s2idle_dev_ops s2idle_dev_ops;
>   };
>   
>   /* Global to allow setting externally without requiring driver data */
> @@ -5030,6 +5032,12 @@ static int asus_wmi_add(struct platform_device *pdev)
>   	if (err)
>   		goto fail_platform;
>   
> +	if (asus->driver->quirks->lps0_begin_delay) {
> +		asus->s2idle_dev_ops.begin_delay =
> +			asus->driver->quirks->lps0_begin_delay;
> +		acpi_register_lps0_dev(&asus->s2idle_dev_ops);
> +	}
> +
>   	if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_INIT) {
>   		if (acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
>   					&& dmi_check_system(asus_rog_ally_device))
> @@ -5184,6 +5192,8 @@ static int asus_wmi_add(struct platform_device *pdev)
>   fail_custom_fan_curve:
>   fail_platform_profile_setup:
>   fail_fan_boost_mode:
> +	if (asus->driver->quirks->lps0_begin_delay)
> +		acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
>   fail_platform:
>   	kfree(asus);
>   	return err;
> @@ -5207,6 +5217,8 @@ static void asus_wmi_remove(struct platform_device *device)
>   	asus_fan_set_auto(asus);
>   	throttle_thermal_policy_set_default(asus);
>   	asus_wmi_battery_exit(asus);
> +	if (asus->driver->quirks->lps0_begin_delay)
> +		acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
>   
>   	kfree(asus);
>   }
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 5cd4392b964e..2a8c51d39bf8 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -52,6 +52,20 @@ struct quirk_entry {
>   	 */
>   	int no_display_toggle;
>   	u32 xusb2pr;
> +	/*
> +	 * Ally devices uninitialize after the display off DSM of modern
> +	 * stanby, after a predetermined fade animation on their RGB.

standby

> +	 * If the USB subsystem puts the controller into D3 before that,
> +	 * it loses its state and (i) for original allies, it leaves the

Allies (or Allys) to make it easier to read for non-native speakers.

> +	 * xpad device connected, causing spurious wake-ups and higher
> +	 * power draw, (ii) for newer allies using the adaptive protocol

Ditto.

> +	 * causes the controller to reboot on resume if mcu_powersave is
> +	 * false. Therefore, allow adding a delay for the affected devices.
> +	 * (if MCU powersave is true the controller always reboots, but

Is MCU powersave a system firmware (UEFI) option?

> +	 * this also causes an unwelcome 5-7s delay on resume, this issue
> +	 * is present on all firmwares)
> +	 */
> +	int lps0_begin_delay;
>   };
>   
>   struct asus_wmi_driver {


Kind regards,

Paul

Re: [systemd-devel] [RFC v2 04/10] platform/x86: asus-wmi: add s2idle begin delay for Ally devices

[Antheas Kapenekakis]

On Tue, 28 Apr 2026 at 08:34, Paul Menzel
<pmenzel+systemd-devel@molgen.mpg.de> wrote:
>
> Dear Antheas,
>
>
> Thank you for your patch.
>
> Am 25.04.26 um 23:57 schrieb Antheas Kapenekakis:
> > Asus ROG Ally devices have a small fade animation during their suspend
> > sequence. This animation is triggered by the Display Off firmware
> > notification. After it, they power off, disconnect, and save their
>
> Does *they* mean the system firmware?

Hi,
thanks

MCU firmware, will reword to say controller

> > state. Without a delay after that notification, if the device is placed
> > into D3, it causes it to reboot during resume, or particularly for
> > original Allies, also leave the xpad device connected, causing spurious
> > wakeups when a gamepad button is pressed and higher standby drain.
> >
> > Therefore, introduce a small delay quirk and bind it to an s2idle dev
> > ops device. Place this quirk in the asus-wmi driver, as the USB gamepad
> > disconnects, making it hard to place in hid-asus, and to allow for
> > proper functionality when the hid-asus driver is blacklisted.
>
> Reading the commit message, it’s not clear to me, if it’s a firmware bug
> or not. I’d say, it is. Could Asus fix *additionally* fix it?

It is a linux specific firmware bug that happens because the
assumption of "In 95% of cases, I will have 300ms after the Display
Off notification fires" does not hold in Linux.

Because the controller expects to have the time to fade the RGB, and
then power off.

Currently, the notification fires after the controller has already
powered off. It is mostly fixed by Asus already and even without this
patch the controller has correct functionality. The delay mostly makes
sure that the fade animation is performed correctly, the original ally
yanks its xpad device, and when mcu powersave is off, instead of the
controller rebooting, it comes back instantly after suspend. For
earlier firmwares that had bigger issues, the delay also fixes those.

> Also please list the devices added to the quirk list to the commit message.

Ack.

> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >   drivers/platform/x86/asus-nb-wmi.c | 40 ++++++++++++++++++++++++++++++
> >   drivers/platform/x86/asus-wmi.c    | 12 +++++++++
> >   drivers/platform/x86/asus-wmi.h    | 14 +++++++++++
> >   3 files changed, 66 insertions(+)
> >
> > diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> > index b4677c5bba5b..da963f3a17b1 100644
> > --- a/drivers/platform/x86/asus-nb-wmi.c
> > +++ b/drivers/platform/x86/asus-nb-wmi.c
> > @@ -155,6 +155,10 @@ static struct quirk_entry quirk_asus_z13 = {
> >       .tablet_switch_mode = asus_wmi_kbd_dock_devid,
> >   };
> >
> > +static struct quirk_entry quirk_ally = {
> > +     .lps0_begin_delay = 500,
>
> I’d append the unit to the variable name. (But I also saw that
> `s2idle_dev_ops.begin_delay` does not follow this schema.)

I can add that.

> > +};
> > +
> >   static int dmi_matched(const struct dmi_system_id *dmi)
> >   {
> >       pr_info("Identified laptop model '%s'\n", dmi->ident);
> > @@ -553,6 +557,42 @@ static const struct dmi_system_id asus_quirks[] = {
> >               },
> >               .driver_data = &quirk_asus_z13,
> >       },
> > +     {
> > +             .callback = dmi_matched,
> > +             .ident = "ASUS ROG Ally",
> > +             .matches = {
> > +                     DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> > +                     DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> > +             },
> > +             .driver_data = (void *)&quirk_ally,
> > +     },
> > +     {
> > +             .callback = dmi_matched,
> > +             .ident = "ASUS ROG Ally X",
> > +             .matches = {
> > +                     DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> > +                     DMI_MATCH(DMI_BOARD_NAME, "RC72L"),
> > +             },
> > +             .driver_data = (void *)&quirk_ally,
> > +     },
> > +     {
> > +             .callback = dmi_matched,
> > +             .ident = "ASUS ROG Xbox Ally",
> > +             .matches = {
> > +                     DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> > +                     DMI_MATCH(DMI_BOARD_NAME, "RC73Y"),
> > +             },
> > +             .driver_data = (void *)&quirk_ally,
> > +     },
> > +     {
> > +             .callback = dmi_matched,
> > +             .ident = "ASUS ROG Xbox Ally X",
> > +             .matches = {
> > +                     DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> > +                     DMI_MATCH(DMI_BOARD_NAME, "RC73X"),
> > +             },
> > +             .driver_data = (void *)&quirk_ally,
> > +     },
> >       {},
> >   };
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 7c0915e097ba..3316415abbdf 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -337,6 +337,8 @@ struct asus_wmi {
> >       struct asus_wmi_debug debug;
> >
> >       struct asus_wmi_driver *driver;
> > +
> > +     struct acpi_s2idle_dev_ops s2idle_dev_ops;
> >   };
> >
> >   /* Global to allow setting externally without requiring driver data */
> > @@ -5030,6 +5032,12 @@ static int asus_wmi_add(struct platform_device *pdev)
> >       if (err)
> >               goto fail_platform;
> >
> > +     if (asus->driver->quirks->lps0_begin_delay) {
> > +             asus->s2idle_dev_ops.begin_delay =
> > +                     asus->driver->quirks->lps0_begin_delay;
> > +             acpi_register_lps0_dev(&asus->s2idle_dev_ops);
> > +     }
> > +
> >       if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_INIT) {
> >               if (acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
> >                                       && dmi_check_system(asus_rog_ally_device))
> > @@ -5184,6 +5192,8 @@ static int asus_wmi_add(struct platform_device *pdev)
> >   fail_custom_fan_curve:
> >   fail_platform_profile_setup:
> >   fail_fan_boost_mode:
> > +     if (asus->driver->quirks->lps0_begin_delay)
> > +             acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
> >   fail_platform:
> >       kfree(asus);
> >       return err;
> > @@ -5207,6 +5217,8 @@ static void asus_wmi_remove(struct platform_device *device)
> >       asus_fan_set_auto(asus);
> >       throttle_thermal_policy_set_default(asus);
> >       asus_wmi_battery_exit(asus);
> > +     if (asus->driver->quirks->lps0_begin_delay)
> > +             acpi_unregister_lps0_dev(&asus->s2idle_dev_ops);
> >
> >       kfree(asus);
> >   }
> > diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> > index 5cd4392b964e..2a8c51d39bf8 100644
> > --- a/drivers/platform/x86/asus-wmi.h
> > +++ b/drivers/platform/x86/asus-wmi.h
> > @@ -52,6 +52,20 @@ struct quirk_entry {
> >        */
> >       int no_display_toggle;
> >       u32 xusb2pr;
> > +     /*
> > +      * Ally devices uninitialize after the display off DSM of modern
> > +      * stanby, after a predetermined fade animation on their RGB.
>
> standby
>
> > +      * If the USB subsystem puts the controller into D3 before that,
> > +      * it loses its state and (i) for original allies, it leaves the
>
> Allies (or Allys) to make it easier to read for non-native speakers.
>
> > +      * xpad device connected, causing spurious wake-ups and higher
> > +      * power draw, (ii) for newer allies using the adaptive protocol
>
> Ditto.
>
> > +      * causes the controller to reboot on resume if mcu_powersave is
> > +      * false. Therefore, allow adding a delay for the affected devices.
> > +      * (if MCU powersave is true the controller always reboots, but
>
> Is MCU powersave a system firmware (UEFI) option?

MCU powersave (as named in this driver), and named Extreme Standby in
Windows, is an Asus WMI param (cannot be set in the BIOS but it is
UEFI) that makes it so the controller completely powers off during
sleep and halves standby power.

Noted on the other comments.

Antheas

> > +      * this also causes an unwelcome 5-7s delay on resume, this issue
> > +      * is present on all firmwares)
> > +      */
> > +     int lps0_begin_delay;
> >   };
> >
> >   struct asus_wmi_driver {
>
>
> Kind regards,
>
> Paul
>

[RFC v2 05/10] HID: asus: remove quirk handling for Ally devices

[Antheas Kapenekakis]

Ally device controllers had suspend issues due to the faulty s2idle
ordering present in the kernel, causing a failure to resume. These
issues were partially addressed with a firmware update, leading to
the introduction of a version check in this driver.

However, after fixing the call ordering in s2idle and adding a small
delay particularly for Ally devices, the controller for these devices
is universally fixed for all firmware versions, including the spurious
resume bug requiring set_ally_mcu_powersave(True) for the controller to
function correctly on newer firmwares.

Therefore, remove the check and cleanup the driver.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 113 ++---------------------------------------
 1 file changed, 4 insertions(+), 109 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index bc93b27f9b13..2d2fe72a3d5b 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -53,10 +53,6 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
-#define ROG_ALLY_REPORT_SIZE 64
-#define ROG_ALLY_X_MIN_MCU 313
-#define ROG_ALLY_MIN_MCU 319
-
 /* Spurious HID codes sent by QUIRK_ROG_NKEY_KEYBOARD devices */
 #define ASUS_SPURIOUS_CODE_0XEA 0xea
 #define ASUS_SPURIOUS_CODE_0XEC 0xec
@@ -99,9 +95,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_MEDION_E1239T		BIT(10)
 #define QUIRK_ROG_NKEY_KEYBOARD		BIT(11)
 #define QUIRK_ROG_CLAYMORE_II_KEYBOARD	BIT(12)
-#define QUIRK_ROG_ALLY_XPAD		BIT(13)
-#define QUIRK_HID_FN_LOCK		BIT(14)
-#define QUIRK_ROG_NKEY_ID1ID2_INIT	BIT(15)
+#define QUIRK_HID_FN_LOCK		BIT(13)
+#define QUIRK_ROG_NKEY_ID1ID2_INIT	BIT(14)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS | \
@@ -631,102 +626,9 @@ static void asus_kbd_backlight_work(struct work_struct *work)
 		hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
 }
 
-/*
- * We don't care about any other part of the string except the version section.
- * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
- * The bytes "5a 05 03 31 00 1a 13" and possibly more come before the version
- * string, and there may be additional bytes after the version string such as
- * "75 00 74 00 65 00" or a postfix such as "_T01"
- */
-static int mcu_parse_version_string(const u8 *response, size_t response_size)
-{
-	const u8 *end = response + response_size;
-	const u8 *p = response;
-	int dots, err, version;
-	char buf[4];
-
-	dots = 0;
-	while (p < end && dots < 2) {
-		if (*p++ == '.')
-			dots++;
-	}
-
-	if (dots != 2 || p >= end || (p + 3) >= end)
-		return -EINVAL;
-
-	memcpy(buf, p, 3);
-	buf[3] = '\0';
-
-	err = kstrtoint(buf, 10, &version);
-	if (err || version < 0)
-		return -EINVAL;
-
-	return version;
-}
-
-static int mcu_request_version(struct hid_device *hdev)
-{
-	u8 *response __free(kfree) = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
-	const u8 request[] = { 0x5a, 0x05, 0x03, 0x31, 0x00, 0x20 };
-	int ret;
-
-	if (!response)
-		return -ENOMEM;
-
-	ret = asus_kbd_set_report(hdev, request, sizeof(request));
-	if (ret < 0)
-		return ret;
-
-	ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID, response,
-				ROG_ALLY_REPORT_SIZE, HID_FEATURE_REPORT,
-				HID_REQ_GET_REPORT);
-	if (ret < 0)
-		return ret;
-
-	ret = mcu_parse_version_string(response, ROG_ALLY_REPORT_SIZE);
-	if (ret < 0) {
-		pr_err("Failed to parse MCU version: %d\n", ret);
-		print_hex_dump(KERN_ERR, "MCU: ", DUMP_PREFIX_NONE,
-			      16, 1, response, ROG_ALLY_REPORT_SIZE, false);
-	}
-
-	return ret;
-}
-
-static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
-{
-	int min_version, version;
-
-	version = mcu_request_version(hdev);
-	if (version < 0)
-		return;
-
-	switch (idProduct) {
-	case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY:
-		min_version = ROG_ALLY_MIN_MCU;
-		break;
-	case USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X:
-		min_version = ROG_ALLY_X_MIN_MCU;
-		break;
-	default:
-		min_version = 0;
-	}
-
-	if (version < min_version) {
-		hid_warn(hdev,
-			"The MCU firmware version must be %d or greater to avoid issues with suspend.\n",
-			min_version);
-	} else {
-		set_ally_mcu_hack(ASUS_WMI_ALLY_MCU_HACK_DISABLED);
-		set_ally_mcu_powersave(true);
-	}
-}
-
 static int asus_kbd_register_leds(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
-	struct usb_interface *intf;
-	struct usb_device *udev;
 	unsigned char kbd_func;
 	int ret;
 
@@ -754,13 +656,6 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 			return ret;
 	}
 
-	if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
-		intf = to_usb_interface(hdev->dev.parent);
-		udev = interface_to_usbdev(intf);
-		validate_mcu_fw_version(hdev,
-			le16_to_cpu(udev->descriptor.idProduct));
-	}
-
 	drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
 					      sizeof(struct asus_kbd_leds),
 					      GFP_KERNEL);
@@ -1493,10 +1388,10 @@ static const struct hid_device_id asus_devices[] = {
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD},
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD},
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD },
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_XGM_2022),
 	},
-- 
2.53.0

[RFC v2 06/10] platform/x86: asus-wmi: Remove Ally s2idle resume fixes

[Antheas Kapenekakis]

Ally device controllers had suspend issues due to the faulty s2idle
ordering present in the kernel, causing a failure to resume. These
issues were partially addressed with a firmware update, leading to
the introduction of a version check in hid-asus and accompanying calls
in this driver.

However, after fixing the call ordering in s2idle and adding a small
delay particularly for Ally devices, the controller for these devices
is universally fixed for all firmware versions, including the spurious
resume bug requiring set_ally_mcu_powersave(True) for the controller to
function correctly on newer firmwares.

Therefore, remove the calls, suspend quirks with CSEE duplicate calling
(it was always called by s2idle through the Display Off _DSM but ignored
while the USB subsystem was down and these calls took place), and
cleanup the driver.

Noteworthy is that we no longer force mcu_powersave to be on boot. This
way, the user's choice in e.g. Windows or from userspace software is
respected. Downstream distributions that care about the additional power
draw in lieu of the significant additional delay in resume introduced by
the setting can enable it with a udev rule (the canonical handling for
this functionality).

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/asus-wmi.c            | 124 ---------------------
 include/linux/platform_data/x86/asus-wmi.h |   8 --
 2 files changed, 132 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3316415abbdf..70c725ca0edd 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -140,33 +140,10 @@ module_param(fnlock_default, bool, 0444);
 #define ASUS_MINI_LED_2024_STRONG	0x01
 #define ASUS_MINI_LED_2024_OFF		0x02
 
-#define ASUS_USB0_PWR_EC0_CSEE "\\_SB.PCI0.SBRG.EC0.CSEE"
-/*
- * The period required to wait after screen off/on/s2idle.check in MS.
- * Time here greatly impacts the wake behaviour. Used in suspend/wake.
- */
-#define ASUS_USB0_PWR_EC0_CSEE_WAIT	600
-#define ASUS_USB0_PWR_EC0_CSEE_OFF	0xB7
-#define ASUS_USB0_PWR_EC0_CSEE_ON	0xB8
-
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static int throttle_thermal_policy_write(struct asus_wmi *);
 
-static const struct dmi_system_id asus_rog_ally_device[] = {
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "RC72L"),
-		},
-	},
-	{ },
-};
-
 static bool ashs_present(void)
 {
 	int i = 0;
@@ -341,9 +318,6 @@ struct asus_wmi {
 	struct acpi_s2idle_dev_ops s2idle_dev_ops;
 };
 
-/* Global to allow setting externally without requiring driver data */
-static enum asus_ally_mcu_hack use_ally_mcu_hack = ASUS_WMI_ALLY_MCU_HACK_INIT;
-
 #if IS_ENABLED(CONFIG_ASUS_WMI_DEPRECATED_ATTRS)
 static void asus_wmi_show_deprecated(void)
 {
@@ -1436,43 +1410,6 @@ static DEVICE_ATTR_RW(nv_temp_target);
 
 /* Ally MCU Powersave ********************************************************/
 
-/*
- * The HID driver needs to check MCU version and set this to false if the MCU FW
- * version is >= the minimum requirements. New FW do not need the hacks.
- */
-void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
-{
-	use_ally_mcu_hack = status;
-	pr_debug("%s Ally MCU suspend quirk\n",
-		 status == ASUS_WMI_ALLY_MCU_HACK_ENABLED ? "Enabled" : "Disabled");
-}
-EXPORT_SYMBOL_NS_GPL(set_ally_mcu_hack, "ASUS_WMI");
-
-/*
- * mcu_powersave should be enabled always, as it is fixed in MCU FW versions:
- * - v313 for Ally X
- * - v319 for Ally 1
- * The HID driver checks MCU versions and so should set this if requirements match
- */
-void set_ally_mcu_powersave(bool enabled)
-{
-	int result, err;
-
-	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_MCU_POWERSAVE, enabled, &result);
-	if (err) {
-		pr_warn("Failed to set MCU powersave: %d\n", err);
-		return;
-	}
-	if (result > 1) {
-		pr_warn("Failed to set MCU powersave (result): 0x%x\n", result);
-		return;
-	}
-
-	pr_debug("%s MCU Powersave\n",
-		 enabled ? "Enabled" : "Disabled");
-}
-EXPORT_SYMBOL_NS_GPL(set_ally_mcu_powersave, "ASUS_WMI");
-
 #if IS_ENABLED(CONFIG_ASUS_WMI_DEPRECATED_ATTRS)
 static ssize_t mcu_powersave_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
@@ -5038,21 +4975,6 @@ static int asus_wmi_add(struct platform_device *pdev)
 		acpi_register_lps0_dev(&asus->s2idle_dev_ops);
 	}
 
-	if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_INIT) {
-		if (acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE)
-					&& dmi_check_system(asus_rog_ally_device))
-			use_ally_mcu_hack = ASUS_WMI_ALLY_MCU_HACK_ENABLED;
-		if (dmi_match(DMI_BOARD_NAME, "RC71")) {
-			/*
-			 * These steps ensure the device is in a valid good state, this is
-			 * especially important for the Ally 1 after a reboot.
-			 */
-			acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
-						   ASUS_USB0_PWR_EC0_CSEE_ON);
-			msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
-		}
-	}
-
 	/* ensure defaults for tunables */
 #if IS_ENABLED(CONFIG_ASUS_WMI_DEPRECATED_ATTRS)
 	asus->ppt_pl2_sppt = 5;
@@ -5306,51 +5228,10 @@ static int asus_hotk_restore(struct device *device)
 	return 0;
 }
 
-static int asus_hotk_prepare(struct device *device)
-{
-	if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_ENABLED) {
-		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
-					   ASUS_USB0_PWR_EC0_CSEE_OFF);
-		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
-	}
-	return 0;
-}
-
-#if defined(CONFIG_SUSPEND)
-static void asus_ally_s2idle_restore(void)
-{
-	if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_ENABLED) {
-		acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE,
-					   ASUS_USB0_PWR_EC0_CSEE_ON);
-		msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT);
-	}
-}
-
-/* Use only for Ally devices due to the wake_on_ac */
-static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = {
-	.restore = asus_ally_s2idle_restore,
-};
-
-static void asus_s2idle_check_register(void)
-{
-	if (acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops))
-		pr_warn("failed to register LPS0 sleep handler in asus-wmi\n");
-}
-
-static void asus_s2idle_check_unregister(void)
-{
-	acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops);
-}
-#else
-static void asus_s2idle_check_register(void) {}
-static void asus_s2idle_check_unregister(void) {}
-#endif /* CONFIG_SUSPEND */
-
 static const struct dev_pm_ops asus_pm_ops = {
 	.thaw = asus_hotk_thaw,
 	.restore = asus_hotk_restore,
 	.resume = asus_hotk_resume,
-	.prepare = asus_hotk_prepare,
 };
 
 /* Registration ***************************************************************/
@@ -5377,11 +5258,7 @@ static int asus_wmi_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	asus_s2idle_check_register();
-
 	ret = asus_wmi_add(pdev);
-	if (ret)
-		asus_s2idle_check_unregister();
 
 	return ret;
 }
@@ -5418,7 +5295,6 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver);
 void asus_wmi_unregister_driver(struct asus_wmi_driver *driver)
 {
 	guard(mutex)(&register_mutex);
-	asus_s2idle_check_unregister();
 
 	platform_device_unregister(driver->platform_device);
 	platform_driver_unregister(&driver->platform_driver);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 554f41b827e1..938a27c6f66e 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -188,8 +188,6 @@ enum asus_hid_event {
 #define ASUS_EV_MAX_BRIGHTNESS 3
 
 #if IS_REACHABLE(CONFIG_ASUS_WMI)
-void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
-void set_ally_mcu_powersave(bool enabled);
 int asus_wmi_get_devstate_dsts(u32 dev_id, u32 *retval);
 int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
 int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
@@ -197,12 +195,6 @@ int asus_hid_register_listener(struct asus_hid_listener *cdev);
 void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
 int asus_hid_event(enum asus_hid_event event);
 #else
-static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
-{
-}
-static inline void set_ally_mcu_powersave(bool enabled)
-{
-}
 static inline int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval)
 {
 	return -ENODEV;
-- 
2.53.0

[RFC v2 07/10] Documentation: Add documentation for the new sysfs hints class

[Antheas Kapenekakis]

The new sysfs hints class allows adding standardized hints to inform
devices of the host state. The initial hint added is the idle hint,
which allows userspace to inform devices of the host's idle state
based on user interaction (e.g., inactive). Add the documentation for
the new class and the currently available activity hint.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 Documentation/ABI/testing/sysfs-class-hint | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-hint

diff --git a/Documentation/ABI/testing/sysfs-class-hint b/Documentation/ABI/testing/sysfs-class-hint
new file mode 100644
index 000000000000..9d6ec956ebb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-hint
@@ -0,0 +1,78 @@
+What:		/sys/class/hint/*/name
+Date:		June 2026
+KernelVersion:	7.2
+Description:	Name of the class device given by the driver.
+
+		RO
+
+What:		/sys/class/hint/*/idle_choices
+Date:		June 2026
+KernelVersion:	7.2
+Description:	If the device supports idle states, this file contains
+		space-separated list of idle states supported for this device.
+		These states can be used to modify the runtime appearance of the
+		device (e.g., turn off the fan for snooze, turn off the keyboard
+		backlight for inactive) in response to user activity and host
+		state. Userspace can keep track of the idle state of the host
+		and inform devices with idle hint support of the current state.
+
+		The following idle states are supported:
+
+		==========	========================================
+		active		The user is actively interacting with the host.
+				The device appearance is unchanged and normal.
+		inactive	The user has stopped interacting with the
+				host and triggered an inactivity timer (e.g.,
+				lockscreen timeout) and the host displays
+				(if any) are off. The device may choose to
+				change its appearance (e.g. turn off its
+				backlight) or suspend if it is user facing
+				(e.g., handheld controller).
+		snooze	  The host enters a "dark resume" state where the
+				host should look like it is suspended. In
+				response, platform devices may turn off the
+				device fan, pulse the power light, or lower
+				the thermal envelope of the device.
+		resume	  While in the snooze state, userspace has decided
+				to prepare for user interaction while remaining
+				in snooze (e.g., user pressed a keyboard
+				button). If the device lowered its thermal
+				envelope for the snooze state, it can use this
+				hint to restore the normal thermal envelope and
+				lower the time to interaction.
+		==========	========================================
+
+		Reference the following diagram for userspace implementations:
+
+		<Suspend> <-> <Snooze> <-> <Inactive> <-> <Active>
+			       ->  <Resume>  /\
+
+		Where Suspend is the normal suspend path (e.g., s2idle). Devices
+		may support a subset of these idle states (e.g., only active and
+		inactive). In this case, userspace should report the closest
+		supported idle state that's higher than the target one.
+
+		Examples:
+			1) Device supports inactive and active:
+			   If the desired state is snooze, choose inactive
+			2) Device supports snooze and active:
+			   If the desired state is inactive, choose active
+			3) Device does not support resume:
+			   If the desired state is resume, stay in snooze
+
+		RO
+
+What:		/sys/class/hint/*/idle
+Date:		June 2026
+KernelVersion:	7.2
+Description:	Reading this file gives the current selected idle hint for
+		this device. Writing this file with one of the strings from
+		idle_choices changes the idle state to the new value.
+
+		Depending on the device, certain transition combinations may
+		be disallowed. Specifically, devices can choose to reject
+		entering resume from a state other than snooze and re-entering
+		snooze from resume. In this case, the device rejects the change
+		with -EINVAL.
+
+		RW
-- 
2.53.0

[RFC v2 08/10] hint: Add hint class ABI for devices to receive updates on host activity

[Antheas Kapenekakis]

The s2idle device requires the ability for userspace to inform of its
idleness state (e.g., inactive, active, snooze) which is used to update
device appearance (e.g., turn off keyboard backlight). Ideally, we would
want to extend this ability to non-ACPI/non-X86 devices and drivers,
e.g. memory mapped ECs, to be able to use this activity hint to for
example turn off their backlight. Moreover, to prevent adding new ABIs,
we would also like the ability to add new types of hints in the future.

Therefore, introduce a /sys/class/hint sysfs interface, with
{idle, idle_choices} to allow userspace to inform devices of
its current idleness. The initial ABI for class hint makes {idle,
idle_choices} optional, which allows for future hint additions that
are unrelated without forcing the implementation of the idle hint.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 MAINTAINERS           |   8 ++
 drivers/base/Kconfig  |   3 +
 drivers/base/Makefile |   1 +
 drivers/base/hint.c   | 283 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/hint.h  |  38 ++++++
 5 files changed, 333 insertions(+)
 create mode 100644 drivers/base/hint.c
 create mode 100644 include/linux/hint.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d1cc0e12fe1f..12712b628449 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7324,6 +7324,14 @@ F:	drivers/devfreq/event/
 F:	include/dt-bindings/pmu/exynos_ppmu.h
 F:	include/linux/devfreq-event.h
 
+DEVICE HINTS
+M:	Antheas Kapenekakis <lkml@antheas.dev>
+L:	driver-core@lists.linux.dev
+S:	Maintained
+F:	Documentation/ABI/testing/sysfs-class-hint
+F:	drivers/base/hint.c
+F:	include/linux/hint.h
+
 DEVICE I/O & IRQ [RUST]
 M:	Danilo Krummrich <dakr@kernel.org>
 M:	Alice Ryhl <aliceryhl@google.com>
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 1786d87b29e2..c5316e8f98d0 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -222,6 +222,9 @@ config DMA_FENCE_TRACE
 	  lockup related problems for dma-buffers shared across multiple
 	  devices.
 
+config HINT
+	bool
+
 config GENERIC_ARCH_TOPOLOGY
 	bool
 	help
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 8074a10183dc..27557351fa20 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
 obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
 obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
 obj-$(CONFIG_ACPI) += physical_location.o
+obj-$(CONFIG_HINT) += hint.o
 
 obj-y			+= test/
 
diff --git a/drivers/base/hint.c b/drivers/base/hint.c
new file mode 100644
index 000000000000..0be4116d68cc
--- /dev/null
+++ b/drivers/base/hint.c
@@ -0,0 +1,283 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/* Hint sysfs interface */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/init.h>
+#include <linux/mutex.h>
+#include <linux/hint.h>
+#include <linux/sysfs.h>
+
+struct hint_handler {
+	const char *name;
+	struct device dev;
+	int minor;
+	struct mutex lock; /* Prevents parallel calls to class device. */
+	unsigned long idle_choices[BITS_TO_LONGS(HINT_IDLE_LAST)];
+	enum hint_idle_option idle;
+	const struct hint_ops *ops;
+};
+
+#define to_hint_handler(d)	(container_of(d, struct hint_handler, dev))
+
+static const char * const hint_idle_names[] = {
+	[HINT_IDLE_SNOOZE]   = "snooze",
+	[HINT_IDLE_RESUME]   = "resume",
+	[HINT_IDLE_INACTIVE] = "inactive",
+	[HINT_IDLE_ACTIVE]   = "active",
+};
+static_assert(ARRAY_SIZE(hint_idle_names) == HINT_IDLE_LAST);
+
+static ssize_t name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct hint_handler *handler = to_hint_handler(dev);
+
+	return sysfs_emit(buf, "%s\n", handler->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static ssize_t idle_choices_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct hint_handler *handler = to_hint_handler(dev);
+	int i, len = 0;
+
+	for_each_set_bit(i, handler->idle_choices, HINT_IDLE_LAST) {
+		if (len == 0)
+			len += sysfs_emit_at(buf, len, "%s", hint_idle_names[i]);
+		else
+			len += sysfs_emit_at(buf, len, " %s", hint_idle_names[i]);
+	}
+	len += sysfs_emit_at(buf, len, "\n");
+
+	return len;
+}
+static DEVICE_ATTR_RO(idle_choices);
+
+static ssize_t idle_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	enum hint_idle_option idle = HINT_IDLE_LAST;
+	struct hint_handler *handler = to_hint_handler(dev);
+	int err;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &handler->lock) {
+		if (!handler->ops->idle_get)
+			return handler->idle;
+
+		err = handler->ops->idle_get(dev, &idle);
+		if (err)
+			return err;
+
+		if (WARN_ON(idle >= HINT_IDLE_LAST))
+			return -EINVAL;
+	}
+
+	return sysfs_emit(buf, "%s\n", hint_idle_names[idle]);
+}
+
+static ssize_t idle_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct hint_handler *handler = to_hint_handler(dev);
+	int index, ret;
+
+	index = sysfs_match_string(hint_idle_names, buf);
+	if (index < 0)
+		return -EINVAL;
+
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &handler->lock) {
+		if (!test_bit(index, handler->idle_choices))
+			return -EOPNOTSUPP;
+
+		if (handler->ops->idle_set) {
+			ret = handler->ops->idle_set(dev, index);
+			if (ret)
+				return ret;
+		}
+		handler->idle = index;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_RW(idle);
+
+static umode_t hint_attr_is_visible(struct kobject *kobj,
+				    struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct hint_handler *handler = to_hint_handler(dev);
+
+	if ((attr == &dev_attr_idle.attr ||
+	     attr == &dev_attr_idle_choices.attr) &&
+	    bitmap_empty(handler->idle_choices, HINT_IDLE_LAST))
+		return 0;
+
+	return attr->mode;
+}
+
+static struct attribute *hint_attrs[] = {
+	&dev_attr_name.attr,
+	&dev_attr_idle_choices.attr,
+	&dev_attr_idle.attr,
+	NULL
+};
+
+static const struct attribute_group hint_group = {
+	.attrs      = hint_attrs,
+	.is_visible = hint_attr_is_visible,
+};
+
+static const struct attribute_group *hint_groups[] = {
+	&hint_group,
+	NULL,
+};
+
+static void hint_device_release(struct device *dev)
+{
+	struct hint_handler *handler = to_hint_handler(dev);
+
+	kfree(handler);
+}
+
+static const struct class hint_class = {
+	.name = "hint",
+	.dev_groups = hint_groups,
+	.dev_release = hint_device_release,
+};
+
+/**
+ * hint_register - Creates and registers a hint class device
+ * @dev: Parent device
+ * @name: Name of the class device
+ * @drvdata: Driver data that will be attached to the class device
+ * @ops: Hint probes and getters/setters
+ *
+ * Return: pointer to the new class device on success, ERR_PTR on failure
+ */
+struct device *hint_register(struct device *dev, const char *name,
+			     void *drvdata,
+			     const struct hint_ops *ops)
+{
+	struct device *adev;
+	int minor, err;
+
+	/* Sanity check */
+	if (WARN_ON_ONCE(!dev || !name || !ops))
+		return ERR_PTR(-EINVAL);
+
+	struct hint_handler *handler __free(kfree) = kzalloc_obj(*handler);
+	if (!handler)
+		return ERR_PTR(-ENOMEM);
+
+	/*
+	 * Hint probes
+	 */
+
+	if (ops->idle_probe) {
+		err = ops->idle_probe(drvdata, handler->idle_choices);
+		if (err) {
+			dev_err(dev, "idle state hint probe failed\n");
+			return ERR_PTR(err);
+		}
+		handler->idle =
+			find_first_bit(handler->idle_choices, HINT_IDLE_LAST);
+	}
+
+	/* create class interface for handler */
+	handler->name = name;
+	handler->ops = ops;
+	handler->minor = minor;
+	handler->dev.class = &hint_class;
+	handler->dev.parent = dev;
+	mutex_init(&handler->lock);
+	dev_set_drvdata(&handler->dev, drvdata);
+	dev_set_name(&handler->dev, name, handler->minor);
+
+	adev = &no_free_ptr(handler)->dev;
+	err = device_register(adev);
+	if (err) {
+		put_device(adev);
+		return ERR_PTR(err);
+	}
+
+	return adev;
+}
+EXPORT_SYMBOL_GPL(hint_register);
+
+/**
+ * hint_remove - Unregisters a hint class device
+ * @dev: Class device
+ */
+void hint_remove(struct device *dev)
+{
+	struct hint_handler *handler;
+
+	if (IS_ERR_OR_NULL(dev))
+		return;
+
+	handler = to_hint_handler(dev);
+
+	guard(mutex)(&handler->lock);
+
+	device_unregister(&handler->dev);
+}
+EXPORT_SYMBOL_GPL(hint_remove);
+
+static void devm_hint_release(struct device *dev, void *res)
+{
+	struct device **adev = res;
+
+	hint_remove(*adev);
+}
+
+/**
+ * devm_hint_register - Device managed version of hint_register
+ * @dev: Parent device
+ * @name: Name of the class device
+ * @drvdata: Driver data that will be attached to the class device
+ * @ops: Activity operations
+ *
+ * Return: pointer to the new class device on success, ERR_PTR on failure
+ */
+struct device *devm_hint_register(struct device *dev, const char *name,
+				  void *drvdata, const struct hint_ops *ops)
+{
+	struct device *adev;
+	struct device **dr;
+
+	dr = devres_alloc(devm_hint_release, sizeof(*dr), GFP_KERNEL);
+	if (!dr)
+		return ERR_PTR(-ENOMEM);
+
+	adev = hint_register(dev, name, drvdata, ops);
+	if (IS_ERR(adev)) {
+		devres_free(dr);
+		return adev;
+	}
+
+	*dr = adev;
+	devres_add(dev, dr);
+
+	return adev;
+}
+EXPORT_SYMBOL_GPL(devm_hint_register);
+
+static int __init hint_init(void)
+{
+	return class_register(&hint_class);
+}
+
+/*
+ * Required for s2idle to be able to register hints.
+ * module_init() would run after it tries to register the device.
+ */
+postcore_initcall(hint_init);
+
+MODULE_AUTHOR("Antheas Kapenekakis <lkml@antheas.dev>");
+MODULE_DESCRIPTION("Activity sysfs interface");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/hint.h b/include/linux/hint.h
new file mode 100644
index 000000000000..56686691442c
--- /dev/null
+++ b/include/linux/hint.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Device hints sysfs interface
+ */
+
+#ifndef _HINT_H_
+#define _HINT_H_
+
+#include <linux/device.h>
+#include <linux/bitops.h>
+
+enum hint_idle_option {
+	HINT_IDLE_ACTIVE,
+	HINT_IDLE_INACTIVE,
+	HINT_IDLE_SNOOZE,
+	HINT_IDLE_RESUME,
+	HINT_IDLE_LAST, /*must always be last */
+};
+
+/**
+ * struct hint_ops - hint probes and get/set operations
+ * @idle_probe:	Callback to setup idle hints available to the device.
+ * @idle_get:	Will be called when showing the current idle hint in sysfs.
+ * @idle_set:	Will be called when storing a new idle hint in sysfs.
+ */
+struct hint_ops {
+	int (*idle_probe)(void *drvdata, unsigned long *choices);
+	int (*idle_get)(struct device *dev, enum hint_idle_option *idle);
+	int (*idle_set)(struct device *dev, enum hint_idle_option idle);
+};
+
+struct device *hint_register(struct device *dev, const char *name,
+			     void *drvdata, const struct hint_ops *ops);
+void hint_remove(struct device *dev);
+struct device *devm_hint_register(struct device *dev, const char *name,
+				  void *drvdata, const struct hint_ops *ops);
+
+#endif  /*_HINT_H_*/
-- 
2.53.0

[RFC v2 09/10] acpi/x86: s2idle: Listen to idle hints to perform MS transitions

[Antheas Kapenekakis]

Modern Standby capable devices allow controlling their appearance by
userspace to appear inactive or asleep while the system is still running.
Expose these states to userspace as idle hints, so userspace can
leverage "dark resume" states to perform background tasks while the
system appears asleep.

If userspace is not idle-aware, transition to snooze and back to active
as part of the normal begin() and end() callbacks, so that normal
functionality (e.g., pulsing the power light) is maintained.

In addition, in case we have fired the intent to turn display on
notification and are in the resume idle state, the transition to s2idle
is undefined behavior. Therefore, momentarily transition to active and
back to snooze and emit an error, instead of bailing.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/acpi/Kconfig      |   1 +
 drivers/acpi/x86/s2idle.c | 226 +++++++++++++++++++++++++++++++-------
 2 files changed, 186 insertions(+), 41 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 6f4b545f7377..08622ace9c67 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -14,6 +14,7 @@ menuconfig ACPI
 	select NLS
 	select CRC32
 	select FIRMWARE_TABLE
+	select HINT if X86 && SUSPEND # s2idle idle hint
 	default y if X86
 	help
 	  Advanced Configuration and Power Interface (ACPI) support for 
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 8b48f999e0e9..357d6f9406dc 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
+#include <linux/hint.h>
 #include <linux/suspend.h>
 
 #include "../sleep.h"
@@ -67,6 +68,9 @@ static guid_t lps0_dsm_guid_microsoft;
 static int lps0_dsm_func_mask_microsoft;
 static int lps0_dsm_state;
 
+static enum hint_idle_option current_idle = HINT_IDLE_ACTIVE;
+static enum hint_idle_option presuspend_idle = HINT_IDLE_ACTIVE;
+
 /* Device constraint entry structure */
 struct lpi_device_info {
 	char *name;
@@ -439,9 +443,171 @@ static const struct acpi_device_id amd_hid_ids[] = {
 	{}
 };
 
+static int acpi_s2idle_idle_probe(void *drvdata, unsigned long *choices)
+{
+	if (!lps0_device_handle || sleep_no_lps0)
+		return 0;
+
+	if (lps0_dsm_func_mask_microsoft > 0) {
+		*choices |= BIT(HINT_IDLE_ACTIVE);
+		if (lps0_dsm_func_mask_microsoft &
+		    (1 << ACPI_LPS0_DISPLAY_OFF | 1 << ACPI_LPS0_DISPLAY_ON))
+			*choices |= BIT(HINT_IDLE_INACTIVE);
+		if (lps0_dsm_func_mask_microsoft &
+		    (1 << ACPI_LPS0_SLEEP_ENTRY | 1 << ACPI_LPS0_SLEEP_EXIT))
+			*choices |= BIT(HINT_IDLE_SNOOZE);
+		if (lps0_dsm_func_mask_microsoft &
+		    (1 << ACPI_LPS0_TURN_ON_DISPLAY))
+			*choices |= BIT(HINT_IDLE_RESUME);
+	}
+
+	if (lps0_dsm_func_mask > 0) {
+		*choices |= BIT(HINT_IDLE_ACTIVE);
+		if (acpi_s2idle_vendor_amd()) {
+			if (lps0_dsm_func_mask &
+			    (1 << ACPI_LPS0_DISPLAY_OFF_AMD |
+			     1 << ACPI_LPS0_DISPLAY_ON_AMD))
+				*choices |= BIT(HINT_IDLE_INACTIVE);
+		} else {
+			if (lps0_dsm_func_mask & (1 << ACPI_LPS0_DISPLAY_OFF |
+						  1 << ACPI_LPS0_DISPLAY_ON))
+				*choices |= BIT(HINT_IDLE_INACTIVE);
+		}
+	}
+
+	return 0;
+}
+
+static int acpi_s2idle_idle_get(struct device *dev, enum hint_idle_option *idle)
+{
+	*idle = current_idle;
+	return 0;
+}
+
+static int acpi_s2idle_idle_set(struct device *dev, enum hint_idle_option idle)
+{
+	if (idle >= HINT_IDLE_LAST)
+		return -EINVAL;
+
+	if (idle == current_idle)
+		return 0;
+
+	acpi_handle_debug(lps0_device_handle,
+			  "Idle state transition from %d to %d\n",
+			  current_idle, idle);
+
+	/* Resume can only be entered if we are on the snooze state. */
+	if (idle == HINT_IDLE_RESUME) {
+		if (current_idle != HINT_IDLE_SNOOZE)
+			return -EINVAL;
+
+		if (lps0_dsm_func_mask_microsoft > 0)
+			acpi_sleep_run_lps0_dsm(ACPI_LPS0_TURN_ON_DISPLAY,
+						lps0_dsm_func_mask_microsoft,
+						lps0_dsm_guid_microsoft);
+
+		current_idle = HINT_IDLE_RESUME;
+		return 0;
+	}
+
+	/*
+	 * The system should not be able to re-enter snooze from resume as it
+	 * is undefined behavior. As part of setting the idle to "Resume",
+	 * userspace promised a transition to "Inactive" or "Active".
+	 */
+	if (current_idle == HINT_IDLE_RESUME &&
+	    idle == HINT_IDLE_SNOOZE)
+		return -EINVAL;
+
+	/*
+	 * When leaving snooze, always fire the resume notification first if
+	 * the device supports it. This is to counteract buggy firmware
+	 * (e.g., Lenovo) that expects the resume notification to fire always.
+	 */
+	if (current_idle == HINT_IDLE_SNOOZE && idle < current_idle &&
+	    lps0_dsm_func_mask_microsoft > 0) {
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_TURN_ON_DISPLAY,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
+	}
+
+	/* Resume is the Snooze state logic-wise. */
+	if (current_idle == HINT_IDLE_RESUME)
+		current_idle = HINT_IDLE_SNOOZE;
+
+	if (current_idle < idle) {
+		for (; current_idle < idle; current_idle++) {
+			switch (current_idle + 1) {
+			case HINT_IDLE_INACTIVE:
+				if (lps0_dsm_func_mask > 0)
+					acpi_sleep_run_lps0_dsm(
+						acpi_s2idle_vendor_amd() ?
+							ACPI_LPS0_DISPLAY_OFF_AMD :
+							ACPI_LPS0_DISPLAY_OFF,
+						lps0_dsm_func_mask,
+						lps0_dsm_guid);
+
+				if (lps0_dsm_func_mask_microsoft > 0)
+					acpi_sleep_run_lps0_dsm(
+						ACPI_LPS0_DISPLAY_OFF,
+						lps0_dsm_func_mask_microsoft,
+						lps0_dsm_guid_microsoft);
+				break;
+			case HINT_IDLE_SNOOZE:
+				if (lps0_dsm_func_mask_microsoft > 0)
+					acpi_sleep_run_lps0_dsm(
+						ACPI_LPS0_SLEEP_ENTRY,
+						lps0_dsm_func_mask_microsoft,
+						lps0_dsm_guid_microsoft);
+				break;
+			default:
+				break;
+			}
+		}
+	} else if (current_idle > idle) {
+		for (; current_idle > idle; current_idle--) {
+			switch (current_idle) {
+			case HINT_IDLE_INACTIVE:
+				if (lps0_dsm_func_mask > 0)
+					acpi_sleep_run_lps0_dsm(
+						acpi_s2idle_vendor_amd() ?
+							ACPI_LPS0_DISPLAY_ON_AMD :
+							ACPI_LPS0_DISPLAY_ON,
+						lps0_dsm_func_mask,
+						lps0_dsm_guid);
+				if (lps0_dsm_func_mask_microsoft > 0)
+					acpi_sleep_run_lps0_dsm(
+						ACPI_LPS0_DISPLAY_ON,
+						lps0_dsm_func_mask_microsoft,
+						lps0_dsm_guid_microsoft);
+				break;
+			case HINT_IDLE_SNOOZE:
+				if (lps0_dsm_func_mask_microsoft > 0)
+					acpi_sleep_run_lps0_dsm(
+						ACPI_LPS0_SLEEP_EXIT,
+						lps0_dsm_func_mask_microsoft,
+						lps0_dsm_guid_microsoft);
+				break;
+			default:
+				break;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static struct hint_ops acpi_s2idle_hint_ops = {
+	.idle_probe = acpi_s2idle_idle_probe,
+	.idle_get = acpi_s2idle_idle_get,
+	.idle_set = acpi_s2idle_idle_set,
+};
+
 static int lps0_device_attach(struct acpi_device *adev,
 			      const struct acpi_device_id *not_used)
 {
+	struct device *hdev;
+
 	if (lps0_device_handle)
 		return 0;
 
@@ -508,6 +674,15 @@ static int lps0_device_attach(struct acpi_device *adev,
 	 */
 	acpi_ec_mark_gpe_for_wake();
 
+	/*
+	 * Add idle hint handler to lps0_device_handle.
+	 */
+	hdev = devm_hint_register(&adev->dev, "s2idle", NULL,
+				  &acpi_s2idle_hint_ops);
+	if (IS_ERR(hdev))
+		acpi_handle_err(adev->handle,
+				"Failed to register idle hint device\n");
+
 	return 0;
 }
 
@@ -538,23 +713,14 @@ static int acpi_s2idle_begin_lps0(void)
 			lpi_constraints_table = ERR_PTR(-ENODATA);
 	}
 
-	/* Display off */
-	if (lps0_dsm_func_mask > 0)
-		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
-					ACPI_LPS0_DISPLAY_OFF_AMD :
-					ACPI_LPS0_DISPLAY_OFF,
-					lps0_dsm_func_mask, lps0_dsm_guid);
-
-	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_OFF,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
-
-	/* Modern Standby entry */
-	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_ENTRY,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
+	presuspend_idle = current_idle;
+	if (current_idle == HINT_IDLE_RESUME) {
+		acpi_handle_err(
+			lps0_device_handle,
+			"Unexpected idle state: Resume. Transitioning to active and back.\n");
+		acpi_s2idle_idle_set(NULL, HINT_IDLE_ACTIVE);
+	}
+	acpi_s2idle_idle_set(NULL, HINT_IDLE_SNOOZE);
 
 	list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
 		if (handler->begin_delay && handler->begin_delay > delay)
@@ -636,30 +802,8 @@ static void acpi_s2idle_end_lps0(void)
 {
 	acpi_s2idle_end();
 
-	if (!lps0_device_handle || sleep_no_lps0)
-		return;
-
-	if (lps0_dsm_func_mask_microsoft > 0) {
-		/* Intent to turn on display */
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_TURN_ON_DISPLAY,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
-		/* Modern Standby exit */
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SLEEP_EXIT,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
-	}
-
-	/* Display on */
-	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_DISPLAY_ON,
-					lps0_dsm_func_mask_microsoft,
-					lps0_dsm_guid_microsoft);
-	if (lps0_dsm_func_mask > 0)
-		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
-					ACPI_LPS0_DISPLAY_ON_AMD :
-					ACPI_LPS0_DISPLAY_ON,
-					lps0_dsm_func_mask, lps0_dsm_guid);
+	if (lps0_device_handle && !sleep_no_lps0)
+		acpi_s2idle_idle_set(NULL, presuspend_idle);
 }
 
 static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
-- 
2.53.0

[RFC v2 10/10] acpi/x86: s2idle: Subtract delay from last DSM fire in begin delay

[Antheas Kapenekakis]

Certain devices require a small delay to settle prior to suspend after
the last DSM firmware notification. Since that notification can now be
fired by userspace, subtract the elapsed time since the last DSM call to
lower or skip the begin quirk delay.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/acpi/x86/s2idle.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 357d6f9406dc..06f08b5a21fd 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -70,6 +70,7 @@ static int lps0_dsm_state;
 
 static enum hint_idle_option current_idle = HINT_IDLE_ACTIVE;
 static enum hint_idle_option presuspend_idle = HINT_IDLE_ACTIVE;
+static unsigned long idle_transition_jiffies;
 
 /* Device constraint entry structure */
 struct lpi_device_info {
@@ -507,6 +508,7 @@ static int acpi_s2idle_idle_set(struct device *dev, enum hint_idle_option idle)
 						lps0_dsm_guid_microsoft);
 
 		current_idle = HINT_IDLE_RESUME;
+		idle_transition_jiffies = jiffies;
 		return 0;
 	}
 
@@ -594,6 +596,7 @@ static int acpi_s2idle_idle_set(struct device *dev, enum hint_idle_option idle)
 		}
 	}
 
+	idle_transition_jiffies = jiffies;
 	return 0;
 }
 
@@ -694,6 +697,7 @@ static struct acpi_scan_handler lps0_handler = {
 static int acpi_s2idle_begin_lps0(void)
 {
 	struct acpi_s2idle_dev_ops *handler;
+	unsigned int elapsed;
 	int delay = 0;
 
 	if (!lps0_device_handle || sleep_no_lps0)
@@ -726,8 +730,15 @@ static int acpi_s2idle_begin_lps0(void)
 		if (handler->begin_delay && handler->begin_delay > delay)
 			delay = handler->begin_delay;
 	}
-	if (delay > 0)
-		msleep(delay);
+	if (delay > 0) {
+		elapsed = jiffies_to_msecs(jiffies - idle_transition_jiffies);
+		if (delay > elapsed) {
+			msleep(delay - elapsed);
+			acpi_handle_debug(lps0_device_handle,
+					  "Waited %d ms for begin quirk\n",
+					  delay - elapsed);
+		}
+	}
 
 	return acpi_s2idle_begin();
 }
-- 
2.53.0