On Mar 26 2024, Werner Sembach wrote: > Hi all, > > Am 25.03.24 um 19:30 schrieb Hans de Goede: > > [snip] > > > > If the kernel already handles the custom protocol into generic HID, the > > > > work for userspace is not too hard because they can deal with a known > > > > protocol and can be cross-platform in their implementation. > > > > > > > > I'm mentioning that cross-platform because SDL used to rely on the > > > > input, LEDs, and other Linux peculiarities and eventually fell back on > > > > using hidraw only because it's way more easier that way. > > > > > > > > The other advantage of LampArray is that according to Microsoft's > > > > document, new devices are going to support it out of the box, so they'll > > > > be supported out of the box directly. > > > > > > > > Most of the time my stance is "do not add new kernel API, you'll regret > > > > it later". So in that case, given that we have a formally approved > > > > standard, I would suggest to use it, and consider it your API. > > > The only new UAPI would be the use_leds_uapi switch to turn on/off the backwards compatibility. I have my reserves with such a kill switch (see below). > > Actually we don't even need that. Typically there is a single HID > > driver handling both keys and the backlight, so userspace cannot > > just unbind the HID driver since then the keys stop working. I don't think Werner meant unbinding the HID driver, just a toggle to enable/disable the basic HID core processing of LampArray. > > > > But with a virtual LampArray HID device the only functionality > > for an in kernel HID driver would be to export a basic keyboard > > backlight control interface for simple non per key backlight control > > to integrate nicely with e.g. GNOME's backlight control. > > Don't forget that in the future there will be devices that natively support > LampArray in their firmware, so for them it is the same device. Yeah, the generic LampArray support will not be able to differentiate "emulated" devices from native ones. > > Regards, > > Werner > > > And then when OpenRGB wants to take over it can just unbind the HID > > driver from the HID device using existing mechanisms for that. Again no, it'll be too unpredicted. > > > > Hmm, I wonder if that will not also kill hidraw support though ... > > I guess getting hidraw support back might require then also manually > > binding the default HID input driver. Bentiss any input on this? To be able to talk over hidraw you need a driver to be bound, yes. But I had the impression that LampArray would be supported by default in hid-input.c, thus making this hard to remove. Having a separate driver will work, but as soon as the LampArray device will also export a multitouch touchpad, we are screwed and will have to make a choice between LampArray and touch... > > > > Background info: as discussed earlier in the thread Werner would like > > to have a basic driver registering a /sys/class/leds/foo::kbd_backlight/ > > device, since those are automatically supported by GNOME (and others) > > and will give basic kbd backlight brightness control in the desktop > > environment. This could be a simple HID driver for > > the hid_allocate_device()-ed virtual HID device, but userspace needs > > to be able to move that out of the way when it wants to take over > > full control of the per key lighting. Do we really need to entirely unregister the led class device? Can't we snoop on the commands and get some "mean value"? > > > > Regards, > > > > Hans > > > > > > > > > > > > > > > > > The control flow for the whole system would look something like this: > > > > > > - System boots > > > > > > - Kernel driver initializes keyboard (maybe stops rainbowpuke boot effects, sets brightness to a default value, or initializes a solid color) > > > > > > - systemd-backlight restores last keyboard backlight brightness > > > > > > - UPower sees sysfs leds entry and exposes it to DBus for DEs to do keyboard brightness handling > > > > > > - If the user wants more control they (auto-)start OpenRGB > > > > > > - OpenRGB disables sysfs leds entry via use_leds_uapi to prevent double control of the same device by UPower > > > > > > - OpenRGB directly interacts with hidraw device via LampArray API to give fine granular control of the backlight > > > > > > - When OpenRGB closes it should reenable the sysfs leds entry That's where your plan falls short: if OpenRGB crashes, or is killed it will not reset that bit. Next question: is OpenRGB supposed to keep the hidraw node opened all the time or not? If it has to keep it open, we should be able to come up with a somewhat similar hack that we have with hid-steam: when the hidraw node is opened, we disable the kernel processing of LampArray. When the node is closed, we re-enable it. But that also means we have to distinguish steam/SDL from OpenRGB... I just carefully read the LampArray spec. And it's simpler than what I expected. But the thing that caught my attention was that it's mentioned that there is no way for the host to query the current color/illumination of the LEDs when the mode is set to AutonomousMode=false. Which means that the kernel should be able to snoop into any commands sent from OpenRGB to the device, compute a mean value and update its internal state. Basically all we need is the "toggle" to put the led class in read-only mode while OpenRGB kicks in. Maybe given that we are about to snoop on the commands sent, we can detect that there is a LampArray command emitted, attach this information to the struct file * in hidraw, and then re-enable rw when that user closes the hidraw node. Cheers, Benjamin > > > > > > - System shutdown > > > > > > - Since OpenRGB reenables the sysfs leds entry, systemd-backlight can correctly store a brightness value for next boot > > > > > > Regards, > > > > > > Werner > > > > > > > Side note to self: I really need to resurrect the hidraw revoke series > > > > so we could export those hidraw node to userspace with uaccess through > > > > logind... > > > > > > > > Cheers, > > > > Benjamin
