[lkml]   [2019]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [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

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
- 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:
> See some of the comments.
> Dan
>> Thanks,
>> Pavel

 \ /
  Last update: 2019-01-17 22:11    [W:0.200 / U:25.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site