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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <35a98b67-d1eb-4aa9-9d3f-025c94cd6b0f@gmx.de>
Date: Tue, 22 Oct 2024 17:18:36 +0200
From: Armin Wolf <W_Armin@....de>
To: Pavel Machek <pavel@....cz>, Hans de Goede <hdegoede@...hat.com>
Cc: Werner Sembach <wse@...edocomputers.com>,
 Benjamin Tissoires <bentiss@...nel.org>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 dri-devel@...ts.freedesktop.org, jelle@...aa.nl, jikos@...nel.org,
 lee@...nel.org, linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-leds@...r.kernel.org, miguel.ojeda.sandonis@...il.com,
 ojeda@...nel.org, onitake@...il.com, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH 1/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO
 NB04 devices

Am 22.10.24 um 11:47 schrieb Pavel Machek:

> Hi!
>
>>> Sorry for taking a bit long to respond.
>>>
>>> This "illumination" subsystem would (from my perspective) act like some sort of LED subsystem
>>> for devices with a high count of LEDs, like some RGB keyboards.
>>>
>>> This would allow us too:
>>> - provide an abstract interface for userspace applications like OpenRGB
>>> - provide an generic LED subsystem emulation on top of the illumination device (optional)
>>> - support future RGB controllers in a generic way
>>>
>>> Advanced features like RGB effects, etc can be added later should the need arise.
>>>
>>> I would suggest that we model it after the HID LampArray interface:
>>>
>>> - interface for querying:
>>>   - number of LEDs
>>>   - supported colors, etc of those LEDs
>>>   - position of those LEDs if available
>>>   - kind (keyboard, ...)
>>>   - latency, etc
>>> - interface for setting multiple LEDs at once
>>> - interface for setting a range of LEDs at once
> How are LEDs ordered? I don't believe range makes much sense.

Range would allow for efficiently changing the color of all LEDs. But i agree
that this can be considered optional and can be added later.

Should we ever prototype such an interface, then providing a method for setting
multiple LEDs at once would be enough.

>>> I do not know if mixing sysfs (for controller attributes like number of LEDs, etc) and IOCTL
>>> (for setting/getting LED colors) is a good idea, any thoughts?
>> I wonder what the advantage of this approach is over simply using HID LampArray
>> (emulation), openRGB is already going to support HID LampArray and since Microsoft
>> is pushing this we will likely see it getting used more and more.
> There's nothing simple about "HID LampArray". Specification is long
> ang ugly... and we don't want to be stuck with with OpenRGB (links to QT!).

And HID LampArray its not easily extendable.

>
>> Using HID LampArray also has the advantage that work has landed and is landing
>> to allow safely handing over raw HID access to userspace programs or even
>> individual graphical apps with the option to revoke that access when it is
>> no longer desired for the app to have access.
> HID raw is not suitable kernel interface.

I agree, using HID raw in this case would be like amdgpu emulating a i915 GPU to
support applications working with a i915 GPU.

>> 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.
> If you don't want "some alternative API", we already have perfectly
> working API for 2D arrays of LEDs. I believe I mentioned it before
> :-). Senzrohssre.
>
> 								Pavel

We may have to support 3D arrays of LEDs, so using a simple framebuffer
would likely cause trouble.

I think of something like this:

illumination class:

sysfs attrs:

  - lamp_count
  - kind (optional)
  - width, height, length (all optional)
  - latency (optional)
  - driver-defined attributes like firmware_version, ... (optional)

ioctl interface:

  - get LED info (id, supported colors, position (optional), key code (optional), ...)
  - get current color of LEDs
  - set multiple LEDs (by ID)

This interface is similar the the HID LampArray interface except that:

  - we can read the current color
  - we can omit optional information
  - we can extend the interface later (animations, etc)

Thanks,
Armin Wolf