All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marek Behún" <marek.behun@nic.cz>
To: Jacek Anaszewski <jacek.anaszewski@gmail.com>, pavel@ucw.cz
Cc: sven@svenschwermer.de, linux-leds@vger.kernel.org,
	devicetree@vger.kernel.org, linux-pwm@vger.kernel.org,
	Sven Schwermer <sven.schwermer@disruptive-technologies.com>,
	robh+dt@kernel.org, thierry.reding@gmail.com,
	u.kleine-koenig@pengutronix.de, lee.jones@linaro.org,
	post@lespocky.de
Subject: Re: [PATCH v3 1/2] dt-bindings: leds: Add multicolor PWM LED bindings
Date: Sat, 29 Jan 2022 00:26:39 +0100	[thread overview]
Message-ID: <20220129002639.33c7d4c0@thinkpad> (raw)
In-Reply-To: <09b46d05-5dd0-a585-2ca3-0bc04e613343@gmail.com>

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. So either we need to rename this LED to MULTI, or the
driver needs to do gamma correction so that the LED behaves RGB.

Pavel, what is your opinion on this?

Marek

  reply	other threads:[~2022-01-28 23:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 10:48 [PATCH v3 0/2] Multicolor PWM LED support sven
2022-01-26 10:48 ` [PATCH v3 1/2] dt-bindings: leds: Add multicolor PWM LED bindings sven
2022-01-27 21:24   ` Jacek Anaszewski
2022-01-28 20:36     ` Marek Behún
2022-01-28 23:04       ` Jacek Anaszewski
2022-01-28 23:26         ` Marek Behún [this message]
2022-01-31  7:10           ` Alexander Dahl
2022-01-31  8:55             ` Sven Schwermer
2022-02-12 11:54         ` Pavel Machek
2022-01-26 10:48 ` [PATCH v3 2/2] leds: Add PWM multicolor driver sven
2022-02-02 12:33   ` Andy Shevchenko
2022-02-06  9:17     ` Sven Schwermer
     [not found]       ` <CAHp75VeSD5bYERp=s9Dzd0xScVc+sYSdc8W4XBfCVXJgyWMPyA@mail.gmail.com>
2022-02-06 11:04         ` Sven Schwermer
2022-02-06 12:25           ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220129002639.33c7d4c0@thinkpad \
    --to=marek.behun@nic.cz \
    --cc=devicetree@vger.kernel.org \
    --cc=jacek.anaszewski@gmail.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=post@lespocky.de \
    --cc=robh+dt@kernel.org \
    --cc=sven.schwermer@disruptive-technologies.com \
    --cc=sven@svenschwermer.de \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.