drivers/leds/rgb/leds-qcom-lpg.c | 174 +++++++++++++++++++++++++++++++-------- 1 file changed, 141 insertions(+), 33 deletions(-)
Currently, when the LED is configured as a RGB LED or a multi-color
LED device, the same pattern is programmed for all LED channels
regardless of the sub-led intensities when triggered by HW pattern.
It results that the LED device is always working in a white-balanced
mode regardless of the intensity settings.
To fix this, scale the pattern data according to the sub-led intensity
and program the HW pattern separately for each LPG channel.
Fixes: 24e2d05d1b68 ("leds: Add driver for Qualcomm LPG")
Fixes: 6ab1f766a80a ("leds: rgb: leds-qcom-lpg: Add support for PPG through single SDAM")
Fixes: 5e9ff626861a ("leds: rgb: leds-qcom-lpg: Include support for PPG with dedicated LUT SDAM")
Assisted-by: Claude:claude-4-6-sonnet
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
drivers/leds/rgb/leds-qcom-lpg.c | 174 +++++++++++++++++++++++++++++++--------
1 file changed, 141 insertions(+), 33 deletions(-)
diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index d7d6518de30f..ca84da563e09 100644
--- a/drivers/leds/rgb/leds-qcom-lpg.c
+++ b/drivers/leds/rgb/leds-qcom-lpg.c
@@ -148,6 +148,24 @@ struct lpg_channel {
unsigned int pattern_hi_idx;
};
+/**
+ * struct lpg_pattern - The LPG pattern normalized from the LED pattern
+ * @data: The pattern data array (caller must kfree)
+ * @len: number of entries to write to the LUT
+ * @delta_t: common step duration in ms
+ * @lo_pause: low-pause duration in ms
+ * @hi_pause: high-pause duration in ms
+ * @ping_pong: true if the pattern support reverse
+ */
+struct lpg_pattern {
+ struct led_pattern *data;
+ unsigned int len;
+ unsigned int delta_t;
+ unsigned int lo_pause;
+ unsigned int hi_pause;
+ bool ping_pong;
+};
+
/**
* struct lpg_led - logical LED object
* @lpg: lpg context reference
@@ -959,23 +977,23 @@ static int lpg_blink_mc_set(struct led_classdev *cdev,
return ret;
}
-static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
- u32 len, int repeat)
+/**
+ * lpg_prepare_pattern() - validate and normalize a leds-trigger-pattern payload
+ *
+ * Collapses the paired-entry format required by the trigger into a compact
+ * array, detects palindrome ping-pong, and extracts timing parameters.
+ * On success the caller owns prep->data and must kfree() it.
+ */
+static int lpg_prepare_pattern(struct lpg *lpg, struct led_pattern *led_pattern,
+ u32 len, int repeat, struct lpg_pattern *prep)
{
- struct lpg_channel *chan;
- struct lpg *lpg = led->lpg;
struct led_pattern *pattern;
unsigned int brightness_a;
unsigned int brightness_b;
- unsigned int hi_pause = 0;
- unsigned int lo_pause = 0;
unsigned int actual_len;
unsigned int delta_t;
- unsigned int lo_idx;
- unsigned int hi_idx;
unsigned int i;
bool ping_pong = true;
- int ret = -EINVAL;
/* Hardware only support oneshot or indefinite loops */
if (repeat != -1 && repeat != 1)
@@ -1001,9 +1019,9 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
for (i = 0; i < len; i += 2) {
if (led_pattern[i].brightness != led_pattern[i + 1].brightness)
- goto out_free_pattern;
+ goto out_free;
if (led_pattern[i + 1].delta_t != 0)
- goto out_free_pattern;
+ goto out_free;
pattern[i / 2].brightness = led_pattern[i].brightness;
pattern[i / 2].delta_t = led_pattern[i].delta_t;
@@ -1016,7 +1034,7 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
* through the entire LUT, so prohibit this.
*/
if (len < 2)
- goto out_free_pattern;
+ goto out_free;
/*
* The LPG plays patterns with at a fixed pace, a "low pause" can be
@@ -1073,13 +1091,13 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
* specify hi pause. Reject other variations.
*/
if (i != actual_len - 1)
- goto out_free_pattern;
+ goto out_free;
}
}
/* LPG_RAMP_DURATION_REG is a 9bit */
if (delta_t >= BIT(9))
- goto out_free_pattern;
+ goto out_free;
/*
* Find "low pause" and "high pause" in the pattern in the LUT case.
@@ -1087,20 +1105,48 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
* duration of all steps.
*/
if (lpg->lut_base || lpg->lut_sdam) {
- lo_pause = pattern[0].delta_t;
- hi_pause = pattern[actual_len - 1].delta_t;
+ prep->lo_pause = pattern[0].delta_t;
+ prep->hi_pause = pattern[actual_len - 1].delta_t;
} else {
if (delta_t != pattern[0].delta_t || delta_t != pattern[actual_len - 1].delta_t)
- goto out_free_pattern;
+ goto out_free;
+ prep->lo_pause = 0;
+ prep->hi_pause = 0;
}
+ prep->data = pattern;
+ prep->len = actual_len;
+ prep->delta_t = delta_t;
+ prep->ping_pong = ping_pong;
+
+ return 0;
+
+out_free:
+ kfree(pattern);
+ return -EINVAL;
+}
+
+static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
+ u32 len, int repeat)
+{
+ struct lpg_channel *chan;
+ struct lpg *lpg = led->lpg;
+ struct lpg_pattern pattern;
+ unsigned int lo_idx;
+ unsigned int hi_idx;
+ unsigned int i;
+ int ret;
+
+ ret = lpg_prepare_pattern(lpg, led_pattern, len, repeat, &pattern);
+ if (ret < 0)
+ return ret;
mutex_lock(&lpg->lock);
if (lpg->lut_base)
- ret = lpg_lut_store(lpg, pattern, actual_len, &lo_idx, &hi_idx);
+ ret = lpg_lut_store(lpg, pattern.data, pattern.len, &lo_idx, &hi_idx);
else
- ret = lpg_lut_store_sdam(lpg, pattern, actual_len, &lo_idx, &hi_idx);
+ ret = lpg_lut_store_sdam(lpg, pattern.data, pattern.len, &lo_idx, &hi_idx);
if (ret < 0)
goto out_unlock;
@@ -1108,12 +1154,12 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
for (i = 0; i < led->num_channels; i++) {
chan = led->channels[i];
- chan->ramp_tick_ms = delta_t;
- chan->ramp_ping_pong = ping_pong;
+ chan->ramp_tick_ms = pattern.delta_t;
+ chan->ramp_ping_pong = pattern.ping_pong;
chan->ramp_oneshot = repeat != -1;
- chan->ramp_lo_pause_ms = lo_pause;
- chan->ramp_hi_pause_ms = hi_pause;
+ chan->ramp_lo_pause_ms = pattern.lo_pause;
+ chan->ramp_hi_pause_ms = pattern.hi_pause;
chan->pattern_lo_idx = lo_idx;
chan->pattern_hi_idx = hi_idx;
@@ -1121,8 +1167,7 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
out_unlock:
mutex_unlock(&lpg->lock);
-out_free_pattern:
- kfree(pattern);
+ kfree(pattern.data);
return ret;
}
@@ -1144,23 +1189,88 @@ static int lpg_pattern_single_set(struct led_classdev *cdev,
}
static int lpg_pattern_mc_set(struct led_classdev *cdev,
- struct led_pattern *pattern, u32 len,
+ struct led_pattern *led_pattern, u32 len,
int repeat)
{
struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
struct lpg_led *led = container_of(mc, struct lpg_led, mcdev);
+ struct lpg *lpg = led->lpg;
+ struct lpg_channel *chan;
+ struct lpg_pattern pattern;
+ struct led_pattern *scaled;
unsigned int triled_mask = 0;
- int ret, i;
+ unsigned int lo_idx;
+ unsigned int hi_idx;
+ unsigned int scale;
+ unsigned int i, j;
+ int ret;
+
+ ret = lpg_prepare_pattern(lpg, led_pattern, len, repeat, &pattern);
+ if (ret < 0)
+ return ret;
+
+ /* Allocate buffer for the per-channel scaled pattern copy */
+ scaled = kmalloc_array(pattern.len, sizeof(*scaled), GFP_KERNEL);
+ if (!scaled) {
+ ret = -ENOMEM;
+ goto out_free_prep;
+ }
for (i = 0; i < led->num_channels; i++)
triled_mask |= led->channels[i]->triled_mask;
- triled_set(led->lpg, triled_mask, 0);
+ triled_set(lpg, triled_mask, 0);
+
+ led_mc_calc_color_components(mc, LED_FULL);
+
+ /*
+ * Each channel gets its own LUT block scaled by subled_info[i].brightness
+ * so the pattern respects the configured colour balance.
+ */
+ mutex_lock(&lpg->lock);
+ for (i = 0; i < led->num_channels; i++) {
+ chan = led->channels[i];
+ scale = mc->subled_info[i].brightness;
+
+ for (j = 0; j < pattern.len; j++) {
+ scaled[j].brightness = DIV_ROUND_CLOSEST(
+ (u32)pattern.data[j].brightness * scale, LED_FULL);
+ scaled[j].delta_t = pattern.data[j].delta_t;
+ }
+
+ if (lpg->lut_base)
+ ret = lpg_lut_store(lpg, scaled, pattern.len, &lo_idx, &hi_idx);
+ else
+ ret = lpg_lut_store_sdam(lpg, scaled, pattern.len, &lo_idx, &hi_idx);
+
+ if (ret < 0) {
+ /* Free LUT slots already allocated for previous channels */
+ while (i-- > 0) {
+ chan = led->channels[i];
+ lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
+ chan->pattern_lo_idx = 0;
+ chan->pattern_hi_idx = 0;
+ }
+ goto out_unlock;
+ }
+
+ chan->ramp_tick_ms = pattern.delta_t;
+ chan->ramp_ping_pong = pattern.ping_pong;
+ chan->ramp_oneshot = repeat != -1;
+ chan->ramp_lo_pause_ms = pattern.lo_pause;
+ chan->ramp_hi_pause_ms = pattern.hi_pause;
+ chan->pattern_lo_idx = lo_idx;
+ chan->pattern_hi_idx = hi_idx;
+ }
+
+out_unlock:
+ mutex_unlock(&lpg->lock);
+ kfree(scaled);
+out_free_prep:
+ kfree(pattern.data);
- ret = lpg_pattern_set(led, pattern, len, repeat);
if (ret < 0)
return ret;
- led_mc_calc_color_components(mc, LED_FULL);
lpg_brightness_set(led, cdev, mc->subled_info);
return 0;
@@ -1174,11 +1284,9 @@ static int lpg_pattern_clear(struct lpg_led *led)
mutex_lock(&lpg->lock);
- chan = led->channels[0];
- lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
-
for (i = 0; i < led->num_channels; i++) {
chan = led->channels[i];
+ lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
lpg_sdam_configure_triggers(chan, 0);
lpg_clear_pbs_trigger(chan->lpg, chan->lut_mask);
chan->pattern_lo_idx = 0;
---
base-commit: 1795fd2dbe84ef4d393b69a0b2a3b371f810bde5
change-id: 20260605-lpg-rgb-color-balance-fix-82436649abf3
Best regards,
--
Fenglin Wu <fenglin.wu@oss.qualcomm.com>
On Fri, Jun 05, 2026 at 01:18:24AM -0700, Fenglin Wu wrote:
> Currently, when the LED is configured as a RGB LED or a multi-color
> LED device, the same pattern is programmed for all LED channels
> regardless of the sub-led intensities when triggered by HW pattern.
> It results that the LED device is always working in a white-balanced
> mode regardless of the intensity settings.
>
> To fix this, scale the pattern data according to the sub-led intensity
> and program the HW pattern separately for each LPG channel.
>
> Fixes: 24e2d05d1b68 ("leds: Add driver for Qualcomm LPG")
> Fixes: 6ab1f766a80a ("leds: rgb: leds-qcom-lpg: Add support for PPG through single SDAM")
> Fixes: 5e9ff626861a ("leds: rgb: leds-qcom-lpg: Include support for PPG with dedicated LUT SDAM")
> Assisted-by: Claude:claude-4-6-sonnet
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
> drivers/leds/rgb/leds-qcom-lpg.c | 174 +++++++++++++++++++++++++++++++--------
> 1 file changed, 141 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
> index d7d6518de30f..ca84da563e09 100644
> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> @@ -148,6 +148,24 @@ struct lpg_channel {
> unsigned int pattern_hi_idx;
> };
>
> +/**
> + * struct lpg_pattern - The LPG pattern normalized from the LED pattern
> + * @data: The pattern data array (caller must kfree)
> + * @len: number of entries to write to the LUT
> + * @delta_t: common step duration in ms
> + * @lo_pause: low-pause duration in ms
> + * @hi_pause: high-pause duration in ms
> + * @ping_pong: true if the pattern support reverse
> + */
> +struct lpg_pattern {
> + struct led_pattern *data;
This looks like an overkill. Can you embed the struct here instead of
embedding a pointer?
> + unsigned int len;
> + unsigned int delta_t;
> + unsigned int lo_pause;
> + unsigned int hi_pause;
> + bool ping_pong;
> +};
> +
> /**
> * struct lpg_led - logical LED object
> * @lpg: lpg context reference
--
With best wishes
Dmitry
© 2016 - 2026 Red Hat, Inc.