Re: [RFC PATCH 1/9] leds: Load trigger modules on-demand if used as hw control trigger

[Mark Pearson]

On Fri, Mar 13, 2026, at 10:01 AM, Rong Zhang wrote:
> Hi Mark,
>
> On Thu, 2026-03-12 at 17:46 -0400, Mark Pearson wrote:
>> 
>> On Thu, Mar 12, 2026, at 2:01 PM, Rong Zhang wrote:
>> > Hi Andrew,
>> > 
>> > On Wed, 2026-03-11 at 22:29 +0100, Andrew Lunn wrote:
>> > > > We just can't prevent the EC from responding to the Fn+Space shortcut.
>> > > > So it's essentially user's choice to switch to the hw control trigger
>> > > > and make it offloaded to hardware (sorry if my cover letter and replies
>> > > > didn't express this well).
>> > > 
>> > > Do you have any control over the EC?
>> > > 
>> > > You have a two bosses dilemma. You need to eliminate one of the
>> > > bosses. Either the EC controls the LED, or Linux does. Having both
>> > > controlling it is just going to work out badly.
>> > 
>> > I agree that the manufacturers designed the interface poorly :-/
>> > 
>> > I am not affiliated with any laptop manufacturers so I am just speaking
>> > for myself:
>> > 
>> > IMO the real boss is the user. Both the shortcut (Fn+Space) and the
>> > ACPI interface are just "message channels" for the EC to know about the
>> > user's choice.
>> > 
>> > Being able to press such a shortcut always implies that the user is
>> > physically in front of the device. In this case it no longer about
>> > whether Linux or the EC controls the LED, but both should respect
>> > user's choice. That was why brightness_hw_changed was introduced to
>> > respect user's choice and pass it to the userspace. So far there has
>> > been ~10 drivers utilizing the brightness_hw_changed interface.
>> > 
>> I am affiliated with a laptop manufacturer :) Happy to take suggestions on what should be improved or is missing (can't promise anything but happy to consider it and take it for review).
>> 
>> We can set the brightness, get the status, and the FW sends events when it changes - all supported on Linux (for Lenovo devices). This looks like a pretty decent API to me. What is it missing?
>
> IMO, one missing thing is that there's no approach for the OS to
> prevent the EC from responding to Fn+Space. Hence no 100% pure software
> control is possible. We end up have a mixed SW + EC control.
>
> Another missing thing is that there's no approach for the OS to
> query/set the ALS-to-brightness curve (or trip points, whatever you
> call it) of the EC driven auto brightness mode. Therefore, should we
> have a kernel mode ALS-based software trigger, we would never know if
> our curve could be offloaded.
>
> That being said, I don't think either of these two missing things is a
> big deal, since most laptops (if not all) are just designed to work
> like this and I don't think we would have a kernel mode ALS-based
> software trigger any soon. No 100% pure software control wasn't, isn't
> and shouldn't be a blocker of using an LED classdev. As I've said,
> that's exactly why brightness_hw_changed was introduced.
>
Got it - thanks for the input.

My personal opinion (not feedback from the FW team)
 - disabling the FN+spacebar is not a useful feature. It's not exactly something you would trigger accidentally. I can't think when people would need to disable it rather than just choosing to not use it.
 - Programming the curve would be more interesting - but becomes a bit of a pro-user use case. Happy to suggest it, but it's a nice-to-have feature IMO.

Agreed that neither of these seem a valid reason to block the implementation.

>> 
>> I don't understand the two bosses issue I'm afraid. The user ('Linux' in your description?) tells the EC what it wants the LED to be, and the EC does it. The EC is not a boss.
>
> The "user" means the one (i.e., a human) who is using the device. And
> "message channels" mean:
>
> User -> pressing Fn+Space -> EC -> update keyboard backlight
>
> User -> LED classdev / manufacturer utilities -> ACPI -> EC -> update
> keyboard backlight
>
> They are all about the user, as a human, tells their intention to the
> device. Of course there may be some userspace software or kernel
> trigger blinking the LED, but again, that's still the user's choice and
> intention. Hence I don't think EC is a boss either, and the user is the
> real boss.
>
I think we're in agreement :)

>> 
>> > > 
>> > > > As my previous reply said, it's common that an LED driver can't prevent
>> > > > hardware from changing its state autonomously. Prior to the
>> > > > introduction of auto brightness mode, brightness_hw_changed is enough
>> > > > to handle this. The issue only emerges when recent models start to
>> > > > provide an auto brightness mode based on the ALS sensor.
>> > > 
>> > > Do you have a software driven brightness mode based on an ALS? What
>> > > API do you use to control this? Can you use that API, and accelerate
>> > > it?
>> > 
>> > All devices I've seen implement an EC driven auto brightness mode based
>> > on an ALS.
>> > 
>> > @Mark, do you know any device implementing a software driven auto
>> > brightness mode?
>> > 
>> 
>> I don't - to my knowledge in auto mode it's always driven by the HW/FW.
>
> Thanks.
>
>> 
>> If there was a SW approach it would read the sensor and set the brightness to low/medium/high (and not to auto) so I'm struggling to understand the issue here. What am I missing?
>
> My understanding of Andrew's words (see also his previous replies) is:
>
>    hw control trigger is fundamentally an accelerated (offloaded)
>    software trigger. Only if there is a software-driven ALS-based
>    trigger and the curve matches the FW one can we treat the auto
>    brightness mode as a hw control trigger.
>
> But those laptops with an ALS and keyboard backlight are not designed
> to work like this, and the curve may be very specific to the ALS and
> the luminance of the keyboard backlight. So I asked you to confirm if
> there is any device designed to use an software driven auto brightness
> mode (even under Windows).
>
> Hmm, forgot to ask about that... Is there any device comes with ALS-
> based auto brightness mode but Linux cannot read the the ALS? If such
> devices exist, the "accelerated software trigger" model is no longer
> relevant.
>
I don't know of any SW driven auto brightness mode.

Afraid I also don't know about reading the ALS. I'll see if I can find out - but I don't think it's directly important to this series which is about supporting the new HW automated feature.
We're not trying to implement an automated SW based control feature with this series :)

> Also that's why we have private LED triggers -- it's useful when the
> HW/FW has its own triggering functionality. For example, "cros-ec-led"
> has a private trigger and declares it as its hw control trigger.
>

ack

>> 
>> > > 
>> > > > FYI, desktop environments (e.g., GNOME, KDE) can control the backlight
>> > > > brightness of keyboards through sliders and heavily depend on
>> > > > brightness_hw_changed to update the sliders and display OSD once the
>> > > > shortcut is pressed.
>> > > 
>> > > Hold up. Terminology problem. I'm a networking guy, i know networking
>> > > terms. By slider, do you mean a software scroll bar sort of thing? 
>> > 
>> > Yes. See
>> > https://blogs.kde.org/2024/09/04/brightness-controls-for-all-your-displays/
>> > 
>> > (it was about display backlight but it also showed the keyboard one in
>> > the same image)
>> > 
>> > > I'm
>> > > an XFCE users. I can control the display backlight with a slider on
>> > > the battery charge applet. And i can use Fn F4/F5. I've not seen a
>> > > software scroll bar for the keyboard backlight, but i think
>> > > <CTRL><SPC> allows me to change the keyboard backlight.
>> > > 
>> > > So we have a slider, which is purely software, Linux. And we have key
>> > > presses, which you are calling shortcut, which the EC acts on, and
>> > > might tell Linux, maybe, but not about the key press, but the action
>> > > the EC took because of the key press.
>> > 
>> > "might tell", "maybe"
>> > 
>> > It always tells the OS that the state of keyboard backlight has
>> > changed.
>> > 
>> > > 
>> > > You have some API to the EC to ask it nicely to act on the software
>> > > slide, but it is the EC which really controls the LED, not Linux.
>> > > 
>> > > To me a Linux LED is a poor fit for what you want, and i think a
>> > > trigger is even worse. The problems you have are because the
>> > > LED+trigger model, plus using the hardware for acceleration, does not
>> > > fit with the EC actually controlling the hardware.
>> > > 
>> > > I would suggest you look at the API the EC exports and find a better
>> > > model for it.
>> > 
>> > An LED classdev may be unable to perfectly fit this, but nothing is
>> > perfect and so far it's the best thing we have. It's a fortunate to
>> > have the LED subsystem. Windows, without a similar interface, ends up
>> > being filled with disgusting software pre-installed by the
>> > manufacturer.
>> > 
>> 
>> Afraid I don't understand what we are debating here.
>> 
>> Isn't the whole goal of this patch to make it so LED classdev is a better fit to address missing functionality? Why would switching to something else (I have no idea what) be better? Especially given the the keyboard backlight is currently a LED device, and changing that would potentially break things for users.
>> 
>> From my perspective if I could just tear this out and have a Lenovo only keyboard_backlight implementation under (for example) /sys/devices/thinkpad_acpi it would be so much easier. But I don't think it is the right thing to do. My experience is if we define a common approach then all vendors will use it going forward - which is better for the Linux experience overall.
>> Or we don't have fully implemented features for Linux users? That's kinda sucky.
>
> I agreed with you. Just some supplemental information:
>
> ideapad-laptop has an custom attribute "fn_lock". This used to be
> the only way for userspace to turn on/off FnLock. The attribute does
> not support any notification mechanism.
>
> Since devices with FnLock also come with an LED indicating the status
> of FnLock, an LED classdev has been added with a new
> LED_FUNCTION_FNLOCK macro for dt-bindings and UAPI. See commit
> 7ab6c64663a0 ("dt-bindings: leds: Add LED_FUNCTION_FNLOCK") and commit
> 07f48f668fac ("platform/x86: ideapad-laptop: add FnLock LED class
> device"). Since then, userspace receives notifications through
> brightness_hw_changed when the user presses Fn+Esc.
>
> I think this shows that an LED classdev, as a common interface, has its
> vitality even when being used in a very specific use case.
>
>> 
>> I don't think the two bosses argument is valid (or at least I don't understand it). Are there any other critical implementation details that make this a poor choice and will bite us in the long run?
>> 
>> I personally find the implementation more complicated than I originally expected, but having looked at it and understood better what Rong was proposing I understand the benefits and I think it works. We're still checking it out on Thinkpad to confirm that, but this patch is a RFC so I think that's part of the process.
>> 
>> > IMO the presence of brightness_hw_changed proves that an LED classdev,
>> > as long as appropriate interfaces are provided, can work well with such
>> > hardware. And I don't think there is too much difference between EC
>> > setting a static brightness value due to a shortcut being pressed and
>> > EC turning on/off the auto brightness mode due to the same shortcut --
>> > if we can handle the former well, we can also implement a similar
>> > mechanism for the latter.
>> > 
>> > 
>> > Do you have any recommendations for a "better model"?
>> > 
>> > Did you mean do not register LED classdevs at all? This isn't really
>> > viable and will break userspace. Some drivers has been using LED
>> > classdevs for keyboard backlight for over a decade. And many
>> > `*::kbd_backlight' LEDs rely on brightness_hw_changed, so it's very
>> > common that we can't take 100% control over EC. LED classdevs and EC-
>> > controlled keyboard backlight have lived in harmony for a long time.
>> > 
>> > If we still register the keybaord backlight as an LED classdev but use
>> > a custom attribute to report/set the auto brightness mode. IMO this is
>> > even uglier than LED+trigger, as writing to such a non-brightness
>> > attribute will interfere with the brightness attribute and the active
>> > trigger and vice versa. Even if we rule out the EC's action, such
>> > interference still exists as long as users use the attribute.
>> > 
>> > 
>> > As for the software-vs-hardware priority issue, how about adding an
>> > attribute "hw_change_policy" so that users can select if the software
>> > state should be always reimposed to hardware?
>> 
>> Is this needed? When wouldn't this be the case?
>> 
>> If the SW sets a specific brightness that should become the setting. It would override any previous choices and tell the HW that is what is wanted now - don't change it (until the user says otherwise).
>> If we're in auto mode and the HW changes the brightness - it doesn't change the setting from auto mode, just the reported brightness level if queried.
>
> My understanding of Andrew's words is:
>
>    Linux LED should always be the boss. Tell the HW: don't change it *even
>    if the user presses Fn+Space*. Failing to accomplish this means that we
>    are in "a two bosses dilemma", hence "a Linux LED is a poor fit for
>    what you want" and "a trigger is even worse".
>
> Since Andrew cares about the software's precedence, the best thing we
> can do is adding an attribute for users to select their preference. The
> attribute's default value must not be reimpose, otherwise it breaks
> existing userspace programs relying on brightness_hw_changed and
> confuses most users.
>

For the current HW, the attribute is not going to be supported. There's no way to disable FN+space as it goes directly to the BIOS (which then notifies the OS).

I guess the OS can get the notification and then say 'nope - despite the fact they just pressed the FN + spacebar, they actually don't want anything to change', and revert the setting to the previous setting. 
If the user had to specifically set the attribute to do this it's fine - but it feels like something that nobody would ever use and a whole bunch of extra complexity, to me

> But yeah, personally I don't think it's needed either. It's been 9
> years since the introduction of brightness_hw_changed, and there's no
> complaint about HW/FW overriding the software configured brightness.
> After all, it's the user who decides to press the shortcut and asks the
> EC to change the brightness or turn on/off the auto brightness mode.
>

I think you and I are both in agreement. 

Andrew - are you still against the current proposal or have your concerns been addressed?

How do we proceed on finding something that we can move forward with?

Mark