Message-ID: <6b32fb73-0544-4a68-95ba-e82406a4b188@gmx.de>
Date: Wed, 17 Jan 2024 20:03:57 +0100
From: Armin Wolf <W_Armin@....de>
To: Hans de Goede <hdegoede@...hat.com>,
 Werner Sembach <wse@...edocomputers.com>, Pavel Machek <pavel@....cz>,
 Jani Nikula <jani.nikula@...ux.intel.com>, jikos@...nel.org,
 Jelle van der Waa <jelle@...aa.nl>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>, Lee Jones
 <lee@...nel.org>, linux-kernel@...r.kernel.org,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 linux-input@...r.kernel.org, ojeda@...nel.org, linux-leds@...r.kernel.org
Subject: Re: Userspace API for per key backlight for non HID (no hidraw)
 keyboards

Am 17.01.24 um 17:50 schrieb Hans de Goede:

> Hi All,
>
> On 11/27/23 11:59, Werner Sembach wrote:
>
> <snip>
>
>> I also stumbled across a new Problem:
>>
>> We have an upcoming device that has a per-key keyboard backlight, but does the control completely via a wmi/acpi interface. So no usable hidraw here for a potential userspace driver implementation ...
>>
>> So a quick summary for the ideas floating in this thread so far:
>>
>> 1. Expand leds interface allowing arbitrary modes with semi arbitrary optional attributes:
>>
>>      - Pro:
>>
>>          - Still offers all default attributes for use with UPower
>>
>>          - Fairly simple to implement from the preexisting codebase
>>
>>          - Could be implemented for all (to me) known internal keyboard backlights
>>
>>      - Con:
>>
>>          - Violates the simplicity paradigm of the leds interface (e.g. with this one leds entry controls possible multiple leds)
> So what you are suggesting here is having some way (a-z + other sysfs attr?)
> to use a single LED class device and then extend that to allow setting all
> keys ?
>
> This does not seem like a good idea to me and this will also cause issues
> when doing animations in software, since this API will likely be slow.
>
> And if the API is not slow, then it will likely involve some sort
> of binary sysfs file for setting multiple keys rather then 1
> file per key which would break the normal 1 file per setting sysfs
> paradigm.
>
>> 2. Implement per-key keyboards as auxdisplay
>>
>>      - Pro:
>>
>>          - Already has a concept for led positions
> With a "concept" you mean simple x,y positioning or is
> there something more advanced here that I'm aware of ?
>
>>          - Is conceptually closer to "multiple leds forming a singular entity"
>>
>>      - Con:
>>
>>          - No preexisting UPower support
>>
>>          - No concept for special hardware lightning modes
>>
>>          - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
> Hmm, so there is very little documentation on this and what
> docs there is: Documentation/admin-guide/auxdisplay/cfag12864b.rst
> as well as the example program how to uses this suggests that
> this is using the old /dev/fb# interface which we are sorta
> trying to retire.
>
>
>> 3. Implement in input subsystem
>>
>>      - Pro:
>>
>>          - Preexisting concept for keys and key purpose
>>
>>      - Con:
>>
>>          - Not in scope for subsystem
>>
>>          - No other preexisting light infrastructure
> Dmitry actually recently nacked the addition of
> a LED_MIC_MUTE define to include/uapi/linux/input-event-codes.h
> which was intended to be able to allow the input LED support
> with standard HID mic-mute leds (spk-mute is already supported
> this way).
>
> Dmitry was very clear that no new LEDs must be added and
> that any new LED support should be done through the LED
> subsytem, so I do not think that something like this
> is going to fly.
>
>
>> 4. Implement a simple leds driver only supporting a small subset of the capabilities and make it disable-able for a userspace driver to take over
>>
>>      - Pro:
>>
>>          - Most simple to implement basic support
>>
>>          - In scope for led subsystem simplicity paradigm
>>
>>      - Con:
>>
>>          - Not all built in keyboard backlights can be implemented in a userspace only driver
> Right, so this is basically what we have been discussing in the other
> part of the thread with the:
>
> /sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
>
> proposal to unregister the kernel's LED class device and then
> allow userspace to do whatever it wants through /dev/hidraw
> without the kernel also trying to access the backlight
> functionality at the same time.
>
> AFAIK there already is a bunch of userspace support for
> per key addressable kbd RGB backlights using hidraw support,
> so this way we can use the momentum / code of these existing
> projects, at least for existing hidraw keyboards and adding
> support for:
>
> /sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
>
> to these existing projects should be simple.
>
> Yet this will not work for your mentioned "control completely
> via a wmi/acpi interface". Still I think we should go the same
> route for those adding a misc-char device or something like
> that to allow making WMI calls from userspace (like Windows
> can do). Maybe with an allow list per GUID to only allow
> specific calls, so that we can avoid possible dangerous calls.
>
> Armin Wolf recently became the WMI bus maintainer.
>
> Armin, we are discussing how to deal with (laptop) keyboards
> which have a separate RGB LED per key and how to control
> those LEDs.
>
> So far per key addressable keyboard backlighting has always
> been HID based, so any existing support is just userspace
> based using /dev/hidraw. In my experience the problem with
> supporting gaming peripherals is that there is interest in it,
> but not really enough interest to keep a sustained momentum
> behind projects, especially not when it comes to taking code
> from a fun weekend hack to upstreaming them into bigger
> projects like the kernel.
>
> So I would like to offer some sort of easy accessible
> API to userspace for accessing this, basically allowing
> userspace drivers for the LED part of the keyboard which
> in some cases will involve making WMI calls from userspace.
>
> So, Armin, what do you think about a way of allowing
> (filtered) WMI calls from userspace through say
> a misc-char device + ioctls or something like that?
>
> Werner atm I personally do think that option 4. from
> your list is the way to go. Mainly because designing
> a generic kernel API for all bells and whistles of gaming
> hw is very tricky and would require a large ongoing
> effort which I just don't see happening (based on
> past experience).
>
> Regards,
>
> Hans
>
Hi,

i can understand your concerns, but i strongly advise against a generic WMI userspace API.
The reasons for this are:

1. We are still unable to parse (and use) the binary MOF buffers describing the WMI devices,
so all of that would require the driver parsing a raw byte buffer. In this case a separate
misc device managed by the driver would basically do the same.

2. Many WMI implementations are like RWEverything implemented inside the ACPI firmware, so
most devices will require the driver to do excessive filtering. Many implementations also do
no proper input validation either so the driver has to know all possible use cases since he
has to protect the buggy ACPI firmware from userspace attacks.

Regarding point number 2, i just had to contact Asus so that they remove a broken WMI interface
from my motherboard or else a simple application could crash the Windows kernel. This firmware
is (sadly) being designed as an internal API and thus unstable beyond all imagination.

For HID devices, a userspace driver might be OK since they are somewhat isolated from the remaining
hardware, but WMI is basically a kernel bypass for ACPI firmware calls, so userspace could easily
attack the kernel is way.

Personally, i would prefer extending the LED subsystem to support zone-like devices with many LEDs,
as this would prevent userspace from having to tinker with the hardware behind the kernels back.
Other highly device-specific features could be implemented with a driver-specific misc device.

Regarding the speed, it depends on the underlying WMI interface design if smooth animations are
even possible, since many WMI interfaces are quite slow. Can you share the Binary MOF buffers
describing the WMI interfaces in question?

Thanks,
Armin Wolf

Powered by blists - more mailing lists