All of lore.kernel.org
 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] platform/x86: asus-wmi: fix FA401 series keyboard sleep strobe
Date: Wed, 2 Sep 2026 12:36:09 +0300 (EEST)	[thread overview]
Message-ID: <a429f57e-0e3d-65b3-09a6-748760fa86e2@linux.intel.com> (raw)
In-Reply-To: <20260902015256.23434-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; the sleep strobe requires a non-zero brightness to
> activate.
> 
> Use a quirk flag (kbd_rgb_state_quirk) set via DMI match table rather than
> ad-hoc dmi_match() calls. 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>
> ---
>  drivers/platform/x86/asus-nb-wmi.c | 14 ++++++++++
>  drivers/platform/x86/asus-wmi.c    | 43 +++++++++++++++++++++++++++++-
>  drivers/platform/x86/asus-wmi.h    |  1 +
>  3 files changed, 57 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..8d7c9e8bd 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -129,6 +129,14 @@ 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)

Can those values be somehow named? Is there a field that could then use 
FIELD_PREP() or are these individual bits that could be named and or'ed 
together?

Alternatively, if no better naming exists for these, convert them to 
GENMASK().

> +
>  #define ASUS_MINI_LED_MODE_MASK		0x03
>  /* Standard modes for devices with only on/off */
>  #define ASUS_MINI_LED_OFF		0x00
> @@ -5153,7 +5161,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 +5173,15 @@ 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;
> +
>  	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 +5412,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) {
> +		u8 brightness = 0x80 | 0x03; /* level 3 (max) + light-on bit */

Please use a named define for "light-on bit" instead of the literal.

As level 3 is mentioned also in the other comment, you can drop this 
comment.

> +
> +		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 |
> +					  TUF_RGB_STATE_BOOT | TUF_RGB_STATE_AWAKE |
> +					  TUF_RGB_STATE_SLEEP | TUF_RGB_STATE_KEYBOARD,
> +					  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:36 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02  1:52 [PATCH] platform/x86: asus-wmi: fix FA401 series keyboard sleep strobe Idotoho Reimon Simanjuntak
2026-09-02  9:36 ` Ilpo Järvinen [this message]

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=a429f57e-0e3d-65b3-09a6-748760fa86e2@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.