Re: [PATCH v2] platform/x86: add support for Acer Predator LEDs

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 6/21/21 9:23 PM, Enrico Weigelt, metux IT consult wrote:
> On 16.06.21 19:50, Hans de Goede wrote:
> 
> Hi,
> 
>>> hmm, keyboard backlight ... don't we already have something for that
>>> in input subsys ? I believe that some lone LEDs aren't the right subsys
>>> for those stuff.
>>
>> Actually the standardized userspace API for exporting keyboard backlights
>> is using the LED class sysfs API, e.g.:
>>
>> cat /sys/class/leds/tpacpi\:\:kbd_backlight/brightnes
> 
> Sounds like we don't have an API for that particular case at all.
> Everbody just exposes LED class devices and userland always needs
> hardware specific code to practically use it.

The LED API actually has specific features which are typically
only used with kbd-backlights, such as the brightness_hw_changed
attribute which was specifically added to allow userspace to
monitor when a laptops embedded controller changes the kbd-backlight
brightness in response to a Fn + somekey hotkey keypress, so that
userspace can show an on-screen-display notification that the
kbd brightness has changed (like how it typically does for
audio volume changes too) and also showing the new brightness
level. See: Documentation/ABI/testing/sysfs-class-led for
the docs for the /sys/class/leds/<led>/brightness_hw_changed

So yes this very much is the standardized API for dealing with
kbd-backlights and has been so far years.

> We should at least have some standard mechanism for get least getting
> the connection between an input device and it's backlight device(s).
> 
>> And the same for Dell and other kbd backlights, also the upower
>> daemon even has code for dealing with kbd-backlights:
>> https://gitlab.freedesktop.org/upower/upower/-/blob/master/src/up-kbd-backlight.c
>> exporting them over its dbus API so that non-root users can
>> control them.
> 
> Looks like a very complicated way to do that. But actually I've never
> understood why I should use this strange upower thing anways :p

Just because you don't have a use for it does not mean that it
is not useful (and widely used) in cases where people use Linux
as a desktop OS, rather then for more embedded cases.

>> Basically using the LED class for kbd-backlight functionality
>> basically is the defacto standard under Linux, so exposing this
>> through the LED class is definitely the right thing to do.
> 
> In general, LED class isn't so bad, as it already gives us LED control
> (*1), but I don't see any portable way for finding the corresponding
> LED for some input device. In DRM I see the backlight as subdevice.

With USB-HID keyboards the LED class device will have the same HID-device
as parent as the input device. If there is no HID parent-device, then any
foo_kbd_backlight device will belong to the atkbd (PS/2) input-device.

Regards,

Hans