Re: [PATCH v4] HID: generic: add LampArray support via hid-lamparray helper

[Werner Sembach]

Hi,

Am 27.08.26 um 20:47 schrieb Cristian Mazzotta:
> Am 25.03.26 um 11:35 schrieb Werner Sembach:
> > as Tim went on to other endeavors, I'm taking over this patch. I have
> > yet to test it myself, but i was already in the feedback loop while he
> > was writing it, so I'm not unfamiliar with the code.
taking Aaron into the loop who actually ended up taking over the development.
> Hi Werner,
>
> I recently got a new laptop (Acer Predator PT14-52T) with absolutely zero
> support for any RGB outside of Windows 11; while studying the behavior of
> the Windows driver, I found out about LampArray devices and so I ended up
> landing here.
>
> This laptop has 2 LampArray devices: a USB attached keyboard (05AF:767A)
> and an I2C ENE controller (0CF2:5130) for 3 zones in total. The original
> motivation was power draw during s2idle; the lights would stay on and
> constantly draw power. I measured 12.35W drop to 2.84W when I
> blanked the lights and measured with a simple usb power meter, so I've
> been working off of Tim's code for the past little bit.

The plan for the in kernel driver was to intentionally only provide basic 
support. Reason is the current UAPI of the leds subsystem does not offer the 
full flexibility lamp array offers, so it's a challange to map that already. 
Also most current DEs also only support very basic controls interacting with the 
leds subsystem.

If more control is needed a userspace driver should disable the leds subsystem 
control and directly control the device via hidraw. One such driver will 
probably be openRGB in the near future, but it only has initial, currently 
broken support for lamp array.

> Getting full support required touching most of the driver:
>
>   - Multi-zone support with individual zone states instead of a single
>     monolithic zone taking every device.
The driver tries to represent every device as one leds device with one zone to 
not confuse the DEs
>   - lamparray_read_lamp_count() used hid_hw_request(), which is
>     asynchronous, so the value was read before the transfer completed.
>     Switched to hid_hw_raw_request()
Probably something Aaron already fixed on his dev branch, but thanks for 
pointing it out.
>   - lamparray_parse_update_report() would bind to the last HID_LAIP_LAMP_COUNT
>     (usage 0x0003) parsed rather than the first, so LampCount ended up
>     being on the wrong field. This has been fixed by binding the first match.
Not familiar enough with the code myself to comment on that, but maybe a hint 
for Aaron
>   - Added suspend and resume hooks, including an autonomous mode refresh
>     on S4 that my laptop needed
Same as above
>   - use_leds_uapi=0 now leaves the LED class devs registered, but ignores
>     all writes rather than unregistering them.
Unregistering is important for userspace to know what happens, otherwise kde for 
example would still offer brightness control while it isn't actually doing 
anything.
>   - Also added ABI documentation since it was missing previously
Thanks, I think Aaron did something there too.
> About half of the driver changed, so I don't think it would be easily
> reviewable as a delta from v4.
>
> Would you prefer that I post it as a v5 with Tim's patch as a base, or
> should I just send you the changes that you can add to your own version?
> I don't want to duplicate work if you already have a v5 in progress.
>
> I have hardware in front of me so I can test whatever is useful.

I think it's best for Aaron sending his version first and then you reacting on 
that on the LKML and getting in touch. It's unfortunate that parallel work 
happened here.

Also you might wanna take a look at the openRGB userspace implementation for 
LampArray control. It still needs a lot of work. I too dabbled in there but 
still not quite enough.

Best regards,

Werner

> Thank you,
> Cristian Mazzotta