Hi Vesa, On 11/13/2018 08:10 AM, Vesa Jääskeläinen wrote: > Hi Jacek, > > On 12/11/2018 18.02, Jacek Anaszewski wrote: >>>> Support for RGB LEDs, or other variations of LED synchronization >>>> have been approached several times, but without satisfying result >>>> so far. >>>> >>>> Generally the problem boils down to the issue of how triggers >>>> should handle multiple synchronized LED class devices in a backwards >>>> compatible way with monochrome LEDs. >>>> >>>> At some point the HSV [0] approach was proposed, but there was a >>>> problem >>>> with getting uniform colors across devices. Especially white. >>>> Certainly a calibration mechanism would be required. >>>> >>>> [0] https://lkml.org/lkml/2017/8/30/423 >>> >>> We do not usually use PWM controlled leds so our calibration has been HW >>> engineer tuning the resistors for the leds to get acceptable color for >>> different colors variations. >>> >>> If one wants to have fixed colors for such device then one could use >>> this similar method to define them or/and free form interface to enter >>> RGB values there. Thou with PWM RGB leds you probably want to have more >>> animation there which probably would require some additional support >>> code. >>> >>> One way to do atomic PWM RGB color change with sysfs could be: >>> >>> echo "21 223 242" > color >>> >>> or >>> >>> echo "21 223 242" > rgb >>> >>> or: >>> >>> echo "r:21 g:232 b:242" > color (or something similar) >>> >>> and if there is know registered name then just write it to "color" which >>> would pick registered color rgb values to led instances rgb value. >>> >>> Now for PWM RGB led one could use "brightness" and "rgb" value to >>> calculate actual color with some color space formula (like hsv in [0]). >>> >>> Doing white with RGB LED is a bit hard so usually you want to get RGBW >>> LED (or RGBAW LED) if "real" white is something that is needed. This >>> could then be "rgbw" entry and "color" to pick from fixed set. >>> >>> These presets in device tree for "color" could be considered one way of >>> doing calibration for particular hardware. >>> >>> So in device tree for RGB PWM led it could be like: >>> >>> color-orange { >>> rgb = <249 197 9> >>> } >>> >>> color-warm-white { >>> rgb = <255 253 249> >>> } >>> >>> How would that sound like? >> >> Thank you for the description of your approach. >> >> Predefined DT color definitions make an interesting option. Nonetheless, >> your design assumes that for RGB LEDs max_brightness would have to be >> always 1. >> >> While it would solve the issue, it wouldn't allow to benefit from >> the whole potential of RGB LEDs - think of having adjustable "color >> brightness" (like in HSV color model). > > What I tried to describe above was that these could also work with HSV > model also with larger brigtness values too. > > Let's say we do following sequence to change from off state to user > configured intensity value: > > # Turn off leds > echo 0 > brightness > > # Select requested color > echo "orange" > color > > # Use configured LED intensity from user configuration > echo 84 > brightness > > Driver would now use rgb value <249 197 9> and brightness <84> to > calculate (with HSV solution) real PWM values for the led component > colors. With led controller this is probably easier to get linear > feedback but for generic PWM controller you may want to utilize PWM > curve feature like defined for backlight driver. > > When thinking this I instantly see my self with thinking PS4's > sliding/pulsing color animation for orange/blue/white. And controlling > this animation steps manually from user space probably is not the best > idea so like with blinking you would need more accurately timed action > within kernel I suppose (or hardware with the support). > > With that idea forward: > > # Turning animation sequencer off causes instant changes for values > echo "off" > animation-sequencer > > # User 1 second for animation (no effect yet as sequencer is off) > echo 1000 > animation-time-ms (or so) > > # Turn off leds (instance change) > echo 0 > brightness > > # Select requested color (instant change) > echo "orange" > color > > # Change to linear animation sequencer > echo "linear" > animation-sequencer > > # Now everything from this point is subject to animation sequencer > # Sequencer remember active state and next state (as seen in sysfs) > > # Use configured LED intensity from user configuration (no effect yet) > echo 84 > brightness > > # Trigger transition sequence > echo load > animation-sequencer > > # This would cause 1 second animation from black to orange with user > intensivity of 84 > > # Wait for transition to complete and some extra > sleep 4 > > # Change color to blue with animation > echo "blue" > color > echo load > animation-sequencer > > # This would cause driver to change from orange to blue in current > intensivity level during 1 second period > > # -- end -- > > Now that went a bit ahead of time but I believe it demonstrates the > potential for this approach with combined with HSL functionality for > future. We have just contributed pattern trigger - it could be adjusted to RGB LED purposes too. See drivers/leds/trigger/ledtrig-pattern.c and Documentation/ABI/testing/sysfs-class-led-trigger-pattern in Linus' tree. >> How abut allowing for providing an array of color intensity/brightness >> levels per each color? In the simplest case there could be a single >> level per color, but it should be possible to provide up to 255 levels. > > I believe that brightness and color should be separate topics. I didn't make myself clear enough - I like the approach with the color sysfs file. By the "color brightness" I mean color arrays, that would have to be defined per each color. Elements of such an array would make the "color brightness/lightness" range. > We have in our devices option in user interface to configure intesivity > for backlight and in one device also intensivity for status led (this is > the only PWM led we have :)). (I believe this might be currently > implemented without actual led driver and directly use PWM interface) > > Even in when reduced intensivity we want the color to be set to > specified value even thou it would not be as bright as full intensivity > would be. > > What do you think? I think that we've just agreed the preliminary requirements for a new RGB LED class interface. -- Best regards, Jacek Anaszewski