Message-ID: <1fb08a74-62c7-4d0c-ba5d-648e23082dcb@tuxedocomputers.com>
Date: Wed, 20 Mar 2024 12:16:40 +0100
From: Werner Sembach <wse@...edocomputers.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Lee Jones <lee@...nel.org>, jikos@...nel.org,
 linux-kernel@...r.kernel.org, Jelle van der Waa <jelle@...aa.nl>,
 Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 linux-input@...r.kernel.org, ojeda@...nel.org, linux-leds@...r.kernel.org,
 Pavel Machek <pavel@....cz>, Gregor Riepl <onitake@...il.com>
Subject: Re: Future handling of complex RGB devices on Linux v3

Hi Hans and the others,

Am 22.02.24 um 14:14 schrieb Werner Sembach:
> Hi,
>
> Thanks everyone for the exhaustive feedback. And at least this thread is a 
> good comprehesive reference for the future ^^.
>
> To recap the hopefully final UAPI for complex RGB lighting devices:
>
> - By default there is a singular /sys/class/leds/* entry that treats the 
> device as if it was a single zone RGB keyboard backlight with no special effects.
>
> - There is an accompanying misc device with the sysfs attributes "name", 
> "device_type",  "firmware_version_string", "serial_number" for device 
> identification and "use_leds_uapi" that defaults to 1.
>
>     - If set to 0 the /sys/class/leds/* entry disappears. The driver should 
> keep the last state the backlight was in active if possible.
>
>     - If set 1 it appears again. The driver should bring it back to a static 1 
> zone setting while avoiding flicker if possible.
>
> - If the device is not controllable by for example hidraw, the misc device 
> might also implement additional ioctls or sysfs attributes to allow a more 
> complex low level control for the keyboard backlight. This is will be a highly 
> vendor specific UAPI.
So in the OpenRGB issue thread 
https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/dynamic-lighting-devices 
aka HID LampArray was mentioned. I did dismiss it because I thought that is only 
relevant for firmware, but I now stumbled upon the Virtual HID Framework (VHF) 
https://learn.microsoft.com/en-us/windows-hardware/drivers/hid/virtual-hid-framework--vhf- 
and now I wonder if an equivalent exists for Linux? A quick search did not yield 
any results for me.

If a virtual HID device is possible and the WMI interface can reasonably be 
mapped to the LampArray API this might be the best starting point:

- Implement a Virtual HID device with LampArray

- Implement LampArray in OpenRGB

- (Optional) Implement a generic LampArray leds subsystem driver that maps to 
the single zone control and ads the use_leds_uapi sysfs switch to the virtual 
HID device

- (Optional) Implement vendor specific controls for 
AutonomousMode/built-in-firmware-effects via custom HID commands

- (Optional) Implement Virtual HID devices for actual HID devices that don't 
support LampArray in firmware (Open question: How to prevent userspace/OpenRGB 
from interacting with original HID when the virtual HID device is not in 
AutonomousMode? How to associate the original and virtual HID device to each 
other that userspace can easily recognize this relation? Or is it possible to 
add virtual HID commands on top of a real HID device, making it look exactly 
like the pure virtual devices for userspace?)

The LampArray API hereby is made with the intention to be used for multi leds 
devices, like per-key-backlight keyboards, unlike the leds UAPI. And it is 
coming anyway with new RGB devices soon. So it would not conflict with a "don't 
introduce unnecessary UAPI interfaces" principle. Are there any plans already of 
Wrapping LampArray in some kind ioctl/sysfs API? Or just have it used via 
hidraw? Or was there no discussion about it till now?

Regards,

Werner

>
>     - The actual logic interacting with this low level UAPI is implemented by 
> a userspace driver
>
> Implementation wise: For the creation of the misc device with the 
> use_leds_uapi switch a helper function/macro might be useful? Wonder if it 
> should go into leds.h, led-class-multicolor.h, or a new header file?
>
> - Out of my head it would look something like this:
>
> led_classdev_add_optional_misc_control(
>     struct led_classdev *led_cdev,
>     char* name,
>     char* device_type,
>     char* firmware_version_string,
>     char* serial_number,
>     void (*deregister_led)(struct led_classdev *led_cdev),
>     void (*reregister_led)(struct led_classdev *led_cdev))
>
> Let me know your thoughts and hopefully I can start implementing it soon for 
> one of our devices.
>
> Kind regards,
>
> Werner Sembach
>

Powered by blists - more mailing lists