devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Dan Murphy <dmurphy@ti.com>,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, dachaac@gmail.com,
	robh+dt@kernel.org
Subject: RGB LED class Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver
Date: Fri, 18 Jan 2019 01:02:35 +0100	[thread overview]
Message-ID: <20190118000235.GB27661@amd> (raw)
In-Reply-To: <8dfa2854-2814-6874-ab8e-1858e9a18acf@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3189 bytes --]

Hi!


> >I am willing to work with you on the HSV and adapting the LP50xx part to this framework.
> >Or any RGB framework for that matter.  I still don't agree with the kernel needing to declare colors
> >  maybe color capabilities but not specific colors.
> 
> Dan, if you have a bandwidth for LED RGB class implementation
> then please go ahead. It would be good to compare colors produced
> by software HSV->RGB algorithm to what can be achieved with
> LEDn_BRIGHTNESS feature.

Don't get me wrong, I'd like to see LED RGB class implementation. But
it will delay merge of this driver.

If we want to do that, we should first discuss the requirements, and
then come up with interface.. and only then we can talk about the
driver code.

That's why I believe preferable way would be to merge the driver using
the existing interface.

Of course, first designing RGB LED class and then merging the
driver.. is okay with me. But lets not rush the class because there's
driver waiting for it.

> The requirements for LED RGB class as I would see it:
> 
> sysfs interface:
> 
> brightness-model: space separated list of available options:
> - rgb (default):
>   - creates color file with "red green blue" decimal values
>   - creates brightness file
> 	a) for devices with hardware support for adjusting color
>            intensity it maps to corresponding register
>         b) for the rest writing any value greater than 0 will result
>            in setting all color registers to max
> - hsv:
>   - creates color file with "h s v" values - it shall
>     use software HSV->RGB algorithm for setting color registers
> 
> - any other custom color ranges defined in DT, but it can be covered
>   later
> - other options?

First, I think we want to decide if RGB LED should be presented as
3 LEDs or as 1 LED... and what to do with existing RGB leds being
presented as 3 LEDs.

I don't think we want to support both RGB and HSV in the kernel. It is
math, and not a nice one.

Yes, both have advantages and disadvantages, but having _both_ in
kernel has disadvantages of both.

One way I could imagine the interface:

    RGB LED presented as one LED.

    brightness -- controls brightness of whole RGB module.

    pwm_channels -- "1000 240 300" -- "red part should be full on, green
    should be pwm controlled to 240/1000, blue should be 300/1000"

    pwm_white -- "1000 500 400" -- tells userspace what to write to PWM
    channels to get approximately white color.

    This would assume that RGB LEDs are always pwm controlled. That
    seems to be true for hardware I seen.

    + no complex math in kernel

    + userspace knows enough to display arbitrary colors

    + userspace can use full range of available PWM intensities

    + existing triggers will work nicely

    - userland needs to do non-trivial math to get colors it wants

    - not sure how to migrate existing devices

Thoughts? Other possible interfaces?

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  reply	other threads:[~2019-01-18  0:02 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-14 21:17 [PATCH v2 1/2] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers Dan Murphy
2019-01-14 21:17 ` [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver Dan Murphy
2019-01-15 21:47   ` Jacek Anaszewski
2019-01-15 22:22     ` Pavel Machek
2019-01-16  0:20       ` Dan Murphy
2019-01-16 10:55         ` Pavel Machek
2019-01-16 18:41           ` Dan Murphy
2019-01-16 22:04             ` Pavel Machek
2019-01-16 23:33               ` Dan Murphy
2019-01-17 10:06                 ` Pavel Machek
2019-01-17 13:27                   ` Dan Murphy
2019-01-17 21:10             ` Jacek Anaszewski
2019-01-18  0:02               ` Pavel Machek [this message]
2019-01-18 15:57                 ` RGB LED class " Dan Murphy
2019-01-28 23:03                   ` Pavel Machek
2019-01-18 22:13                 ` Jacek Anaszewski
2019-01-19 21:36                   ` Pavel Machek
2019-01-20 15:30                     ` Jacek Anaszewski
2019-01-21 19:38                       ` Jacek Anaszewski
2019-01-28 23:04                       ` Pavel Machek
2019-01-18 13:45               ` Dan Murphy
2019-01-18 13:58                 ` Dan Murphy
2019-01-20  6:42                   ` Vesa Jääskeläinen
2019-01-22 21:39                     ` Jacek Anaszewski
2019-01-22 22:44                       ` Dan Murphy
2019-01-23 21:52                         ` Jacek Anaszewski
2019-01-24 21:00                           ` Dan Murphy
2019-01-24 21:55                             ` Jacek Anaszewski
2019-01-29 13:56                               ` Dan Murphy
2019-01-29 20:19                                 ` Jacek Anaszewski
2019-01-29 20:26                                   ` Dan Murphy
2019-01-29 21:45                                     ` Pavel Machek
2019-01-29 21:46                                       ` Dan Murphy
2019-01-29 21:53                                     ` Jacek Anaszewski
2019-01-20 15:32                 ` Jacek Anaszewski
2019-01-17 21:08           ` Jacek Anaszewski
2019-01-19 19:11             ` Vesa Jääskeläinen
2019-01-19 21:46               ` Pavel Machek
2019-01-19 22:44                 ` Vesa Jääskeläinen
2019-01-20  6:51                 ` Vesa Jääskeläinen
2019-01-21 13:27               ` Dan Murphy
2019-01-21 15:12                 ` Vesa Jääskeläinen
2019-01-24 20:32     ` Dan Murphy
2019-01-24 21:14       ` Jacek Anaszewski
2019-01-15 21:42 ` [PATCH v2 1/2] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers Jacek Anaszewski

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=20190118000235.GB27661@amd \
    --to=pavel@ucw.cz \
    --cc=dachaac@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmurphy@ti.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).