Add a new hid-lamparray helper module and integrate it with the hid-generic
driver.

While more complex lamparray handling should be done in userspace via hidraw,
providing a small module to add basic lamparray support makes it possible for
userspace software to interact with lamparrays by simply using well-known APIs
of the LED subsystem. One use-case would be to enable desktop environments to
support keyboard backlight control out of the box for HID lamparray devices
without having to implement the whole HID protocol themselves.

This patch is based on previous discussions:
https://lore.kernel.org/all/1fb08a74-62c7-4d0c-ba5d-648e23082dcb@tuxedocomputers.com/

The helper provides basic support for devices exposing a Lighting/LampArray
application collection (usage page 0x59) and registers a single-zone RGB LED
representation via the LED subsystem.

hid-generic now checks for LampArray support after hid_parse() and optionally
registers a lamparray instance. Failures in the helper do not abort device probe
to keep the driver logic otherwise unchanged.

LampArray resources are released on driver remove.

This commit was successfully tested on the Microsoft MacroPad reference
implementation (https://github.com/microsoft/RP2040MacropadHidSample 1d6c3ad)
and in combination with the tuxedo_nb04_wmi driver, albeit only fully functional
with a recent fix posted to the LKML
(https://lore.kernel.org/all/20260826081149.235487-2-aer@tuxedocomputers.com).

Others reported the driver to work on the Acer Predator PT14-52T and the ASUS
Prime B650-Plus, with the former requiring some patches on top of v5, which
should now be included in v6.

v6:
- Read attribute reports synchronously (by Cristian Mazotta)
- Raise log level of fatal probe errors (by Cristian Mazotta)
- Transfer control when use_leds_uapi changes (by Cristian Mazotta)
- Blank lamps across suspend and restore on resume (by Cristian Mazotta)
- Add quirk for devices that ignore intensities (reported by Armin Wolf)
- Better locking, fixing rare races in uapi registration
- Use scope guards for locking and allocations where possible
- Remove obsolete calls to led_mc_calc_color_components and
  hid_device_io_start/stop
- Updated and slightly improved documentation
v5:
- Proper hardware detection (no quirks necessary anymore)
- Add documentation for new sysfs knob
- Pass limits of the device to sysfs (intensities & brightness)
- More flexible Kconfig (use tristate)
- Improved locking
- Several memory leak and (de-)initialization fixes
- Don't read current color values from hardware (the HID spec does not
  offer this option)
- Remove redundant report dump functionality
v4:
- Restrict CONFIG_HID_LAMPARRAY to built-in configurations only to fix
  additional randconfig build errors
v3:
- Squash V1 and V2 into one patch
v2:
- Fix Kconfig to avoid build errors when LEDS_CLASS_MULTICOLOR is
  disabled

Aaron Erhardt (2):
  HID: lamparray: add new LampArray helper module
  HID: generic: add LampArray support via hid-lamparray helper

Cristian Mazzotta (1):
  HID: lamparray: blank lamps across suspend and restore on resume

 .../ABI/testing/sysfs-driver-hid-lamparray    |  15 +
 drivers/hid/Kconfig                           |  18 +
 drivers/hid/Makefile                          |   2 +
 drivers/hid/hid-generic.c                     |  70 +-
 drivers/hid/hid-lamparray.c                   | 907 ++++++++++++++++++
 include/linux/hid-lamparray.h                 | 117 +++
 6 files changed, 1128 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-lamparray
 create mode 100644 drivers/hid/hid-lamparray.c
 create mode 100644 include/linux/hid-lamparray.h

-- 
2.43.0

On 16.09.26 16:48, Aaron Erhardt wrote:
> Add a new hid-lamparray helper module and integrate it with the hid-generic
> driver.

Thanks for picking these up!

I do have two corrections on patch 3 however, and both were mine
originally, so my bad:

 - The measurement in the commit message and in the comment above
   lamparray_suspend() is wrong. It should be 12.35 W with lamps lit and
   3.14 W blanked, which is 9.21 W or about 75% of s2idle draw, not
   2.84 W / 77%. The 2.84 W was an earlier test on a local branch that
   includes full multi-zone support. It's worth noting in the comment that
   the 3.14 W still includes the lid zone, which single-zone control
   cannot reach on this device.

 - The kerneldoc for lamparray_suspend() still says it "writes zeroes to
   the rgb values only, keeping the brightness", but the call is now
   lamparray_hw_set_state(ldev, 0, 0, 0, 0). The code is fine, but the doc
   should follow it.

On your question about the default state: I think the lights "look like
they don't work" can be a real conclusion, but the cause is the intensity
default rather than the brightness default.

With led_init_state NULL, last_r/g/b stay zero, register_led copies
them into subleds[].intensity, and brightness_set reads r/g/b back out
of subled_info[].intensity. So every brightness write sends
(0, 0, 0, brightness), which is black at any brightness. The
zero-brightness quirk does not change this since it only forces RGB to
zero when brightness is already zero.

The LED class device is therefore inert rather than just dark;
systemd-backlight restoring a saved brightness, or a DE slider, or
UPower, all write brightness and see nothing happen. This is because
nothing in that stack writes multi_intensity first.

Defaulting to autonomous mode would not fix that. It would replace an
inert knob with an ignored one: the cached RGB and brightness would
describe nothing the hardware is doing, and a DE would have to
discover and write use_leds_uapi to make the node real. That is
driver-specific knowledge that desktop environments are unlikely to
carry.

On a second look, I would suggest keeping brightness at LED_OFF, but
defaulting the intensities to max_r/max_g/max_b. The device is still
dark at probe, autonomous mode is still disabled so the cached state
matches the hardware, and the first brightness write from existing
userspace lights it up. Combined with the quirk you added for Armin's
device, it stays dark even on firmware that ignores the intensity channel.

One hardware data point, since discoverability came up: on the Acer
Predator PT14-52T the keyboard brightness keys are handled entirely in
the EC and never reach the LED class device, so they cannot be relied
on to show the user that anything is controllable.

On multi-collection, I don't see myself getting to it soon. I might work
on it during fall or winter break of my university, however I cannot
guarantee it, so please do not consider it blocked on me. The ENE
controller exposes two LampArrayAttributesReport collections and
lamparray_parse_update_report() only keeps the last.

I have a local branch which includes working multi-collection support.
It's based off of Tim's v4, and it's what I referred to in the original
v4 thread; I'd be happy to send over my branch as a reference, though
porting it to v6 is not trivial.

Am 16.09.26 um 18:15 schrieb Cristian Mazzotta:
> On 16.09.26 16:48, Aaron Erhardt wrote:
>> Add a new hid-lamparray helper module and integrate it with the hid-generic
>> driver.
> 
> Thanks for picking these up!
> 
> I do have two corrections on patch 3 however, and both were mine
> originally, so my bad:
> 
>  - The measurement in the commit message and in the comment above
>    lamparray_suspend() is wrong. It should be 12.35 W with lamps lit and
>    3.14 W blanked, which is 9.21 W or about 75% of s2idle draw, not
>    2.84 W / 77%. The 2.84 W was an earlier test on a local branch that
>    includes full multi-zone support. It's worth noting in the comment that
>    the 3.14 W still includes the lid zone, which single-zone control
>    cannot reach on this device.
> 
>  - The kerneldoc for lamparray_suspend() still says it "writes zeroes to
>    the rgb values only, keeping the brightness", but the call is now
>    lamparray_hw_set_state(ldev, 0, 0, 0, 0). The code is fine, but the doc
>    should follow it.

Ack. Will be fixed in v7.

> 
> On your question about the default state: I think the lights "look like
> they don't work" can be a real conclusion, but the cause is the intensity
> default rather than the brightness default.
> 
> With led_init_state NULL, last_r/g/b stay zero, register_led copies
> them into subleds[].intensity, and brightness_set reads r/g/b back out
> of subled_info[].intensity. So every brightness write sends
> (0, 0, 0, brightness), which is black at any brightness. The
> zero-brightness quirk does not change this since it only forces RGB to
> zero when brightness is already zero.
> 
> The LED class device is therefore inert rather than just dark;
> systemd-backlight restoring a saved brightness, or a DE slider, or
> UPower, all write brightness and see nothing happen. This is because
> nothing in that stack writes multi_intensity first.
> 
> Defaulting to autonomous mode would not fix that. It would replace an
> inert knob with an ignored one: the cached RGB and brightness would
> describe nothing the hardware is doing, and a DE would have to
> discover and write use_leds_uapi to make the node real. That is
> driver-specific knowledge that desktop environments are unlikely to
> carry.
> 
> On a second look, I would suggest keeping brightness at LED_OFF, but
> defaulting the intensities to max_r/max_g/max_b. The device is still
> dark at probe, autonomous mode is still disabled so the cached state
> matches the hardware, and the first brightness write from existing
> userspace lights it up. Combined with the quirk you added for Armin's
> device, it stays dark even on firmware that ignores the intensity channel.

I think this is a good idea. In general, the default should probably
either set intensities to max_(r|g|b) or brightness to max_brightness.
This would reduce the amount of writes necessary to see a result and I
agree that probably the brightness knob is most likely to be used by
userspace. Still, I'd like to have some more feedback, especially from
the maintainers before integrating this behavior in v7.

> 
> One hardware data point, since discoverability came up: on the Acer
> Predator PT14-52T the keyboard brightness keys are handled entirely in
> the EC and never reach the LED class device, so they cannot be relied
> on to show the user that anything is controllable.
> 
> On multi-collection, I don't see myself getting to it soon. I might work
> on it during fall or winter break of my university, however I cannot
> guarantee it, so please do not consider it blocked on me. The ENE
> controller exposes two LampArrayAttributesReport collections and
> lamparray_parse_update_report() only keeps the last.
> 
> I have a local branch which includes working multi-collection support.
> It's based off of Tim's v4, and it's what I referred to in the original
> v4 thread; I'd be happy to send over my branch as a reference, though
> porting it to v6 is not trivial.

I could have a look at your branch, but I think it's better to wait for
this series to be merged before exploring more advanced features. I
can't guarantee that I will work on that actively, especially since I
don't have access to such hardware, but I'm curious to see what changes
you have applied to make it work with v4.