Re: [PATCH 1/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO NB04 devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Am 22.10.24 um 17:02 schrieb Armin Wolf:
Am 22.10.24 um 11:37 schrieb Pavel Machek:

Hi!

Personally I really like the idea to just emulate a HID LampArray device
for this instead or rolling our own API.  I believe there need to be
strong arguments to go with some alternative NIH API and I have not
heard such arguments yet.

Using a virtual HID LampArray already creates two issues:

1. We have to supply device size data (length, width, height), but the driver
cannot know this.

2. It is very difficult to extend the HID LampArray interface, for example
there is no way to read the current LED color from the hardware or switch
between different modes.

A sysfs- and/or ioctl-based interface would allow us to:

1. Threat some data as optional.

2. Extend the interface later should the need arise.

Looking at the tuxedo-drivers code, it seems that the WMI interface also reports:

- preset color
- device type (touchpad, keyboard, ...)
- keyboard type (US/UK)

Making this information available through the HID LampArray protocol would be
difficult (except for the device type information).

Agreed on everything Hans said.

I'll personnaly fight against any new "illumination" API as long as we
don't have committed users. This is the same policy the DRM folks
are
Well, and I'll personally fight against user<->kernel protocol as
crazy as HID LampArray is.

OpenRGB is not suitable hardware driver.
                                Pavel

I agree.

The point is that we need to design a userspace API since we cannot just allow
userspace to access the raw device like with HID devices.

And since we are already forced to come up with a userspace API, then maybe it would make sense to build a extendable userspace API or else we might end up in the exact
same situation later should another similar driver appear.

Since the HID LampArray is a hardware interface standard, we AFAIK cannot easily extend it.

Also i like to point out that OpenRGB seems to be willing to use this new "illumination" API as long as the underlying hardware interface is properly documented so that they can implement
support for it under Windows.

I would even volunteer to write the necessary OpenRGB backend since i already contributed to
the project in the past.

Just wanting to leave my 2 cents here: I'm in theory fine with both approaches (hidraw LampArray or wrapping it in some kind of new UAPI which at least has the LampArray feature set).

I also don't think that OpenRGB has a problem with a new Linux exclusive API as long as someone is doing the implementation work. After all the reason why OpenRGB was started is to unify all the different vendor APIs under one UI. So one more or less doesn't matter.

BUT: I already did work for the hidraw LampArray approach and OpenRGB already did work for that as well (albeit I didn't yet managed to get the draft running) and we already had a lengthy discussion about this in the last thread. (This one https://lore.kernel.org/all/20231011190017.1230898-1-wse@xxxxxxxxxxxxxxxxxxx/) with all the same arguments.

e.g. Expansion of the API: How should that look like? It would have to be basically an own extension for every keyboard manufacturer because every one supports different built in modes with different values to tweak.

So I'm siding with Hans and Benjamin on this one.

My only plan for the current patch besides some more code beautification: Move the device-sku specific values (key map, and key positions) to a bpf driver.

The question in my mind currently is: Is the patch merge ready with just that? Or must the OpenRGB implemenation also be finished before the merge?

Best regards,

Werner


Thanks,
Armin Wolf





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux