[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <08580ec5-1d7b-4612-8a3f-75bc2f40aad2@app.fastmail.com>
Date: Wed, 06 Aug 2025 15:02:25 -0400
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: "Rong Zhang" <i@...g.moe>, "Ike Panhc" <ikepanhc@...il.com>,
"Derek J . Clark" <derekjohn.clark@...il.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
"Hans de Goede" <hansg@...nel.org>
Cc:
"platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] platform/x86: ideapad-laptop: Fully support auto kbd backlight
Hi Rong,
On Tue, Aug 5, 2025, at 10:01 AM, Rong Zhang wrote:
> Currently, the auto brightness mode of keyboard backlight maps to
> brightness=0 in LED classdev. The only method to switch to such a mode
> is by pressing the manufacturer-defined shortcut (Fn+Space). However, 0
> is a multiplexed brightness value; writing 0 simply results in the
> backlight being turned off.
>
> With brightness processing code decoupled from LED classdev, we can now
> fully support the auto brightness mode. In this mode, the keyboard
> backlight is controlled by the EC according to the ambient light sensor
> (ALS).
>
> To utilize this, a sysfs node is exposed to the userspace:
> /sys/class/leds/platform::kbd_backlight/als_enabled. The name is chosen
> to align with dell-laptop, which provides a similar feature.
>
> Signed-off-by: Rong Zhang <i@...g.moe>
> ---
> .../ABI/testing/sysfs-platform-ideapad-laptop | 12 ++++
> drivers/platform/x86/lenovo/ideapad-laptop.c | 65 ++++++++++++++++++-
> 2 files changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> index 5ec0dee9e707..a2b78aa60aaa 100644
> --- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> +++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> @@ -50,3 +50,15 @@ Description:
> Controls whether the "always on USB charging" feature is
> enabled or not. This feature enables charging USB devices
> even if the computer is not turned on.
> +
> +What: /sys/class/leds/platform::kbd_backlight/als_enabled
> +Date: July 2025
> +KernelVersion: 6.17
> +Contact: platform-driver-x86@...r.kernel.org
> +Description:
> + This file allows to control the automatic keyboard
> + illumination mode on some systems that have an ambient
> + light sensor. Write 1 to this file to enable the auto
> + mode, 0 to disable it. In this mode, the actual
> + brightness level is not available and reading the
> + "brightness" file always returns 0.
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c
> b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 5014c1d0b633..49f2fc68add4 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -1712,6 +1712,57 @@ static void ideapad_kbd_bl_notify(struct
> ideapad_private *priv)
> ideapad_kbd_bl_notify_known(priv, brightness);
> }
>
> +static ssize_t als_enabled_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct ideapad_private *priv = container_of(led_cdev, struct
> ideapad_private, kbd_bl.led);
> + int hw_brightness;
> +
> + hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> + if (hw_brightness < 0)
> + return hw_brightness;
> +
> + return sysfs_emit(buf, "%d\n", hw_brightness ==
> KBD_BL_AUTO_MODE_HW_BRIGHTNESS);
> +}
> +
> +static ssize_t als_enabled_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct ideapad_private *priv = container_of(led_cdev, struct
> ideapad_private, kbd_bl.led);
> + bool state;
> + int err;
> +
> + err = kstrtobool(buf, &state);
> + if (err)
> + return err;
> +
> + /*
> + * Auto (ALS) mode uses a predefined HW brightness value. It is
> + * impossible to disable it without setting another brightness value.
> + * Set the brightness to 0 when disabling is requested.
> + */
> + err = ideapad_kbd_bl_hw_brightness_set(priv, state ?
> KBD_BL_AUTO_MODE_HW_BRIGHTNESS : 0);
> + if (err)
> + return err;
> +
> + /* Both HW brightness values map to 0 in the LED classdev. */
> + ideapad_kbd_bl_notify_known(priv, 0);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(als_enabled);
> +
> +static struct attribute *ideapad_kbd_bl_als_attrs[] = {
> + &dev_attr_als_enabled.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(ideapad_kbd_bl_als);
> +
> static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> {
> int brightness, err;
> @@ -1722,10 +1773,20 @@ static int ideapad_kbd_bl_init(struct
> ideapad_private *priv)
> if (WARN_ON(priv->kbd_bl.initialized))
> return -EEXIST;
>
> - if (ideapad_kbd_bl_check_tristate(priv->kbd_bl.type)) {
> + switch (priv->kbd_bl.type) {
> + case KBD_BL_TRISTATE_AUTO:
> + /* The sysfs node will be
> /sys/class/leds/platform::kbd_backlight/als_enabled */
> + priv->kbd_bl.led.groups = ideapad_kbd_bl_als_groups;
> + fallthrough;
> + case KBD_BL_TRISTATE:
> priv->kbd_bl.led.max_brightness = 2;
> - } else {
> + break;
> + case KBD_BL_STANDARD:
> priv->kbd_bl.led.max_brightness = 1;
> + break;
> + default:
> + /* This has already been validated by ideapad_check_features(). */
> + unreachable();
> }
>
> brightness = ideapad_kbd_bl_brightness_get(priv);
> --
> 2.50.1
We're looking to implement this feature on the Thinkpads, so this change is timely :)
I did wonder if we should be making changes at the LED class level? Something similar to LED_BRIGHT_HW_CHANGED maybe as a way to advertise that auto mode is supported and some hooks to support that in sysfs?
I know it would be more work, but I'm guessing this is going to be a common feature across multiple vendors it might need doing at a common layer.
As a note - on the Thinkpads we've had to look at getting the correct Intel ISH firmware loaded (and we're working on getting that upstream to linux-firmware). Is that needed on the Ideapads for the feature to work well or not?
Mark
Powered by blists - more mailing lists