[RFC PATCH 0/1] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[RFC PATCH 0/1] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Qiling Zhu]

Hi Mark, Ilpo, Rong,

Recent ThinkPads with an ambient light sensor (ALS) support an auto
mode for the keyboard backlight: the EC picks the backlight level
according to ambient light. The mode is EC keyboard backlight level 3,
above the two fixed levels exposed so far, and it is reachable only
through the Fn+Space shortcut, which is handled entirely by the
firmware. Userspace can neither select the mode nor observe whether it
is active.

This series wires the auto mode up as a private hardware control
trigger, "tpacpi-kbd-auto", on top of the hw control trigger framework
that recently landed in mainline [1]. With it:

  - selecting the trigger writes EC level 3 (auto mode on); writing
    "none" turns the backlight off
  - the trigger state (bracketed in the trigger sysfs attribute, and
    the new trigger_may_offload_to_hw attribute) is the userspace
    view of auto mode; reading brightness returns -ENODATA while
    auto mode is active, per the framework semantics
  - Fn+Space keeps cycling through levels and auto mode as before,
    and the trigger state now follows hardware-initiated transitions
    automatically

This is the thinkpad_acpi counterpart of the ideapad-laptop wiring
from the same framework series, which is still queued; this patch
depends only on the LED core parts already in mainline and can be
applied independently.

For background: the February discussion settled that auto mode must
not be exposed as a brightness value [2], but no agreement was
reached back then on how to expose it instead; the hardware control
trigger approach was established later by the framework series [1].

Tested on a ThinkPad X1 Carbon Gen 14 (MT 21V8) running linux-next
next-20261030: 61 scripted user-visible assertions covering the
software paths, the Fn+Space event chain and the unchanged legacy
behavior, plus manual exercising of both the Fn+Space cycling and
desktop-environment brightness controls; checkpatch --strict clean.

Deliberately unchanged (details in the commit message):

  - max_brightness stays 2; auto mode is not multiplexed into the
    brightness value
  - writing 3 to /proc/acpi/ibm/kbdlight keeps returning -EINVAL
  - module unload still turns the keyboard backlight off

Two open questions, the main reason for the RFC tag:

  1. MLCG returns capability bits in the upper 16 bits; does one of
     them indicate auto mode support? If yes, I would gate trigger
     registration on it. Mark, does the EC documentation say?
  2. On models without an ALS, the EC behavior when asked for level 3
     via MLCS is unknown to me. If it is undefined rather than
     rejected, optimistic registration would need a rethink (e.g.
     DMI whitelisting).

Rong, happy to rebase this on top of your series or fold it in, if
you prefer to keep both wirings together.

Comments welcome.

Regards,
Qiling Zhu

This patch was developed with LLM assistance; I reviewed every design
decision against the framework sources and validated the behavior on
real hardware myself.

[1] https://lore.kernel.org/r/20260618-leds-trigger-hw-changed-v2-0-c28c44053cf3@rong.moe/
[2] https://lore.kernel.org/r/20260203232219.11683-1-vishnuocv@gmail.com/

Qiling Zhu (1):
  platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

 drivers/platform/x86/lenovo/Kconfig         |   1 +
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 104 ++++++++++++++++----
 2 files changed, 88 insertions(+), 17 deletions(-)

-- 
2.56.0

[RFC PATCH 1/1] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Qiling Zhu]

Recent ThinkPads with an ambient light sensor (ALS) support an auto mode
for the keyboard backlight: the EC controls the backlight according to
ambient light. The mode is EC keyboard backlight level 3, in addition to
the two fixed brightness levels currently exposed by the driver, and is
currently reachable only through the Fn+Space shortcut, which is handled
directly by the firmware.

Wire the auto mode up as a private hardware control trigger, named
"tpacpi-kbd-auto", on top of the LED hw control trigger framework:

  - selecting the trigger writes level 3 (activate); the offloaded state
    is derived from the current EC level
  - when Fn+Space cycles into or out of auto mode, the driver notifies
    the LED core about the hardware-initiated hardware control
    transition, so the trigger state (shown bracketed in the trigger
    sysfs attribute) follows along
  - if the hardware is already in auto mode at driver initialization,
    the trigger is set as the default one

Deliberately unchanged:

  - max_brightness stays 2, auto mode is not multiplexed into the
    brightness value
  - writing 3 to /proc/acpi/ibm/kbdlight keeps returning -EINVAL, while
    reads keep reporting the current EC level verbatim (including 3)
  - module unload still turns the keyboard backlight off

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---

Note: Documentation/admin-guide/laptops/thinkpad-acpi.rst has no kbdlight
section at all, so there is no in-tree documentation to update; the
userspace-facing semantics of hardware control triggers are documented
in Documentation/leds/leds-class.rst.

Open questions (details in the cover letter): whether a MLCG capability
bit indicates auto mode support, and the EC behavior on ALS-less models
when asked for level 3.

 drivers/platform/x86/lenovo/Kconfig         |   1 +
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 104 ++++++++++++++++----
 2 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
index 516594993..74fc66a44 100644
--- a/drivers/platform/x86/lenovo/Kconfig
+++ b/drivers/platform/x86/lenovo/Kconfig
@@ -75,6 +75,7 @@ config THINKPAD_ACPI
 	depends on RFKILL || RFKILL = n
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on BACKLIGHT_CLASS_DEVICE
+	depends on LEDS_TRIGGERS_HW_CHANGED
 	depends on I2C
 	depends on DRM
 	select ACPI_PLATFORM_PROFILE
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 4f81855ad..cd5c17d5f 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5102,6 +5102,9 @@ static struct ibm_struct video_driver_data = {
  * Keyboard backlight subdriver
  */
 
+/* EC level that selects ALS-controlled auto mode for the keyboard backlight */
+#define TPACPI_KBD_LIGHT_AUTO_LEVEL	3
+
 static enum led_brightness kbdlight_brightness;
 static DEFINE_MUTEX(kbdlight_mutex);
 
@@ -5197,13 +5200,33 @@ static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
 	return level;
 }
 
+static struct led_hw_trigger_type kbdlight_auto_trigger_type;
+
+static int kbdlight_auto_trigger_activate(struct led_classdev *led_cdev)
+{
+	return kbdlight_set_level(TPACPI_KBD_LIGHT_AUTO_LEVEL);
+}
+
+static bool kbdlight_auto_trigger_hw_offloaded(struct led_classdev *led_cdev)
+{
+	return kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+}
+
+static struct led_trigger kbdlight_auto_trigger = {
+	.name		= "tpacpi-kbd-auto",
+	.trigger_type	= &kbdlight_auto_trigger_type,
+	.activate	= kbdlight_auto_trigger_activate,
+	.hw_offloaded	= kbdlight_auto_trigger_hw_offloaded,
+};
+
 static struct tpacpi_led_classdev tpacpi_led_kbdlight = {
 	.led_classdev = {
 		.name		= "tpacpi::kbd_backlight",
 		.max_brightness	= 2,
-		.flags		= LED_BRIGHT_HW_CHANGED,
+		.flags		= LED_BRIGHT_HW_CHANGED | LED_TRIG_HW_CHANGED,
 		.brightness_set_blocking = &kbdlight_sysfs_set,
 		.brightness_get	= &kbdlight_sysfs_get,
+		.trigger_type	= &kbdlight_auto_trigger_type,
 	}
 };
 
@@ -5224,6 +5247,26 @@ static int __init kbdlight_init(struct ibm_init_struct *iibm)
 	kbdlight_brightness = kbdlight_sysfs_get(NULL);
 	tp_features.kbdlight = 1;
 
+	/*
+	 * Register the auto-mode trigger before the LED classdev: if the
+	 * hardware is already in auto mode, the trigger can then be set as
+	 * the default one and gets activated during LED classdev
+	 * registration.
+	 */
+	rc = devm_led_trigger_register(&tpacpi_pdev->dev,
+				       &kbdlight_auto_trigger);
+	if (rc) {
+		tp_features.kbdlight = 0;
+		return rc;
+	}
+
+	tpacpi_led_kbdlight.led_classdev.hw_control_trigger =
+		kbdlight_auto_trigger.name;
+
+	if (kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		tpacpi_led_kbdlight.led_classdev.default_trigger =
+			kbdlight_auto_trigger.name;
+
 	rc = led_classdev_register(&tpacpi_pdev->dev,
 				   &tpacpi_led_kbdlight.led_classdev);
 	if (rc < 0) {
@@ -5313,6 +5356,47 @@ static void kbdlight_resume(void)
 	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
 }
 
+/* Notify the LED core when the hardware cycles into or out of auto mode. */
+static void kbdlight_notify_hw_control(struct led_classdev *led_cdev,
+				       int old_level, int new_level)
+{
+	bool was_auto = old_level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+	bool is_auto = new_level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+
+	if (is_auto != was_auto)
+		led_trigger_notify_hw_control_changed(led_cdev, is_auto);
+}
+
+/*
+ * The EC cycles the keyboard backlight level (and auto mode) itself on
+ * Fn+Space; check the current level and update the LED core state.
+ */
+static void kbdlight_notify_change(void)
+{
+	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
+	enum led_brightness brightness;
+
+	guard(mutex)(&kbdlight_mutex);
+
+	/*
+	 * Check the brightness actually changed, setting the brightness
+	 * through kbdlight_set_level() also triggers this event.
+	 */
+	brightness = kbdlight_sysfs_get(NULL);
+	if (kbdlight_brightness == brightness)
+		return;
+
+	/*
+	 * Calling kbdlight_notify_hw_control() is safe while holding
+	 * kbdlight_mutex: the notification is only queued, and the worker
+	 * re-acquires the mutex (if at all) after this critical section has
+	 * been left.
+	 */
+	kbdlight_notify_hw_control(led_cdev, kbdlight_brightness, brightness);
+	kbdlight_brightness = brightness;
+	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);
+}
+
 static struct ibm_struct kbdlight_driver_data = {
 	.name = "kbdlight",
 	.read = kbdlight_read,
@@ -11581,22 +11665,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
 		/* Key events are suppressed by default hotkey_user_mask */
 		return false;
 	case TP_HKEY_EV_KBD_LIGHT:
-		if (tp_features.kbdlight) {
-			enum led_brightness brightness;
-
-			guard(mutex)(&kbdlight_mutex);
-
-			/*
-			 * Check the brightness actually changed, setting the brightness
-			 * through kbdlight_set_level() also triggers this event.
-			 */
-			brightness = kbdlight_sysfs_get(NULL);
-			if (kbdlight_brightness != brightness) {
-				kbdlight_brightness = brightness;
-				led_classdev_notify_brightness_hw_changed(
-					&tpacpi_led_kbdlight.led_classdev, brightness);
-			}
-		}
+		if (tp_features.kbdlight)
+			kbdlight_notify_change();
 		/* Key events are suppressed by default hotkey_user_mask */
 		return false;
 	case TP_HKEY_EV_DFR_CHANGE_ROW:
-- 
2.56.0


base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

Re: [RFC PATCH 0/1] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Rong Zhang]

Hi Qiling,

Thanks for working on this.

On Sun, 2026-11-01 at 18:01 +0800, Qiling Zhu wrote:
> Hi Mark, Ilpo, Rong,
> 
> Recent ThinkPads with an ambient light sensor (ALS) support an auto
> mode for the keyboard backlight: the EC picks the backlight level
> according to ambient light. The mode is EC keyboard backlight level 3,
> above the two fixed levels exposed so far, and it is reachable only
> through the Fn+Space shortcut, which is handled entirely by the
> firmware. Userspace can neither select the mode nor observe whether it
> is active.
> 
> This series wires the auto mode up as a private hardware control
> trigger, "tpacpi-kbd-auto", on top of the hw control trigger framework
> that recently landed in mainline [1]. With it:
> 
>   - selecting the trigger writes EC level 3 (auto mode on); writing
>     "none" turns the backlight off
>   - the trigger state (bracketed in the trigger sysfs attribute, and
>     the new trigger_may_offload_to_hw attribute) is the userspace
>     view of auto mode; reading brightness returns -ENODATA while
>     auto mode is active, per the framework semantics
>   - Fn+Space keeps cycling through levels and auto mode as before,
>     and the trigger state now follows hardware-initiated transitions
>     automatically
> 
> This is the thinkpad_acpi counterpart of the ideapad-laptop wiring
> from the same framework series, which is still queued; this patch
> depends only on the LED core parts already in mainline and can be
> applied independently.
> 
> For background: the February discussion settled that auto mode must
> not be exposed as a brightness value [2], but no agreement was
> reached back then on how to expose it instead; the hardware control
> trigger approach was established later by the framework series [1].
> 
> Tested on a ThinkPad X1 Carbon Gen 14 (MT 21V8) running linux-next
> next-20261030: 61 scripted user-visible assertions covering the
> software paths, the Fn+Space event chain and the unchanged legacy
> behavior, plus manual exercising of both the Fn+Space cycling and
> desktop-environment brightness controls; checkpatch --strict clean.
> 
> Deliberately unchanged (details in the commit message):
> 
>   - max_brightness stays 2; auto mode is not multiplexed into the
>     brightness value
>   - writing 3 to /proc/acpi/ibm/kbdlight keeps returning -EINVAL
>   - module unload still turns the keyboard backlight off
> 
> Two open questions, the main reason for the RFC tag:
> 
>   1. MLCG returns capability bits in the upper 16 bits; does one of
>      them indicate auto mode support? If yes, I would gate trigger
>      registration on it. Mark, does the EC documentation say?
>   2. On models without an ALS, the EC behavior when asked for level 3
>      via MLCS is unknown to me. If it is undefined rather than
>      rejected, optimistic registration would need a rethink (e.g.
>      DMI whitelisting).
> 
> Rong, happy to rebase this on top of your series or fold it in, if
> you prefer to keep both wirings together.

(+CC Vishnu)

AFAIK, Vishnu is also working on adding the same support using the hw
control trigger framework. Maybe they can answer your questions or
integrate your patch into theirs.

Thanks,
Rong

> 
> Comments welcome.
> 
> Regards,
> Qiling Zhu
> 
> This patch was developed with LLM assistance; I reviewed every design
> decision against the framework sources and validated the behavior on
> real hardware myself.
> 
> [1] https://lore.kernel.org/r/20260618-leds-trigger-hw-changed-v2-0-c28c44053cf3@rong.moe/
> [2] https://lore.kernel.org/r/20260203232219.11683-1-vishnuocv@gmail.com/
> 
> Qiling Zhu (1):
>   platform/x86: thinkpad_acpi: Add support for auto keyboard backlight
> 
>  drivers/platform/x86/lenovo/Kconfig         |   1 +
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 104 ++++++++++++++++----
>  2 files changed, 88 insertions(+), 17 deletions(-)

Re: [RFC PATCH 1/1] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Rong Zhang]

Hi Qiling

On Sun, 2026-11-01 at 18:01 +0800, Qiling Zhu wrote:
> Recent ThinkPads with an ambient light sensor (ALS) support an auto mode
> for the keyboard backlight: the EC controls the backlight according to
> ambient light. The mode is EC keyboard backlight level 3, in addition to
> the two fixed brightness levels currently exposed by the driver, and is
> currently reachable only through the Fn+Space shortcut, which is handled
> directly by the firmware.
> 
> Wire the auto mode up as a private hardware control trigger, named
> "tpacpi-kbd-auto", on top of the LED hw control trigger framework:
> 
>   - selecting the trigger writes level 3 (activate); the offloaded state
>     is derived from the current EC level
>   - when Fn+Space cycles into or out of auto mode, the driver notifies
>     the LED core about the hardware-initiated hardware control
>     transition, so the trigger state (shown bracketed in the trigger
>     sysfs attribute) follows along
>   - if the hardware is already in auto mode at driver initialization,
>     the trigger is set as the default one
> 
> Deliberately unchanged:
> 
>   - max_brightness stays 2, auto mode is not multiplexed into the
>     brightness value
>   - writing 3 to /proc/acpi/ibm/kbdlight keeps returning -EINVAL, while
>     reads keep reporting the current EC level verbatim (including 3)
>   - module unload still turns the keyboard backlight off
> 
> Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
> ---
> 
> Note: Documentation/admin-guide/laptops/thinkpad-acpi.rst has no kbdlight
> section at all, so there is no in-tree documentation to update; the
> userspace-facing semantics of hardware control triggers are documented
> in Documentation/leds/leds-class.rst.
> 
> Open questions (details in the cover letter): whether a MLCG capability
> bit indicates auto mode support, and the EC behavior on ALS-less models
> when asked for level 3.
> 
>  drivers/platform/x86/lenovo/Kconfig         |   1 +
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 104 ++++++++++++++++----
>  2 files changed, 88 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> index 516594993..74fc66a44 100644
> --- a/drivers/platform/x86/lenovo/Kconfig
> +++ b/drivers/platform/x86/lenovo/Kconfig
> @@ -75,6 +75,7 @@ config THINKPAD_ACPI
>  	depends on RFKILL || RFKILL = n
>  	depends on ACPI_VIDEO || ACPI_VIDEO = n
>  	depends on BACKLIGHT_CLASS_DEVICE
> +	depends on LEDS_TRIGGERS_HW_CHANGED

Nah. No driver should depend on or select LEDS_TRIGGERS_HW_CHANGED.
Everything still works without it.

That's also why no driver depends on or selects
LEDS_BRIGHTNESS_HW_CHANGED.

Both options are only enabled if a user (or a distribution) opts into it
while configuring Kconfig options.

>  	depends on I2C
>  	depends on DRM
>  	select ACPI_PLATFORM_PROFILE
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index 4f81855ad..cd5c17d5f 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -5102,6 +5102,9 @@ static struct ibm_struct video_driver_data = {
>   * Keyboard backlight subdriver
>   */
>  
> +/* EC level that selects ALS-controlled auto mode for the keyboard backlight */
> +#define TPACPI_KBD_LIGHT_AUTO_LEVEL	3
> +
>  static enum led_brightness kbdlight_brightness;
>  static DEFINE_MUTEX(kbdlight_mutex);
>  
> @@ -5197,13 +5200,33 @@ static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
>  	return level;
>  }
>  
> +static struct led_hw_trigger_type kbdlight_auto_trigger_type;
> +
> +static int kbdlight_auto_trigger_activate(struct led_classdev *led_cdev)
> +{
> +	return kbdlight_set_level(TPACPI_KBD_LIGHT_AUTO_LEVEL);
> +}
> +
> +static bool kbdlight_auto_trigger_hw_offloaded(struct led_classdev *led_cdev)
> +{
> +	return kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL;
> +}
> +
> +static struct led_trigger kbdlight_auto_trigger = {
> +	.name		= "tpacpi-kbd-auto",
> +	.trigger_type	= &kbdlight_auto_trigger_type,
> +	.activate	= kbdlight_auto_trigger_activate,
> +	.hw_offloaded	= kbdlight_auto_trigger_hw_offloaded,
> +};
> +
>  static struct tpacpi_led_classdev tpacpi_led_kbdlight = {
>  	.led_classdev = {
>  		.name		= "tpacpi::kbd_backlight",
>  		.max_brightness	= 2,
> -		.flags		= LED_BRIGHT_HW_CHANGED,
> +		.flags		= LED_BRIGHT_HW_CHANGED | LED_TRIG_HW_CHANGED,
>  		.brightness_set_blocking = &kbdlight_sysfs_set,
>  		.brightness_get	= &kbdlight_sysfs_get,
> +		.trigger_type	= &kbdlight_auto_trigger_type,
>  	}
>  };
>  
> @@ -5224,6 +5247,26 @@ static int __init kbdlight_init(struct ibm_init_struct *iibm)
>  	kbdlight_brightness = kbdlight_sysfs_get(NULL);
>  	tp_features.kbdlight = 1;
>  
> +	/*
> +	 * Register the auto-mode trigger before the LED classdev: if the
> +	 * hardware is already in auto mode, the trigger can then be set as
> +	 * the default one and gets activated during LED classdev
> +	 * registration.
> +	 */
> +	rc = devm_led_trigger_register(&tpacpi_pdev->dev,
> +				       &kbdlight_auto_trigger);
> +	if (rc) {
> +		tp_features.kbdlight = 0;
> +		return rc;
> +	}
> +
> +	tpacpi_led_kbdlight.led_classdev.hw_control_trigger =
> +		kbdlight_auto_trigger.name;
> +
> +	if (kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL)
> +		tpacpi_led_kbdlight.led_classdev.default_trigger =
> +			kbdlight_auto_trigger.name;
> +
>  	rc = led_classdev_register(&tpacpi_pdev->dev,
>  				   &tpacpi_led_kbdlight.led_classdev);
>  	if (rc < 0) {
> @@ -5313,6 +5356,47 @@ static void kbdlight_resume(void)
>  	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
>  }
>  
> +/* Notify the LED core when the hardware cycles into or out of auto mode. */
> +static void kbdlight_notify_hw_control(struct led_classdev *led_cdev,
> +				       int old_level, int new_level)
> +{
> +	bool was_auto = old_level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
> +	bool is_auto = new_level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
> +
> +	if (is_auto != was_auto)
> +		led_trigger_notify_hw_control_changed(led_cdev, is_auto);
> +}
> +
> +/*
> + * The EC cycles the keyboard backlight level (and auto mode) itself on
> + * Fn+Space; check the current level and update the LED core state.
> + */
> +static void kbdlight_notify_change(void)
> +{
> +	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
> +	enum led_brightness brightness;
> +
> +	guard(mutex)(&kbdlight_mutex);
> +
> +	/*
> +	 * Check the brightness actually changed, setting the brightness
> +	 * through kbdlight_set_level() also triggers this event.
> +	 */
> +	brightness = kbdlight_sysfs_get(NULL);
> +	if (kbdlight_brightness == brightness)
> +		return;
> +
> +	/*
> +	 * Calling kbdlight_notify_hw_control() is safe while holding
> +	 * kbdlight_mutex: the notification is only queued, and the worker
> +	 * re-acquires the mutex (if at all) after this critical section has
> +	 * been left.
> +	 */
> +	kbdlight_notify_hw_control(led_cdev, kbdlight_brightness, brightness);
> +	kbdlight_brightness = brightness;
> +	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);

Don't pass TPACPI_KBD_LIGHT_AUTO_LEVEL to
led_classdev_notify_brightness_hw_changed(). Please pass 0 instead.

> +}
> +
>  static struct ibm_struct kbdlight_driver_data = {
>  	.name = "kbdlight",
>  	.read = kbdlight_read,

I guess kbdlight_write() would need to call kbdlight_notify_hw_control()
to update the trigger's state as well.

Meanwhile, kbdlight_suspend() and kbdlight_resume() will override the
trigger.

> @@ -11581,22 +11665,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
>  		/* Key events are suppressed by default hotkey_user_mask */
>  		return false;
>  	case TP_HKEY_EV_KBD_LIGHT:
> -		if (tp_features.kbdlight) {
> -			enum led_brightness brightness;
> -
> -			guard(mutex)(&kbdlight_mutex);
> -
> -			/*
> -			 * Check the brightness actually changed, setting the brightness
> -			 * through kbdlight_set_level() also triggers this event.
> -			 */
> -			brightness = kbdlight_sysfs_get(NULL);
> -			if (kbdlight_brightness != brightness) {
> -				kbdlight_brightness = brightness;
> -				led_classdev_notify_brightness_hw_changed(
> -					&tpacpi_led_kbdlight.led_classdev, brightness);
> -			}
> -		}
> +		if (tp_features.kbdlight)
> +			kbdlight_notify_change();

Please split pure refactor (moving existing code) and feature additions
into two patches.

Thanks,
Rong

>  		/* Key events are suppressed by default hotkey_user_mask */
>  		return false;
>  	case TP_HKEY_EV_DFR_CHANGE_ROW:

[RFC PATCH v2 0/2] Add support for auto keyboard backlight

[Qiling Zhu]

Hi Mark, Rong, Ilpo,

Thanks for the review comments on v1. This v2 addresses them and is
now split into a pure refactor (1/2) and the feature addition (2/2),
as requested.

Summary of the changes:

* Kconfig: the "depends on LEDS_TRIGGERS_HW_CHANGED" is gone. While
  at it I also added a "select LEDS_TRIGGERS": without it the trigger
  helpers become no-op stubs and the whole feature would silently
  disappear in configurations that don't opt into LEDS_TRIGGERS,
  which seemed worse than a small select. HW_CHANGED support stays
  optional; without it the trigger still works, only the
  hardware-initiated transitions are not reported. The select goes
  beyond what was suggested, so please let me know if you'd rather
  drop it.

* The auto mode is never reported to the LED core as level 3: a
  single helper maps it to a brightness of 0, used by both
  brightness_get and the brightness_hw_changed notification, so
  level 3 has no path into the LED core at all. Internal state
  tracking compares raw EC levels.

* Writing a regular level through /proc/acpi/ibm/kbdlight now drops
  the trigger when leaving auto mode. Instead of raising the
  hardware-control notification and letting the worker handle it, I
  went through the same path brightness_store() uses: drop the
  trigger and hand the new level to the LED core, which applies it
  after the LED_OFF queued by dropping the trigger. That way proc
  and sysfs behave alike.

* suspend/resume no longer touch the LED while auto mode is active
  (v1 would have restored brightness 2 there and left the trigger
  state inconsistent, as Rong pointed out — thank you). The LED core
  is now kept out of suspend entirely in auto mode; resume just
  re-reads the EC level and reconciles. My reasoning is that in auto
  mode the EC owns the backlight, so the driver should not make
  decisions on its behalf — the firmware already turns the backlight
  off for suspend and restores it on resume by itself. This is based
  on observing my own machine (s2idle), so I may be wrong about
  other models. The non-auto paths are unchanged: suspend still
  turns the backlight off and resume still restores the previous
  level.

One thing Sashiko (the Linux Foundation's AI review system)
brought up in its review of v1, and which I then verified in the
code, seems worth raising here: led_trigger_hw_control_changed_worker()
reads the transition flag before acquiring the LED's trigger_lock,
and the read and the transition are not atomic. If, while a worker
carrying "deactivate" is still on its way, userspace re-selects the
private trigger, the worker then drops the trigger that userspace
just set, and no further event arrives to reconcile the state.
I haven't reliably reproduced this, but the window looks real, and
the ideapad wiring seems equally exposed. My feeling is that this
would be better addressed in the trigger core than worked around in
each driver, but I may be missing something — Rong, I'd appreciate
your thoughts.

A few observations that may be useful for the discussion:

* I disassembled the DSDT and instrumented the driver to see what
  MLCS returns: it is the EC status word, and bit 31 looks like an
  error flag — the DSDT skips the hotkey event when it is set. On my
  machine it reads 0 for all levels, with no level echo, and the
  error path is not reachable. That is why activation verifies with a
  read-back rather than the return value.

* The 0x1012 event is raised by the DSDT itself: MLCS issues it when
  the write succeeded, the level actually changed, and the hotkey is
  not masked. This explains why driver-initiated writes also see the
  event and why rewriting the same level doesn't.

* The Fn+Space cycle order on this machine is 0 -> 3 -> 1 -> 2 -> 0,
  confirmed by testing; sharing in case it is useful for comparison
  across models. Nothing in the code or tests relies on it.

An open question from v1 that I'm still unsure about: on models
without an ALS the auto mode presumably doesn't exist — how should
the driver handle that? Mark, do you know what the EC does when
asked for level 3 on such machines, and whether there is a reliable
way to tell at probe time whether auto mode is supported? For now
the trigger is registered unconditionally on keyboard-backlight-
capable machines.

Vishnu, Rong mentioned in the v1 discussion that you are also working
on the same support using the hw control trigger framework — how is
that going? If you have answers to the questions above, or if there
is a chance to combine the two efforts, I'd be happy to coordinate.

Tested on the same ThinkPad X1 Carbon Gen 14 (linux-next
next-20261030) with targeted tests and on-hardware verification of
the affected paths, including suspend/resume; checkpatch --strict
clean.

Regards,
Qiling Zhu

This patch was developed with LLM assistance; I reviewed every design
decision against the framework sources and validated the behavior on
real hardware myself.

Qiling Zhu (2):
  platform/x86: thinkpad_acpi: Extract kbdlight_notify_change() helper
  platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

 drivers/platform/x86/lenovo/Kconfig         |   1 +
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 179 +++++++++++++++++---
 2 files changed, 159 insertions(+), 21 deletions(-)

-- 
2.56.0

[RFC PATCH v2 1/2] platform/x86: thinkpad_acpi: Extract kbdlight_notify_change() helper

[Qiling Zhu]

TP_HKEY_EV_KBD_LIGHT handling in tpacpi_driver_event() reads back the EC
level, updates the cached brightness and notifies the LED core inline.
Extract this block into a separate kbdlight_notify_change() helper, in
preparation for reusing it in further changes.

No functional change intended.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 41 +++++++++++++--------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 4f81855ad..3260eaa62 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5313,6 +5313,29 @@ static void kbdlight_resume(void)
 	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
 }
 
+/*
+ * The EC cycles the keyboard backlight level itself on Fn+Space; check
+ * the current level and update the LED core state.
+ */
+static void kbdlight_notify_change(void)
+{
+	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
+	enum led_brightness brightness;
+
+	guard(mutex)(&kbdlight_mutex);
+
+	/*
+	 * Check the brightness actually changed, setting the brightness
+	 * through kbdlight_set_level() also triggers this event.
+	 */
+	brightness = kbdlight_sysfs_get(NULL);
+	if (kbdlight_brightness == brightness)
+		return;
+
+	kbdlight_brightness = brightness;
+	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);
+}
+
 static struct ibm_struct kbdlight_driver_data = {
 	.name = "kbdlight",
 	.read = kbdlight_read,
@@ -11581,22 +11604,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
 		/* Key events are suppressed by default hotkey_user_mask */
 		return false;
 	case TP_HKEY_EV_KBD_LIGHT:
-		if (tp_features.kbdlight) {
-			enum led_brightness brightness;
-
-			guard(mutex)(&kbdlight_mutex);
-
-			/*
-			 * Check the brightness actually changed, setting the brightness
-			 * through kbdlight_set_level() also triggers this event.
-			 */
-			brightness = kbdlight_sysfs_get(NULL);
-			if (kbdlight_brightness != brightness) {
-				kbdlight_brightness = brightness;
-				led_classdev_notify_brightness_hw_changed(
-					&tpacpi_led_kbdlight.led_classdev, brightness);
-			}
-		}
+		if (tp_features.kbdlight)
+			kbdlight_notify_change();
 		/* Key events are suppressed by default hotkey_user_mask */
 		return false;
 	case TP_HKEY_EV_DFR_CHANGE_ROW:
-- 
2.56.0

[RFC PATCH v2 2/2] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Qiling Zhu]

Recent ThinkPads with an ambient light sensor (ALS) support an auto mode
for the keyboard backlight: the EC controls the backlight according to
ambient light. The mode is EC keyboard backlight level 3, above the two
fixed levels exposed so far, and is currently reachable only through the
Fn+Space shortcut, which is handled directly by the firmware.

Wire the auto mode up as a private hardware control trigger, named
"tpacpi-kbd-auto", on top of the LED hw control trigger framework:

  - selecting the trigger writes level 3 (activate); the offloaded state
    is derived from the current EC level
  - when Fn+Space cycles into or out of auto mode, the driver notifies
    the LED core about the hardware-initiated hardware control
    transition, so the trigger state (shown bracketed in the trigger
    sysfs attribute) follows along
  - if the EC is already in auto mode at driver initialization, the
    trigger is set as the default one

The auto mode is not a brightness level: max_brightness stays 2 and the
LED core is always told a brightness of 0 while in auto mode. The proc
interface keeps reporting the raw EC level, including 3, and keeps
rejecting writes of 3 with -EINVAL; writing a regular level through it
while in auto mode drops the trigger, like writing a brightness through
sysfs does. In auto mode suspend and resume are left to the EC and the
firmware; on resume the restored EC level is observed and reconciled.

LEDS_TRIGGERS is selected to keep the trigger functional in
stripped-down configurations.

Deliberately unchanged: module unload still resets the backlight to off;
retaining auto mode across unload is left for future work.

Note that Documentation/admin-guide/laptops/thinkpad-acpi.rst has no
kbdlight section at all, so there is no in-tree documentation to update;
userspace-facing semantics of hardware control triggers are documented
in Documentation/leds/leds-class.rst.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/Kconfig         |   1 +
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 152 ++++++++++++++++++--
 2 files changed, 141 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
index 516594993..77d159a3a 100644
--- a/drivers/platform/x86/lenovo/Kconfig
+++ b/drivers/platform/x86/lenovo/Kconfig
@@ -83,6 +83,7 @@ config THINKPAD_ACPI
 	select NVRAM
 	select NEW_LEDS
 	select LEDS_CLASS
+	select LEDS_TRIGGERS
 	select INPUT_SPARSEKMAP
 	help
 	  This is a driver for the IBM and Lenovo ThinkPad laptops. It adds
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 3260eaa62..7b46ad6f9 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5102,6 +5102,14 @@ static struct ibm_struct video_driver_data = {
  * Keyboard backlight subdriver
  */
 
+/* EC level that selects ALS-controlled auto mode for the keyboard backlight */
+#define TPACPI_KBD_LIGHT_AUTO_LEVEL	3
+
+/*
+ * Cached EC level, including TPACPI_KBD_LIGHT_AUTO_LEVEL. All state
+ * tracking compares EC levels; the auto mode is translated to a
+ * brightness value only when reporting to the LED core.
+ */
 static enum led_brightness kbdlight_brightness;
 static DEFINE_MUTEX(kbdlight_mutex);
 
@@ -5180,6 +5188,15 @@ static bool kbdlight_is_supported(void)
 	return status & BIT(9);
 }
 
+/* The auto mode is not a brightness level; report it as off to the LED core. */
+static enum led_brightness kbdlight_level_to_brightness(int level)
+{
+	if (level == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		return 0;
+
+	return level;
+}
+
 static int kbdlight_sysfs_set(struct led_classdev *led_cdev,
 			enum led_brightness brightness)
 {
@@ -5194,16 +5211,54 @@ static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
 	if (level < 0)
 		return 0;
 
-	return level;
+	return kbdlight_level_to_brightness(level);
+}
+
+static struct led_hw_trigger_type kbdlight_auto_trigger_type;
+
+static int kbdlight_auto_trigger_activate(struct led_classdev *led_cdev)
+{
+	/*
+	 * The EC may already be in auto mode (cycled there on Fn+Space, or
+	 * active since before the driver loaded), in which case there is
+	 * nothing to program.
+	 */
+	if (kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		return 0;
+
+	if (kbdlight_set_level(TPACPI_KBD_LIGHT_AUTO_LEVEL))
+		return -EIO;
+
+	/*
+	 * Check that the EC actually accepted the auto mode; on error the
+	 * trigger core reverts the LED to the "none" trigger.
+	 */
+	if (kbdlight_get_level() != TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		return -EIO;
+
+	return 0;
 }
 
+static bool kbdlight_auto_trigger_hw_offloaded(struct led_classdev *led_cdev)
+{
+	return kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+}
+
+static struct led_trigger kbdlight_auto_trigger = {
+	.name		= "tpacpi-kbd-auto",
+	.trigger_type	= &kbdlight_auto_trigger_type,
+	.activate	= kbdlight_auto_trigger_activate,
+	.hw_offloaded	= kbdlight_auto_trigger_hw_offloaded,
+};
+
 static struct tpacpi_led_classdev tpacpi_led_kbdlight = {
 	.led_classdev = {
 		.name		= "tpacpi::kbd_backlight",
 		.max_brightness	= 2,
-		.flags		= LED_BRIGHT_HW_CHANGED,
+		.flags		= LED_BRIGHT_HW_CHANGED | LED_TRIG_HW_CHANGED,
 		.brightness_set_blocking = &kbdlight_sysfs_set,
 		.brightness_get	= &kbdlight_sysfs_get,
+		.trigger_type	= &kbdlight_auto_trigger_type,
 	}
 };
 
@@ -5221,9 +5276,30 @@ static int __init kbdlight_init(struct ibm_init_struct *iibm)
 		return -ENODEV;
 	}
 
-	kbdlight_brightness = kbdlight_sysfs_get(NULL);
+	kbdlight_brightness = kbdlight_get_level();
+	if (kbdlight_brightness < 0)
+		kbdlight_brightness = 0;
 	tp_features.kbdlight = 1;
 
+	/*
+	 * Register the auto-mode trigger before the LED classdev: if the
+	 * EC is already in auto mode, the trigger can then be set as the
+	 * default one and gets activated during LED classdev registration.
+	 */
+	rc = devm_led_trigger_register(&tpacpi_pdev->dev,
+				       &kbdlight_auto_trigger);
+	if (rc) {
+		tp_features.kbdlight = 0;
+		return rc;
+	}
+
+	tpacpi_led_kbdlight.led_classdev.hw_control_trigger =
+		kbdlight_auto_trigger.name;
+
+	if (kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		tpacpi_led_kbdlight.led_classdev.default_trigger =
+			kbdlight_auto_trigger.name;
+
 	rc = led_classdev_register(&tpacpi_pdev->dev,
 				   &tpacpi_led_kbdlight.led_classdev);
 	if (rc < 0) {
@@ -5246,9 +5322,22 @@ static int kbdlight_set_level_and_update(int level)
 	int ret;
 	struct led_classdev *led_cdev;
 
-	ret = kbdlight_set_level(level);
 	led_cdev = &tpacpi_led_kbdlight.led_classdev;
 
+	/*
+	 * Leaving auto mode through the proc interface has to drop the
+	 * trigger, like writing a brightness through sysfs does. Route the
+	 * new level through the LED core as well, so that it is applied
+	 * after the LED_OFF queued by dropping the trigger.
+	 */
+	if (led_trigger_is_hw_controlled(led_cdev)) {
+		led_trigger_remove_hw_control(led_cdev);
+		led_set_brightness(led_cdev, level);
+		return 0;
+	}
+
+	ret = kbdlight_set_level(level);
+
 	if (ret == 0 && !(led_cdev->flags & LED_SUSPENDED))
 		led_cdev->brightness = level;
 
@@ -5293,6 +5382,8 @@ static int kbdlight_write(char *buf)
 	return kbdlight_set_level_and_update(level);
 }
 
+static void kbdlight_notify_change(void);
+
 static void kbdlight_suspend(void)
 {
 	struct led_classdev *led_cdev;
@@ -5301,38 +5392,75 @@ static void kbdlight_suspend(void)
 		return;
 
 	led_cdev = &tpacpi_led_kbdlight.led_classdev;
+
+	/*
+	 * In auto mode the EC drives the backlight and the firmware turns
+	 * it off for suspend and restores it on resume by itself; keep the
+	 * LED core out of it entirely.
+	 */
+	if (kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		return;
+
 	led_update_brightness(led_cdev);
 	led_classdev_suspend(led_cdev);
 }
 
 static void kbdlight_resume(void)
 {
+	struct led_classdev *led_cdev;
+
 	if (!tp_features.kbdlight)
 		return;
 
-	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
+	led_cdev = &tpacpi_led_kbdlight.led_classdev;
+
+	/* Only balance a suspend that actually suspended the LED. */
+	if (led_cdev->flags & LED_SUSPENDED) {
+		led_classdev_resume(led_cdev);
+		return;
+	}
+
+	/*
+	 * Auto mode was left running over suspend; observe what the
+	 * firmware actually restored.
+	 */
+	kbdlight_notify_change();
 }
 
 /*
- * The EC cycles the keyboard backlight level itself on Fn+Space; check
- * the current level and update the LED core state.
+ * The EC cycles the keyboard backlight level (and auto mode) itself on
+ * Fn+Space; check the current level and update the LED core state.
  */
 static void kbdlight_notify_change(void)
 {
 	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
 	enum led_brightness brightness;
+	bool was_auto, is_auto;
+	int level;
 
 	guard(mutex)(&kbdlight_mutex);
 
 	/*
-	 * Check the brightness actually changed, setting the brightness
-	 * through kbdlight_set_level() also triggers this event.
+	 * Check the level actually changed, setting the brightness through
+	 * kbdlight_set_level() also triggers this event.
 	 */
-	brightness = kbdlight_sysfs_get(NULL);
-	if (kbdlight_brightness == brightness)
+	level = kbdlight_get_level();
+	if (kbdlight_brightness == level)
 		return;
 
-	kbdlight_brightness = brightness;
+	/*
+	 * Notifying the trigger core is safe while holding
+	 * kbdlight_mutex: the transition is only queued, and the worker
+	 * re-reads the EC level (via hw_offloaded) after this critical
+	 * section has been left.
+	 */
+	is_auto = level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+	was_auto = kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+	if (is_auto != was_auto)
+		led_trigger_notify_hw_control_changed(led_cdev, is_auto);
+
+	brightness = kbdlight_level_to_brightness(level);
+	kbdlight_brightness = level;
 	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);
 }
 
-- 
2.56.0

Re: [RFC PATCH v2 0/2] Add support for auto keyboard backlight

[Rong Zhang]

Hi Qiling,

On Mon, 2026-11-02 at 23:10 +0800, Qiling Zhu wrote:
> Hi Mark, Rong, Ilpo,
> 
> Thanks for the review comments on v1. This v2 addresses them and is
> now split into a pure refactor (1/2) and the feature addition (2/2),
> as requested.

You don't need to refresh the series in a hurry, unless you've got enough
reviews or may miss the cycle. Asking an LLM agent to refresh the series
is easy, but a human being needs time to review your series.

Please leave some time for others to respond.

> 
> Summary of the changes:
> 
> * Kconfig: the "depends on LEDS_TRIGGERS_HW_CHANGED" is gone. While
>   at it I also added a "select LEDS_TRIGGERS": without it the trigger
>   helpers become no-op stubs and the whole feature would silently
>   disappear in configurations that don't opt into LEDS_TRIGGERS,
>   which seemed worse than a small select. HW_CHANGED support stays
>   optional; without it the trigger still works, only the
>   hardware-initiated transitions are not reported. The select goes
>   beyond what was suggested, so please let me know if you'd rather
>   drop it.
> 
> * The auto mode is never reported to the LED core as level 3: a
>   single helper maps it to a brightness of 0, used by both
>   brightness_get and the brightness_hw_changed notification, so
>   level 3 has no path into the LED core at all. Internal state
>   tracking compares raw EC levels.
> 
> * Writing a regular level through /proc/acpi/ibm/kbdlight now drops
>   the trigger when leaving auto mode. Instead of raising the
>   hardware-control notification and letting the worker handle it, I
>   went through the same path brightness_store() uses: drop the
>   trigger and hand the new level to the LED core, which applies it
>   after the LED_OFF queued by dropping the trigger. That way proc
>   and sysfs behave alike.
> 
> * suspend/resume no longer touch the LED while auto mode is active
>   (v1 would have restored brightness 2 there and left the trigger
>   state inconsistent, as Rong pointed out — thank you). The LED core
>   is now kept out of suspend entirely in auto mode; resume just
>   re-reads the EC level and reconciles. My reasoning is that in auto
>   mode the EC owns the backlight, so the driver should not make
>   decisions on its behalf — the firmware already turns the backlight
>   off for suspend and restores it on resume by itself. This is based
>   on observing my own machine (s2idle), so I may be wrong about
>   other models. The non-auto paths are unchanged: suspend still
>   turns the backlight off and resume still restores the previous
>   level.

If the firmware properly handles it when hw brightness != 3, you can skip
led_classdev_{suspend,resume}() completely as long as the device supports
auto mode. They are only needed when the firmware doesn't take care of
the keyboard backlight. Skipping them should make our life mush easier.

Of course, we still need an approach to determine the support. That's why
we should leave some time for Vishnu and Mark to answer the question.

> 
> One thing Sashiko (the Linux Foundation's AI review system)
> brought up in its review of v1, and which I then verified in the
> code, seems worth raising here: led_trigger_hw_control_changed_worker()
> reads the transition flag before acquiring the LED's trigger_lock,
> and the read and the transition are not atomic. If, while a worker
> carrying "deactivate" is still on its way, userspace re-selects the
> private trigger, the worker then drops the trigger that userspace
> just set, and no further event arrives to reconcile the state.
> I haven't reliably reproduced this, but the window looks real, and
> the ideapad wiring seems equally exposed. My feeling is that this
> would be better addressed in the trigger core than worked around in
> each driver, but I may be missing something — Rong, I'd appreciate
> your thoughts.

It is an unfortunate side-effect of the workqueue deferral mechanism. The
mechanism was introduced in order that:

- the LED driver can notify hw control change in an atomic context
- the LED driver can properly serialize notification handling, trigger
[de]activation, brightness_set[_blocking]() and hw_offloaded() callback
without worrying about lock reentrance

As for ideapad-laptop, it needs the second guarantee, as all said
functions or callbacks need to acquire the same mutex to prevent TOCTOU
issues.

IMHO the race condition is not a big deal. The interface was designed
with an assumption: it's the user who decides to press the shortcut
(Fn+Space) and/or select the trigger via sysfs. It can only be barely
reproduced in fuzz tests with scripts and robotic fingers, which is as
boring as fuzzing the bind/unbind interface to stress naive drivers...

That being said, I am thinking of working around this by querying
hw_offloaded() in the worker. I will find some time to prepare a patch to
see if the approach works and submit it. It won't change the interface,
so patches can be applied in an arbitrary order. Please ignore the issue
for the time being (OTOH, you don't need to rebase on top of the said
patch when it's ready).

Thanks,
Rong

> 
> A few observations that may be useful for the discussion:
> 
> * I disassembled the DSDT and instrumented the driver to see what
>   MLCS returns: it is the EC status word, and bit 31 looks like an
>   error flag — the DSDT skips the hotkey event when it is set. On my
>   machine it reads 0 for all levels, with no level echo, and the
>   error path is not reachable. That is why activation verifies with a
>   read-back rather than the return value.
> 
> * The 0x1012 event is raised by the DSDT itself: MLCS issues it when
>   the write succeeded, the level actually changed, and the hotkey is
>   not masked. This explains why driver-initiated writes also see the
>   event and why rewriting the same level doesn't.
> 
> * The Fn+Space cycle order on this machine is 0 -> 3 -> 1 -> 2 -> 0,
>   confirmed by testing; sharing in case it is useful for comparison
>   across models. Nothing in the code or tests relies on it.
> 
> An open question from v1 that I'm still unsure about: on models
> without an ALS the auto mode presumably doesn't exist — how should
> the driver handle that? Mark, do you know what the EC does when
> asked for level 3 on such machines, and whether there is a reliable
> way to tell at probe time whether auto mode is supported? For now
> the trigger is registered unconditionally on keyboard-backlight-
> capable machines.
> 
> Vishnu, Rong mentioned in the v1 discussion that you are also working
> on the same support using the hw control trigger framework — how is
> that going? If you have answers to the questions above, or if there
> is a chance to combine the two efforts, I'd be happy to coordinate.
> 
> Tested on the same ThinkPad X1 Carbon Gen 14 (linux-next
> next-20261030) with targeted tests and on-hardware verification of
> the affected paths, including suspend/resume; checkpatch --strict
> clean.
> 
> Regards,
> Qiling Zhu
> 
> This patch was developed with LLM assistance; I reviewed every design
> decision against the framework sources and validated the behavior on
> real hardware myself.
> 
> Qiling Zhu (2):
>   platform/x86: thinkpad_acpi: Extract kbdlight_notify_change() helper
>   platform/x86: thinkpad_acpi: Add support for auto keyboard backlight
> 
>  drivers/platform/x86/lenovo/Kconfig         |   1 +
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 179 +++++++++++++++++---
>  2 files changed, 159 insertions(+), 21 deletions(-)

Re: [RFC PATCH v2 2/2] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Rong Zhang]

Hi Qiling,

On Mon, 2026-11-02 at 23:10 +0800, Qiling Zhu wrote:
> Recent ThinkPads with an ambient light sensor (ALS) support an auto mode
> for the keyboard backlight: the EC controls the backlight according to
> ambient light. The mode is EC keyboard backlight level 3, above the two
> fixed levels exposed so far, and is currently reachable only through the
> Fn+Space shortcut, which is handled directly by the firmware.
> 
> Wire the auto mode up as a private hardware control trigger, named
> "tpacpi-kbd-auto", on top of the LED hw control trigger framework:
> 
>   - selecting the trigger writes level 3 (activate); the offloaded state
>     is derived from the current EC level
>   - when Fn+Space cycles into or out of auto mode, the driver notifies
>     the LED core about the hardware-initiated hardware control
>     transition, so the trigger state (shown bracketed in the trigger
>     sysfs attribute) follows along
>   - if the EC is already in auto mode at driver initialization, the
>     trigger is set as the default one
> 
> The auto mode is not a brightness level: max_brightness stays 2 and the
> LED core is always told a brightness of 0 while in auto mode. The proc
> interface keeps reporting the raw EC level, including 3, and keeps
> rejecting writes of 3 with -EINVAL; writing a regular level through it
> while in auto mode drops the trigger, like writing a brightness through
> sysfs does. In auto mode suspend and resume are left to the EC and the
> firmware; on resume the restored EC level is observed and reconciled.
> 
> LEDS_TRIGGERS is selected to keep the trigger functional in
> stripped-down configurations.
> 
> Deliberately unchanged: module unload still resets the backlight to off;
> retaining auto mode across unload is left for future work.
> 
> Note that Documentation/admin-guide/laptops/thinkpad-acpi.rst has no
> kbdlight section at all, so there is no in-tree documentation to update;
> userspace-facing semantics of hardware control triggers are documented
> in Documentation/leds/leds-class.rst.
> 
> Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
> ---
>  drivers/platform/x86/lenovo/Kconfig         |   1 +
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 152 ++++++++++++++++++--
>  2 files changed, 141 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> index 516594993..77d159a3a 100644
> --- a/drivers/platform/x86/lenovo/Kconfig
> +++ b/drivers/platform/x86/lenovo/Kconfig
> @@ -83,6 +83,7 @@ config THINKPAD_ACPI
>  	select NVRAM
>  	select NEW_LEDS
>  	select LEDS_CLASS
> +	select LEDS_TRIGGERS
>  	select INPUT_SPARSEKMAP
>  	help
>  	  This is a driver for the IBM and Lenovo ThinkPad laptops. It adds
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index 3260eaa62..7b46ad6f9 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -5102,6 +5102,14 @@ static struct ibm_struct video_driver_data = {
>   * Keyboard backlight subdriver
>   */
>  
> +/* EC level that selects ALS-controlled auto mode for the keyboard backlight */
> +#define TPACPI_KBD_LIGHT_AUTO_LEVEL	3
> +
> +/*
> + * Cached EC level, including TPACPI_KBD_LIGHT_AUTO_LEVEL. All state
> + * tracking compares EC levels; the auto mode is translated to a
> + * brightness value only when reporting to the LED core.
> + */
>  static enum led_brightness kbdlight_brightness;
>  static DEFINE_MUTEX(kbdlight_mutex);
>  
> @@ -5180,6 +5188,15 @@ static bool kbdlight_is_supported(void)
>  	return status & BIT(9);
>  }
>  
> +/* The auto mode is not a brightness level; report it as off to the LED core. */
> +static enum led_brightness kbdlight_level_to_brightness(int level)
> +{
> +	if (level == TPACPI_KBD_LIGHT_AUTO_LEVEL)
> +		return 0;
> +
> +	return level;
> +}
> +
>  static int kbdlight_sysfs_set(struct led_classdev *led_cdev,
>  			enum led_brightness brightness)
>  {
> @@ -5194,16 +5211,54 @@ static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
>  	if (level < 0)
>  		return 0;
>  
> -	return level;
> +	return kbdlight_level_to_brightness(level);
> +}
> +
> +static struct led_hw_trigger_type kbdlight_auto_trigger_type;
> +
> +static int kbdlight_auto_trigger_activate(struct led_classdev *led_cdev)
> +{
> +	/*
> +	 * The EC may already be in auto mode (cycled there on Fn+Space, or
> +	 * active since before the driver loaded), in which case there is
> +	 * nothing to program.
> +	 */
> +	if (kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL)
> +		return 0;

I don't think the device needs this, as kbdlight_set_level() should be
idempotent.

> +
> +	if (kbdlight_set_level(TPACPI_KBD_LIGHT_AUTO_LEVEL))
> +		return -EIO;
> +
> +	/*
> +	 * Check that the EC actually accepted the auto mode; on error the
> +	 * trigger core reverts the LED to the "none" trigger.
> +	 */
> +	if (kbdlight_get_level() != TPACPI_KBD_LIGHT_AUTO_LEVEL)
> +		return -EIO;

Nah. Please wait until Vishnu and Mark answer about the approach to
determine the support for auto mode.

> +
> +	return 0;
>  }
>  
> +static bool kbdlight_auto_trigger_hw_offloaded(struct led_classdev *led_cdev)
> +{
> +	return kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL;
> +}
> +
> +static struct led_trigger kbdlight_auto_trigger = {
> +	.name		= "tpacpi-kbd-auto",
> +	.trigger_type	= &kbdlight_auto_trigger_type,
> +	.activate	= kbdlight_auto_trigger_activate,
> +	.hw_offloaded	= kbdlight_auto_trigger_hw_offloaded,
> +};
> +
>  static struct tpacpi_led_classdev tpacpi_led_kbdlight = {
>  	.led_classdev = {
>  		.name		= "tpacpi::kbd_backlight",
>  		.max_brightness	= 2,
> -		.flags		= LED_BRIGHT_HW_CHANGED,
> +		.flags		= LED_BRIGHT_HW_CHANGED | LED_TRIG_HW_CHANGED,
>  		.brightness_set_blocking = &kbdlight_sysfs_set,
>  		.brightness_get	= &kbdlight_sysfs_get,
> +		.trigger_type	= &kbdlight_auto_trigger_type,
>  	}
>  };
>  
> @@ -5221,9 +5276,30 @@ static int __init kbdlight_init(struct ibm_init_struct *iibm)
>  		return -ENODEV;
>  	}
>  
> -	kbdlight_brightness = kbdlight_sysfs_get(NULL);
> +	kbdlight_brightness = kbdlight_get_level();
> +	if (kbdlight_brightness < 0)
> +		kbdlight_brightness = 0;
>  	tp_features.kbdlight = 1;
>  
> +	/*
> +	 * Register the auto-mode trigger before the LED classdev: if the
> +	 * EC is already in auto mode, the trigger can then be set as the
> +	 * default one and gets activated during LED classdev registration.
> +	 */
> +	rc = devm_led_trigger_register(&tpacpi_pdev->dev,
> +				       &kbdlight_auto_trigger);
> +	if (rc) {
> +		tp_features.kbdlight = 0;
> +		return rc;
> +	}
> +
> +	tpacpi_led_kbdlight.led_classdev.hw_control_trigger =
> +		kbdlight_auto_trigger.name;
> +
> +	if (kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL)
> +		tpacpi_led_kbdlight.led_classdev.default_trigger =
> +			kbdlight_auto_trigger.name;
> +
>  	rc = led_classdev_register(&tpacpi_pdev->dev,
>  				   &tpacpi_led_kbdlight.led_classdev);
>  	if (rc < 0) {
> @@ -5246,9 +5322,22 @@ static int kbdlight_set_level_and_update(int level)
>  	int ret;
>  	struct led_classdev *led_cdev;
>  
> -	ret = kbdlight_set_level(level);
>  	led_cdev = &tpacpi_led_kbdlight.led_classdev;
>  
> +	/*
> +	 * Leaving auto mode through the proc interface has to drop the
> +	 * trigger, like writing a brightness through sysfs does. Route the
> +	 * new level through the LED core as well, so that it is applied
> +	 * after the LED_OFF queued by dropping the trigger.
> +	 */
> +	if (led_trigger_is_hw_controlled(led_cdev)) {

You don't need this, and it causes a TOCTOU issue between
is_hw_controlled() and remove_hw_control().

> +		led_trigger_remove_hw_control(led_cdev);
> +		led_set_brightness(led_cdev, level);

???

Can't you simply refactor and call kbdlight_notify_change() here?

In detail:

- PATCH 1: split kbdlight_notify_change() into a function that accepts a
hw brightness level and a wrapper that queries the hardware
- PATCH 2: call the former in kbdlight_set_level_and_update(), like what
commit 07f48f668fac ("platform/x86: ideapad-laptop: add FnLock LED class
device") did
- PATCH 3: add support for auto mode

With those changes, kbdlight_set_level_and_update() should automatically
removes the triggers without the need of poking the LED core harshly.

Besides, IMO the attribute should really be deprecated with a warning
stating that writing to the attribute may desynchronize the LED core, so
that we can reduce the maintenance burden instead of infinitely working
around all sorts of such issues [1]. I am not an expert at this driver,
so I will defer it to other maintainers.

> +		return 0;
> +	}
> +
> +	ret = kbdlight_set_level(level);
> +
>  	if (ret == 0 && !(led_cdev->flags & LED_SUSPENDED))
>  		led_cdev->brightness = level;

[1]: for example, here is a pre-existing race condition.

>  
> @@ -5293,6 +5382,8 @@ static int kbdlight_write(char *buf)
>  	return kbdlight_set_level_and_update(level);
>  }
>  
> +static void kbdlight_notify_change(void);
> +
>  static void kbdlight_suspend(void)
>  {
>  	struct led_classdev *led_cdev;
> @@ -5301,38 +5392,75 @@ static void kbdlight_suspend(void)
>  		return;
>  
>  	led_cdev = &tpacpi_led_kbdlight.led_classdev;
> +
> +	/*
> +	 * In auto mode the EC drives the backlight and the firmware turns
> +	 * it off for suspend and restores it on resume by itself; keep the
> +	 * LED core out of it entirely.
> +	 */
> +	if (kbdlight_get_level() == TPACPI_KBD_LIGHT_AUTO_LEVEL)
> +		return;
> +

As I've said, getting rid of led_classdev_{suspend,resume}() completely
when the device supports auto mode will make our life much easier.

>  	led_update_brightness(led_cdev);
>  	led_classdev_suspend(led_cdev);
>  }
>  
>  static void kbdlight_resume(void)
>  {
> +	struct led_classdev *led_cdev;
> +
>  	if (!tp_features.kbdlight)
>  		return;
>  
> -	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
> +	led_cdev = &tpacpi_led_kbdlight.led_classdev;
> +
> +	/* Only balance a suspend that actually suspended the LED. */
> +	if (led_cdev->flags & LED_SUSPENDED) {
> +		led_classdev_resume(led_cdev);
> +		return;
> +	}
> +
> +	/*
> +	 * Auto mode was left running over suspend; observe what the
> +	 * firmware actually restored.
> +	 */
> +	kbdlight_notify_change();

If you'd like to do so, it should be in a separated patch, too.

Thanks,
Rong

>  }
>  
>  /*
> - * The EC cycles the keyboard backlight level itself on Fn+Space; check
> - * the current level and update the LED core state.
> + * The EC cycles the keyboard backlight level (and auto mode) itself on
> + * Fn+Space; check the current level and update the LED core state.
>   */
>  static void kbdlight_notify_change(void)
>  {
>  	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
>  	enum led_brightness brightness;
> +	bool was_auto, is_auto;
> +	int level;
>  
>  	guard(mutex)(&kbdlight_mutex);
>  
>  	/*
> -	 * Check the brightness actually changed, setting the brightness
> -	 * through kbdlight_set_level() also triggers this event.
> +	 * Check the level actually changed, setting the brightness through
> +	 * kbdlight_set_level() also triggers this event.
>  	 */
> -	brightness = kbdlight_sysfs_get(NULL);
> -	if (kbdlight_brightness == brightness)
> +	level = kbdlight_get_level();
> +	if (kbdlight_brightness == level)
>  		return;
>  
> -	kbdlight_brightness = brightness;
> +	/*
> +	 * Notifying the trigger core is safe while holding
> +	 * kbdlight_mutex: the transition is only queued, and the worker
> +	 * re-reads the EC level (via hw_offloaded) after this critical
> +	 * section has been left.
> +	 */
> +	is_auto = level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
> +	was_auto = kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL;
> +	if (is_auto != was_auto)
> +		led_trigger_notify_hw_control_changed(led_cdev, is_auto);
> +
> +	brightness = kbdlight_level_to_brightness(level);
> +	kbdlight_brightness = level;
>  	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);
>  }
>  

Re: [RFC PATCH 0/1] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Vishnu Sankar]

Hi Qiling,

Thanks a lot for the patches.


On Mon, Nov 2, 2026 at 3:02 AM Rong Zhang <i@rong.moe> wrote:
>
> Hi Qiling,
>
> Thanks for working on this.
>
> On Sun, 2026-11-01 at 18:01 +0800, Qiling Zhu wrote:
> > Hi Mark, Ilpo, Rong,
> >
> > Recent ThinkPads with an ambient light sensor (ALS) support an auto
> > mode for the keyboard backlight: the EC picks the backlight level
> > according to ambient light. The mode is EC keyboard backlight level 3,
> > above the two fixed levels exposed so far, and it is reachable only
> > through the Fn+Space shortcut, which is handled entirely by the
> > firmware. Userspace can neither select the mode nor observe whether it
> > is active.
> >
> > This series wires the auto mode up as a private hardware control
> > trigger, "tpacpi-kbd-auto", on top of the hw control trigger framework
> > that recently landed in mainline [1]. With it:
> >
> >   - selecting the trigger writes EC level 3 (auto mode on); writing
> >     "none" turns the backlight off
> >   - the trigger state (bracketed in the trigger sysfs attribute, and
> >     the new trigger_may_offload_to_hw attribute) is the userspace
> >     view of auto mode; reading brightness returns -ENODATA while
> >     auto mode is active, per the framework semantics
> >   - Fn+Space keeps cycling through levels and auto mode as before,
> >     and the trigger state now follows hardware-initiated transitions
> >     automatically
> >
> > This is the thinkpad_acpi counterpart of the ideapad-laptop wiring
> > from the same framework series, which is still queued; this patch
> > depends only on the LED core parts already in mainline and can be
> > applied independently.
> >
> > For background: the February discussion settled that auto mode must
> > not be exposed as a brightness value [2], but no agreement was
> > reached back then on how to expose it instead; the hardware control
> > trigger approach was established later by the framework series [1].
> >
> > Tested on a ThinkPad X1 Carbon Gen 14 (MT 21V8) running linux-next
> > next-20261030: 61 scripted user-visible assertions covering the
> > software paths, the Fn+Space event chain and the unchanged legacy
> > behavior, plus manual exercising of both the Fn+Space cycling and
> > desktop-environment brightness controls; checkpatch --strict clean.
> >
> > Deliberately unchanged (details in the commit message):
> >
> >   - max_brightness stays 2; auto mode is not multiplexed into the
> >     brightness value
> >   - writing 3 to /proc/acpi/ibm/kbdlight keeps returning -EINVAL
> >   - module unload still turns the keyboard backlight off
> >
> > Two open questions, the main reason for the RFC tag:
> >
> >   1. MLCG returns capability bits in the upper 16 bits; does one of
> >      them indicate auto mode support? If yes, I would gate trigger
> >      registration on it. Mark, does the EC documentation say?
> >   2. On models without an ALS, the EC behavior when asked for level 3
> >      via MLCS is unknown to me. If it is undefined rather than
> >      rejected, optimistic registration would need a rethink (e.g.
> >      DMI whitelisting).
> >
> > Rong, happy to rebase this on top of your series or fold it in, if
> > you prefer to keep both wirings together.
>
> (+CC Vishnu)
>
> AFAIK, Vishnu is also working on adding the same support using the hw
> control trigger framework. Maybe they can answer your questions or
> integrate your patch into theirs.

Thanks, Rong.
Mark and I discussed this, and we are happy to go ahead with Qiling's
implementation rather than duplicate the work.

For some background, we had previously implemented the ThinkPad side
in thinkpad_acpi against Rong's hardware-control-trigger series and
tested it successfully on our hardware. We had been waiting for the
common LED infrastructure before taking that work further, so it is
good to see this moving forward now.

Regarding the open capability question, I checked the Lenovo
keyboard-backlight firmware definitions. MLCG provides an explicit
Auto-mode capability indication:

bit 20 = 0: Manual mode only
bit 20 = 1: Auto mode is available

So I think MLCG bit 20 is the appropriate indication to use when
deciding whether tpacpi-kbd-auto should be registered, rather than
probing level 3 or relying on ALS presence.

One clarification regarding "AUTO" mode: Auto mode on newer ThinkPads
is not necessarily ALS-dependent. Some systems use the ambient light
sensor as part of the automatic keyboard-backlight behavior, while
there is also an activity-only based Auto behavior where the backlight
turns on following keyboard activity and turns off after a period of
inactivity. Therefore, absence of ALS does not by itself mean that
Auto mode is unsupported. The firmware-reported Auto capability should
be the better discriminator.

Regarding MLCS bit 31 discussed earlier, please ignore that bit for
Auto-mode capability detection. The explicit capability indication is
MLCG bit 20.

Based on this, my suggestion would be to gate Auto-trigger
registration on MLCG bit 20 and leave the actual Auto behavior to the
firmware.

I am happy to test the next revision on additional ThinkPad hardware as well.

>
> Thanks,
> Rong
>
> >
> > Comments welcome.
> >
> > Regards,
> > Qiling Zhu
> >
> > This patch was developed with LLM assistance; I reviewed every design
> > decision against the framework sources and validated the behavior on
> > real hardware myself.
> >
> > [1] https://lore.kernel.org/r/20260618-leds-trigger-hw-changed-v2-0-c28c44053cf3@rong.moe/
> > [2] https://lore.kernel.org/r/20260203232219.11683-1-vishnuocv@gmail.com/
> >
> > Qiling Zhu (1):
> >   platform/x86: thinkpad_acpi: Add support for auto keyboard backlight
> >
> >  drivers/platform/x86/lenovo/Kconfig         |   1 +
> >  drivers/platform/x86/lenovo/thinkpad_acpi.c | 104 ++++++++++++++++----
> >  2 files changed, 88 insertions(+), 17 deletions(-)



--

Regards,

      Vishnu Sankar

Re: [RFC PATCH v2 0/2] Add support for auto keyboard backlight

[Qiling Zhu]

On Tue, Nov 03, 2026 at 02:13:55AM +0800, Rong Zhang wrote:
> Hi Qiling,
> 
> On Mon, 2026-11-02 at 23:10 +0800, Qiling Zhu wrote:
> > Hi Mark, Rong, Ilpo,
> > 
> > Thanks for the review comments on v1. This v2 addresses them and is
> > now split into a pure refactor (1/2) and the feature addition (2/2),
> > as requested.
> 
> You don't need to refresh the series in a hurry, unless you've got enough
> reviews or may miss the cycle. Asking an LLM agent to refresh the series
> is easy, but a human being needs time to review your series.
> 
> Please leave some time for others to respond.

Thanks, Rong, for the detailed review. I'll rework the series
following your suggestions and take some time to understand and
test the changes carefully.

Thanks also for pointing out that I was updating the series
too quickly. I apologize for not leaving enough time for others
to review, and I appreciate your patience in explaining this.
I'll be more mindful of the review process going forward.

And thanks to Vishnu for clarifying the Auto-mode capability
detection. I'll use MLCG bit 20 to determine whether the
Auto trigger should be registered in the next revision.

I'm very open to further suggestions or alternative approaches.
If someone has a better implementation or existing patches that
would be a better fit, I'd be equally happy to build on those
instead. My main goal is to get this support implemented properly.

Thanks again for your time and help.

Regards,
Qiling Zhu

[PATCH v3 0/7] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Qiling Zhu]

Hi all,

Thanks, Rong, for the two rounds of detailed review, Vishnu and Mark
for the firmware-side answers, and Andrea for the bug report that
became the first patch. This v3 addresses the feedback in full; the
series grew from two to seven patches to pick up the suggested
structure and to make the fixes independently backportable.

The series in short:

  1/7  Report auto mode as brightness 0 instead of 3          (stable fix)
  2/7  Extract kbdlight_notify_change() helpers               (refactor)
  3/7  Sync LED core state on level writes          (write-path sync)
  4/7  Detect auto-mode capability via MLCG bit 20            (groundwork)
  5/7  Skip LED suspend/resume on auto-mode machines
  6/7  Add support for auto keyboard backlight                 (the feature)
  7/7  Reconcile EC level on resume

Changes since v2, by comment:

* The notification path is now split as suggested: a helper that accepts
  a level and a wrapper that queries the EC (2/7), and the proc write
  path goes through it (3/7), like commit 07f48f668fac did for the
  ideapad FnLock LED. The manual check-then-act trigger removal is gone
  entirely; leaving auto mode through either interface now drops the
  trigger through the LED core.

* LED suspend/resume is skipped entirely on machines that support auto
  mode, keyed on the capability bit rather than the current level (5/7),
  as suggested. My testing confirms the firmware restores the previous
  level on its own in all cases except auto mode across hibernation;
  less state remains in the driver, and the class of races around
  LED_SUSPENDED disappears.

* activate() is now a one-liner with no read-back verification (6/7);
  it only writes the level, using the plain set helper. I did not use
  the updating wrapper there on purpose: the trigger core is in the
  middle of its own activation flow at that point, and the 0x1012 event
  that follows the level change is expected to bring the state back in
  sync through the regular notify path.

* Trigger registration is gated on MLCG bit 20 as per Vishnu's
  confirmation (4/7 + 6/7).

* Andrea's report is fixed by 1/7, which reports auto mode as 0 to the
  LED classdev and is marked for stable. It is deliberately minimal -
  a single check, no further splitting - to keep backporting easy.

* On the proc attribute deprecation: I support the idea, deferring to
  the thinkpad_acpi maintainers.

One addition that was not in v2: testing showed that the firmware does
not restore auto mode across hibernation - the EC comes back with the
backlight off, without an event. Rather than re-attaching the trigger
from the pre-suspend state, 7/7 simply re-reads the EC level on resume
and routes it through the regular notification path. If the firmware
exits auto mode across hibernation, the driver simply reflects the
resulting EC state rather than restoring the previous mode: it stays
out of the firmware's way and only synchronizes, which keeps the code
simple. Happy to hear opinions.

A few notes on the result:

* Writing a level through either interface now also emits
  brightness_hw_changed: both write paths share the same notify helper,
  which keeps the LED core and userspace listeners consistent with the
  new level no matter which interface was used. Rewriting the current
  level emits nothing, matching the firmware's own
  no-event-on-same-level semantics. Extending the notification to
  userspace-initiated changes seemed reasonable to me - the OSD then
  also reflects echo-based changes - but I'm happy to restrict it to
  hardware-initiated transitions if that is considered the better
  semantics.

* All state transitions now run under kbdlight_mutex in single
  critical sections (query + reconcile, write + reconcile), with the
  set and notify helpers expecting the caller to hold it. On the
  ordering with the LED core's trigger_lock, my reasoning is that the
  trigger callbacks (activate, hw_offloaded) are reached with
  trigger_lock held and then take kbdlight_mutex, while the
  notification helper is lock-free (it only queues the worker), so I
  don't see a reverse nesting. Extra eyes on this are welcome.

* The cached level is now the raw EC level including auto mode, so the
  kbdlight_brightness name has drifted a bit from what it holds.
  Renaming it would touch quite a few places; I left it as is for now
  and can do the rename as a follow-up if preferred.

One open question, mainly for Mark and Vishnu: the skip of LED
suspend/resume assumes the firmware turns the backlight off on suspend
and restores it on resume on all auto-mode capable machines, which is
what I observed here for manual levels (suspend and hibernation), auto
across suspend, and auto across hibernation coming back off. My belief
is that most machines handle this fine, but more testing on other
models or a look at the EC documentation from the Lenovo side would be
needed to be sure; if an exception turns up, a DMI quirk could restore
the old path for that machine.

Tested on my ThinkPad X1 Carbon Gen 14 (linux-next next-20261030).

I'm still learning the ways of upstream; thank you all for your time
and reviews, and any further comments are most welcome.

This series was developed with LLM assistance; I reviewed every design
decision against the framework sources and validated the behavior on
real hardware myself.

Regards,
Qiling Zhu

Qiling Zhu (7):
  platform/x86: thinkpad_acpi: Report auto keyboard backlight mode as off
  platform/x86: thinkpad_acpi: Extract kbdlight_notify_change() helpers
  platform/x86: thinkpad_acpi: Sync LED core state on level writes
  platform/x86: thinkpad_acpi: Detect auto-mode capability via MLCG
  platform/x86: thinkpad_acpi: Skip LED suspend/resume on auto-mode machines
  platform/x86: thinkpad_acpi: Add support for auto keyboard backlight
  platform/x86: thinkpad_acpi: Reconcile EC level on resume

[PATCH v3 1/7] platform/x86: thinkpad_acpi: Report auto keyboard backlight mode as off

[Qiling Zhu]

The EC keyboard backlight level 3, which selects the auto mode, is
passed to the LED classdev verbatim by kbdlight_sysfs_get().  The LED
then reports a brightness of 3 with max_brightness 2, both through the
brightness sysfs attribute and through brightness_hw_changed
notifications.

This is not only cosmetic: UPower 1.91.x treats the out-of-range value as
invalid and stops watching brightness_hw_changed altogether, so the desktop
stops following the Fn+Space hotkey once auto mode has been entered.

The auto mode is not a brightness level.  Map it to 0, the value it is
reported as most of the time anyway, in line with what ideapad-laptop
already does for its auto mode.

Fixes: bb28f3d51ff5 ("thinkpad_acpi: Add support for keyboard backlight")
Reported-by: Andrea Rabbaglietti <rabbagliettiandrea@gmail.com>
Closes: https://lore.kernel.org/r/CAENjc+w0Rqy=kDSew7XtNJCSvyPiXPJ5faWdmCr5RGYFhpunCA@mail.gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 4f81855ad..2c3f8cf32 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5102,6 +5102,9 @@ static struct ibm_struct video_driver_data = {
  * Keyboard backlight subdriver
  */
 
+/* EC level that selects the auto mode for the keyboard backlight */
+#define TPACPI_KBD_LIGHT_AUTO_LEVEL 3
+
 static enum led_brightness kbdlight_brightness;
 static DEFINE_MUTEX(kbdlight_mutex);
 
@@ -5191,7 +5194,8 @@ static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
 	int level;
 
 	level = kbdlight_get_level();
-	if (level < 0)
+	/* Auto mode is not a brightness level; report it as off. */
+	if (level < 0 || level == TPACPI_KBD_LIGHT_AUTO_LEVEL)
 		return 0;
 
 	return level;
-- 
2.56.1

[PATCH v3 2/7] platform/x86: thinkpad_acpi: Extract kbdlight_notify_change() helpers

[Qiling Zhu]

TP_HKEY_EV_KBD_LIGHT handling in tpacpi_driver_event() reads back the EC
level, updates the cached brightness and notifies the LED core inline.
Extract this block into separate kbdlight_notify_level() and
kbdlight_notify_change() helpers, in preparation for reusing the former
when writing a level through the proc interface in a subsequent change.

kbdlight_notify_level() expects the caller to hold kbdlight_mutex;
kbdlight_notify_change() queries the EC and notifies under the same
mutex, like the original inline code did.

No functional change intended.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 40 +++++++++++++--------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 2c3f8cf32..c8098f317 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5317,6 +5317,31 @@ static void kbdlight_resume(void)
 	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
 }
 
+/*
+ * Caller must hold kbdlight_mutex.
+ */
+static void kbdlight_notify_level(int level)
+{
+	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
+
+	/*
+	 * Check the brightness actually changed, setting the brightness
+	 * through kbdlight_set_level() also triggers this event.
+	 */
+	if (level == kbdlight_brightness)
+		return;
+
+	kbdlight_brightness = level;
+	led_classdev_notify_brightness_hw_changed(led_cdev, level);
+}
+
+static void kbdlight_notify_change(void)
+{
+	guard(mutex)(&kbdlight_mutex);
+
+	kbdlight_notify_level(kbdlight_sysfs_get(NULL));
+}
+
 static struct ibm_struct kbdlight_driver_data = {
 	.name = "kbdlight",
 	.read = kbdlight_read,
@@ -11586,20 +11611,7 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
 		return false;
 	case TP_HKEY_EV_KBD_LIGHT:
 		if (tp_features.kbdlight) {
-			enum led_brightness brightness;
-
-			guard(mutex)(&kbdlight_mutex);
-
-			/*
-			 * Check the brightness actually changed, setting the brightness
-			 * through kbdlight_set_level() also triggers this event.
-			 */
-			brightness = kbdlight_sysfs_get(NULL);
-			if (kbdlight_brightness != brightness) {
-				kbdlight_brightness = brightness;
-				led_classdev_notify_brightness_hw_changed(
-					&tpacpi_led_kbdlight.led_classdev, brightness);
-			}
+			kbdlight_notify_change();
 		}
 		/* Key events are suppressed by default hotkey_user_mask */
 		return false;
-- 
2.56.1

[PATCH v3 3/7] platform/x86: thinkpad_acpi: Sync LED core state on level writes

[Qiling Zhu]

kbdlight_set_level_and_update() pokes the led_classdev brightness cache
directly after writing the new EC level, without notifying userspace and
without any locking.  Commit 06da5325d02e ("platform/x86: thinkpad_acpi:
Stop setting led_classdev brightness directly") already removed this
pattern from the sysfs path but had to keep it for the proc interface,
which has no LED core path of its own.

Move the LED core state update into kbdlight_notify_level(): stop
updating the cached level in kbdlight_set_level() and have both the
sysfs and the proc write paths go through kbdlight_set_level_and_update(),
which writes the EC level and updates the cache under kbdlight_mutex,
and reports the new level through the LED core, like commit
07f48f668fac ("platform/x86: ideapad-laptop: add FnLock LED class
device") did for the ideapad FnLock LED.

As a result, writing a level through either interface now emits a
brightness_hw_changed notification, so userspace listeners see level
changes from both of them.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 23 +++++++++++++--------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index c8098f317..657a19d11 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5108,17 +5108,17 @@ static struct ibm_struct video_driver_data = {
 static enum led_brightness kbdlight_brightness;
 static DEFINE_MUTEX(kbdlight_mutex);
 
+/*
+ * Caller must hold kbdlight_mutex.
+ */
 static int kbdlight_set_level(int level)
 {
 	if (!hkey_handle)
 		return -ENXIO;
 
-	guard(mutex)(&kbdlight_mutex);
-
 	if (!acpi_evalf(hkey_handle, NULL, "MLCS", "dd", level))
 		return -EIO;
 
-	kbdlight_brightness = level;
 	return 0;
 }
 
@@ -5183,10 +5183,12 @@ static bool kbdlight_is_supported(void)
 	return status & BIT(9);
 }
 
+static int kbdlight_set_level_and_update(int level);
+
 static int kbdlight_sysfs_set(struct led_classdev *led_cdev,
 			enum led_brightness brightness)
 {
-	return kbdlight_set_level(brightness);
+	return kbdlight_set_level_and_update(brightness);
 }
 
 static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
@@ -5245,18 +5247,21 @@ static void kbdlight_exit(void)
 	led_classdev_unregister(&tpacpi_led_kbdlight.led_classdev);
 }
 
+static void kbdlight_notify_level(int level);
+
 static int kbdlight_set_level_and_update(int level)
 {
 	int ret;
-	struct led_classdev *led_cdev;
+
+	guard(mutex)(&kbdlight_mutex);
 
 	ret = kbdlight_set_level(level);
-	led_cdev = &tpacpi_led_kbdlight.led_classdev;
+	if (ret)
+		return ret;
 
-	if (ret == 0 && !(led_cdev->flags & LED_SUSPENDED))
-		led_cdev->brightness = level;
+	kbdlight_notify_level(level);
 
-	return ret;
+	return 0;
 }
 
 static int kbdlight_read(struct seq_file *m)
-- 
2.56.1

[PATCH v3 4/7] platform/x86: thinkpad_acpi: Detect auto-mode capability via MLCG

[Qiling Zhu]

The upper bits of the MLCG return value carry capability information.
Bit 20 indicates whether the EC supports the auto mode of the keyboard
backlight (0 = manual levels only, 1 = auto mode available), as defined
by the firmware.

Record it in tp_features.kbdlight_auto, in preparation for using it to
gate the auto-mode trigger registration and the LED suspend/resume
skipping in subsequent changes.

Link: https://lore.kernel.org/r/CABxCQKuEP5wMSstO0MK1MhaqXTKrPhUSN3V2mrE_42j4cXsXZQ@mail.gmail.com
Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 657a19d11..681b340a1 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -363,6 +363,7 @@ static struct {
 		TP_HOTKEY_TABLET_USES_GMMS,
 	} hotkey_tablet;
 	u32 kbdlight:1;
+	u32 kbdlight_auto:1;
 	u32 light:1;
 	u32 light_status:1;
 	u32 bright_acpimode:1;
@@ -5161,6 +5162,13 @@ static bool kbdlight_is_supported(void)
 	}
 
 	vdbg_printk(TPACPI_DBG_INIT, "kbdlight MLCG returned 0x%x\n", status);
+
+	/*
+	 * MLCG bit 20 indicates auto mode support: 0 = manual levels only,
+	 * 1 = auto mode available (per the firmware definition).
+	 */
+	tp_features.kbdlight_auto = !!(status & BIT(20));
+
 	/*
 	 * Guessed test for keyboard backlight:
 	 *
-- 
2.56.1

[PATCH v3 5/7] platform/x86: thinkpad_acpi: Skip LED suspend/resume on auto-mode machines

[Qiling Zhu]

On machines whose EC supports the auto mode (MLCG bit 20) the firmware
takes care of the keyboard backlight across suspend by itself: on a
ThinkPad X1 Carbon Gen 14 it turns the backlight off on suspend and
restores the previous level on resume for the manual levels, and keeps
auto mode across suspend; after hibernation from auto mode the
backlight comes back off.

Skip led_classdev_suspend() and led_classdev_resume() entirely on those
machines: the LED core no longer enters the suspended state, which also
removes the window in which userspace writes and pending
set_brightness work would race against a suspended LED.

Machines without auto-mode support keep the existing behavior.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 681b340a1..f9e2d324d 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5317,6 +5317,14 @@ static void kbdlight_suspend(void)
 	if (!tp_features.kbdlight)
 		return;
 
+	/*
+	 * On machines with auto-mode support the firmware takes care of
+	 * the keyboard backlight across suspend by itself; keep the LED
+	 * core out of it.
+	 */
+	if (tp_features.kbdlight_auto)
+		return;
+
 	led_cdev = &tpacpi_led_kbdlight.led_classdev;
 	led_update_brightness(led_cdev);
 	led_classdev_suspend(led_cdev);
@@ -5327,6 +5335,9 @@ static void kbdlight_resume(void)
 	if (!tp_features.kbdlight)
 		return;
 
+	if (tp_features.kbdlight_auto)
+		return;
+
 	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
 }
 
-- 
2.56.1

[PATCH v3 6/7] platform/x86: thinkpad_acpi: Add support for auto keyboard backlight

[Qiling Zhu]

Wire the EC keyboard backlight auto mode (level 3) up as a private
hardware control trigger, named "tpacpi-kbd-auto", on top of the LED
hw control trigger framework.  On machines that support auto mode
(MLCG bit 20) the trigger is registered and set as the LED's trigger
type.

Selecting the trigger switches the backlight to auto mode; the
offloaded state is derived from the cached EC level.  When Fn+Space
cycles into or out of auto mode, the driver notifies the trigger core,
so the trigger state shown in the trigger sysfs attribute follows
hardware-initiated transitions.  If the EC is already in auto mode at
driver initialization, the trigger is set as the default one.  Writing
a regular level through either sysfs or proc leaves auto mode, and the
trigger is dropped by the LED core.

Registering the trigger is allowed to fail; basic backlight control
keeps working in that case.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/Kconfig         |  1 +
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 95 +++++++++++++++++++--
 2 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
index 516594993..77d159a3a 100644
--- a/drivers/platform/x86/lenovo/Kconfig
+++ b/drivers/platform/x86/lenovo/Kconfig
@@ -83,6 +83,7 @@ config THINKPAD_ACPI
 	select NVRAM
 	select NEW_LEDS
 	select LEDS_CLASS
+	select LEDS_TRIGGERS
 	select INPUT_SPARSEKMAP
 	help
 	  This is a driver for the IBM and Lenovo ThinkPad laptops. It adds
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index f9e2d324d..f11e61417 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5106,7 +5106,12 @@ static struct ibm_struct video_driver_data = {
 /* EC level that selects the auto mode for the keyboard backlight */
 #define TPACPI_KBD_LIGHT_AUTO_LEVEL 3
 
-static enum led_brightness kbdlight_brightness;
+/*
+ * Cached EC level, including TPACPI_KBD_LIGHT_AUTO_LEVEL.  All state
+ * tracking compares EC levels; the auto mode is translated to a
+ * brightness value only when reporting to the LED core.
+ */
+static int kbdlight_brightness;
 static DEFINE_MUTEX(kbdlight_mutex);
 
 /*
@@ -5191,6 +5196,38 @@ static bool kbdlight_is_supported(void)
 	return status & BIT(9);
 }
 
+static struct led_hw_trigger_type kbdlight_auto_trigger_type;
+
+static enum led_brightness kbdlight_level_to_brightness(int level)
+{
+	/* Auto mode is not a brightness level; report it as off. */
+	if (level == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+		return 0;
+
+	return level;
+}
+
+static int kbdlight_auto_trigger_activate(struct led_classdev *led_cdev)
+{
+	guard(mutex)(&kbdlight_mutex);
+
+	return kbdlight_set_level(TPACPI_KBD_LIGHT_AUTO_LEVEL);
+}
+
+static bool kbdlight_auto_trigger_hw_offloaded(struct led_classdev *led_cdev)
+{
+	guard(mutex)(&kbdlight_mutex);
+
+	return kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+}
+
+static struct led_trigger kbdlight_auto_trigger = {
+	.name		= "tpacpi-kbd-auto",
+	.trigger_type	= &kbdlight_auto_trigger_type,
+	.activate	= kbdlight_auto_trigger_activate,
+	.hw_offloaded	= kbdlight_auto_trigger_hw_offloaded,
+};
+
 static int kbdlight_set_level_and_update(int level);
 
 static int kbdlight_sysfs_set(struct led_classdev *led_cdev,
@@ -5204,11 +5241,11 @@ static enum led_brightness kbdlight_sysfs_get(struct led_classdev *led_cdev)
 	int level;
 
 	level = kbdlight_get_level();
-	/* Auto mode is not a brightness level; report it as off. */
-	if (level < 0 || level == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+
+	if (level < 0)
 		return 0;
 
-	return level;
+	return kbdlight_level_to_brightness(level);
 }
 
 static struct tpacpi_led_classdev tpacpi_led_kbdlight = {
@@ -5235,9 +5272,37 @@ static int __init kbdlight_init(struct ibm_init_struct *iibm)
 		return -ENODEV;
 	}
 
-	kbdlight_brightness = kbdlight_sysfs_get(NULL);
+	kbdlight_brightness = kbdlight_get_level();
+	if (kbdlight_brightness < 0)
+		kbdlight_brightness = 0;
+
 	tp_features.kbdlight = 1;
 
+	if (tp_features.kbdlight_auto) {
+		rc = devm_led_trigger_register(&tpacpi_pdev->dev,
+					       &kbdlight_auto_trigger);
+		if (rc) {
+			/*
+			 * Auto-mode trigger unavailable; basic backlight
+			 * control keeps working.
+			 */
+			dev_warn(&tpacpi_pdev->dev,
+				 "auto-mode trigger unavailable\n");
+		} else {
+			struct led_classdev *led_cdev =
+				&tpacpi_led_kbdlight.led_classdev;
+
+			led_cdev->flags |= LED_TRIG_HW_CHANGED;
+			led_cdev->trigger_type = &kbdlight_auto_trigger_type;
+			led_cdev->hw_control_trigger =
+				kbdlight_auto_trigger.name;
+
+			if (kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL)
+				led_cdev->default_trigger =
+					kbdlight_auto_trigger.name;
+		}
+	}
+
 	rc = led_classdev_register(&tpacpi_pdev->dev,
 				   &tpacpi_led_kbdlight.led_classdev);
 	if (rc < 0) {
@@ -5347,6 +5412,8 @@ static void kbdlight_resume(void)
 static void kbdlight_notify_level(int level)
 {
 	struct led_classdev *led_cdev = &tpacpi_led_kbdlight.led_classdev;
+	bool is_auto, was_auto;
+	int brightness;
 
 	/*
 	 * Check the brightness actually changed, setting the brightness
@@ -5355,15 +5422,29 @@ static void kbdlight_notify_level(int level)
 	if (level == kbdlight_brightness)
 		return;
 
+	is_auto = level == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+	was_auto = kbdlight_brightness == TPACPI_KBD_LIGHT_AUTO_LEVEL;
+
+	if (is_auto != was_auto)
+		led_trigger_notify_hw_control_changed(led_cdev, is_auto);
+
 	kbdlight_brightness = level;
-	led_classdev_notify_brightness_hw_changed(led_cdev, level);
+	brightness = kbdlight_level_to_brightness(level);
+
+	led_classdev_notify_brightness_hw_changed(led_cdev, brightness);
 }
 
 static void kbdlight_notify_change(void)
 {
+	int level;
+
 	guard(mutex)(&kbdlight_mutex);
 
-	kbdlight_notify_level(kbdlight_sysfs_get(NULL));
+	level = kbdlight_get_level();
+	if (level < 0)
+		return;
+
+	kbdlight_notify_level(level);
 }
 
 static struct ibm_struct kbdlight_driver_data = {
-- 
2.56.1

[PATCH v3 7/7] platform/x86: thinkpad_acpi: Reconcile EC level on resume

[Qiling Zhu]

The firmware does not always restore the keyboard backlight level across
sleep: after hibernation from auto mode the EC comes back with the
backlight off, without emitting a notification.  The cached level and a
possibly still-active auto trigger then desynchronize from the hardware
until the next user action.

Check the EC level on resume on auto-mode machines and route it through
the regular notification path, which also deactivates the auto trigger
if the firmware left auto mode.

Signed-off-by: Qiling Zhu <kmatzkaczor244@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index f11e61417..802e0d53b 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5395,13 +5395,22 @@ static void kbdlight_suspend(void)
 	led_classdev_suspend(led_cdev);
 }
 
+static void kbdlight_notify_change(void);
+
 static void kbdlight_resume(void)
 {
 	if (!tp_features.kbdlight)
 		return;
 
-	if (tp_features.kbdlight_auto)
+	if (tp_features.kbdlight_auto) {
+		/*
+		 * The firmware may not restore auto mode: after hibernation
+		 * from level 3 it comes back with the backlight off.  Check
+		 * the EC level and reconcile.
+		 */
+		kbdlight_notify_change();
 		return;
+	}
 
 	led_classdev_resume(&tpacpi_led_kbdlight.led_classdev);
 }
-- 
2.56.1