Re: Clarification questions regarding multicolor leds

[Werner Sembach <wse@tuxedocomputers.com>]

Hi,

thanks for the answers, comments below.

Am 21.08.22 um 20:18 schrieb Pavel Machek:
> Hi!
>> I'm currently implementing the multicolors sysfs leds interface for several
>> Clevo and Tongfang barebones, but I'm unsure how to actually map the leds to
>> the multicolor interface:
>>
>> The keyboards come in 5 variants:
>>
>> Single zone RGB + Brightness
>>
>> Single Zone RGB
>>
>> 3 Zone RGB + Shared Brightness
>>
>> Per Key RGB
>>
>> Per Key RGB + Shared Brightness
>>
>> First question: How do I map multiple zones or per-key leds?
>>
>> Should I register a seperate ::kbd_backlight for zone/key? resulting in
>> ::kbd_backlight, ::kbd_backlight_1, ::kbd_backlight_2, ::kbd_backlight_3,
>> etc?
> For a zone, yes.
Ok
>
>> Should I give them more desciptive names like ::kbd_backlight_left,
>> ::kbd_backlight_center, ::kbd_backlight_right, ::kbd_backlight_a,
>> ::kbd_backlight_b, ::kbd_backlight_enter?
>>
>> Or Should I only create a single ::kbd_backlight instance and map the
>> different zones to subleds? So there are number of zones * 3 subleds, with
>> each tripplet controlling the rgb values of one zone/key? This would help
>> performance, as for the per-key backlight, the firmware in the backend wants
>> an array for all keys at once. So setting each key seperatly would mean
>> sending the whole array for each key individually. And I think what most
>> people want to do is to set the whole keyboard at once anyway and nit key by
>> key.
> Not sure what to do there. And not sure if LED subsystem is suitable
> for this, actually. This starts to look like a display...

Well, the per-key keyboard actually has a 6x20 grid of leds under it of 
which each can be set to a different color.

So in a kind of way it is a Display? Especially if someone wants to 
program a complex animation to it.

What would be a better place/way to implement this? With the numeration 
implementation you have ::kbd_backlight(_0) up to ::kbd_backlight_119. 
And 120 file accesses for each frame of a potentially custom programmed 
keyboard backlight animation.

Also, userspace has no clear way of knowing how these 120 leds are 
actually placed under the keyboard.

Suggestion for the TODO list: Find a suitable interface for single 
devices with many individually controlable leds (per-key-rgb keyboards, 
led stipes with individually controllable leds, rgb ram with 
individually controllable leds, etc.)

A quick idea: Maybe add a multi_coord and a multi_coord_max entry? The 
first first one giving the coordinate of the led on the device (eg a 
keyboard) in an abstract messuring unit in the form "<x> <y>". Defined 
by 0 0 as the upper left corner of the device and multi_coord_max as the 
lower right. In the case the device is "one-dimensional", e.g. a led 
stripe, the second value of coord_max is 0. Alternativly, no 
multi_coord_max, and multi_coord is just 2 float values between 0 and 100.

>
>> Second question: For the keyboards with shared brightness, is it ok to have
>> the brightness values of ::kbd_backlight, ::kbd_backlight_1 etc. just in
>> sync? I did not see a way to have a ::kbd_backlight without the brightness
>> sysfs entry (then I would have just given the brightness switch to
>> ::kbd_backlight and not to ::kbd_backlight_1 and ::kbd_backlight_2)
> Can we simply ignore shared brightness to get reasonable API?
So not use the brightness parameter of the firmware, but do everything 
with RGB values?
>
>> Third question: The 3 zone RGB and the per-key keyboards have firmware
>> accelerated modes, like breathing and rainbow. How do I make them accessible
>> via the multicolor leds interface? the blinking pattern interface does not
>> really match the usecase as these modes are a simple single value toggle
>> (0=static color, 1=breathing, 2=ignore color settings and play predefined
>> moving rainbow pattern, etc).
> Take a look at drivers/leds/trigger/ledtrig-pattern.c . That's
> interface we'd like.

Isn't that the interface described here 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt 
? That does not really map to the firmware, es the breathing mode for 
example can't be adjusted, it just goes from max to off and back in a 
predefined timeframe.

As far as I see there is no interface for vendor specific modes. So what 
would the correct place be so expose this to userspace? Only thing I can 
think auf atm is just a sysfs entry in the platform driver complementing 
the leds settings. Albeit a little bit ugly because then you have 2 
different places controlling the leds.

>
> Best regards,
> 								Pavel

Kind regards,

Werner