* [PATCH] drm/i915/color: Fix plane color pipeline programming bugs
@ 2026-05-21 18:00 Sean Paul
2026-05-21 18:39 ` Jani Nikula
0 siblings, 1 reply; 6+ messages in thread
From: Sean Paul @ 2026-05-21 18:00 UTC (permalink / raw)
To: intel-gfx
Cc: Sean Paul, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, David Airlie, Simona Vetter, intel-xe, dri-devel
From: Sean Paul <seanpaul@google.com>
Fix two bugs in the plane-level color pipeline programming:
1. Fix a step discontinuity in the Post-CSC Gamma LUT when SDR dimming
is active by clamping Segment 2 to the last user-provided LUT entry
value instead of hardcoding it to 1.0 (1 << 24).
2. Fix a typo in the loop condition in xelpd_program_plane_pre_csc_lut
for Segment 2 degamma programming, changing 'while (i++ > 130)' to
'while (i++ < 130)'. Also clamp Segment 2 to the last user-provided
LUT entry value instead of hardcoding it to 1.0 (1 << 24) to fix
a step discontinuity similar to the Post-CSC fix.
Signed-off-by: Sean Paul <seanpaul@google.com>
---
drivers/gpu/drm/i915/display/intel_color.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 2d318e922671..9b807b024ec3 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3953,6 +3953,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
enum plane_id plane = to_intel_plane(state->plane)->id;
const struct drm_color_lut32 *pre_csc_lut = plane_state->hw.degamma_lut->data;
u32 i, lut_size;
+ u32 lut_val = 1 << 24;
if (icl_is_hdr_plane(display, plane)) {
lut_size = 128;
@@ -3963,7 +3964,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
if (pre_csc_lut) {
for (i = 0; i < lut_size; i++) {
- u32 lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
+ lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
intel_de_write_dsb(display, dsb,
PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
@@ -3975,8 +3976,8 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
do {
intel_de_write_dsb(display, dsb,
PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
- (1 << 24));
- } while (i++ > 130);
+ lut_val);
+ } while (i++ < 130);
} else {
for (i = 0; i < lut_size; i++) {
u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
@@ -4023,11 +4024,11 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
lut_val);
}
- /* Segment 2 */
+ /* Segment 2 - clamp to the last LUT value to prevent step discontinuity */
do {
intel_de_write_dsb(display, dsb,
PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
- (1 << 24));
+ lut_val);
} while (i++ < 34);
} else {
/*TODO: Add for segment 0 */
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs
2026-05-21 18:00 [PATCH] drm/i915/color: Fix plane color pipeline programming bugs Sean Paul
@ 2026-05-21 18:39 ` Jani Nikula
2026-05-21 19:28 ` Sean Paul
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2026-05-21 18:39 UTC (permalink / raw)
To: Sean Paul, intel-gfx
Cc: Sean Paul, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
David Airlie, Simona Vetter, intel-xe, dri-devel
On Thu, 21 May 2026, Sean Paul <sean@poorly.run> wrote:
> From: Sean Paul <seanpaul@google.com>
>
> Fix two bugs in the plane-level color pipeline programming:
> 1. Fix a step discontinuity in the Post-CSC Gamma LUT when SDR dimming
> is active by clamping Segment 2 to the last user-provided LUT entry
> value instead of hardcoding it to 1.0 (1 << 24).
> 2. Fix a typo in the loop condition in xelpd_program_plane_pre_csc_lut
> for Segment 2 degamma programming, changing 'while (i++ > 130)' to
> 'while (i++ < 130)'. Also clamp Segment 2 to the last user-provided
> LUT entry value instead of hardcoding it to 1.0 (1 << 24) to fix
> a step discontinuity similar to the Post-CSC fix.
One fix per patch, please.
For #2 there's already [1].
BR,
Jani.
[1] https://lore.kernel.org/r/20260519075245.383864-1-pranay.samala@intel.com
>
> Signed-off-by: Sean Paul <seanpaul@google.com>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 2d318e922671..9b807b024ec3 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -3953,6 +3953,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
> enum plane_id plane = to_intel_plane(state->plane)->id;
> const struct drm_color_lut32 *pre_csc_lut = plane_state->hw.degamma_lut->data;
> u32 i, lut_size;
> + u32 lut_val = 1 << 24;
>
> if (icl_is_hdr_plane(display, plane)) {
> lut_size = 128;
> @@ -3963,7 +3964,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
>
> if (pre_csc_lut) {
> for (i = 0; i < lut_size; i++) {
> - u32 lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
> + lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
>
> intel_de_write_dsb(display, dsb,
> PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
> @@ -3975,8 +3976,8 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
> do {
> intel_de_write_dsb(display, dsb,
> PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
> - (1 << 24));
> - } while (i++ > 130);
> + lut_val);
> + } while (i++ < 130);
> } else {
> for (i = 0; i < lut_size; i++) {
> u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
> @@ -4023,11 +4024,11 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
> lut_val);
> }
>
> - /* Segment 2 */
> + /* Segment 2 - clamp to the last LUT value to prevent step discontinuity */
> do {
> intel_de_write_dsb(display, dsb,
> PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
> - (1 << 24));
> + lut_val);
> } while (i++ < 34);
> } else {
> /*TODO: Add for segment 0 */
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs
2026-05-21 18:39 ` Jani Nikula
@ 2026-05-21 19:28 ` Sean Paul
2026-05-21 19:39 ` [PATCH v2 1/2] drm/i915/color: Fix step discontinuity in Post-CSC Gamma LUT Sean Paul
2026-05-25 9:42 ` Claude review: Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Sean Paul @ 2026-05-21 19:28 UTC (permalink / raw)
To: Jani Nikula
Cc: Sean Paul, intel-gfx, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, David Airlie, Simona Vetter, intel-xe, dri-devel
On Thu, May 21, 2026 at 2:39 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Thu, 21 May 2026, Sean Paul <sean@poorly.run> wrote:
> > From: Sean Paul <seanpaul@google.com>
> >
> > Fix two bugs in the plane-level color pipeline programming:
> > 1. Fix a step discontinuity in the Post-CSC Gamma LUT when SDR dimming
> > is active by clamping Segment 2 to the last user-provided LUT entry
> > value instead of hardcoding it to 1.0 (1 << 24).
> > 2. Fix a typo in the loop condition in xelpd_program_plane_pre_csc_lut
> > for Segment 2 degamma programming, changing 'while (i++ > 130)' to
> > 'while (i++ < 130)'. Also clamp Segment 2 to the last user-provided
> > LUT entry value instead of hardcoding it to 1.0 (1 << 24) to fix
> > a step discontinuity similar to the Post-CSC fix.
>
> One fix per patch, please.
Ack
>
> For #2 there's already [1].
This isn't in drm-tip or drm-intel afaict. I'll drop it out of my set,
but could you please apply it?
Sean
>
> BR,
> Jani.
>
> [1] https://lore.kernel.org/r/20260519075245.383864-1-pranay.samala@intel.com
>
> >
> > Signed-off-by: Sean Paul <seanpaul@google.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_color.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> > index 2d318e922671..9b807b024ec3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -3953,6 +3953,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
> > enum plane_id plane = to_intel_plane(state->plane)->id;
> > const struct drm_color_lut32 *pre_csc_lut = plane_state->hw.degamma_lut->data;
> > u32 i, lut_size;
> > + u32 lut_val = 1 << 24;
> >
> > if (icl_is_hdr_plane(display, plane)) {
> > lut_size = 128;
> > @@ -3963,7 +3964,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
> >
> > if (pre_csc_lut) {
> > for (i = 0; i < lut_size; i++) {
> > - u32 lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
> > + lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
> >
> > intel_de_write_dsb(display, dsb,
> > PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
> > @@ -3975,8 +3976,8 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
> > do {
> > intel_de_write_dsb(display, dsb,
> > PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
> > - (1 << 24));
> > - } while (i++ > 130);
> > + lut_val);
> > + } while (i++ < 130);
> > } else {
> > for (i = 0; i < lut_size; i++) {
> > u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
> > @@ -4023,11 +4024,11 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
> > lut_val);
> > }
> >
> > - /* Segment 2 */
> > + /* Segment 2 - clamp to the last LUT value to prevent step discontinuity */
> > do {
> > intel_de_write_dsb(display, dsb,
> > PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
> > - (1 << 24));
> > + lut_val);
> > } while (i++ < 34);
> > } else {
> > /*TODO: Add for segment 0 */
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/2] drm/i915/color: Fix step discontinuity in Post-CSC Gamma LUT
2026-05-21 18:39 ` Jani Nikula
2026-05-21 19:28 ` Sean Paul
@ 2026-05-21 19:39 ` Sean Paul
2026-05-21 19:39 ` [PATCH v2 2/2] drm/i915/color: Fix Pre-CSC degamma LUT bounds Sean Paul
2026-05-25 9:42 ` Claude review: Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Sean Paul @ 2026-05-21 19:39 UTC (permalink / raw)
To: intel-gfx
Cc: jani.nikula, Sean Paul, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, David Airlie, Simona Vetter, intel-xe, dri-devel
From: Sean Paul <seanpaul@google.com>
Fix a step discontinuity in the Post-CSC Gamma LUT when SDR dimming
is active by clamping Segment 2 to the last user-provided LUT entry
value instead of hardcoding it to 1.0 (1 << 24).
Link: https://lore.kernel.org/intel-gfx/20260521180143.2143262-1-sean@poorly.run/ #v1
Signed-off-by: Sean Paul <seanpaul@google.com>
Changes in v2:
- Split out into separate patches for pre/post csc fixes
- Dropped loop bounds fix in favor of [1]
[1]- https://lore.kernel.org/r/20260519075245.383864-1-pranay.samala@intel.com
---
drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 2d318e922671..90d5dc2ef3e4 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -4023,11 +4023,11 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
lut_val);
}
- /* Segment 2 */
+ /* Segment 2 - clamp to the last LUT value to prevent step discontinuity */
do {
intel_de_write_dsb(display, dsb,
PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
- (1 << 24));
+ lut_val);
} while (i++ < 34);
} else {
/*TODO: Add for segment 0 */
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] drm/i915/color: Fix Pre-CSC degamma LUT bounds
2026-05-21 19:39 ` [PATCH v2 1/2] drm/i915/color: Fix step discontinuity in Post-CSC Gamma LUT Sean Paul
@ 2026-05-21 19:39 ` Sean Paul
0 siblings, 0 replies; 6+ messages in thread
From: Sean Paul @ 2026-05-21 19:39 UTC (permalink / raw)
To: intel-gfx
Cc: jani.nikula, Sean Paul, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, David Airlie, Simona Vetter, intel-xe, dri-devel
From: Sean Paul <seanpaul@google.com>
Fix a typo in the loop condition in xelpd_program_plane_pre_csc_lut
for Segment 2 degamma programming, changing 'while (i++ > 130)' to
'while (i++ < 130)'. Also clamp Segment 2 to the last user-provided
LUT entry value instead of hardcoding it to 1.0 (1 << 24) to fix
a step discontinuity.
Link: https://lore.kernel.org/intel-gfx/20260521180143.2143262-1-sean@poorly.run/ #v1
Signed-off-by: Sean Paul <seanpaul@google.com>
Changes in v2:
- Split out into separate patches for pre/post csc fixes
- Dropped loop bounds fix in favor of [1]
[1]- https://lore.kernel.org/r/20260519075245.383864-1-pranay.samala@intel.com
---
drivers/gpu/drm/i915/display/intel_color.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 90d5dc2ef3e4..469011c83f9b 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3953,6 +3953,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
enum plane_id plane = to_intel_plane(state->plane)->id;
const struct drm_color_lut32 *pre_csc_lut = plane_state->hw.degamma_lut->data;
u32 i, lut_size;
+ u32 lut_val = 1 << 24;
if (icl_is_hdr_plane(display, plane)) {
lut_size = 128;
@@ -3963,7 +3964,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
if (pre_csc_lut) {
for (i = 0; i < lut_size; i++) {
- u32 lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
+ lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 24);
intel_de_write_dsb(display, dsb,
PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
@@ -3975,7 +3976,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
do {
intel_de_write_dsb(display, dsb,
PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
- (1 << 24));
+ lut_val);
} while (i++ > 130);
} else {
for (i = 0; i < lut_size; i++) {
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Claude review: Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs
2026-05-21 18:39 ` Jani Nikula
2026-05-21 19:28 ` Sean Paul
2026-05-21 19:39 ` [PATCH v2 1/2] drm/i915/color: Fix step discontinuity in Post-CSC Gamma LUT Sean Paul
@ 2026-05-25 9:42 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 9:42 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs
Author: Jani Nikula <jani.nikula@linux.intel.com>
Patches: 5
Reviewed: 2026-05-25T19:42:43.331225
---
This mbox contains **three patches**: the v1 combined patch (`[PATCH]`) and the v2 series split into two (`[PATCH v2 1/2]` and `[PATCH v2 2/2]`). The v1 is superseded by v2. The review focuses on the v2 series.
The fixes are correct in substance — both address a real step-discontinuity bug in the i915 plane color LUT programming where Segment 2 was hardcoded to `1 << 24` instead of continuing from the last user-provided LUT entry. However, **Patch v2 2/2 has a stale commit message that must be fixed before merging**.
The code changes are small, well-scoped, and the split into separate pre-CSC and post-CSC patches is a good improvement over v1.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-25 9:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 18:00 [PATCH] drm/i915/color: Fix plane color pipeline programming bugs Sean Paul
2026-05-21 18:39 ` Jani Nikula
2026-05-21 19:28 ` Sean Paul
2026-05-21 19:39 ` [PATCH v2 1/2] drm/i915/color: Fix step discontinuity in Post-CSC Gamma LUT Sean Paul
2026-05-21 19:39 ` [PATCH v2 2/2] drm/i915/color: Fix Pre-CSC degamma LUT bounds Sean Paul
2026-05-25 9:42 ` Claude review: Re: [PATCH] drm/i915/color: Fix plane color pipeline programming bugs Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox