Re: Generic RGB LED support was Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi Vesa,

On 1/9/19 8:11 AM, Vesa Jääskeläinen wrote:
> Hi Pavel,
> 
> On 09/01/2019 0.59, Pavel Machek wrote:
>> Hi!
>>
>>>>>> Grab yourself an RGB LED and play with it; you'll see what the
>>>>>> problems are. It is hard to explain colors over email...
>>>>>
>>>>> Video [0] gives some overview of lp5024 capabilities.
>>>>>
>>>>> I don't see any problems in exposing separate red,green,blue
>>>>> files and brightness for the devices with hardware support for
>>>>> that.
>>>>
>>>> Well, that's what we do today, as three separate LEDs, right?
>>>
>>> No. It doesn't allow for setting color intensity by having
>>> the color fixed beforehand. Below is relevant excerpt from
>>> the lp5024 documentation. This is not something that can be
>>> mapped to RGB color space, but rather to HSV/HSL, with the
>>> reservation that the hardware implementation uses PWM
>>> for setting color intensity.
>>
>> So they have feature where they have independent controls for each
>> channel, then one common control per three channels. Other chips have
>> common control for all the LEDs, for example. We don't support that
>> currently; lets focus on the RGB thing first.
>>
>>>> I don't have problem with that, either; other drivers already do
>>>> that. He's free to use existing same interface.
>>>>
>>>> But that is insufficient, as it does not allow simple stuff, such as
>>>> turning led "white".
>>>>
>>>> So... perhaps we should agree on requirements, first, and then we can
>>>> discuss solutions?
>>>>
>>>> Requirements for RGB LED interface:
>>>>
>>>> 1) Userspace should be able to set the white color
>>>>
>>>> 2) Userspace should be able to arbitrary color from well known list
>>>> and it should approximately match what would CRT, LCD or OLED 
>>>> monitor display
>>>
>>> The difference is that monitor display driver is pre-calibrated
>>> for given display by the manufacturer. With the LED controllers the
>>> manufacturer has no control over what LEDs will be connected to the
>>> iouts. Therefore it should be not surprising that colors produced
>>> by custom LEDs are not as user would expect when comparing to
>>> the RGB color displayed on the monitor display.
>>
>> It is true that _chip_ manufacturer can not know what LEDs will be
>> connected. But _system_ manufacturer can and should know that, and
>> should tell be able to tell us in the dts.
>>
>>> This renders your requirement 2) infeasible with use of custom LEDs
>>> any fixed algorithm, since the final effect will always heavily depend
>>> on the LED circuit design.
>>
>> Depending on LED circuit design and actual LEDs connected is okay.. we
>> just need to get information from _system_ designer (not chip
>> designer), and pass it to a place where color is computed.
>>
>>>> a) RGB LEDs are usually not balanced. Setting 100% PWM on
>>>> red/green/blue channels will result in nothing close to white
>>>> light. In fact, to get white light on N900, blue and green channel's
>>>> PWM needs to be set pretty low, as in 5%.
>>>>
>>>> b) LED class does not define any relation between "brightness" in
>>>> sysfs and ammount of light in lumens. Some drivers use close to linear
>>>> relation, some use exponential relation. Human eyes percieve logarithm
>>>> of lumens. RGB color model uses even more complex function.
>>>
>>> One general question: do you have any solutions in store?
>>
>> I played with LEDs on N900 over the weekend, yes.
>>
>> And getting reasonable colors seems to be possible, when a) and b) are
>> solved... a) seems to be more important than b).
>>
>> Now... this does not tell us how we should design kernel<->user
>> interface, but it should tell us that main goals - 1) and 2) are
>> possible.
> 
> I was thinking about this calibration and color correctness thing and I 
> am thinking a bit that it should be partly in kernel and partly in user 
> space.
> 
> For displays and printers there are defined icc-profiles that define how 
> colors are mapped to particular device in cases when you want to have 
> accurate color representation. In theory to get accurate LED output one 
> could model LEDs with icc profile and then pick your color and convert 
> it with icc profile to actual raw hardware values. Then this raw 
> hardware value could be written from user space to kernel.

icc-profiles idea sounds interesting. We would have to adjust hsv->rgb
algorithm to take the profiles into account. I wonder how that would
work in practice.

> In kernel we could provide raw hardware value support and in case of PWM 
> IC controlled LEDs we could provide curve mapping to linearize the 
> behavior of values entered to enable use in cases where close enough is 
> good enough.
> 
> Eg. if you want to have "white" then you have your color space picker 
> like sRGB(255,255,255). Then you would define icc profile it might 
> translate to hardware raw values 253%, 230%, 225%.

> Then you would write this to kernel with:
> 
> # set red, green and blue color elements
> echo "253 230 225" > color
> 
> In this case however behavior of brightness node is challening in 
> accuracy domain. Britghtness value 255 would of course provide 1:1 
> mapping in this case.
> 
> To go right to correct color and brightness at one go we could have 
> optional brightness at the end of color value array:
> 
> # set red, green and blue color element and brightness setting:
> echo "253 230 225 255" > color
> 
> If you want to have fancier behavior for brightness in your system then 
> we probably need to have configurable brightness model.
> 
> - hardware, like in lp5024 case would map to hardware register (would be 
> omitted if there is no such register)
> - onoff, would act like light switch ON/OFF eg. either configured value 
> or all zeroes.
> - scaled, would multiply the color elements
> - hsl, would use hsl formula
> - and this can be extended later with some other models and allows us to 
> start with with some models now.
> 
> We could define this in devicetree and from sysfs a bit like with trigger:
> 
> $ cat brightness_model
> [hardware] onoff scaled hsl
> 
> $ echo "hsl" > brightness_model
> 
> $ cat brightness_model
> hardware onoff scaled [hsl]
> 
> Then we could have "color_names" or such sysfs entry to determine allow 
> user space auto detection of led elements):
> 
> $ cat color_names
> red green blue
> 
> I suppose this model would provide flexibilty for multiple cases. Make 
> it simple for most uses, allow accuracy with icc profiles for advanced 
> users, would allow atomic color setting.
> 
> I have updated (not yet in github) my tests to use color array model and 
> color_names already and can play with brightness_model thing if this is 
> something that is good path?
> 
> What do you think?

-- 
Best regards,
Jacek Anaszewski