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

Started by Enrico Weigelt, metux IT consult on Jun 16, 2021 · 6 messages from 2 authors · Last reply Jun 22, 2021

open the first message on its own page

Message details and related content
On 16.06.21 17:56, leo60228 wrote:
quoted
Can you please tell a bit more what these LEDs are actually used for ?
Do they have some names or symbols ? Are they also controlled by
something else (e.g. numlock or shiftlock leds)
They're used for the keyboard backlight. This functionality is pretty common on gaming laptops.
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.

CC'ing linux-input list: guys, could you have a look at this ? is input
sysbsys already capable of handling this ?



--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
Message details and related content
Hi,

On 6/16/21 7:20 PM, Enrico Weigelt, metux IT consult wrote:
On 16.06.21 17:56, leo60228 wrote:
quoted
quoted
Can you please tell a bit more what these LEDs are actually used for ?
Do they have some names or symbols ? Are they also controlled by
something else (e.g. numlock or shiftlock leds)
They're used for the keyboard backlight. This functionality is pretty common on gaming laptops.
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

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.

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.

Since these are zones however, we probably wat to avoid the
kbd_backlight suffix of the name, otherwise upower will
pick the first device it enumerates and control that, while
leaving the other zones alone, which is not what we want.

Regards,

Hans
Message details and related content
On 16.06.21 19:50, Hans de Goede wrote:

Hi,
quoted
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.

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
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.



--mtx


*1) just recognized that on my toshiba portege (TOS6208) it only works
    for readout - writing to "brightness" does nothing at all
-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
Message details and related content
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,
quoted
quoted
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).
quoted
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.
quoted
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

input devices vs. keyboard backlights [WAS: [PATCH v2] platform/x86: add support for Acer Predator LEDs]

Message details and related content
On 21.06.21 21:43, Hans de Goede wrote:

Hi,
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
yes, that's great. but it seems we're still lacking a direct standard
way for associating an input device with the corresponding backlight
LED.

Or am I missing something ?
quoted
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.
I'm actually using Linux on desktop for over 25 years now.

But let's go back to the kernel side.
quoted
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.
Still a lot of if's and guessing :(

Why can't we make it appear the same way like the other leds (eg. caps-
lock) ?

Here's how it looks on my Portege:

| ~ ls -l /dev/input/input0:
|
| drwxr-xr-x 2 root root    0 Jun 22 11:42 capabilities
| lrwxrwxrwx 1 root root    0 Jun 22 11:42 device -> ../../../serio0
| drwxr-xr-x 3 root root    0 Jun 22 11:42 event0
| drwxr-xr-x 2 root root    0 Jun 22 11:42 id
| drwxr-xr-x 3 root root    0 Jun 22 11:37 input0::capslock
| drwxr-xr-x 3 root root    0 Jun 22 11:42 input0::numlock
| drwxr-xr-x 3 root root    0 Jun 22 11:42 input0::scrolllock
| -r--r--r-- 1 root root 4096 Jun 22 11:42 modalias
| -r--r--r-- 1 root root 4096 Jun 22 11:42 name
| -r--r--r-- 1 root root 4096 Jun 22 11:42 phys
| drwxr-xr-x 2 root root    0 Jun 22 11:42 power
| -r--r--r-- 1 root root 4096 Jun 22 11:42 properties
| lrwxrwxrwx 1 root root    0 Jun 22 11:42 subsystem ->
../../../../../../class/input
| -rw-r--r-- 1 root root 4096 Jun 22 11:42 uevent
| -r--r--r-- 1 root root 4096 Jun 22 11:42 uniq

I'd imagine also having some "input0::backlight" here.

These leds seem to be sub-devices of the keyboard device, that's perhaps
tricky to do with the current backlight drivers - but maybe just add
some symlink to it ?


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren

5 more lines

Re: input devices vs. keyboard backlights [WAS: [PATCH v2] platform/x86: add support for Acer Predator LEDs]

Message details and related content
Hi,

On 6/22/21 12:19 PM, Enrico Weigelt, metux IT consult wrote:
On 21.06.21 21:43, Hans de Goede wrote:

Hi,
quoted
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
yes, that's great. but it seems we're still lacking a direct standard
way for associating an input device with the corresponding backlight
LED.
True.
Or am I missing something ?
In most cases the lacking way to associate the 2 is not a problem
because almost all systems have only a single keyboard backlight
and all existing standard Linux userspace code around kbd-backlights
(like upower) has been written with this assumption.

This is actually a problem when you plug in a fancy USB kbd with
backlight into a laptop where the laptop kbd also has a backlight ...

Fixing this would be great, but it requires fixes all over
the stack and associating the backlight with a specific kbd
is probably the smallest of the issues to solve here.
quoted
quoted
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.
I'm actually using Linux on desktop for over 25 years now.

But let's go back to the kernel side.
quoted
quoted
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.
Still a lot of if's and guessing :(

Why can't we make it appear the same way like the other leds (eg. caps-
lock) ?
Here's how it looks on my Portege:

| ~ ls -l /dev/input/input0:
So you have an input0 symlink under /dev/input to /sys/class/input/input0/
that is non standard, but that is not really relevant you get the same
output if you directly do:

ls -l /sys/class/input/input0/
|
| drwxr-xr-x 2 root root    0 Jun 22 11:42 capabilities
| lrwxrwxrwx 1 root root    0 Jun 22 11:42 device -> ../../../serio0
| drwxr-xr-x 3 root root    0 Jun 22 11:42 event0
| drwxr-xr-x 2 root root    0 Jun 22 11:42 id
| drwxr-xr-x 3 root root    0 Jun 22 11:37 input0::capslock
| drwxr-xr-x 3 root root    0 Jun 22 11:42 input0::numlock
| drwxr-xr-x 3 root root    0 Jun 22 11:42 input0::scrolllock
| -r--r--r-- 1 root root 4096 Jun 22 11:42 modalias
| -r--r--r-- 1 root root 4096 Jun 22 11:42 name
| -r--r--r-- 1 root root 4096 Jun 22 11:42 phys
| drwxr-xr-x 2 root root    0 Jun 22 11:42 power
| -r--r--r-- 1 root root 4096 Jun 22 11:42 properties
| lrwxrwxrwx 1 root root    0 Jun 22 11:42 subsystem ->
../../../../../../class/input
| -rw-r--r-- 1 root root 4096 Jun 22 11:42 uevent
| -r--r--r-- 1 root root 4096 Jun 22 11:42 uniq

I'd imagine also having some "input0::backlight" here.

These leds seem to be sub-devices of the keyboard device,
They are child devices of the input-device instantiated for the
keyboard device, the keyboard device itself is likely actually:

/sys/devices/platform/i8042/serio0

If you do "ls -l /sys/class/input/input0" then you will see
that this is a symlink pointing to

/sys/devices/platform/i8042/serio0/input/input0

The LED class devices for the kbd LEDs are instantiated
by the input-subsys itself, hence they use the input-device
as parent.

For HID based keyboards the HID device which is the parent
of the input-device will also be the parent of the kbd-backlight
LED class device since both are instantiated by the HID driver.

E.g. if I plug in a Logitech G510 keyboard I get this:

[hans@x1 ~]$ ls -l /sys/bus/hid/devices/0003:046D:C22D.000D/{leds,input}/
'/sys/bus/hid/devices/0003:046D:C22D.000D/input/':
total 0
drwxr-xr-x. 6 root root 0 Jun 22 12:32 input53
drwxr-xr-x. 6 root root 0 Jun 22 12:32 input54

'/sys/bus/hid/devices/0003:046D:C22D.000D/leds/':
total 0
drwxr-xr-x. 3 root root 0 Jun 22 12:32 g15::kbd_backlight
drwxr-xr-x. 3 root root 0 Jun 22 12:32 g15::macro_preset1
drwxr-xr-x. 3 root root 0 Jun 22 12:32 g15::macro_preset2
drwxr-xr-x. 3 root root 0 Jun 22 12:32 g15::macro_preset3
drwxr-xr-x. 3 root root 0 Jun 22 12:32 g15::macro_record
drwxr-xr-x. 3 root root 0 Jun 22 12:32 g15::power_on_backlight_val

So the input and LED class devices can already be associated
with each other by finding a common parent. This is not
unusual, this is e.g. also how video4linux tv-apps like
xawtv and tvtime find the alsa record/input device to get

35 more lines

Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help