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@xxxxxxxxx>
> ---
> 
> 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: