Re: [RFC PATCH 0/9] leds: Add support for hw initiated hw control trigger transition

[Lee Jones <lee@kernel.org>]

I'd like for the netdev folks to take a look at this please.

> Some laptops can tune their keyboard backlight according to ambient
> light sensors (auto mode). This capability is essentially a hw control
> trigger. Meanwhile, such laptops also offer a shrotcut for cycling
> through brightness levels and auto mode. For example, on ThinkBook,
> pressing Fn+Space cycles keyboard backlight levels in the following
> sequence:
> 
>   1 => 2 => 0 => auto => 1 ...
> 
> Recent ThinkPad models should have similar sequence too.
> 
> However, there are some issues preventing us from using hw control
> trigger:
> 
> 1. We want a mechanism to tell userspace which trigger is the hw control
>    trigger, so that userspace can determine if auto mode is on/off or
>    turing it on/off programmatically without obtaining the hw control
>    trigger's name via other channels
> 2. Turing on/off auto mode via the shortcut cannot activate/deactivate
>    the hw control trigger, making the software state out-of-sync
> 3. Even with #1 resolved, deactivating the hw control trigger after
>    receiving the event indicating "auto => 1" has a side effect of
>    emitting LED_OFF, breaking the shortcut cycle
> 
> This RFC series tries to demonstrate a path on solving these issues:
> 
> - Introduce an attribute called trigger_may_offload, so that userspace
>    can determine:
>    - if the LED device supports hw control (supported => visible)
>    - which trigger is the hw control trigger
>    - if the hw control trigger is selected
>    - if the hw control trigger is in hw control (i.e., offloaded)
>      - A callback offloaded() is added so that LED triggers can report
>        their hw control state
> - Add led_trigger_notify_hw_control_changed() interface, so that LED
>   drivers can notify the LED core about hardware initiated hw control
>   state transitions. The LED core will then determine if the transition
>   is allowed and turning on/off the hw control trigger accordingly
> - Tune the logic of trigger deactivation so that it won't emit LED_OFF
>   when the deactivation is triggered by hardware
> 
> The last two patches are included into the RFC series to demonstrate how
> to utilize these interfaces to add support for auto keyboard backlight
> to ThinkBook. They will be submitted separately once the dust settles.
> 
> Currently no Kconfig entry is provided to disable either interface. If
> needed, I will add one later.
> 
> [ Summary of other approaches ]
> 
> < custom attribute >
> 
> Pros:
> - simplicity, KISS
> - no need to touch the LED core
> - extensible as long as it has a sensor-neutral name
>   - a sensor-related name could potentially lead to a mess if a future
>     device implements auto mode based on multiple different sensors
> 
> Cons:
> - must have zero influence on brightness_set[_blocking] callbacks
>   in order not to break triggers
>   - potential interference with triggers and the brightness attribute
> - weird semantic (an attribute other than "brightness" and "trigger"
>   changes the brightness)
> 
> < hw control trigger (this series) >
> 
> Pros:
> - mutually exclusive with other triggers (hence less chaos)
> - semantic correctness
> - acts as an aggregate switch to turn on/off auto mode even a future
>   device implements auto mode based on multiple different sensors
>   - extensibility (through trigger attributes)
> 
> Cons:
> - complexity
> 
> [ Previous discussion threads ]
> 
> https://lore.kernel.org/r/08580ec5-1d7b-4612-8a3f-75bc2f40aad2@app.fastmail.com
> 
> https://lore.kernel.org/r/1dbfcf656cdb4af0299f90d7426d2ec7e2b8ac9e.camel@rong.moe
> 
> Thanks,
> Rong
> 
> Rong Zhang (9):
>   leds: Load trigger modules on-demand if used as hw control trigger
>   leds: Add callback offloaded() to query the state of hw control
>     trigger
>   leds: cros_ec: Implement offloaded() callback for trigger
>   leds: turris-omnia: Implement offloaded() callback for trigger
>   leds: trigger: netdev: Implement offloaded() callback
>   leds: Add trigger_may_offload attribute
>   leds: trigger: Add led_trigger_notify_hw_control_changed() interface
>   platform/x86: ideapad-laptop: Decouple HW & cdev brightness for kbd
>     backlight
>   platform/x86: ideapad-laptop: Fully support auto kbd backlight
> 
>  .../obsolete/sysfs-class-led-trigger-netdev   |  15 ++
>  Documentation/ABI/testing/sysfs-class-led     |  22 ++
>  .../testing/sysfs-class-led-trigger-netdev    |  13 --
>  Documentation/leds/leds-class.rst             |  72 ++++++-
>  drivers/leds/led-class.c                      |  23 +++
>  drivers/leds/led-triggers.c                   | 176 +++++++++++++++-
>  drivers/leds/leds-cros_ec.c                   |   6 +
>  drivers/leds/leds-turris-omnia.c              |   7 +
>  drivers/leds/leds.h                           |   3 +
>  drivers/leds/trigger/ledtrig-netdev.c         |  10 +
>  drivers/platform/x86/lenovo/Kconfig           |   1 +
>  drivers/platform/x86/lenovo/ideapad-laptop.c  | 194 ++++++++++++++----
>  include/linux/leds.h                          |   6 +
>  13 files changed, 492 insertions(+), 56 deletions(-)
>  create mode 100644 Documentation/ABI/obsolete/sysfs-class-led-trigger-netdev
> 
> 
> base-commit: a75cb869a8ccc88b0bc7a44e1597d9c7995c56e5
> -- 
> 2.51.0
> 

-- 
Lee Jones [李琼斯]