LinuxLists.cc - [PATCH v2 1/2] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers

2019-01-14 21:19:12

Subject: [PATCH v2 1/2] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers

Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024 and the LP5018
RGB LED device driver. The LP5036/3024/18 can control RGB LEDs individually
or as part of a control bank group. These devices have the ability
to adjust the mixing control for the RGB LEDs to obtain different colors
independent of the overall brightness of the LED grouping.

Datasheet:
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdf

Signed-off-by: Dan Murphy <[email protected]>
---

v2 - Added the LP5030/36 devices, defined the modules vs banked properties
renamed the file from lp5024 to lp50xx. - https://lore.kernel.org/patchwork/patch/1026514/

.../devicetree/bindings/leds/leds-lp50xx.txt | 143 ++++++++++++++++++
1 file changed, 143 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.txt b/Documentation/devicetree/bindings/leds/leds-lp50xx.txt
new file mode 100644
index 000000000000..7bc6843ddba4
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.txt
@@ -0,0 +1,143 @@
+* Texas Instruments - LP5018/24/30/36 RGB LED driver
+
+The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into
+a LED group or control them individually.
+
+The difference in these RGB LED drivers is the number of supported RGB strings.
+
+Required properties:
+ - compatible:
+ "ti,lp5018"
+ "ti,lp5024"
+ "ti,lp5030"
+ "ti,lp5036"
+ - reg : I2C slave address
+ lp5018/24 - 0x28
+ lp5030/36 - 0x30
+ - #address-cells : 1
+ - #size-cells : 0
+
+Optional properties:
+ - enable-gpios : gpio pin to enable/disable the device.
+ - vled-supply : LED supply
+
+Required child properties:
+ - reg : Is the child node iteration.
+
+Required Child properties but only one should be defined per child:
+Either one of these two properties are required for each node. The
+property ti,led-bank takes precedence over the ti,led-module within the same
+node.
+
+ - ti,led-module : This property denotes the single LED module number
+ that will be controlled in the LED class instance.
+ - ti,led-bank : This property denotes the LED module numbers that will
+ be controlled as a single RGB cluster. Each LED module
+ number will be controlled by a single LED class instance.
+ There can only be one instance of the ti,led-bank
+ property for each device node.
+
+The LED outpus associated with the LED modules are defined in Table 1 of the
+corresponding data sheets.
+
+LP5018 - 6 Total RGB cluster LED outputs 0-5
+LP5024 - 8 Total RGB cluster LED outputs 0-7
+LP5030 - 10 Total RGB cluster LED outputs 0-9
+LP5036 - 12 Total RGB cluster LED outputs 0-11
+
+Optional child properties:
+ - label : see Documentation/devicetree/bindings/leds/common.txt
+ - linux,default-trigger :
+ see Documentation/devicetree/bindings/leds/common.txt
+
+Examples:
+LP5018 and LP5024 example:
+led-controller@29 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "ti,lp5024";
+ reg = <0x29>;
+ enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+ vled-supply = <&vmmcsd_fixed>;
+
+ led@0 {
+ reg = <0>;
+ label = "led1_mod";
+ ti,led-module = <1>;
+ };
+
+ led@1 {
+ reg = <1>;
+ label = "banked_leds";
+ ti,led-bank = <0 2 5 3 >;
+ };
+
+ led@2 {
+ reg = <2>;
+ label = "led4_mod";
+ ti,led-module = <4>;
+ };
+
+ led@3 {
+ reg = <3>;
+ label = "led7_mod";
+ ti,led-module = <7>;
+ };
+
+ led@4 {
+ reg = <4>;
+ label = "led6_mod";
+ ti,led-module = <6>;
+ };
+};
+
+LP5030 and LP5036 example:
+led-controller@30 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "ti,lp5036";
+ reg = <0x30>;
+ enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+ vled-supply = <&vmmcsd_fixed>;
+
+ led@0 {
+ reg = <0>;
+ label = "led1_mod";
+ ti,led-module = <1>;
+ };
+
+ led@1 {
+ reg = <1>;
+ label = "led_banked";
+ ti,led-bank = <0 2 5 3 9 10 >;
+ };
+
+ led@2 {
+ reg = <2>;
+ label = "led4_mod";
+ ti,led-module = <4>;
+ };
+
+ led@3 {
+ reg = <3>;
+ label = "led7_mod";
+ ti,led-module = <7>;
+ };
+
+ led@4 {
+ reg = <4>;
+ label = "led6_mod";
+ ti,led-module = <6>;
+ };
+
+ led@5 {
+ reg = <5>;
+ label = "led8_mod";
+ ti,led-module = <8>;
+ };
+};
+
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/symlink/lp5024.pdf
+http://www.ti.com/lit/ds/symlink/lp5036.pdf
--
2.20.1.98.gecbdaf0899

2019-01-14 21:19:12

by Dan Murphy

[permalink] [raw]

Subject: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

by Jacek Anaszewski

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Hi Dan,

On 1/16/19 7:41 PM, Dan Murphy wrote:
> Hello
>
> On 1/16/19 4:55 AM, Pavel Machek wrote:
>> Hi!
>>
>>> On 1/15/19 4:22 PM, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>> +The 24-bit RGB value passed in follows the pattern 0xXXRRGGBB
>>>>>> +XX - Do not care ignored by the driver
>>>>>> +RR - is the 8 bit Red LED value
>>>>>> +GG - is the 8 bit Green LED value
>>>>>> +BB - is the 8 bit Blue LED value
>>>>>> +
>>>>>> +Example:
>>>>>> +LED module output 4 of the LP5024 will be a yellow color:
>>>>>> +echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color
>>>>>> +
>>>>>> +LED module output 4 of the LP5024 will be dimmed 50%:
>>>>>> +echo 0x80 > /sys/class/leds/lp5024\:led4_mod/brightness
>>>>>> +
>>>>>> +LED banked RGBs of the LP5036 will be a white color:
>>>>>> +echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color
>>>>>
>>>>> This part with example cans remain in Documentation/leds if you
>>>>>> like.
>>>>
>>>> Does it actually work like that on hardware?
>>>
>>> What?
>>
>> If you do echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color,
>> does it actually produce white? With all the different RGB modules
>> manufacturers can use with lp5024P?
>>
>> If you do echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color, does
>> it actually produce yellow, with all the different RGB modules
>> manufacturers can use with lp5024P?
>>
>
> I believe the answer to the general questions is no for any RGB cluster and driver out there.
> Because if you set the same values on each and every RGB device out there you will get varying shades of the color.
> But for this device yes the color does appear to be yellow to me versus what was displayed on my monitor by the HSL picker.
> But everyone interprets colors differently.
>
> If you write the same value for yellow or white on a droid 4 and the N900 do they produce the same color side by side?
> Most probably not.
>
> As you pointed out the PWM needs to be modified to obtain the correct white color to account for LED and other device constraints.
>
> But we need to take into account the light pipe. Pools nowadays have RGB LED spot lights in them. It can
> be set to white. On my pool right off the lens the color has a purplish hue to it. As the light is diffracted into
> the pool the color becomes white. The pool is clear. When I add chemicals to the pool and make it cloudy
> and turn on the lights the color off the lens is now white. This is an example on a large scale but the issue
> scales down to the hand helds and smart home applications.
>
> If the cluster is piped through a flexible optic 0xffffff may produce the "white" you want on its output.
>
> So an expectation of certain color without proper piping based on a single RGB value may be a little unreasonable.
> There may need to be a way to attenuate the values based on the hardware aspect of the equation ie light pipe (or lack thereof) and LED vendor.
> So if we write 0xffffff to the RGB driver the driver could adjust the intensity of the individual LEDs based on the diffraction
> coefficients.
>
> I also think that is an unreasonable expectation here that writing a single value to any LED RGB driver would produce
> a "rest of the world" absolute color. Maybe it can produce something similar but not identical.
> As you indicated in the requirements there is more involved here then just the LED and the values written.
> The colors should be close but may not be identical.
>
> A 10 year old N900 should not be considered the gold standard for color production due to advancements in LED,
> light pipe and LED driver technology.
> The single package RGB clusters on the board I am testing is about the size of a single RGB LED from 10 years ago.
>
> I agree that the interface developed should work on the device but the algorithm derived to obtain the color needs to have
> a hardware aspect to the calculation.
>
>>>> Is it supposed to support "normal" RGB colors as seen on monitors?
>>>
>>> Monitors are not an application for this part.
>>
>> You did not answer the question. When you talk about yellow, is it
>> same yellow the rest of world talks about?
>>
>
> See above. It is close to what was on my monitor displayed.
>
>>>> Because 100% PWM on all channels does not result in white on hardware
>>>> I have.
>>>
>>> I don't know I am usually blinded by the light and have no diffuser over
>>> the LEDs to disperse the light so when I look I see all 3 colors.
>>
>> How can we have useful discussion about colors when you don't see the
>> colors?
>>
>> Place a piece of paper over the LEDs....
>>
>
> Good suggestion for a rough test.
>
>>>> But...
>>>>
>>>> I believe we should have a reasonable design before we do something
>>>> like this. There's no guarantee someone will not use lp50xx with just
>>>> the white LEDs for example. How will this work? Plus existing hardware
>>>> already uses three separate LEDs for RGB LED. Why not provide same
>>>> interface?
>>>
>>> Which existing hardware? Are they using this part?
>>
>> Nokia N900. They are not using this part, but any interface we invent
>> should work there, too.
>>
>
> Yes a common interface would be nice with some sort of hardware tuning coefficient.
>
>>> <rant>
>>> Why are we delaying getting the RGB framework or HSV in?
>>> I would rather design against something you want instead of having
>>> everyone complain about every implementation I post.
>>> </rant>
>>
>> Because you insist on creating new kernel interfaces, when existing
>> interfaces work, and are doing that badly.
>>
>> Because your patches are of lower quality than is acceptable for linux
>> kernel.
>>
>> Because you don't seem to be reading the emails.
>>
>> I sent list of requirements for RGB led support. This does not meet
>> them.
>>
>
> Sigh. You did not answer my question.
>
> Your requirements seem to be centered around monitors but that is only one application of the current
> RGB LED landscape.
>
> 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.

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?

Best regards,
Jacek Anaszewski

> It was agreed to continue forward with this particular implementation.
> At least thats what the email (I apparently did not read) stated.
>
> I need to fix the code to use the space separated value as pointed out and shown by Vesa.
> This will map nicely into this device with the color file as what I implemented is in theory
> they same code except for the space separated values.
>
>>> It is not a normal RGB driver. The device collates the individual RGB
>>> clusters into a single brightness register and you can modify the intensity of the individual
>>> LEDs via other registers. If brightness is 0 then the cluster is OFF regardless of the color
>>> set in the individual registers.
>>
>> I understand that. So just set cluster brightness to 255 and you have
>> normal RGB driver you can control with existing interfaces. You don't
>> have to use every feature your hardware has.
>>
>
> The brightness file is available and adjusts the brightness of the RGB cluster.
> I am not attempting to implement every feature the device has. But I am attempting to use
> the basic features that are available and useful.
>
>> You did not answer the "what if someone uses this with all white LEDs"
>> question.
>>
>
> Are you asking what if someone places a white LED instead of a RGB on the hardware?
> Well then they need to go back and have a review of the data sheet and what they are trying to
> achieve. That would be a misapplication of the LED driver itself and something software cannot fix.
>
> But if they do determine they want to control these white LEDs with this device
> then they can ignore the "color" file and control the cluster via the brightness file like we
> do today. The color file will only change the intensity of the single output (assuming LED module mode)
> or the banked output.
>
> If a user wants to place a RGB cluster down on the hardware and have white as the consistent color
> well then that is fine as the RGB outputs are all set to 0xff and the intensity of the cluster is
> controlled by the brightness file. If they cannot achieve the "white" with the default settings then
> on init they can set the color file once to obtain the "white" color and continue to use the brightness
> file to control the overall brightness of the cluster.
>
> It was determined in the email chain not to expose a brightness file per output as this device
> does not lend itself to that convention.
>
>> You know what? First, submit driver with similar functionality to
>> existing RGB drivers, using same interface existing drivers are
>> using. When that is accepted, we can talk about extending
>> kernel<->user interfaces.
>>
>
> I could do that but then there is no way for users to have any other color but "white" with this driver.
> That defeats the purpose of the device itself.
>
> This is why I would rather align the interfaces with what is being proposed so the interfaces won't change only
> the engine underneath will.
>
> I am not sure if you are aware of this or care but I found this recent blog on this effort:
> https://www.phoronix.com/scan.php?page=news_item&px=Linux-RGB-LED-Interface
> See some of the comments.
>
> Dan
>
>> Thanks,
>> Pavel
>>
>
>

2019-01-17 21:36:17

by Jacek Anaszewski

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

On 1/16/19 11:55 AM, Pavel Machek wrote:
> Hi!
>
>> On 1/15/19 4:22 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>> +The 24-bit RGB value passed in follows the pattern 0xXXRRGGBB
>>>>> +XX - Do not care ignored by the driver
>>>>> +RR - is the 8 bit Red LED value
>>>>> +GG - is the 8 bit Green LED value
>>>>> +BB - is the 8 bit Blue LED value
>>>>> +
>>>>> +Example:
>>>>> +LED module output 4 of the LP5024 will be a yellow color:
>>>>> +echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color
>>>>> +
>>>>> +LED module output 4 of the LP5024 will be dimmed 50%:
>>>>> +echo 0x80 > /sys/class/leds/lp5024\:led4_mod/brightness
>>>>> +
>>>>> +LED banked RGBs of the LP5036 will be a white color:
>>>>> +echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color
>>>>
>>>> This part with example cans remain in Documentation/leds if you
>>>>> like.
>>>
>>> Does it actually work like that on hardware?
>>
>> What?
>
> If you do echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color,
> does it actually produce white? With all the different RGB modules
> manufacturers can use with lp5024P?
>
> If you do echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color, does
> it actually produce yellow, with all the different RGB modules
> manufacturers can use with lp5024P?

Vesa proposed using icc-profiles to make the colors looking similar
on various LEDs. It was in reply to your message with requirements
for RGB LED class.

For this device we can however do without it. Videos showing the
performance of this particular device with a reference design using
RGB LEDs prove it works nice.

>>> Is it supposed to support "normal" RGB colors as seen on monitors?
>>
>> Monitors are not an application for this part.
>
> You did not answer the question. When you talk about yellow, is it
> same yellow the rest of world talks about?
>
>>> Because 100% PWM on all channels does not result in white on hardware
>>> I have.
>>
>> I don't know I am usually blinded by the light and have no diffuser over
>> the LEDs to disperse the light so when I look I see all 3 colors.
>
> How can we have useful discussion about colors when you don't see the
> colors?
>
> Place a piece of paper over the LEDs....
>
>>> But...
>>>
>>> I believe we should have a reasonable design before we do something
>>> like this. There's no guarantee someone will not use lp50xx with just
>>> the white LEDs for example. How will this work? Plus existing hardware
>>> already uses three separate LEDs for RGB LED. Why not provide same
>>> interface?
>>
>> Which existing hardware? Are they using this part?
>
> Nokia N900. They are not using this part, but any interface we invent
> should work there, too.
>
>> <rant>
>> Why are we delaying getting the RGB framework or HSV in?
>> I would rather design against something you want instead of having
>> everyone complain about every implementation I post.
>> </rant>
>
> Because you insist on creating new kernel interfaces, when existing
> interfaces work, and are doing that badly.
>
> Because your patches are of lower quality than is acceptable for linux
> kernel.

Lets keep things civil please.

This sentence should look familiar to you - it's not the first time
you resort to this type of narration.

> Because you don't seem to be reading the emails.
>
> I sent list of requirements for RGB led support. This does not meet
> them.

And you didn't respond to the comments from Vesa. The requirements
has not been acclaimed.

>> It is not a normal RGB driver. The device collates the individual RGB
>> clusters into a single brightness register and you can modify the intensity of the individual
>> LEDs via other registers. If brightness is 0 then the cluster is OFF regardless of the color
>> set in the individual registers.
>
> I understand that. So just set cluster brightness to 255 and you have
> normal RGB driver you can control with existing interfaces. You don't
> have to use every feature your hardware has.

This feature is one of the core advantages of this device.

> You did not answer the "what if someone uses this with all white LEDs"
> question.
>
> You know what? First, submit driver with similar functionality to
> existing RGB drivers, using same interface existing drivers are
> using. When that is accepted, we can talk about extending
> kernel<->user interfaces.

This stands in contradiction with my request.

Provided there will be no other NAKs, I am eager to accept the
interface with color and brightness files.

Moreover, I think that RGB LED class with configurable
brightness-model, and with possible color range adjustments via
icc-profiles or something similar, is the best solution that has been
proposed so far. It is just flexible.

I'd like to capitalize on the ideas shared in this thread and have
finally LED RGB class materialized.

--
Best regards,
Jacek Anaszewski

2019-01-18 00:06:01

Hi!

> >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.
>
> What algorithm would be used for mapping brightness levels to RGB values
> in case of devices without hardware support for that?

Output power = brightness / max_brightness * pwm_channel[x].

> > 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.
>
> Why pwm in the file names? I don't see any gain and only possible
> problems. Many LED controllers use current level and not PWM
> for driving LEDs.

I want to make sure userspace understands this has linear relation to
output power. I'd like to avoid mentioning color because there power
output is not linear with "RGB"

> s/pwm/color/

s/pwm/power/ would work for me.

> Besides white also other color presets could be defined in DT.

They should not be neccessary. When userspace knows what is white and
that power is linear with values in power_channels, it should be able
to do colorspace conversion itself.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachments:

(No filename) (1.91 kB)
signature.asc (188.00 B)
Digital signature Download all attachments

2019-01-19 21:48:25

Dan

On 1/24/19 10:00 PM, Dan Murphy wrote:
> Jacek
>
> On 1/23/19 3:52 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 1/22/19 11:44 PM, Dan Murphy wrote:
>>> Jacek
>>>
>>> On 1/22/19 3:39 PM, Jacek Anaszewski wrote:
>>>> Hi all,
>>>>
>>>> On 1/20/19 7:42 AM, Vesa Jääskeläinen wrote:
>>>>> Hi Dan,
>>>>>
>>>>> On 18/01/2019 15.58, Dan Murphy wrote:
>>>>>> Jacek
>>>>>>
>>>>>> On 1/18/19 7:45 AM, Dan Murphy wrote:
>>>>>>> Jacek
>>>>>>>
>>>>>>> On 1/17/19 3:10 PM, Jacek Anaszewski wrote:
>>>>>>>> Hi Dan,
>>>>>>>>
>>>>>>>> On 1/16/19 7:41 PM, Dan Murphy wrote:
>>>>>>>>> Hello
>>>>>>>>>
>>>>>>>>> On 1/16/19 4:55 AM, Pavel Machek wrote:
>>>>>>>>>> Hi!
>>>>>>>>>>
>>>>>>>>>>> On 1/15/19 4:22 PM, Pavel Machek wrote:
>>>>>>>>>>>> Hi!
>>>>>>>>>>>>
>>>>>>>>>>>>>> +The 24-bit RGB value passed in follows the pattern 0xXXRRGGBB
>>>>>>>>>>>>>> +XX - Do not care ignored by the driver
>>>>>>>>>>>>>> +RR - is the 8 bit Red LED value
>>>>>>>>>>>>>> +GG - is the 8 bit Green LED value
>>>>>>>>>>>>>> +BB - is the 8 bit Blue LED value
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +Example:
>>>>>>>>>>>>>> +LED module output 4 of the LP5024 will be a yellow color:
>>>>>>>>>>>>>> +echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +LED module output 4 of the LP5024 will be dimmed 50%:
>>>>>>>>>>>>>> +echo 0x80 > /sys/class/leds/lp5024\:led4_mod/brightness
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +LED banked RGBs of the LP5036 will be a white color:
>>>>>>>>>>>>>> +echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color
>>>>>>>>>>>>>
>>>>>>>>>>>>> This part with example cans remain in Documentation/leds if you
>>>>>>>>>>>>>> like.
>>>>>>>>>>>>
>>>>>>>>>>>> Does it actually work like that on hardware?
>>>>>>>>>>>
>>>>>>>>>>> What?
>>>>>>>>>>
>>>>>>>>>> If you do echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color,
>>>>>>>>>> does it actually produce white? With all the different RGB modules
>>>>>>>>>> manufacturers can use with lp5024P?
>>>>>>>>>>
>>>>>>>>>> If you do echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color, does
>>>>>>>>>> it actually produce yellow, with all the different RGB modules
>>>>>>>>>> manufacturers can use with lp5024P?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I believe the answer to the general questions is no for any RGB cluster and driver out there.
>>>>>>>>> Because if you set the same values on each and every RGB device out there you will get varying shades of the color.
>>>>>>>>> But for this device yes the color does appear to be yellow to me versus what was displayed on my monitor by the HSL picker.
>>>>>>>>> But everyone interprets colors differently.
>>>>>>>>>
>>>>>>>>> If you write the same value for yellow or white on a droid 4 and the N900 do they produce the same color side by side?
>>>>>>>>> Most probably not.
>>>>>>>>>
>>>>>>>>> As you pointed out the PWM needs to be modified to obtain the correct white color to account for LED and other device constraints.
>>>>>>>>>
>>>>>>>>> But we need to take into account the light pipe. Pools nowadays have RGB LED spot lights in them. It can
>>>>>>>>> be set to white. On my pool right off the lens the color has a purplish hue to it. As the light is diffracted into
>>>>>>>>> the pool the color becomes white. The pool is clear. When I add chemicals to the pool and make it cloudy
>>>>>>>>> and turn on the lights the color off the lens is now white. This is an example on a large scale but the issue
>>>>>>>>> scales down to the hand helds and smart home applications.
>>>>>>>>>
>>>>>>>>> If the cluster is piped through a flexible optic 0xffffff may produce the "white" you want on its output.
>>>>>>>>>
>>>>>>>>> So an expectation of certain color without proper piping based on a single RGB value may be a little unreasonable.
>>>>>>>>> There may need to be a way to attenuate the values based on the hardware aspect of the equation ie light pipe (or lack thereof) and LED vendor.
>>>>>>>>> So if we write 0xffffff to the RGB driver the driver could adjust the intensity of the individual LEDs based on the diffraction
>>>>>>>>> coefficients.
>>>>>>>>>
>>>>>>>>> I also think that is an unreasonable expectation here that writing a single value to any LED RGB driver would produce
>>>>>>>>> a "rest of the world" absolute color. Maybe it can produce something similar but not identical.
>>>>>>>>> As you indicated in the requirements there is more involved here then just the LED and the values written.
>>>>>>>>> The colors should be close but may not be identical.
>>>>>>>>>
>>>>>>>>> A 10 year old N900 should not be considered the gold standard for color production due to advancements in LED,
>>>>>>>>> light pipe and LED driver technology.
>>>>>>>>> The single package RGB clusters on the board I am testing is about the size of a single RGB LED from 10 years ago.
>>>>>>>>>
>>>>>>>>> I agree that the interface developed should work on the device but the algorithm derived to obtain the color needs to have
>>>>>>>>> a hardware aspect to the calculation.
>>>>>>>>>
>>>>>>>>>>>> Is it supposed to support "normal" RGB colors as seen on monitors?
>>>>>>>>>>>
>>>>>>>>>>> Monitors are not an application for this part.
>>>>>>>>>>
>>>>>>>>>> You did not answer the question. When you talk about yellow, is it
>>>>>>>>>> same yellow the rest of world talks about?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> See above. It is close to what was on my monitor displayed.
>>>>>>>>>
>>>>>>>>>>>> Because 100% PWM on all channels does not result in white on hardware
>>>>>>>>>>>> I have.
>>>>>>>>>>>
>>>>>>>>>>> I don't know I am usually blinded by the light and have no diffuser over
>>>>>>>>>>> the LEDs to disperse the light so when I look I see all 3 colors.
>>>>>>>>>>
>>>>>>>>>> How can we have useful discussion about colors when you don't see the
>>>>>>>>>> colors?
>>>>>>>>>>
>>>>>>>>>> Place a piece of paper over the LEDs....
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Good suggestion for a rough test.
>>>>>>>>>
>>>>>>>>>>>> But...
>>>>>>>>>>>>
>>>>>>>>>>>> I believe we should have a reasonable design before we do something
>>>>>>>>>>>> like this. There's no guarantee someone will not use lp50xx with just
>>>>>>>>>>>> the white LEDs for example. How will this work? Plus existing hardware
>>>>>>>>>>>> already uses three separate LEDs for RGB LED. Why not provide same
>>>>>>>>>>>> interface?
>>>>>>>>>>>
>>>>>>>>>>> Which existing hardware? Are they using this part?
>>>>>>>>>>
>>>>>>>>>> Nokia N900. They are not using this part, but any interface we invent
>>>>>>>>>> should work there, too.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yes a common interface would be nice with some sort of hardware tuning coefficient.
>>>>>>>>>
>>>>>>>>>>> <rant>
>>>>>>>>>>> Why are we delaying getting the RGB framework or HSV in?
>>>>>>>>>>> I would rather design against something you want instead of having
>>>>>>>>>>> everyone complain about every implementation I post.
>>>>>>>>>>> </rant>
>>>>>>>>>>
>>>>>>>>>> Because you insist on creating new kernel interfaces, when existing
>>>>>>>>>> interfaces work, and are doing that badly.
>>>>>>>>>>
>>>>>>>>>> Because your patches are of lower quality than is acceptable for linux
>>>>>>>>>> kernel.
>>>>>>>>>>
>>>>>>>>>> Because you don't seem to be reading the emails.
>>>>>>>>>>
>>>>>>>>>> I sent list of requirements for RGB led support. This does not meet
>>>>>>>>>> them.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sigh. You did not answer my question.
>>>>>>>>>
>>>>>>>>> Your requirements seem to be centered around monitors but that is only one application of the current
>>>>>>>>> RGB LED landscape.
>>>>>>>>>
>>>>>>>>> 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.
>>>>>>>>
>>>>>>>> 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
>>>>>>>
>>>>>>> What about other colored LEDs? Presenting RGB for an Amber LED does not seem right.
>>>>>>> Should the LED color come from the DT?
>>>>>>>
>>>>>>
>>>>>> I thought about this, other non-RGB LEDs would not use the RGB framework.
>>>>>> But should they have the same interfaces as RGB?
>>>>>>
>>>>>> Should PWM control be a global interface?
>>>>>
>>>>> In order to being able to set multi color element led at one go I would recommend using then model:
>>>>>
>>>>> color_names: "red green blue white"
>>>>>
>>>>> echo "32 43 0 128" > color
>>>>>
>>>>> This way all elements would be set at same time from user space point of view.
>>>>>
>>>>> This of course requires that it is part of the physical/logical led that is being controlled. If it is a separate thing then it would logically be differently controlled mono color led.
>>>>>
>>>>> If you look what kinds of leds are available lets say from digikey you get all kinds of combos:
>>>>>
>>>>> - red, green, blue
>>>>> - red, green, blue, amber
>>>>> - red, green, blue, white
>>>>> - red, green, blue, white - cool
>>>>> - red, green, blue, white - neutral
>>>>> - red, green, blue, white - warm
>>>>> - red, orange
>>>>> - purple, ultraviolet
>>>>> - amber, blue
>>>>> - amber, blue, cyan, green, red, violet, white - cool
>>>>> - amber, blue, green
>>>>> - amber, green, blue
>>>>> - and then lots of single special colors
>>>>
>>>> It suggested me another solution. Instead of LED RGB class
>>>> we would have LED multi-color class.
>>>>
>>>
>>> I was thinking the same thing this morning. But I was thinking that the RGB
>>> class should be an additional class to stand on its own or can register to the
>>> multi color class.
>>>
>>>> Sysfs interface design:
>>>> -----------------------
>>>>
>>>> colors: directory containing files that map to
>>>>          the brightness of particular LEDs; there
>>>>          would be predefined color names that LED class
>>>>          driver should map iouts to, e.g.:
>>>
>>> Filling in the missing ideas with questions.
>>>
>>> Is it a directory or a file? If it is a directory does that not break the current
>>> directory label model?
>>>
>>> so the path would be /sys/class/leds/colors ? (It is probably not this but needs clarification)
>>> How would this look if I had 2 of the same color LEDs? The Beagle bone black has 2 Blue LEDs.
>>> They are assigned to different triggers and have different directories. Both are GPIO controlled.
>>>
>>> Or are you saying it would be something like (More then likely this is what you intended)
>>> /sys/class/leds/input4::numlock/colors?
>>
>> Yes, this is what I meant.
>>
>
> OK. Thanks for the clarification
>
>>> Maybe it is mandated that "multi" be added to the label when the class is registered so the caller
>>> knows that this is a multi color class and not a single LED color class.
>>
>> Like I am going to come up with standardized color names
>> in my LED naming related patch, the multi-color names
>> can be defined as well, e.g.: rgb, rgbw, rgbwa, rgbwauv etc.
>>
>
> That may be better it is descriptive off the command line.
>
>>> What about providing a file called colors_raw which takes in the raw decimal values to obtain other color
>>> variants when RGB is only available? And this can also present a single write to the kernel with the new
>>> color values.
>>
>> My design already covers that in the form of files in the colors
>> directory. Like I stated: "files that map to the brightness of
>> particular LEDs". Single write is secured by "echo "write" > sync.
>>
>
> OK. So set the new values and then tell the set the sync which will make the device driver write the
> device.
>
> Sounds good. how about echo 1 > sync and we can stay away from a long string conversion

We need also to be able to do a readout.

If we changed "sync" to "rw" then we could come up with intuitive
semantics:

echo 1 > rw // write
echo 0 > rw // read

rw file would have WO permission.

>>> I am not a fan of hard coding preset colors as you can see there are to many of them and variations of the color.
>>> In addition this severely limits the ability of the user. Unless we stick to primary colors only and not secondary
>>> colors.
>>> Defining and hard coding hte colors can get out of control and not maintainable moving forward. Especially
>>> if we start adding defines like white_warm, white_neutral and other variations to the color.
>>
>> We would not limit anything. Every color combination can be achieved
>> by following this exemplary sequence:
>>
>> $ echo 154 > colors/red
>> $ echo 232 > colors/green
>> $ echo 43 > colors/blue
>> # echo "write" > sync     //only this changes hardware state
>>
>> brightness-model is provided only to define mapping of legacy brightness
>> levels (governed by brightness file and led_set_brightness() API) to
>> the specific combination of colors.
>>
>> For instance we can define three brightness levels for green hue:
>>
>> DT definition for it would look like below:
>>
>> rgb-green = <0x277c33 0x37b048 0x47e45d>;
>>
>> LED multi color class would then do the following mapping for
>> each of the three brightness levels for rgb-green brightness model:
>>
>> $ echo rgb-green > brightness_model
>> $ echo 1 > brightness // red=0x27, green=0x7c, blue=0x33
>> $ echo 2 > brightness // red=0x37, green=0xb0, blue=0x48
>> $ echo 3 > brightness // red=0x47, green=0xe4, blue=0x5d
>>
>
> OK I would have to play with this on the LP devices.
>
>>> What about IR LEDs used for night vision products? Do these fall into the multi color class?
>>> We do have a driver I submitted that had an IR LED and a White LED combined. It was created against the
>>> flash class but it could be a multi color LED as well.
>>>
>>> How would traversing through the full color palette work if you were to want to produce a multi
>>> color ring like the LP50xx where the pattern can traverse from one end of the color spectrum and back?
>>> Or in a product like the gaming keyboards that will change color or change backlight brightness?
>>
>> This is not meant as a solution for pattern generator but for
>> consolidated source of multi color light. In the simplest case
>> RGB LED elements like those used for lp5024 reference board,
>> but it could be RGBWAUV LED [0] as well.
>>
>> For patterns traversing many LEDs I see a trigger as the best solution.
>> Hmm, now I see that trigger mechanism actually can serve as very
>> flexible pattern generator.
>>
>> We would need a device that could be configured to register
>> a number of multi-led-patternN triggers, one per LED, and generate
>> events for each trigger in a loop.
>>
>> The device would have to allow for configuring pattern intervals
>> via sysfs, like in case of current pattern trigger.
>>
>> LED class devices would have to register for its events:
>>
>> $/sys/class/leds/led1 echo multi-led-pattern1 > trigger
>> $/sys/class/leds/led2 echo multi-led-pattern2 > trigger
>> $/sys/class/leds/led3 echo multi-led-pattern3 > trigger
>>
>
> A bit off topic but I like the idea. We should save this for another day

Yes, that's another story.

>> The ability to define brightness models in DT would
>> add even more flexibility.
>>
>
> brightness models would be mandatory to support in the driver but an optional
> DT entry.
>
> Is that a correct assumption?

Correct.

>>> Not sure what color LEDs the keyboard manufacturers place on their keyboards but does the interface design
>>> capable of doing this?
>>>
>>> https://www.youtube.com/watch?v=pfKv3g2FeBE
>>> or something like this
>>>
>>> https://www.youtube.com/watch?v=PO3auX3f5C4
>>>
>>> The LP5036 has this capability.
>>>
>>>>    - red
>>>>    - green
>>>>    - blue
>>>>    - white
>>>>    - sync: accepts "write" and "read", which executes
>>>>            write/readout to/from a device respectively.
>>>>
>>>
>>> What are these above, the values or the files under the colors directory?
>>
>> They are just color specific counterparts of monochrome brightness.
>> In terms of lp50xx they would map to OUTn_COLOR registers.
>>
>>> I am assuming they are files.
>>
>> Right.
>>
>>> Are they mandatory or optional?
>>
>> Mandatory, one per each iout to control.
>>
>
> OK And all the LEDs within this directory would be considered a LED cluster?
>
> And if there are 2 like colors of the LED defined in the same cluster would we just see a single
> file and write a common value to that file and the driver would have to update each
> red LED within that cluster. No independent control of like colored LEDs within the
> registered LED cluster.
>
> If the developer wants this level of control they would have to register two separate classes
>
> Correct?

I would abide by one color to one iout mapping. Registering more LEDs
under the same color feels more of a task for trigger.

>>>> brightness-model: defines brightness level to color configuration
>>>>                    mapping
>>>>    - "hardware": for devices with feature like LEDn_BRIGHTNESS of lp50xx
>>>>    - "rgb-<hue>": available only when all three red,green,blue colors
>>>>                   are present in the colors directory.
>>>>                   <hue> is a placeholder for given DT hue presets.
>>>>    - "rgbw-<hue>": Same as above, but available only when white color
>>>>                   (any of amber or white-{cool|neutral|warm} can be
>>>>                   mapped to white) is also available. In this mode
>>>>                   max_brightness equals num-of-hue-presets + 1, and
>>>>                   max_brightness, when set, turns the "white" LED on
>>>
>>> Why do we need white combined here? Should this not be its own entity?
>>
>> To be able to set white color. We're still talking about one LED
>> element (although they can be be physically few LEDs in one case).
>> This is brightness file, so we've got to stick to the semantics.
>> Max brightness level should be the brightest. With RGBW LEDs we
>> fortunately have a means to achieve pure white, that's why
>> rgbw-<hue> would be beneficial. If you increase L component of
>> HSL color space, the max value gives white for all hues.
>> So maybe this brightness-model would be rather called hsl-<hue>.
>>
>> For RGBW LEDs, we would have to allow for more shades of white too,
>> like in [1].
>>
>
> Yep.

But this all would be left to the DT designer decision.

>>> Again I don't like limiting the color palette from the DT. I think that the
>>> user space can see what colors are available for that device and makes its own
>>> decision on what color to present.
>>>
>>> For the RGBw what about RGB amber and RGB purple. Are the white LEDs always part of the
>>> same function trying to be achieved by the system designer? The RGB can be used to denote
>>> notification status and the white can be used to denote that a charger is connected. Motorola
>>> Droid did this.
>>
>> I hope I've just clarified my idea.
>>
>
> Its getting clearer. I would like to see it in code and play with it not as a user
> but as a developer. Make sure the paper model works as well as the real implementation.
>
> Is this model clear to the developer?
> How would a developer define what values are appropriate for the brightness-model?

We could create guidelines e.g. that for hsl-<hue> pattern, the
colors corresponding to brightness levels should be arranged so
that increasing brightness felt like increasing value of
L component of HSL.

But we wouldn't be able to enforce adherence to a particular scheme.

> Does the driver have to become overly complex to support simple color generation?

Not at all. Caching the colors written to the files in colors directory
would be responsibility of LED multi-color class. On echo 1 > sync
the core would call a new op e.g.

color_set(struct led_classdev_multicolor *mcled_cdev)

Driver would read in it colors cached in struct led_classdev_multicolor
and write them to the hardware.

> Thoughts on putting code to idea?
>
>
>>>
>>>>    - "rgb-linear": I'm not sure if it should be available - it will
>>>>                    have unpredictable results
>>>>
>>>> brightness: sets/reads brightness in the way specific to the
>>>>              current brightness-model. When more colors are available
>>>>              (e.g. amber, blue, cyan, green, red, violet, white), they
>>>>              are not touched by write to brightness).
>>>>
>>>> HSV->RGB conversion is left entirely to the userspace, which can set any
>>>> color with use of the proposed interface.
>>>>
>>>> Let's agree on sysfs interface first, and only after that move
>>>> to the DT details.
>>>>
>>>
>>> DT's are meant to describe hardware and not describe the product. Unless Rob does not see
>>> an issue with defining product capabilities in the DT then we should keep that out of the DT.
>>
>> LED element is a device. I see nothing irrelevant for DT in describing the lighting specificity of a device mounted on the board. Please keep
>> in mind that it will not limit the number of colors available to set.
>> It will only allow to define mapping of brightness level to color.
>> We need that for current trigger mechanism to work with LED multi-color
>> class.
>>
>
> I see this now.
>
> Dan
>
>> [0] http://www.cobledarray.com/sale-4942931-10w-rgbwauv-led-diode-6-in-1-high-power-multicolor-led-chip.html
>> [1] https://www.youtube.com/watch?v=NzlFmTqOh9M
>>
>
>

--
Best regards,
Jacek Anaszewski

2019-01-28 23:05:05

by Pavel Machek

[permalink] [raw]

Subject: Re: RGB LED class Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Hi!

> >>What algorithm would be used for mapping brightness levels to RGB values
> >>in case of devices without hardware support for that?
> >
> >Output power = brightness / max_brightness * pwm_channel[x].
>
> IIUC you mean it as a formula for calculating r,g,b, values?
> I.e., on brightness setting we would have to do this calculation for
> each of three channels?
>
> Then, it will result in changing hue as well. That's why we're
> discussing HSV.

It should not change the hue, AFAICT, modulo the rounding errors.

> >>s/pwm/color/
> >
> >s/pwm/power/ would work for me.
>
> Power implies physical units. I'd prefer "intensity".

Intensity would work for me, too.

> >>Besides white also other color presets could be defined in DT.
> >
> >They should not be neccessary. When userspace knows what is white and
> >that power is linear with values in power_channels, it should be able
> >to do colorspace conversion itself.
>
> Have you verified it in practice? Would it allow to convert RGB values
> of the color displayed on the monitor to LED RGB class intensities,
> allowing to achieve similar color on the LED?

Yes, I think so.

My code is in unicsy_demo repository, in monitor/notint.py .

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

Attachments:

(No filename) (1.37 kB)
signature.asc (188.00 B)
Digital signature Download all attachments

2019-01-28 23:05:37

Hi!

Could you, trim the parts of email you do not need in the reply? This
is quite annoying to follow:

On Tue 2019-01-29 14:26:20, Dan Murphy wrote:
> Jacek
>
> On 1/29/19 2:19 PM, Jacek Anaszewski wrote:
> > Hi Dan,
> >
> > On 1/29/19 2:56 PM, Dan Murphy wrote:
> >> Jacek
> >>
> >> On 1/24/19 3:55 PM, Jacek Anaszewski wrote:
> >>> Dan
> >>>
> >>> On 1/24/19 10:00 PM, Dan Murphy wrote:
> >>>> Jacek
> >>>>
> >>>> On 1/23/19 3:52 PM, Jacek Anaszewski wrote:
> >>>>> Dan,
> >>>>>
> >>>>> On 1/22/19 11:44 PM, Dan Murphy wrote:
> >>>>>> Jacek
> >>>>>>
> >>>>>> On 1/22/19 3:39 PM, Jacek Anaszewski wrote:
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> On 1/20/19 7:42 AM, Vesa J??skel?inen wrote:
> >>>>>>>> Hi Dan,
> >>>>>>>>
> >>>>>>>> On 18/01/2019 15.58, Dan Murphy wrote:
> >>>>>>>>> Jacek
> >>>>>>>>>
> >>>>>>>>> On 1/18/19 7:45 AM, Dan Murphy wrote:
> >>>>>>>>>> Jacek
> >>>>>>>>>>
> >>>>>>>>>> On 1/17/19 3:10 PM, Jacek Anaszewski wrote:
> >>>>>>>>>>> Hi Dan,
> >>>>>>>>>>>
> >>>>>>>>>>> On 1/16/19 7:41 PM, Dan Murphy wrote:
> >>>>>>>>>>>> Hello
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 1/16/19 4:55 AM, Pavel Machek wrote:
> >>>>>>>>>>>>> Hi!
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> On 1/15/19 4:22 PM, Pavel Machek wrote:
> >>>>>>>>>>>>>>> Hi!
> >>>>>>>>>>>>>>>
...

> >> If this is OK I can post the patch with this update.
> >>
> >> But I would rather put the file creation in a class and have the colors directory.
> >
> > Yes, please post your work when it is ready.
>
> OK. I will derive the class and try to post it by the EOW.

You can do that, but if you post the description of kernel<->user
interface, first, you may save yourself a bit of coding...

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

Attachments:

(No filename) (1.84 kB)
signature.asc (188.00 B)
Digital signature Download all attachments

2019-01-29 21:49:27

by Dan Murphy

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Pavel

On 1/29/19 3:45 PM, Pavel Machek wrote:
> Hi!
>
> Could you, trim the parts of email you do not need in the reply? This
> is quite annoying to follow:
>

Agreed.

> On Tue 2019-01-29 14:26:20, Dan Murphy wrote:
>> Jacek
>>
>> On 1/29/19 2:19 PM, Jacek Anaszewski wrote:
>>> Hi Dan,
>>>
>>> On 1/29/19 2:56 PM, Dan Murphy wrote:
>>>> Jacek
>>>>
>>>> On 1/24/19 3:55 PM, Jacek Anaszewski wrote:
>>>>> Dan
>>>>>
>>>>> On 1/24/19 10:00 PM, Dan Murphy wrote:
>>>>>> Jacek
>>>>>>
>>>>>> On 1/23/19 3:52 PM, Jacek Anaszewski wrote:
>>>>>>> Dan,
>>>>>>>
>>>>>>> On 1/22/19 11:44 PM, Dan Murphy wrote:
>>>>>>>> Jacek
>>>>>>>>
>>>>>>>> On 1/22/19 3:39 PM, Jacek Anaszewski wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> On 1/20/19 7:42 AM, Vesa J??skel?inen wrote:
>>>>>>>>>> Hi Dan,
>>>>>>>>>>
>>>>>>>>>> On 18/01/2019 15.58, Dan Murphy wrote:
>>>>>>>>>>> Jacek
>>>>>>>>>>>
>>>>>>>>>>> On 1/18/19 7:45 AM, Dan Murphy wrote:
>>>>>>>>>>>> Jacek
>>>>>>>>>>>>
>>>>>>>>>>>> On 1/17/19 3:10 PM, Jacek Anaszewski wrote:
>>>>>>>>>>>>> Hi Dan,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 1/16/19 7:41 PM, Dan Murphy wrote:
>>>>>>>>>>>>>> Hello
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 1/16/19 4:55 AM, Pavel Machek wrote:
>>>>>>>>>>>>>>> Hi!
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 1/15/19 4:22 PM, Pavel Machek wrote:
>>>>>>>>>>>>>>>>> Hi!
>>>>>>>>>>>>>>>>>
> ...
>
>>>> If this is OK I can post the patch with this update.
>>>>
>>>> But I would rather put the file creation in a class and have the colors directory.
>>>
>>> Yes, please post your work when it is ready.
>>
>> OK. I will derive the class and try to post it by the EOW.
>
> You can do that, but if you post the description of kernel<->user
> interface, first, you may save yourself a bit of coding...
>

OK I will put together the interface doc first and push a RFC.
Then put some code together

Dan

> Thanks,
> Pavel
>

--
------------------
Dan Murphy

2019-01-29 21:53:51

by Jacek Anaszewski

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

Dan,

On 1/29/19 9:26 PM, Dan Murphy wrote:
> Jacek
>
> On 1/29/19 2:19 PM, Jacek Anaszewski wrote:
>> Hi Dan,
>>
>> On 1/29/19 2:56 PM, Dan Murphy wrote:
>>> Jacek
>>>
>>> On 1/24/19 3:55 PM, Jacek Anaszewski wrote:
>>>> Dan
>>>>
>>>> On 1/24/19 10:00 PM, Dan Murphy wrote:
>>>>> Jacek
>>>>>
>>>>> On 1/23/19 3:52 PM, Jacek Anaszewski wrote:
>>>>>> Dan,
>>>>>>
>>>>>> On 1/22/19 11:44 PM, Dan Murphy wrote:
>>>>>>> Jacek
>>>>>>>
>>>>>>> On 1/22/19 3:39 PM, Jacek Anaszewski wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> On 1/20/19 7:42 AM, Vesa Jääskeläinen wrote:
>>>>>>>>> Hi Dan,
>>>>>>>>>
>>>>>>>>> On 18/01/2019 15.58, Dan Murphy wrote:
>>>>>>>>>> Jacek
>>>>>>>>>>
>>>>>>>>>> On 1/18/19 7:45 AM, Dan Murphy wrote:
>>>>>>>>>>> Jacek
>>>>>>>>>>>
>>>>>>>>>>> On 1/17/19 3:10 PM, Jacek Anaszewski wrote:
>>>>>>>>>>>> Hi Dan,
>>>>>>>>>>>>
>>>>>>>>>>>> On 1/16/19 7:41 PM, Dan Murphy wrote:
>>>>>>>>>>>>> Hello
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 1/16/19 4:55 AM, Pavel Machek wrote:
>>>>>>>>>>>>>> Hi!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 1/15/19 4:22 PM, Pavel Machek wrote:
>>>>>>>>>>>>>>>> Hi!
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> +The 24-bit RGB value passed in follows the pattern 0xXXRRGGBB
>>>>>>>>>>>>>>>>>> +XX - Do not care ignored by the driver
>>>>>>>>>>>>>>>>>> +RR - is the 8 bit Red LED value
>>>>>>>>>>>>>>>>>> +GG - is the 8 bit Green LED value
>>>>>>>>>>>>>>>>>> +BB - is the 8 bit Blue LED value
>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>> +Example:
>>>>>>>>>>>>>>>>>> +LED module output 4 of the LP5024 will be a yellow color:
>>>>>>>>>>>>>>>>>> +echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color
>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>> +LED module output 4 of the LP5024 will be dimmed 50%:
>>>>>>>>>>>>>>>>>> +echo 0x80 > /sys/class/leds/lp5024\:led4_mod/brightness
>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>> +LED banked RGBs of the LP5036 will be a white color:
>>>>>>>>>>>>>>>>>> +echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> This part with example cans remain in Documentation/leds if you
>>>>>>>>>>>>>>>>>> like.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Does it actually work like that on hardware?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> What?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If you do echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color,
>>>>>>>>>>>>>> does it actually produce white? With all the different RGB modules
>>>>>>>>>>>>>> manufacturers can use with lp5024P?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If you do echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color, does
>>>>>>>>>>>>>> it actually produce yellow, with all the different RGB modules
>>>>>>>>>>>>>> manufacturers can use with lp5024P?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I believe the answer to the general questions is no for any RGB cluster and driver out there.
>>>>>>>>>>>>> Because if you set the same values on each and every RGB device out there you will get varying shades of the color.
>>>>>>>>>>>>> But for this device yes the color does appear to be yellow to me versus what was displayed on my monitor by the HSL picker.
>>>>>>>>>>>>> But everyone interprets colors differently.
>>>>>>>>>>>>>
>>>>>>>>>>>>> If you write the same value for yellow or white on a droid 4 and the N900 do they produce the same color side by side?
>>>>>>>>>>>>> Most probably not.
>>>>>>>>>>>>>
>>>>>>>>>>>>> As you pointed out the PWM needs to be modified to obtain the correct white color to account for LED and other device constraints.
>>>>>>>>>>>>>
>>>>>>>>>>>>> But we need to take into account the light pipe. Pools nowadays have RGB LED spot lights in them. It can
>>>>>>>>>>>>> be set to white. On my pool right off the lens the color has a purplish hue to it. As the light is diffracted into
>>>>>>>>>>>>> the pool the color becomes white. The pool is clear. When I add chemicals to the pool and make it cloudy
>>>>>>>>>>>>> and turn on the lights the color off the lens is now white. This is an example on a large scale but the issue
>>>>>>>>>>>>> scales down to the hand helds and smart home applications.
>>>>>>>>>>>>>
>>>>>>>>>>>>> If the cluster is piped through a flexible optic 0xffffff may produce the "white" you want on its output.
>>>>>>>>>>>>>
>>>>>>>>>>>>> So an expectation of certain color without proper piping based on a single RGB value may be a little unreasonable.
>>>>>>>>>>>>> There may need to be a way to attenuate the values based on the hardware aspect of the equation ie light pipe (or lack thereof) and LED vendor.
>>>>>>>>>>>>> So if we write 0xffffff to the RGB driver the driver could adjust the intensity of the individual LEDs based on the diffraction
>>>>>>>>>>>>> coefficients.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I also think that is an unreasonable expectation here that writing a single value to any LED RGB driver would produce
>>>>>>>>>>>>> a "rest of the world" absolute color. Maybe it can produce something similar but not identical.
>>>>>>>>>>>>> As you indicated in the requirements there is more involved here then just the LED and the values written.
>>>>>>>>>>>>> The colors should be close but may not be identical.
>>>>>>>>>>>>>
>>>>>>>>>>>>> A 10 year old N900 should not be considered the gold standard for color production due to advancements in LED,
>>>>>>>>>>>>> light pipe and LED driver technology.
>>>>>>>>>>>>> The single package RGB clusters on the board I am testing is about the size of a single RGB LED from 10 years ago.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I agree that the interface developed should work on the device but the algorithm derived to obtain the color needs to have
>>>>>>>>>>>>> a hardware aspect to the calculation.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Is it supposed to support "normal" RGB colors as seen on monitors?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Monitors are not an application for this part.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> You did not answer the question. When you talk about yellow, is it
>>>>>>>>>>>>>> same yellow the rest of world talks about?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> See above. It is close to what was on my monitor displayed.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Because 100% PWM on all channels does not result in white on hardware
>>>>>>>>>>>>>>>> I have.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I don't know I am usually blinded by the light and have no diffuser over
>>>>>>>>>>>>>>> the LEDs to disperse the light so when I look I see all 3 colors.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> How can we have useful discussion about colors when you don't see the
>>>>>>>>>>>>>> colors?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Place a piece of paper over the LEDs....
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Good suggestion for a rough test.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> But...
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I believe we should have a reasonable design before we do something
>>>>>>>>>>>>>>>> like this. There's no guarantee someone will not use lp50xx with just
>>>>>>>>>>>>>>>> the white LEDs for example. How will this work? Plus existing hardware
>>>>>>>>>>>>>>>> already uses three separate LEDs for RGB LED. Why not provide same
>>>>>>>>>>>>>>>> interface?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Which existing hardware? Are they using this part?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Nokia N900. They are not using this part, but any interface we invent
>>>>>>>>>>>>>> should work there, too.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yes a common interface would be nice with some sort of hardware tuning coefficient.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <rant>
>>>>>>>>>>>>>>> Why are we delaying getting the RGB framework or HSV in?
>>>>>>>>>>>>>>> I would rather design against something you want instead of having
>>>>>>>>>>>>>>> everyone complain about every implementation I post.
>>>>>>>>>>>>>>> </rant>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Because you insist on creating new kernel interfaces, when existing
>>>>>>>>>>>>>> interfaces work, and are doing that badly.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Because your patches are of lower quality than is acceptable for linux
>>>>>>>>>>>>>> kernel.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Because you don't seem to be reading the emails.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I sent list of requirements for RGB led support. This does not meet
>>>>>>>>>>>>>> them.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Sigh. You did not answer my question.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Your requirements seem to be centered around monitors but that is only one application of the current
>>>>>>>>>>>>> RGB LED landscape.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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.
>>>>>>>>>>>>
>>>>>>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>> What about other colored LEDs? Presenting RGB for an Amber LED does not seem right.
>>>>>>>>>>> Should the LED color come from the DT?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I thought about this, other non-RGB LEDs would not use the RGB framework.
>>>>>>>>>> But should they have the same interfaces as RGB?
>>>>>>>>>>
>>>>>>>>>> Should PWM control be a global interface?
>>>>>>>>>
>>>>>>>>> In order to being able to set multi color element led at one go I would recommend using then model:
>>>>>>>>>
>>>>>>>>> color_names: "red green blue white"
>>>>>>>>>
>>>>>>>>> echo "32 43 0 128" > color
>>>>>>>>>
>>>>>>>>> This way all elements would be set at same time from user space point of view.
>>>>>>>>>
>>>>>>>>> This of course requires that it is part of the physical/logical led that is being controlled. If it is a separate thing then it would logically be differently controlled mono color led.
>>>>>>>>>
>>>>>>>>> If you look what kinds of leds are available lets say from digikey you get all kinds of combos:
>>>>>>>>>
>>>>>>>>> - red, green, blue
>>>>>>>>> - red, green, blue, amber
>>>>>>>>> - red, green, blue, white
>>>>>>>>> - red, green, blue, white - cool
>>>>>>>>> - red, green, blue, white - neutral
>>>>>>>>> - red, green, blue, white - warm
>>>>>>>>> - red, orange
>>>>>>>>> - purple, ultraviolet
>>>>>>>>> - amber, blue
>>>>>>>>> - amber, blue, cyan, green, red, violet, white - cool
>>>>>>>>> - amber, blue, green
>>>>>>>>> - amber, green, blue
>>>>>>>>> - and then lots of single special colors
>>>>>>>>
>>>>>>>> It suggested me another solution. Instead of LED RGB class
>>>>>>>> we would have LED multi-color class.
>>>>>>>>
>>>>>>>
>>>>>>> I was thinking the same thing this morning. But I was thinking that the RGB
>>>>>>> class should be an additional class to stand on its own or can register to the
>>>>>>> multi color class.
>>>>>>>
>>>>>>>> Sysfs interface design:
>>>>>>>> -----------------------
>>>>>>>>
>>>>>>>> colors: directory containing files that map to
>>>>>>>>            the brightness of particular LEDs; there
>>>>>>>>            would be predefined color names that LED class
>>>>>>>>            driver should map iouts to, e.g.:
>>>>>>>
>>>>>>> Filling in the missing ideas with questions.
>>>>>>>
>>>>>>> Is it a directory or a file? If it is a directory does that not break the current
>>>>>>> directory label model?
>>>>>>>
>>>>>>> so the path would be /sys/class/leds/colors ? (It is probably not this but needs clarification)
>>>>>>> How would this look if I had 2 of the same color LEDs? The Beagle bone black has 2 Blue LEDs.
>>>>>>> They are assigned to different triggers and have different directories. Both are GPIO controlled.
>>>>>>>
>>>>>>> Or are you saying it would be something like (More then likely this is what you intended)
>>>>>>> /sys/class/leds/input4::numlock/colors?
>>>>>>
>>>>>> Yes, this is what I meant.
>>>>>>
>>>>>
>>>>> OK. Thanks for the clarification
>>>>>
>>>>>>> Maybe it is mandated that "multi" be added to the label when the class is registered so the caller
>>>>>>> knows that this is a multi color class and not a single LED color class.
>>>>>>
>>>>>> Like I am going to come up with standardized color names
>>>>>> in my LED naming related patch, the multi-color names
>>>>>> can be defined as well, e.g.: rgb, rgbw, rgbwa, rgbwauv etc.
>>>>>>
>>>>>
>>>>> That may be better it is descriptive off the command line.
>>>>>
>>>>>>> What about providing a file called colors_raw which takes in the raw decimal values to obtain other color
>>>>>>> variants when RGB is only available? And this can also present a single write to the kernel with the new
>>>>>>> color values.
>>>>>>
>>>>>> My design already covers that in the form of files in the colors
>>>>>> directory. Like I stated: "files that map to the brightness of
>>>>>> particular LEDs". Single write is secured by "echo "write" > sync.
>>>>>>
>>>>>
>>>>> OK. So set the new values and then tell the set the sync which will make the device driver write the
>>>>> device.
>>>>>
>>>>> Sounds good. how about echo 1 > sync and we can stay away from a long string conversion
>>>>
>>>> We need also to be able to do a readout.
>>>>
>>>> If we changed "sync" to "rw" then we could come up with intuitive
>>>> semantics:
>>>>
>>>> echo 1 > rw // write
>>>> echo 0 > rw // read
>>>>
>>>> rw file would have WO permission.
>>>
>>> I have re-written the lp50xx driver to show red, green, blue, sync and sync_enable files for each registered LED.
>>
>> Did you change your mind regarding LED multi color class implementation?
>>
>
> No. I just did not want to put effort into the class if the interfaces were not agreed upon.
> So if we are good with the interfaces I can start the MC class.
I'm good with that. It would be nice to see some input from the
other people.

>>> I added the sync_enable file so that the there can be real time control of the individual LEDs as well.
>>
>> Yes, it's a good idea.
>>
>>> Turning on sync means that the user will write to one of the color files and the setting won't take affect until
>>> sync is written.
>>>
>>> If sync is off then the LED register is updated immediately.
>>>
>>> Being able to turn on and off syncing maybe better. A developer may choose to set up the color and then sync
>>> or they may want to ramp a specific color. This will help reduce user space writes but also reduce the number
>>> of peripheral writes when color values do not change.
>>>
>>> I am having difficulty in creating the colors directory in the device driver.
>>> This appears to need to be done in the class when the class pointer is available.
>>
>> Not necessarily, you need only dev->kobj. Please follow the implementation of drivers/usb/core/ledtrig-usbport.c. However I would
>> really prefer if it became a part of new LED multi color class.
>>
>
> It will. I should be able to create the colors directory there as well.
> I need to figure out how to cleanly tell the MC class what LEDs are available.

Of course the solution from ledtrig-usbport.c is equally good for the
class.

Related to the LED color identifications in the multi color class -
we'd need some defines in e.g. include/dt-bindings/leds/colors.h:

#define LED_COLOR_RED 0
#define LED_COLOR_GREEN 1
#define LED_COLOR_BLUE 2

Then, in the device tree we could have:

led-controller@8 {
...
led@0 {
multi-color = <LED_COLOR_RED
LED_COLOR_GREEN
LED_COLOR_BLUE>;
reg = <0x0>; // in case of lp5024 this would identify
// RGB LED module;
};
};

For pwm LEDs we would need a bit different approach, so I'd not
strive to create generic bindings for LED multi color class.

We need generic interface only for LED multi color class registration.
e.g.:

struct led_classdev_multicolor {
/* led class device */
struct led_classdev led_cdev; // for legacy brightness support
struct led_classdev *colors; // dynamically allocated array

void (*multicolor_set) (struct led_classdev_multicolor *led_mcdev);
void (*unicolor_set) (struct led_classdev *led_cdev,
enum led_brightness); // [color]_store ssyfs
// callback will call it
};

led_classdev_multicolor_register(struct device *parent,
struct led_classdev_multicolor *led_mcdev)
...

>>>>>>> I am not a fan of hard coding preset colors as you can see there are to many of them and variations of the color.
>>>>>>> In addition this severely limits the ability of the user. Unless we stick to primary colors only and not secondary
>>>>>>> colors.
>>>>>>> Defining and hard coding hte colors can get out of control and not maintainable moving forward. Especially
>>>>>>> if we start adding defines like white_warm, white_neutral and other variations to the color.
>>>>>>
>>>>>> We would not limit anything. Every color combination can be achieved
>>>>>> by following this exemplary sequence:
>>>>>>
>>>>>> $ echo 154 > colors/red
>>>>>> $ echo 232 > colors/green
>>>>>> $ echo 43 > colors/blue
>>>>>> # echo "write" > sync     //only this changes hardware state
>>>>>>
>>>
>>> The code works more like this
>>> $ :/sys/class/leds/lp5024:led1_mod# ls
>>> blue            device          max_brightness red             sync            trigger
>>> brightness      green           power           subsystem       sync_enable     uevent
>>>
>>> $ cat sync_enable
>>> enabled        // I can change this to return an int instead
>>> $ echo 154 > red
>>> $ echo 232 > green
>>> $ echo 43 > blue
>>> $ echo 1 > sync     //this changes hardware state
>>>
>>> $ echo 0 > sync_enable
>>> $ cat sync_enable
>>> disabled
>>> $ echo 154 > red    // changes red LED immediately
>>> $ echo 232 > green    // changes green LED immediately
>>> $ echo 43 > blue    // changes blue LED immediately
>>> $ echo 1 > sync     // Has no affect on the hardware
>>>
>>> If this is OK I can post the patch with this update.
>>>
>>> But I would rather put the file creation in a class and have the colors directory.
>>
>> Yes, please post your work when it is ready.
>
> OK. I will derive the class and try to post it by the EOW.
>
>>
>>>>>> brightness-model is provided only to define mapping of legacy brightness
>>>>>> levels (governed by brightness file and led_set_brightness() API) to
>>>>>> the specific combination of colors.
>>>>>>
>>>>>> For instance we can define three brightness levels for green hue:
>>>>>>
>>>>>> DT definition for it would look like below:
>>>>>>
>>>>>> rgb-green = <0x277c33 0x37b048 0x47e45d>;
>>>>>>
>>>>>> LED multi color class would then do the following mapping for
>>>>>> each of the three brightness levels for rgb-green brightness model:
>>>>>>
>>>>>> $ echo rgb-green > brightness_model
>>>>>> $ echo 1 > brightness // red=0x27, green=0x7c, blue=0x33
>>>>>> $ echo 2 > brightness // red=0x37, green=0xb0, blue=0x48
>>>>>> $ echo 3 > brightness // red=0x47, green=0xe4, blue=0x5d
>>>>>>
>>>>>
>>>>> OK I would have to play with this on the LP devices.
>>>
>>> I have not done anything with this.
>>>
>
> I will omit this until we figure out how to nicely present this to user space from the MC class.

Could you share what particularly is your concern?

> Or maybe it is something you can add once we have the class ready.

OK. But for the cases when neither LEDn_BRIGHTNESS feature nor
any DT color ranges are provided please do provide one brightness
model "onoff", that for brightness==1 will set brightness of all colors
to max, and turn them off on brightness==0.

>>>>>>> What about IR LEDs used for night vision products? Do these fall into the multi color class?
>>>>>>> We do have a driver I submitted that had an IR LED and a White LED combined. It was created against the
>>>>>>> flash class but it could be a multi color LED as well.
>>>>>>>
>>>>>>> How would traversing through the full color palette work if you were to want to produce a multi
>>>>>>> color ring like the LP50xx where the pattern can traverse from one end of the color spectrum and back?
>>>>>>> Or in a product like the gaming keyboards that will change color or change backlight brightness?
>>>>>>
>>>>>> This is not meant as a solution for pattern generator but for
>>>>>> consolidated source of multi color light. In the simplest case
>>>>>> RGB LED elements like those used for lp5024 reference board,
>>>>>> but it could be RGBWAUV LED [0] as well.
>>>>>>
>>>>>> For patterns traversing many LEDs I see a trigger as the best solution.
>>>>>> Hmm, now I see that trigger mechanism actually can serve as very
>>>>>> flexible pattern generator.
>>>>>>
>>>>>> We would need a device that could be configured to register
>>>>>> a number of multi-led-patternN triggers, one per LED, and generate
>>>>>> events for each trigger in a loop.
>>>>>>
>>>>>> The device would have to allow for configuring pattern intervals
>>>>>> via sysfs, like in case of current pattern trigger.
>>>>>>
>>>>>> LED class devices would have to register for its events:
>>>>>>
>>>>>> $/sys/class/leds/led1 echo multi-led-pattern1 > trigger
>>>>>> $/sys/class/leds/led2 echo multi-led-pattern2 > trigger
>>>>>> $/sys/class/leds/led3 echo multi-led-pattern3 > trigger
>>>>>>
>>>>>
>>>>> A bit off topic but I like the idea. We should save this for another day
>>>>
>>>> Yes, that's another story.
>>>>
>>>>>> The ability to define brightness models in DT would
>>>>>> add even more flexibility.
>>>>>>
>>>>>
>>>>> brightness models would be mandatory to support in the driver but an optional
>>>>> DT entry.
>>>>>
>>>>> Is that a correct assumption?
>>>>
>>>> Correct.
>>>>
>>>>>>> Not sure what color LEDs the keyboard manufacturers place on their keyboards but does the interface design
>>>>>>> capable of doing this?
>>>>>>>
>>>>>>> https://www.youtube.com/watch?v=pfKv3g2FeBE
>>>>>>> or something like this
>>>>>>>
>>>>>>> https://www.youtube.com/watch?v=PO3auX3f5C4
>>>>>>>
>>>>>>> The LP5036 has this capability.
>>>>>>>
>>>>>>>>      - red
>>>>>>>>      - green
>>>>>>>>      - blue
>>>>>>>>      - white
>>>>>>>>      - sync: accepts "write" and "read", which executes
>>>>>>>>              write/readout to/from a device respectively.
>>>>>>>>
>>>>>>>
>>>>>>> What are these above, the values or the files under the colors directory?
>>>>>>
>>>>>> They are just color specific counterparts of monochrome brightness.
>>>>>> In terms of lp50xx they would map to OUTn_COLOR registers.
>>>>>>
>>>>>>> I am assuming they are files.
>>>>>>
>>>>>> Right.
>>>>>>
>>>>>>> Are they mandatory or optional?
>>>>>>
>>>>>> Mandatory, one per each iout to control.
>>>>>>
>>>>>
>>>>> OK And all the LEDs within this directory would be considered a LED cluster?
>>>>>
>>>>> And if there are 2 like colors of the LED defined in the same cluster would we just see a single
>>>>> file and write a common value to that file and the driver would have to update each
>>>>> red LED within that cluster. No independent control of like colored LEDs within the
>>>>> registered LED cluster.
>>>>>
>>>>> If the developer wants this level of control they would have to register two separate classes
>>>>>
>>>>> Correct?
>>>>
>>>> I would abide by one color to one iout mapping. Registering more LEDs
>>>> under the same color feels more of a task for trigger.
>>>>
>>>>>>>> brightness-model: defines brightness level to color configuration
>>>>>>>>                      mapping
>>>>>>>>      - "hardware": for devices with feature like LEDn_BRIGHTNESS of lp50xx
>>>>>>>>      - "rgb-<hue>": available only when all three red,green,blue colors
>>>>>>>>                     are present in the colors directory.
>>>>>>>>                     <hue> is a placeholder for given DT hue presets.
>>>>>>>>      - "rgbw-<hue>": Same as above, but available only when white color
>>>>>>>>                     (any of amber or white-{cool|neutral|warm} can be
>>>>>>>>                     mapped to white) is also available. In this mode
>>>>>>>>                     max_brightness equals num-of-hue-presets + 1, and
>>>>>>>>                     max_brightness, when set, turns the "white" LED on
>>>>>>>
>>>>>>> Why do we need white combined here? Should this not be its own entity?
>>>>>>
>>>>>> To be able to set white color. We're still talking about one LED
>>>>>> element (although they can be be physically few LEDs in one case).
>>>>>> This is brightness file, so we've got to stick to the semantics.
>>>>>> Max brightness level should be the brightest. With RGBW LEDs we
>>>>>> fortunately have a means to achieve pure white, that's why
>>>>>> rgbw-<hue> would be beneficial. If you increase L component of
>>>>>> HSL color space, the max value gives white for all hues.
>>>>>> So maybe this brightness-model would be rather called hsl-<hue>.
>>>>>>
>>>>>> For RGBW LEDs, we would have to allow for more shades of white too,
>>>>>> like in [1].
>>>>>>
>>>>>
>>>>> Yep.
>>>>
>>>> But this all would be left to the DT designer decision.
>>>>
>>>>>>> Again I don't like limiting the color palette from the DT. I think that the
>>>>>>> user space can see what colors are available for that device and makes its own
>>>>>>> decision on what color to present.
>>>>>>>
>>>>>>> For the RGBw what about RGB amber and RGB purple. Are the white LEDs always part of the
>>>>>>> same function trying to be achieved by the system designer? The RGB can be used to denote
>>>>>>> notification status and the white can be used to denote that a charger is connected. Motorola
>>>>>>> Droid did this.
>>>>>>
>>>>>> I hope I've just clarified my idea.
>>>>>>
>>>>>
>>>>> Its getting clearer. I would like to see it in code and play with it not as a user
>>>>> but as a developer. Make sure the paper model works as well as the real implementation.
>>>>>
>>>>> Is this model clear to the developer?
>>>>> How would a developer define what values are appropriate for the brightness-model?
>>>>
>>>> We could create guidelines e.g. that for hsl-<hue> pattern, the
>>>> colors corresponding to brightness levels should be arranged so
>>>> that increasing brightness felt like increasing value of
>>>> L component of HSL.
>>>>
>>>> But we wouldn't be able to enforce adherence to a particular scheme.
>>>>
>>>>> Does the driver have to become overly complex to support simple color generation?
>>>>
>>>> Not at all. Caching the colors written to the files in colors directory
>>>> would be responsibility of LED multi-color class. On echo 1 > sync
>>>> the core would call a new op e.g.
>>>>
>>>> color_set(struct led_classdev_multicolor *mcled_cdev)
>>>>
>>>> Driver would read in it colors cached in struct led_classdev_multicolor
>>>> and write them to the hardware.
>>>>
>>>>> Thoughts on putting code to idea?
>>>>>
>>>>>
>>>>>>>
>>>>>>>>      - "rgb-linear": I'm not sure if it should be available - it will
>>>>>>>>                      have unpredictable results
>>>>>>>>
>>>>>>>> brightness: sets/reads brightness in the way specific to the
>>>>>>>>                current brightness-model. When more colors are available
>>>>>>>>                (e.g. amber, blue, cyan, green, red, violet, white), they
>>>>>>>>                are not touched by write to brightness).
>>>>>>>>
>>>>>>>> HSV->RGB conversion is left entirely to the userspace, which can set any
>>>>>>>> color with use of the proposed interface.
>>>>>>>>
>>>>>>>> Let's agree on sysfs interface first, and only after that move
>>>>>>>> to the DT details.
>>>>>>>>
>>>>>>>
>>>>>>> DT's are meant to describe hardware and not describe the product. Unless Rob does not see
>>>>>>> an issue with defining product capabilities in the DT then we should keep that out of the DT.
>>>>>>
>>>>>> LED element is a device. I see nothing irrelevant for DT in describing the lighting specificity of a device mounted on the board. Please keep
>>>>>> in mind that it will not limit the number of colors available to set.
>>>>>> It will only allow to define mapping of brightness level to color.
>>>>>> We need that for current trigger mechanism to work with LED multi-color
>>>>>> class.
>>>>>>
>>>>>
>>>>> I see this now.
>>>>>
>>>>> Dan
>>>>>
>>>>>> [0] http://www.cobledarray.com/sale-4942931-10w-rgbwauv-led-diode-6-in-1-high-power-multicolor-led-chip.html
>>>>>> [1] https://www.youtube.com/watch?v=NzlFmTqOh9M
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>

--
Best regards,
Jacek Anaszewski