[PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

[PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

[Fenglin Wu]

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>
---
Changes in v6:
- Fixed comments form Sashiko, and transfer 'pattern.data' to a
  '__free(kree)' local variable to get rid of the goto.
- Link to v5: https://patch.msgid.link/20260707-lpg-rgb-color-balance-fix-v5-1-99e2d73084fc@oss.qualcomm.com

Changes in v5:
- Add no_free_ptr(pattern) to avoid it being cleaned up
- Link to v4: https://patch.msgid.link/20260629-lpg-rgb-color-balance-fix-v4-1-4db8592fb3c5@oss.qualcomm.com

Changes in v4:
- Fixing LLVM compilation issue: avoid jumping over guard(mutex) initialization
- Link to v3: https://patch.msgid.link/20260629-lpg-rgb-color-balance-fix-v3-1-17796a06d799@oss.qualcomm.com

Changes in v3:
- update to use __free() and guard(mutex) for easy cleanup
- Link to v2: https://patch.msgid.link/20260624-lpg-rgb-color-balance-fix-v2-1-c01b0e50caf6@oss.qualcomm.com

Changes in v2:
- Change to use tab for the indention in the comments of 'struct lpg_pattern'
- Remove the comment in lpg_prepare_pattern() as the function name is
  self-explantory.
- Link to v1: https://patch.msgid.link/20260605-lpg-rgb-color-balance-fix-v1-1-3233644a3385@oss.qualcomm.com
---
 drivers/leds/rgb/leds-qcom-lpg.c | 172 +++++++++++++++++++++++++++++----------
 1 file changed, 130 insertions(+), 42 deletions(-)

diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index d7d6518de30f..24b1f570f524 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,15 @@ 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)
+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)
@@ -995,15 +1005,16 @@ static int lpg_pattern_set(struct lpg_led *led, struct led_pattern *led_pattern,
 	if (len % 2)
 		return -EINVAL;
 
-	pattern = kzalloc_objs(*pattern, len / 2);
+	struct led_pattern *pattern __free(kfree) = kzalloc_objs(*pattern, len / 2);
+
 	if (!pattern)
 		return -ENOMEM;
 
 	for (i = 0; i < len; i += 2) {
 		if (led_pattern[i].brightness != led_pattern[i + 1].brightness)
-			goto out_free_pattern;
+			return -EINVAL;
 		if (led_pattern[i + 1].delta_t != 0)
-			goto out_free_pattern;
+			return -EINVAL;
 
 		pattern[i / 2].brightness = led_pattern[i].brightness;
 		pattern[i / 2].delta_t = led_pattern[i].delta_t;
@@ -1016,7 +1027,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;
+		return -EINVAL;
 
 	/*
 	 * The LPG plays patterns with at a fixed pace, a "low pause" can be
@@ -1073,13 +1084,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;
+				return -EINVAL;
 		}
 	}
 
 	/* LPG_RAMP_DURATION_REG is a 9bit */
 	if (delta_t >= BIT(9))
-		goto out_free_pattern;
+		return -EINVAL;
 
 	/*
 	 * Find "low pause" and "high pause" in the pattern in the LUT case.
@@ -1087,43 +1098,64 @@ 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;
+			return -EINVAL;
+		prep->lo_pause = 0;
+		prep->hi_pause = 0;
 	}
 
+	prep->data = no_free_ptr(pattern);
+	prep->len = actual_len;
+	prep->delta_t = delta_t;
+	prep->ping_pong = ping_pong;
 
-	mutex_lock(&lpg->lock);
+	return 0;
+}
+
+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;
+
+	struct led_pattern *prep_data __free(kfree) = pattern.data;
+
+	guard(mutex)(&lpg->lock);
 
 	if (lpg->lut_base)
-		ret = lpg_lut_store(lpg, pattern, actual_len, &lo_idx, &hi_idx);
+		ret = lpg_lut_store(lpg, prep_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, prep_data, pattern.len, &lo_idx, &hi_idx);
 
 	if (ret < 0)
-		goto out_unlock;
+		return ret;
 
 	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;
 	}
 
-out_unlock:
-	mutex_unlock(&lpg->lock);
-out_free_pattern:
-	kfree(pattern);
-
 	return ret;
 }
 
@@ -1144,23 +1176,81 @@ 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;
 	unsigned int triled_mask = 0;
-	int ret, i;
-
-	for (i = 0; i < led->num_channels; i++)
-		triled_mask |= led->channels[i]->triled_mask;
-	triled_set(led->lpg, triled_mask, 0);
+	unsigned int lo_idx;
+	unsigned int hi_idx;
+	unsigned int scale;
+	unsigned int i, j;
+	int ret;
 
-	ret = lpg_pattern_set(led, pattern, len, repeat);
+	ret = lpg_prepare_pattern(lpg, led_pattern, len, repeat, &pattern);
 	if (ret < 0)
 		return ret;
 
+	struct led_pattern *prep_data __free(kfree) = pattern.data;
+
+	/* Allocate buffer for the per-channel scaled pattern copy */
+	struct led_pattern *scaled __free(kfree) =
+			   kmalloc_array(pattern.len, sizeof(*scaled), GFP_KERNEL);
+	if (!scaled)
+		return -ENOMEM;
+
+	for (i = 0; i < led->num_channels; i++)
+		triled_mask |= led->channels[i]->triled_mask;
+	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.
+	 */
+	guard(mutex)(&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)prep_data[j].brightness * scale, LED_FULL);
+			scaled[j].delta_t = prep_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;
+			}
+
+			return ret;
+		}
+
+		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;
+	}
+
 	lpg_brightness_set(led, cdev, mc->subled_info);
 
 	return 0;
@@ -1174,11 +1264,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>

Re: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver permanently leaks a hardware LUT slot and fails to trigger patterns when a 2-step palindrome pattern evaluates to an `actual_len` of 1.
--

--- Patch [1]: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode ---
commit c5b003f7bc9326bf3784a70ad883c39b215ae496
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
Subject: leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

This commit splits out pattern preparation into lpg_prepare_pattern() and
adds support for per-channel scaling so the hardware pattern respects
multi-color LED balances. It also updates the LUT storage paths to handle
the dynamically allocated scaled patterns.

> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
> index d7d6518de30f2..24b1f570f5244 100644
> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> +++ b/drivers/leds/rgb/leds-qcom-lpg.c

[ ... ]

> @@ -1087,43 +1098,64 @@ 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;
> +			return -EINVAL;
> +		prep->lo_pause = 0;
> +		prep->hi_pause = 0;
>  	}
>  
> +	prep->data = no_free_ptr(pattern);
> +	prep->len = actual_len;

[Severity: High]
This is a pre-existing issue, but does this logic permanently leak a
hardware LUT slot if a 2-step palindrome pattern is provided?

If a user-space process provides a 2-step pattern where both steps have the
same brightness, the ping_pong logic earlier in lpg_prepare_pattern() will
calculate actual_len = (2 + 1) / 2, which evaluates to 1.

When this length-1 pattern is subsequently stored, it allocates 1 slot where
lo_idx and hi_idx are equal. Because they are equal, lpg_brightness_set()
treats the configuration as a static brightness instead of playing the
pattern.

Later, when the pattern is cleared and lpg_lut_free() is called with this
lo_idx and hi_idx, it hits this early return:

static void lpg_lut_free(struct lpg *lpg, unsigned int lo_idx, unsigned int hi_idx)
{
	int len;

	len = hi_idx - lo_idx + 1;
	if (len == 1)
		return;

	bitmap_clear(lpg->lut_bitmap, lo_idx, len);
}

Does this early return bypass bitmap_clear() and permanently leak the LUT
slot for the remainder of the device lifecycle?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-lpg-rgb-color-balance-fix-v6-1-b49d51528f61@oss.qualcomm.com?part=1

Re: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

[Fenglin Wu]

On 7/17/2026 12:55 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The driver permanently leaks a hardware LUT slot and fails to trigger patterns when a 2-step palindrome pattern evaluates to an `actual_len` of 1.
> --
> 
> --- Patch [1]: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode ---
> commit c5b003f7bc9326bf3784a70ad883c39b215ae496
> Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> Subject: leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode
> 
> This commit splits out pattern preparation into lpg_prepare_pattern() and
> adds support for per-channel scaling so the hardware pattern respects
> multi-color LED balances. It also updates the LUT storage paths to handle
> the dynamically allocated scaled patterns.
> 
>> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
>> index d7d6518de30f2..24b1f570f5244 100644
>> --- a/drivers/leds/rgb/leds-qcom-lpg.c
>> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> 
> [ ... ]
> 
>> @@ -1087,43 +1098,64 @@ 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;
>> +			return -EINVAL;
>> +		prep->lo_pause = 0;
>> +		prep->hi_pause = 0;
>>  	}
>>  
>> +	prep->data = no_free_ptr(pattern);
>> +	prep->len = actual_len;
> 
> [Severity: High]
> This is a pre-existing issue, but does this logic permanently leak a
> hardware LUT slot if a 2-step palindrome pattern is provided?
> 
> If a user-space process provides a 2-step pattern where both steps have the
> same brightness, the ping_pong logic earlier in lpg_prepare_pattern() will
> calculate actual_len = (2 + 1) / 2, which evaluates to 1.
> 
> When this length-1 pattern is subsequently stored, it allocates 1 slot where
> lo_idx and hi_idx are equal. Because they are equal, lpg_brightness_set()
> treats the configuration as a static brightness instead of playing the
> pattern.
> 
> Later, when the pattern is cleared and lpg_lut_free() is called with this
> lo_idx and hi_idx, it hits this early return:
> 
> static void lpg_lut_free(struct lpg *lpg, unsigned int lo_idx, unsigned int hi_idx)
> {
> 	int len;
> 
> 	len = hi_idx - lo_idx + 1;
> 	if (len == 1)
> 		return;
> 
> 	bitmap_clear(lpg->lut_bitmap, lo_idx, len);
> }
> 
> Does this early return bypass bitmap_clear() and permanently leak the LUT
> slot for the remainder of the device lifecycle?
> 

I think this should go into a separate patch, since it isn’t introduced
by this change. On the fix itself, rather than checking "(len == 1)" to
clear the bitmap, we should add a new flag to track whether the bitmap
has been allocated and only free it when that flag is true.

@Jones, what do you think? Would you prefer that I include this fix in
the current change, or should I push a separate fix after this one?

Thanks
Fenglin

Re: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

[Lee Jones]

On Tue, 25 Aug 2026, Fenglin Wu wrote:

> 
> 
> On 7/17/2026 12:55 PM, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] The driver permanently leaks a hardware LUT slot and fails to trigger patterns when a 2-step palindrome pattern evaluates to an `actual_len` of 1.
> > --
> > 
> > --- Patch [1]: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode ---
> > commit c5b003f7bc9326bf3784a70ad883c39b215ae496
> > Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> > Subject: leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode
> > 
> > This commit splits out pattern preparation into lpg_prepare_pattern() and
> > adds support for per-channel scaling so the hardware pattern respects
> > multi-color LED balances. It also updates the LUT storage paths to handle
> > the dynamically allocated scaled patterns.
> > 
> >> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
> >> index d7d6518de30f2..24b1f570f5244 100644
> >> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> >> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> > 
> > [ ... ]
> > 
> >> @@ -1087,43 +1098,64 @@ 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;
> >> +			return -EINVAL;
> >> +		prep->lo_pause = 0;
> >> +		prep->hi_pause = 0;
> >>  	}
> >>  
> >> +	prep->data = no_free_ptr(pattern);
> >> +	prep->len = actual_len;
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does this logic permanently leak a
> > hardware LUT slot if a 2-step palindrome pattern is provided?
> > 
> > If a user-space process provides a 2-step pattern where both steps have the
> > same brightness, the ping_pong logic earlier in lpg_prepare_pattern() will
> > calculate actual_len = (2 + 1) / 2, which evaluates to 1.
> > 
> > When this length-1 pattern is subsequently stored, it allocates 1 slot where
> > lo_idx and hi_idx are equal. Because they are equal, lpg_brightness_set()
> > treats the configuration as a static brightness instead of playing the
> > pattern.
> > 
> > Later, when the pattern is cleared and lpg_lut_free() is called with this
> > lo_idx and hi_idx, it hits this early return:
> > 
> > static void lpg_lut_free(struct lpg *lpg, unsigned int lo_idx, unsigned int hi_idx)
> > {
> > 	int len;
> > 
> > 	len = hi_idx - lo_idx + 1;
> > 	if (len == 1)
> > 		return;
> > 
> > 	bitmap_clear(lpg->lut_bitmap, lo_idx, len);
> > }
> > 
> > Does this early return bypass bitmap_clear() and permanently leak the LUT
> > slot for the remainder of the device lifecycle?
> > 
> 
> I think this should go into a separate patch, since it isn’t introduced
> by this change. On the fix itself, rather than checking "(len == 1)" to
> clear the bitmap, we should add a new flag to track whether the bitmap
> has been allocated and only free it when that flag is true.
> 
> @Jones, what do you think? Would you prefer that I include this fix in
> the current change, or should I push a separate fix after this one?

It's Lee! =:-)

I don't tend to look at existing issues, but if you'd like to submit a
patch for it, great!

-- 
Lee Jones

Re: [PATCH v6] leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode

[Konrad Dybcio]

On 7/17/26 6:41 AM, 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>
> ---

GPT came up with the following fixes/suggestions:

1] This one makes sense at a glance:

    leds: rgb: leds-qcom-lpg: Skip LUT allocation for off colors
    
    Multicolor hardware pattern setup programs a separate LUT range for every
    component. A component with zero calculated brightness is disabled before
    the pattern is applied, so its all-zero range is never used.
    
    Skip allocating it to preserve the shared LUT capacity.
    
    Fixes: 2882fa0dc1cf ("leds: rgb: leds-qcom-lpg: Fix LED color balancing in HW pattern mode")

diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index 24b1f570f524..e32388a16537 100644
--- a/drivers/leds/rgb/leds-qcom-lpg.c
+++ b/drivers/leds/rgb/leds-qcom-lpg.c
@@ -1219,6 +1219,13 @@ static int lpg_pattern_mc_set(struct led_classdev *cdev,
                chan = led->channels[i];
                scale = mc->subled_info[i].brightness;
 
+               /* An off component neither needs nor uses a LUT range. */
+               if (!scale) {
+                       chan->pattern_lo_idx = 0;
+                       chan->pattern_hi_idx = 0;
+                       continue;
+               }
+
                for (j = 0; j < pattern.len; j++) {
                        scaled[j].brightness = DIV_ROUND_CLOSEST(
                                (u32)prep_data[j].brightness * scale, LED_FULL);


2] This one.. I'm not convinced..

    leds: rgb: leds-qcom-lpg: Reuse LUT patterns for equal colors
    
    Multicolor hardware patterns with equal nonzero component brightnesses
    produce identical LUT data. Reuse their LUT range instead of allocating
    and programming duplicate data.
    
    Free each shared range once when clearing a pattern or unwinding a failed
    allocation.

diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index e32388a16537..1c4d550c1ed0 100644
--- a/drivers/leds/rgb/leds-qcom-lpg.c
+++ b/drivers/leds/rgb/leds-qcom-lpg.c
@@ -1175,6 +1175,34 @@ static int lpg_pattern_single_set(struct led_classdev *cdev,
 	return 0;
 }
 
+static void lpg_pattern_free(struct lpg_led *led, unsigned int count)
+{
+	struct lpg_channel *chan;
+	unsigned int i, j;
+
+	for (i = 0; i < count; i++) {
+		chan = led->channels[i];
+		if (chan->pattern_lo_idx == chan->pattern_hi_idx)
+			continue;
+
+		for (j = 0; j < i; j++) {
+			if (chan->pattern_lo_idx == led->channels[j]->pattern_lo_idx &&
+			    chan->pattern_hi_idx == led->channels[j]->pattern_hi_idx)
+				break;
+		}
+
+		if (j == i)
+			lpg_lut_free(chan->lpg, chan->pattern_lo_idx,
+				     chan->pattern_hi_idx);
+	}
+
+	for (i = 0; i < count; i++) {
+		chan = led->channels[i];
+		chan->pattern_lo_idx = 0;
+		chan->pattern_hi_idx = 0;
+	}
+}
+
 static int lpg_pattern_mc_set(struct led_classdev *cdev,
 			      struct led_pattern *led_pattern, u32 len,
 			      int repeat)
@@ -1226,6 +1254,17 @@ static int lpg_pattern_mc_set(struct led_classdev *cdev,
 			continue;
 		}
 
+		for (j = 0; j < i; j++) {
+			if (scale == mc->subled_info[j].brightness) {
+				chan->pattern_lo_idx = led->channels[j]->pattern_lo_idx;
+				chan->pattern_hi_idx = led->channels[j]->pattern_hi_idx;
+				break;
+			}
+		}
+
+		if (j != i)
+			continue;
+
 		for (j = 0; j < pattern.len; j++) {
 			scaled[j].brightness = DIV_ROUND_CLOSEST(
 				(u32)prep_data[j].brightness * scale, LED_FULL);
@@ -1238,14 +1277,7 @@ static int lpg_pattern_mc_set(struct led_classdev *cdev,
 			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;
-			}
-
+			lpg_pattern_free(led, i);
 			return ret;
 		}
 
@@ -1271,13 +1303,12 @@ static int lpg_pattern_clear(struct lpg_led *led)
 
 	mutex_lock(&lpg->lock);
 
+	lpg_pattern_free(led, led->num_channels);
+
 	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;
-		chan->pattern_hi_idx = 0;
 	}
 
 	mutex_unlock(&lpg->lock);