Message-ID: <69b9a9df97f4d10e2d11d6b0eb81bbf41fb4cbde.camel@rong.moe>
Date: Tue, 10 Feb 2026 02:14:41 +0800
From: Rong Zhang <i@...g.moe>
To: Mark Pearson <mpearson-lenovo@...ebb.ca>, Hans de Goede
 <hansg@...nel.org>,  Vishnu Sankar <vishnuocv@...il.com>
Cc: Henrique de Moraes Holschuh <hmh@....eng.br>, "Derek J . Clark"	
 <derekjohn.clark@...il.com>, Ilpo Järvinen	
 <ilpo.jarvinen@...ux.intel.com>, ibm-acpi-devel@...ts.sourceforge.net, 
 "platform-driver-x86@...r.kernel.org"	
 <platform-driver-x86@...r.kernel.org>, linux-kernel@...r.kernel.org, Vishnu
 Sankar <vsankar@...ovo.com>
Subject: Re: [PATCH] thinkpad_acpi: Add Auto mode support with dynamic
 max_brightness

Hi Mark,

Thanks for your reply.

On Mon, 2026-02-09 at 10:46 -0500, Mark Pearson wrote:
> 
> On Sun, Feb 8, 2026, at 3:58 PM, Rong Zhang wrote:
> > Hi Hans, Vishnu and Mark,
> > 
> > On Sun, 2026-02-08 at 11:54 +0100, Hans de Goede wrote:
> > > Hi Vishnu,
> > > 
> > > On 4-Feb-26 00:22, Vishnu Sankar wrote:
> > > > Dynamically detect keyboard backlight capabilities and set
> > > > max_brightness correctly (2 for old models, 3 for new models
> > > > with Auto mode).
> > > 
> > > Thank you for your patch.
> > > 
> > > If I understand this correctly, writing 3 as level does not
> > > make the backlight more bright then writing 2, but instead
> > > it puts the backlight in some auto mode ?
> > > 
> > > If I've that correct then  userspace should keep seeing
> > > a range of 0 - 2 and the special auto mode value should
> > > be reported / be made settable through a separate als_enabled
> > > sysfs attribute under the LED class device. See:
> > > 
> > > Documentation/ABI/testing/sysfs-platform-dell-laptop
> > > 
> > > You can add extra attributes there by setting the groups
> > > member of the struct led_classdev, see kbd_led_groups[]
> > > in drivers/platform/x86/dell/dell-laptop.c, except that
> > > you should use a .is_visible callback to only show this
> > > on hw which supports it and you only need 1 group with
> > > 1 attribute.
> > 
> > When I implemented "als_enabled" for ideapad-laptop, Mark Pearson
> > suggested it'd better to introduce "something similar to
> > LED_BRIGHT_HW_CHANGED" rather than using custom attributes, as "this is
> > going to be a common feature across multiple vendors it might need
> > doing at a common layer". Also, auto mode can be activated by HW as a
> > result of user input, so we need an approach to notify userspace just
> > like what LED_BRIGHT_HW_CHANGED does. More importantly, the read value
> > of the brightness attribute becomes nonsense when auto mode is on. This
> > matches the semantic of hw control trigger.
> > 
> > I agreed with Mark and had a proposal of allowing HW to initiate a
> > transition from "none" to hw control trigger and vice versa. See the
> > thread in
> > https://lore.kernel.org/all/08580ec5-1d7b-4612-8a3f-75bc2f40aad2@app.fastmail.com/
> > 
> > I hadn't push it further due to other things taking the priority,
> > though I already had a PoC back to then. I quickly rebased the PoC with
> > some cleanups and put it here for preview:
> > 
> > https://github.com/Rongronggg9/linux/tree/leds-trigger-hw-changed
> > 
> > I will find some time to refine it and send an RFC series.
> > 
> Hi Rong,
> 
> Thanks for highlighting this (have to be honest - I'd forgotten we'd discussed it).
> I think my suggestion may have been understood and I wonder your approach is more complicated than needed.

If there is a mechanism to set the brightness on specific events or
conditions, it is a trigger. If the trigger is controlled by hardware,
it's a hw control trigger. That's why I propose using a private hw
control trigger to represent this to make it semantically correct.

> I was thinking we add a new flag to the led_classdev. e.g
> #define LED_AUTO_BRIGHTNESS         BIT(26)

Implementing it this way is still complicated as far as I can imagine:

- A new attribute to expose the capability as you've said.
- We need to extend brightness_get/brightness_set[_blocking] interfaces
to accept/emit a special brightness value to represent auto mode.
- We should handle brightness setting requests from usersapce and from
led triggers separately: the former can put the LED into auto mode
while the latter cannot.
- Deprecate brightness and brightness_hw_changed while introducing new
attributes. We can't extend existing attributes as I will explain
later. That's the most frustrating part :-/

And this approach becomes a bit weird if a future SKU comes with its
auto mode tunable: you will have some device attributes which are only
meaningful when auto mode is active. This is all because they are
fundamentally trigger attributes in the first place...

> Then the platform driver can set this flag and in led_classdev_register_ext we'd handle it appropriately to create a sysfs (e.g. auto_brightness_capable) node so user space knows auto is supported.
> Other than that:
>  - When the brightness is read and auton is being used - return "auto" instead of a value. Hopefully that doesn't break anything for user space?

It will likely break something. We can't extend an interface with new
data types.

For example, existing userspace programs may have being using these for
long: 

- POSIX shell: [ -eq, -ne, -gt, -ge, -lt, -le ]
- Bash: let, (( ))
- C: atoi(), atol(), atoll(), fscanf(), vfscanf()
- Python: int()
- Regex: [0-9], \d (PCRE), [[:digit:]] (POSIX)
- ... And more similar things dealing with integers

I think it's not worth deprecating the existing interface just to
introduce something that is not fundamentally "brightness".

>  - When the brightness is set, you can use a value or 'auto" as you desire (Documentation would need updating to allow this)
> Really I was just looking for a way to advertise to user space that a auto option would be supported :)

That's my goal too.

I admit that my proposal is complicated and may need a lot of time to
make it into its right path. It may even be rejected by LED folks. But
it's the best approach I can think of considering our requirements on
the interface:

1. It shouldn't break any existing interfaces.
2. It's exposed to userspace for getting or setting its status.
3. HW status transition should reach userspace (similar to
LED_BRIGHT_HW_CHANGED).

I will see if I can finish my RFC patch this week or next. If it's
rejected probably we will have to continue on "als_enabled"...

Thanks,
Rong

> Mark
> 
> 

Powered by blists - more mailing lists