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

[Jacek Anaszewski]

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