Clarification questions regarding multicolor leds

Clarification questions regarding multicolor leds

[Werner Sembach]

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?

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.

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)

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).

Forth question: What is actual the purpose of mc_subled.brightness and 
mc_subled.channel?

Kind regards,

Werner

Re: Clarification questions regarding multicolor leds

[Werner Sembach]

Am 19.08.22 um 09:45 schrieb Werner Sembach:
> 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?
>
> 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.
>
> 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)
>
> 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).
>
> Forth question: What is actual the purpose of mc_subled.brightness and 
> mc_subled.channel?
>
> Kind regards,
>
> Werner
>
Another Question: Is max brightness == max color intensity?

Re: Clarification questions regarding multicolor leds

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]

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.

> 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...

> 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?

> 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.

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Clarification questions regarding multicolor leds

[Werner Sembach]

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

Re: Clarification questions regarding multicolor leds

[Jacek Anaszewski]

Hi Werner,

On 8/19/22 09:45, Werner Sembach wrote:
[...]
> Forth question: What is actual the purpose of mc_subled.brightness and 
> mc_subled.channel?

brightness is for storing subled brightness calculated in
led_mc_calc_color_components() and channel is for storing
information about actual iout that is related to the given
sub-LED. It was introduced for the cases like lp55xx driver
family (see e.g. drivers/leds/leds-lp55xx-common.c).

-- 
Best regards,
Jacek Anaszewski