Messages in this thread | | | Subject | Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver | From | Jacek Anaszewski <> | Date | Sun, 20 Jan 2019 16:32:25 +0100 |
| |
Dan,
On 1/18/19 2:45 PM, 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?
This is out of this discussion scope. Here we're discussing RGB LEDs. I believe LEDn_BRIGHTNESS of lp50xx would also not work as intended with colors other than RGB.
Best regards, Jacek Anaszewski
> How to validate that the color is real? > Or do we present a list of possible colors and validate that the color is appropriate? > > I believe this could leverage the work you are doing on the LED label for color. > >> - creates brightness file >> a) for devices with hardware support for adjusting color >> intensity it maps to corresponding register > > If we group LEDs as proposed we will have independent devices that give each LED a separate brightness control register. > We would need to write to each brightness register here. > >> 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 >>>> >>> >>> >> >> > >
| |