Skip to contentSkip to conversation

leds: core: add support for HSV to RGB conversion

PATCH Superseded Heiner Kallweit 2 participants 4 messages Feb 25, 2016 – Mar 1, 2016 LED

v4 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Heiner Kallweit

author
#0 lore raw

PATCH v4 4/4 leds: core: add support for HSV to RGB conversion

Export a function to convert HSV color values to RGB. It's intended to be called by drivers for RGB LEDs.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.
v4:
- Export led_hsv_to_rgb and let the device driver call it instead of doing the conversion in the core

---
 drivers/leds/led-core.c     |  7 ++++++-
 drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/leds.h        |  8 ++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)
drivers/leds/led-core.c +6 -1
index e75b0c8..e3ee971 100644
@@ -295,7 +295,12 @@ int led_update_brightness(struct led_classdev *led_cdev)
 {
 	int ret = 0;
 
-	if (led_cdev->brightness_get) {
+	/*
+	 * for now reading back the color is not supported as multiple
+	 * HSV -> RGB -> HSV conversions may distort the color due to
+	 * rounding issues in the conversion algorithm
+	 */
+	if (led_cdev->brightness_get && !(led_cdev->flags & LED_DEV_CAP_HSV)) {
 		ret = led_cdev->brightness_get(led_cdev);
 		if (ret >= 0) {
 			led_cdev->brightness = ret;
drivers/leds/led-rgb-core.c +36 -0
index b81daeb..4691795 100644
@@ -40,3 +40,39 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
 	return brightness |
 	       min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
 }
+
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv)
+{
+	int h = min_t(int, (hsv >> 16) & 0xff, 251);
+	int s = (hsv >> 8) & 0xff;
+	int v = hsv & 0xff;
+	int f, p, q, t, r, g, b;
+
+	if (!v)
+		return 0;
+	if (!s)
+		return (v << 16) + (v << 8) + v;
+
+	f = DIV_ROUND_CLOSEST((h % 42) * 255, 42);
+	p = v - DIV_ROUND_CLOSEST(s * v, 255);
+	q = v - DIV_ROUND_CLOSEST(f * s * v, 255 * 255);
+	t = v - DIV_ROUND_CLOSEST((255 - f) * s * v, 255 * 255);
+
+	switch (h / 42) {
+	case 0:
+		r = v; g = t; b = p; break;
+	case 1:
+		r = q; g = v; b = p; break;
+	case 2:
+		r = p; g = v; b = t; break;
+	case 3:
+		r = p; g = q; b = v; break;
+	case 4:
+		r = t; g = p; b = v; break;
+	case 5:
+		r = v; g = p; b = q; break;
+	}
+
+	return (r << 16) + (g << 8) + b;
+}
+EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
include/linux/leds.h +8 -0
index 1e98b2e..27725c6 100644
@@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
 	return led_cdev->flags & LED_SYSFS_DISABLE;
 }
 
+/**
+ * led_hsv_to_rgb - convert a hsv color value to rgb color model
+ * @hsv: the hsv value to convert
+ *
+ * Returns: the resulting rgb value
+ */
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
+
 /*
  * LED Triggers
  */
Signature · 1 line
2.7.1
#1 lore raw

Hi Heiner,

Quoted 26 lines · Heiner Kallweit

Export a function to convert HSV color values to RGB. It's intended to be called by drivers for RGB LEDs.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.
v4:
- Export led_hsv_to_rgb and let the device driver call it instead of doing the conversion in the core

---
  drivers/leds/led-core.c     |  7 ++++++-
  drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
  include/linux/leds.h        |  8 ++++++++
  3 files changed, 50 insertions(+), 1 deletion(-)
drivers/leds/led-core.c +0 -0
index e75b0c8..e3ee971 100644

drivers/leds/led-core.c int led_update_brightness(struct led_classdev *led_cdev) · line 302

Quoted 9 lines
drivers/leds/led-core.c +5 -1
@@ -295,7 +295,12 @@ int led_update_brightness(struct led_classdev *led_cdev)
  {
  	int ret = 0;
 
-	if (led_cdev->brightness_get) {
+	/*
+	 * for now reading back the color is not supported as multiple
+	 * HSV -> RGB -> HSV conversions may distort the color due to
+	 * rounding issues in the conversion algorithm
+	 */

This is very unpleasant. Is it really not feasible to work this around?

Quoted 68 lines
drivers/leds/led-core.c +1 -0
+	if (led_cdev->brightness_get && !(led_cdev->flags & LED_DEV_CAP_HSV)) {
  		ret = led_cdev->brightness_get(led_cdev);
  		if (ret >= 0) {
  			led_cdev->brightness = ret;
drivers/leds/led-rgb-core.c +36 -0
index b81daeb..4691795 100644
@@ -40,3 +40,39 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
  	return brightness |
  	       min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
  }
+
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv)
+{
+	int h = min_t(int, (hsv >> 16) & 0xff, 251);
+	int s = (hsv >> 8) & 0xff;
+	int v = hsv & 0xff;
+	int f, p, q, t, r, g, b;
+
+	if (!v)
+		return 0;
+	if (!s)
+		return (v << 16) + (v << 8) + v;
+
+	f = DIV_ROUND_CLOSEST((h % 42) * 255, 42);
+	p = v - DIV_ROUND_CLOSEST(s * v, 255);
+	q = v - DIV_ROUND_CLOSEST(f * s * v, 255 * 255);
+	t = v - DIV_ROUND_CLOSEST((255 - f) * s * v, 255 * 255);
+
+	switch (h / 42) {
+	case 0:
+		r = v; g = t; b = p; break;
+	case 1:
+		r = q; g = v; b = p; break;
+	case 2:
+		r = p; g = v; b = t; break;
+	case 3:
+		r = p; g = q; b = v; break;
+	case 4:
+		r = t; g = p; b = v; break;
+	case 5:
+		r = v; g = p; b = q; break;
+	}
+
+	return (r << 16) + (g << 8) + b;
+}
+EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
include/linux/leds.h +8 -0
index 1e98b2e..27725c6 100644
@@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
  	return led_cdev->flags & LED_SYSFS_DISABLE;
  }
 
+/**
+ * led_hsv_to_rgb - convert a hsv color value to rgb color model
+ * @hsv: the hsv value to convert
+ *
+ * Returns: the resulting rgb value
+ */
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
+
  /*
   * LED Triggers
   */
Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#2 lore raw

Am 29.02.2016 um 17:13 schrieb Jacek Anaszewski:

Quoted 32 lines

Hi Heiner,

Quoted 29 lines · Heiner Kallweit

Export a function to convert HSV color values to RGB. It's intended to be called by drivers for RGB LEDs.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.
v4:
- Export led_hsv_to_rgb and let the device driver call it instead of doing the conversion in the core

---
  drivers/leds/led-core.c     |  7 ++++++-
  drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
  include/linux/leds.h        |  8 ++++++++
  3 files changed, 50 insertions(+), 1 deletion(-)
drivers/leds/led-core.c +0 -0
index e75b0c8..e3ee971 100644
@@ -295,7 +295,12 @@ int led_update_brightness(struct led_classdev *led_cdev)
  {
      int ret = 0;
Quoted 9 lines

drivers/leds/led-core.c int led_update_brightness(struct led_classdev *led_cdev) · line 302

Quoted 6 lines
drivers/leds/led-core.c +5 -1
-    if (led_cdev->brightness_get) {
+    /*
+     * for now reading back the color is not supported as multiple
+     * HSV -> RGB -> HSV conversions may distort the color due to
+     * rounding issues in the conversion algorithm
+     */

This is very unpleasant. Is it really not feasible to work this around?

With the 8 bit granularity of the color components it's not possible to guarantee that a conversion followed by its reverse operation always results in the same value. (at least for the H and S component, for V it's no problem)

Just one example: H=1 S=1 V=3 converted to RGB will result in R=G=B=3 and the reverse conversion will result in H=0 S=0 V=3

I think there's only a theoretical impact of not supporting reading from the device because who except the driver should access the device. If you think it is relevant then I could think of something like this:
Overwrite the stored brightness value only if the value read from the device differs "significantly" from the stored value. This would indicate that somebody outside the driver changed the LED value and we should reflect this in the stored value. What do you think?

Quoted 70 lines
Quoted 68 lines
drivers/leds/led-core.c +1 -0
+    if (led_cdev->brightness_get && !(led_cdev->flags & LED_DEV_CAP_HSV)) {
          ret = led_cdev->brightness_get(led_cdev);
          if (ret >= 0) {
              led_cdev->brightness = ret;
drivers/leds/led-rgb-core.c +36 -0
index b81daeb..4691795 100644
@@ -40,3 +40,39 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
      return brightness |
             min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
  }
+
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv)
+{
+    int h = min_t(int, (hsv >> 16) & 0xff, 251);
+    int s = (hsv >> 8) & 0xff;
+    int v = hsv & 0xff;
+    int f, p, q, t, r, g, b;
+
+    if (!v)
+        return 0;
+    if (!s)
+        return (v << 16) + (v << 8) + v;
+
+    f = DIV_ROUND_CLOSEST((h % 42) * 255, 42);
+    p = v - DIV_ROUND_CLOSEST(s * v, 255);
+    q = v - DIV_ROUND_CLOSEST(f * s * v, 255 * 255);
+    t = v - DIV_ROUND_CLOSEST((255 - f) * s * v, 255 * 255);
+
+    switch (h / 42) {
+    case 0:
+        r = v; g = t; b = p; break;
+    case 1:
+        r = q; g = v; b = p; break;
+    case 2:
+        r = p; g = v; b = t; break;
+    case 3:
+        r = p; g = q; b = v; break;
+    case 4:
+        r = t; g = p; b = v; break;
+    case 5:
+        r = v; g = p; b = q; break;
+    }
+
+    return (r << 16) + (g << 8) + b;
+}
+EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
include/linux/leds.h +8 -0
index 1e98b2e..27725c6 100644
@@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
      return led_cdev->flags & LED_SYSFS_DISABLE;
  }
 
+/**
+ * led_hsv_to_rgb - convert a hsv color value to rgb color model
+ * @hsv: the hsv value to convert
+ *
+ * Returns: the resulting rgb value
+ */
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
+
  /*
   * LED Triggers
   */
#3 lore raw
Quoted 46 lines · Heiner Kallweit

Am 29.02.2016 um 17:13 schrieb Jacek Anaszewski:

Quoted 32 lines

Hi Heiner,

Quoted 29 lines · Heiner Kallweit

Export a function to convert HSV color values to RGB. It's intended to be called by drivers for RGB LEDs.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.
v4:
- Export led_hsv_to_rgb and let the device driver call it instead of doing the conversion in the core

---
   drivers/leds/led-core.c     |  7 ++++++-
   drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
   include/linux/leds.h        |  8 ++++++++
   3 files changed, 50 insertions(+), 1 deletion(-)
drivers/leds/led-core.c +0 -0
index e75b0c8..e3ee971 100644
@@ -295,7 +295,12 @@ int led_update_brightness(struct led_classdev *led_cdev)
   {
       int ret = 0;
Quoted 9 lines

drivers/leds/led-core.c int led_update_brightness(struct led_classdev *led_cdev) · line 302

Quoted 6 lines
drivers/leds/led-core.c +5 -1
-    if (led_cdev->brightness_get) {
+    /*
+     * for now reading back the color is not supported as multiple
+     * HSV -> RGB -> HSV conversions may distort the color due to
+     * rounding issues in the conversion algorithm
+     */

This is very unpleasant. Is it really not feasible to work this around?

With the 8 bit granularity of the color components it's not possible to guarantee that a conversion followed by its reverse operation always results in the same value. (at least for the H and S component, for V it's no problem)

Quoted 5 lines

Just one example: H=1 S=1 V=3 converted to RGB will result in R=G=B=3 and the reverse conversion will result in H=0 S=0 V=3

I think there's only a theoretical impact of not supporting reading from the device because who except the driver should access the device.

I know about LED flash device that can reduce brightness level in case battery voltage level decreases below certain threshold.

Maybe in case of low current LED RGB controllers such a feature is not likely to appear.

Quoted 6 lines

If you think it is relevant
then I could think of something like this:
Overwrite the stored brightness value only if the value read from the device differs "significantly" from the stored value. This would indicate that somebody outside the driver changed the LED value and we should reflect this in the stored value. What do you think?

I think that only the hardware itself can adjust the brightness. After thinking a while about that I came to conclusion that in case of RGB LEDs it wouldn't make sense if hardware was able to change the colors arbitrarily, as it would distort the color requested by the user.

I'd leave this piece of code intact, but instead add WARN_ON in led_classdev_register, with the argument evaluating to true if brightness_get op is defined and the LED is of RGB type.

BTW: s/LED_DEV_CAP_HSV/LED_DEV_CAP_RGB/

Quoted 73 lines
Quoted 70 lines
Quoted 68 lines
drivers/leds/led-core.c +1 -0
+    if (led_cdev->brightness_get && !(led_cdev->flags & LED_DEV_CAP_HSV)) {
           ret = led_cdev->brightness_get(led_cdev);
           if (ret >= 0) {
               led_cdev->brightness = ret;
drivers/leds/led-rgb-core.c +36 -0
index b81daeb..4691795 100644
@@ -40,3 +40,39 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
       return brightness |
              min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
   }
+
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv)
+{
+    int h = min_t(int, (hsv >> 16) & 0xff, 251);
+    int s = (hsv >> 8) & 0xff;
+    int v = hsv & 0xff;
+    int f, p, q, t, r, g, b;
+
+    if (!v)
+        return 0;
+    if (!s)
+        return (v << 16) + (v << 8) + v;
+
+    f = DIV_ROUND_CLOSEST((h % 42) * 255, 42);
+    p = v - DIV_ROUND_CLOSEST(s * v, 255);
+    q = v - DIV_ROUND_CLOSEST(f * s * v, 255 * 255);
+    t = v - DIV_ROUND_CLOSEST((255 - f) * s * v, 255 * 255);
+
+    switch (h / 42) {
+    case 0:
+        r = v; g = t; b = p; break;
+    case 1:
+        r = q; g = v; b = p; break;
+    case 2:
+        r = p; g = v; b = t; break;
+    case 3:
+        r = p; g = q; b = v; break;
+    case 4:
+        r = t; g = p; b = v; break;
+    case 5:
+        r = v; g = p; b = q; break;
+    }
+
+    return (r << 16) + (g << 8) + b;
+}
+EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
include/linux/leds.h +8 -0
index 1e98b2e..27725c6 100644
@@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
       return led_cdev->flags & LED_SYSFS_DISABLE;
   }
 
+/**
+ * led_hsv_to_rgb - convert a hsv color value to rgb color model
+ * @hsv: the hsv value to convert
+ *
+ * Returns: the resulting rgb value
+ */
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
+
   /*
    * LED Triggers
    */
Signature · 2 lines
Best regards,
Jacek Anaszewski