mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Idotoho Reimon Simanjuntak <idotohors@gmail.com>
Cc: Corentin Chary <corentin.chary@gmail.com>,
	 "Luke D. Jones" <luke@ljones.dev>,
	Denis Benato <denis.benato@linux.dev>,
	 Hans de Goede <hansg@kernel.org>,
	platform-driver-x86@vger.kernel.org,
	 LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] platform/x86: asus-wmi: fix FA401 series keyboard sleep strobe
Date: Wed, 2 Sep 2026 12:42:54 +0300 (EEST)	[thread overview]
Message-ID: <b8cdf49b-91d4-2e86-a8ea-04d1db119225@linux.intel.com> (raw)
In-Reply-To: <20260902032353.16106-1-idotohors@gmail.com>

On Wed, 2 Sep 2026, Idotoho Reimon Simanjuntak wrote:

> The FA401 series accepts the TUF keyboard RGB power-state command but does
> not advertise the device through the normal DSTS presence probe. Register
> the state attributes for this series and re-assert keyboard brightness in
> the PM prepare callback so the EC enters S0ix with the correct state.
> 
> Without the brightness re-assertion, the display server blanks the keyboard
> before the kernel suspend path runs, leaving the EC with brightness=0 and
> preventing the sleep strobe from activating. Because kbd_led_wk may already
> be zeroed by the display server at prepare time, always re-assert level 3
> (max) brightness when sleep backlight is enabled.
> 
> Use a quirk flag (kbd_rgb_state_quirk) set via DMI match table rather than
> ad-hoc dmi_match() calls. Cache power-state flags from sysfs writes and
> initialize the cache to the default enabled state during probe, so the
> workaround also applies before userspace writes the attribute. Do not add
> a module parameter; the sysfs power-state setting is authoritative.
> Use the standard dev_pm_ops .prepare callback rather than overloading the
> Ally-specific LPS0 ops. Define self-documenting macros for the TUF RGB
> state bitfields at file scope.
> 
> Signed-off-by: Idotoho Reimon Simanjuntak <idotohors@gmail.com>
> ---
> Changes in v2:
>  - Cache RGB power state flags from sysfs writes instead of attempting DSTS read.
>  - Initialize state flags to enable all modes during driver probe.
>  - Move TUF_RGB_STATE_* bitfield macros to file scope.
>  - Drop module parameter in favor of DMI quirk + sysfs control.
>  drivers/platform/x86/asus-nb-wmi.c | 14 +++++++
>  drivers/platform/x86/asus-wmi.c    | 61 +++++++++++++++++++++++++++++-
>  drivers/platform/x86/asus-wmi.h    |  1 +
>  3 files changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index aeb461b16..fb06013ca 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -155,6 +155,11 @@ static struct quirk_entry quirk_asus_z13 = {
>  	.tablet_switch_mode = asus_wmi_kbd_dock_devid,
>  };
>  
> +static struct quirk_entry quirk_asus_fa401 = {
> +	.wapf = 0,
> +	.kbd_rgb_state_quirk = true,
> +};
> +
>  static int dmi_matched(const struct dmi_system_id *dmi)
>  {
>  	pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -163,6 +168,15 @@ static int dmi_matched(const struct dmi_system_id *dmi)
>  }
>  
>  static const struct dmi_system_id asus_quirks[] = {
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUSTeK COMPUTER INC. FA401",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_BOARD_NAME, "FA401"),
> +		},
> +		.driver_data = &quirk_asus_fa401,
> +	},
>  	{
>  		.callback = dmi_matched,
>  		.ident = "ASUSTeK COMPUTER INC. Q500A",
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index a65090429..a0b678272 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -129,6 +129,18 @@ module_param(fnlock_default, bool, 0444);
>  #define ASUS_SCREENPAD_BRIGHT_MAX 255
>  #define ASUS_SCREENPAD_BRIGHT_DEFAULT 60
>  
> +/* TUF RGB power state bitfields for DEVS(ASUS_WMI_DEVID_TUF_RGB_STATE) */
> +#define TUF_RGB_STATE_CMD		0xbd
> +#define TUF_RGB_STATE_SAVE		BIT(8)
> +#define TUF_RGB_STATE_BOOT		(0x03 << 16)
> +#define TUF_RGB_STATE_AWAKE		(0x0c << 16)
> +#define TUF_RGB_STATE_SLEEP		(0x30 << 16)
> +#define TUF_RGB_STATE_KEYBOARD		(0xc0 << 16)
> +#define TUF_RGB_STATE_ALL_MODES		(TUF_RGB_STATE_BOOT | \
> +					 TUF_RGB_STATE_AWAKE | \
> +					 TUF_RGB_STATE_SLEEP | \
> +					 TUF_RGB_STATE_KEYBOARD)
> +
>  #define ASUS_MINI_LED_MODE_MASK		0x03
>  /* Standard modes for devices with only on/off */
>  #define ASUS_MINI_LED_OFF		0x00
> @@ -308,6 +320,8 @@ struct asus_wmi {
>  	u32 nv_temp_target;
>  
>  	u32 kbd_rgb_dev;
> +	u32 kbd_rgb_state_flags;
> +	bool kbd_rgb_state_valid;
>  	bool kbd_rgb_state_available;
>  	bool oobe_state_available;
>  
> @@ -1120,8 +1134,13 @@ static ssize_t kbd_rgb_state_store(struct device *dev,
>  				 const char *buf, size_t count)
>  {
>  	u32 flags, cmd, boot, awake, sleep, keyboard;
> +	struct led_classdev *led;
> +	struct asus_wmi *asus;
>  	int err;
>  
> +	led = dev_get_drvdata(dev);
> +	asus = container_of(led, struct asus_wmi, kbd_led);
> +
>  	if (sscanf(buf, "%d %d %d %d %d", &cmd, &boot, &awake, &sleep, &keyboard) != 5)
>  		return -EINVAL;
>  
> @@ -1138,6 +1157,9 @@ static ssize_t kbd_rgb_state_store(struct device *dev,
>  	if (keyboard)
>  		flags |= BIT(7);
>  
> +	asus->kbd_rgb_state_flags = flags << 16;
> +	asus->kbd_rgb_state_valid = true;
> +
>  	/* 0xbd is the required default arg0 for the method. Nothing happens otherwise */
>  	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
>  			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xbd | cmd << 8 | (flags << 16), 0, NULL);

Interesting, is this using TUF_RGB_STATE_CMD as literal? Perhaps you 
should expand this into a patch series and make that conversion in 
preparatory patch.

> @@ -5153,7 +5175,6 @@ static int asus_wmi_add(struct platform_device *pdev)
>  
>  	asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU);
>  	asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU);
> -	asus->kbd_rgb_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE);
>  
>  	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_MINI_LED_MODE))
>  		asus->mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
> @@ -5166,6 +5187,19 @@ static int asus_wmi_add(struct platform_device *pdev)
>  		asus->gpu_mux_dev = ASUS_WMI_DEVID_GPU_MUX_VIVO;
>  #endif /* IS_ENABLED(CONFIG_ASUS_WMI_DEPRECATED_ATTRS) */
>  
> +	/*
> +	 * FA401 series accepts the TUF RGB-state DEVS command but does not
> +	 * advertise the device through DSTS. Keep the normal DSTS probe for
> +	 * other models and force registration when the quirk is set.
> +	 */
> +	asus->kbd_rgb_state_available =
> +		asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE) ||
> +		asus->driver->quirks->kbd_rgb_state_quirk;
> +	if (asus->driver->quirks->kbd_rgb_state_quirk) {
> +		asus->kbd_rgb_state_flags = TUF_RGB_STATE_ALL_MODES;
> +		asus->kbd_rgb_state_valid = true;
> +	}
> +
>  	asus->oobe_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_OOBE);
>  
>  	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY))
> @@ -5396,11 +5430,36 @@ static int asus_hotk_restore(struct device *device)
>  
>  static int asus_hotk_prepare(struct device *device)
>  {
> +	struct asus_wmi *asus = dev_get_drvdata(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);
>  	}
> +
> +	/*
> +	 * The display server may blank the keyboard backlight before the
> +	 * kernel suspend path runs. Re-assert brightness and power-state
> +	 * flags so the EC enters S0ix with the correct state. Always use
> +	 * level 3 (max) because kbd_led_wk may already be zeroed by the
> +	 * display server at this point, and the sleep strobe requires a
> +	 * non-zero brightness to activate.
> +	 */
> +	if (asus && asus->driver->quirks->kbd_rgb_state_quirk &&
> +	    asus->kbd_rgb_state_available && asus->kbd_rgb_state_valid &&
> +	    (asus->kbd_rgb_state_flags & TUF_RGB_STATE_SLEEP)) {
> +		u8 brightness = 0x80 | 0x03; /* level 3 (max) + light-on bit */
> +
> +		asus_wmi_set_devstate(ASUS_WMI_DEVID_KBD_BACKLIGHT,
> +				      brightness, NULL);
> +		asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
> +					  ASUS_WMI_DEVID_TUF_RGB_STATE,
> +					  TUF_RGB_STATE_CMD | TUF_RGB_STATE_SAVE |
> +					  asus->kbd_rgb_state_flags,

This makes things more confusing overall.

One should use FIELD_PREP() consistently for bitfields where a shift is 
needed but here you write a pre-shifted value. In the other place, this 
driver shifts (which should be converted to FIELD_PREP()) while making the 
evaluate call.

> +					  0, NULL);
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 5cd4392b9..69d224774 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -52,6 +52,7 @@ struct quirk_entry {
>  	 */
>  	int no_display_toggle;
>  	u32 xusb2pr;
> +	bool kbd_rgb_state_quirk;
>  };
>  
>  struct asus_wmi_driver {
> 

-- 
 i.


  reply	other threads:[~2026-09-02  9:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02  3:23 Idotoho Reimon Simanjuntak
2026-09-02  9:42 ` Ilpo Järvinen [this message]
2026-09-02 10:46   ` ido
2026-09-02 11:54   ` Denis Benato
2026-09-02 16:42     ` ido

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8cdf49b-91d4-2e86-a8ea-04d1db119225@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=corentin.chary@gmail.com \
    --cc=denis.benato@linux.dev \
    --cc=hansg@kernel.org \
    --cc=idotohors@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luke@ljones.dev \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®