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

From: Vesa JÃÃskelÃinen
Date: Fri Jan 04 2019 - 14:49:20 EST

Hi Pavel,

On 04/01/2019 1.34, Pavel Machek wrote:

Regarding led_scale_color_elements() - I checked it in GIMP and
the results are not satisfactory when increasing brightness.
Even if we managed to fix it, the result would not be guaranteed
to be the same across all devices.

No and they will never be the same. I was told by our hardware expert that
it is rather impossible to get linearly behaving LED control without special
curve fitting trimmed for particular hardware and LED component in use. And
if you go and change LED component/vendor it would need to be "calibrated"
again if such accuracy would be required. Also LEDs age and that has also
effect on this.

Well, it is not possible to "perfectly" calibrate LCD monitors,
either. Yet, color tables make sense for them.

And we should aim for the same thing.

And yes, it may mean re-doing calibration when vendor changes. And it
will mean some math and some understanding of colors.

And... LEDs are linear-enough as it is. That is not a problem. But RGB
does _not_ expect linear response. That's why colors are _way_ off currently.

Example what I was given was some LEDs are off for let's say 20% of PWM linearity and then there is non-linear curve for PWM value vs. intensity (this was in context of white LCD backlight).

One case where we currently are interested in linear intensity is LCD background color. For that we have ramp defined in device tree. There would probably be simple fix to get that curve fitting to work better but let's keep that as another topic for now.

I was thinking that if we get "calibration" curve support in PWM LED brightness we could then just use that as one solution within kernel and let LCD PWM brightness support use same functionality from LED framework. (eg. LCD backlight would bind to PWM controlled mono color LED node)

Other case is that we need to dim LEDs in low light situations.

As such I have nothing against adding support for HSL or other color space based brightness calculations. It might just need to be configurable what kind of mode is being used based on hardware solution in place. HSL seem to need a bit of fixed point math. Got floating point version working already but that does not work with kernel space so one needs to adapt it to fixed point.

One problem here is to figure out is if user configures some color -- is it configured with 100% brightness eg. Should one calculate RGB->HSL and then scale L with brightness and calculate RGB back for setting color -- or does it mean replacing L with brightness value?

Additional problem is then if you have let's say yellow LED color element there. How would one scale that then? Linear is of course easy. If you need to configure this to some color space then how does one define the color in device tree so that such color space conversion is possible? One possible solution is to calibrate the curve (like what you do with LCD calibration) and then just assume brightness as linear brightness value in that case.

Or if you have multi-color LED with red and green but no other color elements. How does brightness scaling work with this one?

I have another proposal, being a mix of what has been discussed so far:

ÂÂ RGB LED class will expose following files:
ÂÂ a) available by default:
ÂÂÂÂ - red, green, blue
ÂÂÂÂÂÂ Writing any of these file will result in writing corresponding
ÂÂÂÂÂÂ device register.

Problem with this is that we are basically back at square one and one cannot
do "atomic" color change with this.

In order to set or activate new values one would need "load values" file or
such that when writing to it would activate new values. However it becomes
quite clumsy interface at that point as you need to handle multiple writes
to multiple files and makes those operations rather slow.

If you don't like the interface, create an shared library. It may be
neccessary, anyway, for the color operations.

You say it is "rather slow" to change all 3 colors. How long does it
take, and how long do you need it to take?

Some times in embedded linux systems you can see "stalls" in operation of an application flow eg. time is spent in different places. I believe "slowest" CPU with embedded linux we are using is Atmel's AT91 series (ARM9) and executing code in there can at times be a bit time consuming.

I have seen delays like 18ms in TI's AM335x CPU series (Cortex-A8) and not even too high load situations (eg. breaking some serial protocols because of breaks in transmission in-between transfer without being a problem in application code as such). It is completely different story when there is a bit of load in system or some special situation in the kernel/OS.

Running high priority thread for configuring LEDs to avoid problems sounds like a wrong solution.

Co-worker of mine tried multi-color LED brightness sliding from user space with current PWM led driver and it was visible from eye that it was not timed smoothly.

For GPIO LED we decided to use out-of-tree solution for multi-color LEDs because we didn't want to see wrong colors in LEDs or sliding colors. For this we have color preset table in device tree and with it color change is more or less atomic. And also it supports kernel based LED blinking with whatever color you have configured there first.

For PWM controlled multi-color LEDs we don't yet have a solution. We need configurable color kernel based blinking.

With Jacek's proposed interface one could do kernel based blinking if brightness is simulated or calculated (if trigger's ON brightness can be configured). Only problem I suppose is color transition from A to B state and after that the blinking would work nicely as target color would already be known by driver.

If we could figure out acceptable solution for color transition problem then I suppose all parties would be happy?

Vesa JÃÃskelÃinen