Re: [PATCH v3 1/2] dt-bindings: leds: Add multicolor PWM LED bindings

[Alexander Dahl <ada@thorsis.com>]

Hello,

Am Sat, Jan 29, 2022 at 12:26:39AM +0100 schrieb Marek Behún:
> On Sat, 29 Jan 2022 00:04:01 +0100
> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> 
> > On 1/28/22 9:36 PM, Marek Behún wrote:
> > > On Thu, 27 Jan 2022 22:24:21 +0100
> > > Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> > >   
> > >> Hi Sven,
> > >>
> > >> On 1/26/22 11:48 AM, sven@svenschwermer.de wrote:  
> > >>> From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
> > >>>
> > >>> This allows to group multiple PWM-connected monochrome LEDs into
> > >>> multicolor LEDs, e.g. RGB LEDs.
> > >>>
> > >>> Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
> > >>> ---  
> > >> [...]  
> > >>> +
> > >>> +additionalProperties: false
> > >>> +
> > >>> +examples:
> > >>> +  - |
> > >>> +    #include <dt-bindings/leds/common.h>
> > >>> +
> > >>> +    rgb-led {
> > >>> +        compatible = "pwm-leds-multicolor";
> > >>> +
> > >>> +        multi-led {
> > >>> +          color = <LED_COLOR_ID_RGB>;
> > >>> +          function = LED_FUNCTION_INDICATOR;
> > >>> +          max-brightness = <65535>;  
> > >>
> > >> It doesn't make much sense to have such a big resolution of global
> > >> multi color brightness. 255 will be sufficient.  
> > > 
> > > If the PWM supports it, why not?
> > > On Omnia the default is 255, and since it is PWM, the change from 0/255
> > > to 1/255 is much bigger then from, say, 15/255 to 16/255. So if 1/255
> > > is too bright, you are then unable to set it less bright. I think 1024
> > > or ever 65535 makes sense with PWMs.  
> > 
> > With values other than 255 we will not achieve 24-bit RGB, which is one
> > problem, and the other one is non-linear brightness that can be achieved
> > with PWM. So probably we would need to add an additional note in the
> > documentation [0], saying that changing global brightness allows to
> > preserve combined LED hue only when all sub-leds are linear, and that it
> > will not be the case for PWM LEDs.
> > 
> > And I propose to change multi-led 'color' DT property value from
> > LED_COLOR_ID_RGB to LED_COLOR_ID_MULTI to avoid the impression that it
> > will work as traditional 24-bit RGB.
> > 
> > [0] Documentation/leds/leds-class-multicolor.rst
> 
> I know that color curves were being discussed at the time multicolor
> was being introduced, and AFAIK Pavel didn't like it, but I don't
> remember the reasons anymore.
> 
> As far as I understand it though, for PWM LEDs there is an equation for
> gamma correction. 

That's right, and it gets a little more complicated if you have RGB
instead of a single LED.  A start for reading might be this:

https://hackaday.com/2016/08/23/rgb-leds-how-to-master-gamma-and-hue-for-perfect-brightness/

(I had bookmarked that back when I was hacking on firmware for an 8bit
microcontroller controlling an RGB LED through soft PWM few years
ago.  A very simple solution is a precalculated static lookup table.)

> So either we need to rename this LED to MULTI, or the
> driver needs to do gamma correction so that the LED behaves RGB.

Do those devices marked as RGB currently, have that gamma correction
integrated on chip? Examples?

Greets
Alex