[v6 0/9] drm/i915/color: Enable SDR plane color pipeline

[v6 0/9] drm/i915/color: Enable SDR plane color pipeline

[Chaitanya Kumar Borah]

This series adds color pipeline support for SDR planes in i915 and
exposes the functionality to userspace through the DRM colorop
framework.

In contrast to HDR planes, SDR planes have LUTs with smaller sizes
and a fixed function CSC block in contrast to a programmable CTM.

The series first introduces a new DRM colorop type,
DRM_COLOROP_FIXED_MATRIX, which represents fixed-function CSC blocks where
userspace selects predefined hardware conversion modes instead of
programming arbitrary matrices. It also adds the YUV correction HW block.
With this the color pipeline can accept both limited range and full range
YUV framebuffers.

With that, the SDR plane color pipeline looks like.

[YUV Range Correct] -> [1D LUT] -> [CSC] -> [1D LUT]

v2:
 - Naming changes [Pekka]
 - Adds YUV color range correct block

v3:
 - Fix NC matrix enum name and string [Melissa]
 - Extract and simplify glk_plane_color_ctl_input_csc into
   helpers (new prep patches)
 - Restructure SDR pre/post-CSC LUT loops to match HDR function
   pattern
 - Drop HDR pre-CSC LUT clamping fix (submitted separately)
 - Rebase

v4:
 - For CI
 - Rebase

v5:
- Do not skip programming legacy property if color pipeline is not set.
  Support for YUV buffers in color pipeline uAPI is a work in progress.
  Therefore, color-pipeline-aware userspace might still use the legacy
  COLOR_RANGE and COLOR_ENCODING property while keeping the color
  pipeline to Bypass. Therefore, skip programming based on legacy
  properties only when a color pipeline is set.

v6:
- Drop YCbCr limited-to-full and RGB709-to-RGB2020 matrices
  until there is a non-IGT userspace implementation. [Robert]
- Drop pre-CSC LUT and YUV range correct blocks from the
  pipeline in favor of a simpler [CSC] -> [1D LUT] pipeline.
  The Fixed Matrix CSC handles both full and limited range
  YCbCr conversions internally.


IGT series (needs update): https://patchwork.freedesktop.org/series/165021/
A rough updated IGT implementation with some additional test can be found here.
https://github.com/ckborah/igt-sandbox/tree/color/sdr-sw-ref

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Melissa Wen <mwen@igalia.com>
Cc: Simon Ser <contact@emersion.fr>
Cc: Alex Hung <alex.hung@amd.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: Pranay Samala <pranay.samala@intel.com>
Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Naveen Kumar <naveen1.kumar@intel.com>
Cc: Robert Mader <robert.mader@collabora.com>

Chaitanya Kumar Borah (7):
  drm/colorop: Add DRM_COLOROP_FIXED_MATRIX
  drm/i915/color: Add CSC on SDR plane color pipeline
  drm/i915/display: extract glk_plane_color_ctl_input_csc helper
  drm/i915/display: simplify glk_plane_color_ctl_input_csc
  drm/i915/display: Program CSC on SDR planes based on Fixed Matrix
    Colorop
  drm/i915/color: Add support for 1D LUT in SDR planes
  drm/i915/color: Add color pipeline support for SDR planes

Pranay Samala (2):
  drm/i915/color: Extract HDR post-CSC LUT programming to helper
    function
  drm/i915/color: Program Plane Post CSC registers for SDR planes

 drivers/gpu/drm/drm_atomic.c                  |   4 +
 drivers/gpu/drm/drm_atomic_uapi.c             |   7 +
 drivers/gpu/drm/drm_colorop.c                 | 107 +++++++++++++++
 drivers/gpu/drm/i915/display/intel_color.c    |  76 ++++++++---
 .../drm/i915/display/intel_color_pipeline.c   |  30 ++++-
 .../drm/i915/display/intel_display_limits.h   |   1 +
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/display/intel_plane.c    |  53 +++++++-
 .../drm/i915/display/skl_universal_plane.c    |  42 +++---
 include/drm/drm_colorop.h                     | 127 ++++++++++++++++++
 include/uapi/drm/drm_mode.h                   |  12 ++
 11 files changed, 420 insertions(+), 40 deletions(-)

-- 
2.50.1

[v6 1/9] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Chaitanya Kumar Borah]

Introduce DRM_COLOROP_FIXED_MATRIX, a new colorop type representing a
hardware that performs a fixed matrix operation.

Unlike CTM-based colorops, this block does not expose programmable
coefficients. Instead, userspace selects one of the predefined
hardware modes via a new FIXED_MATRIX_TYPE enum property. Supported modes
include common YCbCr->RGB and RGB709->RGB2020 conversions.

v5:
 - Drop YCbCr limited to full and RGB709 to RGB2020 matrices
   until we have non-IGT userspace that shows their use (Robert)

v4:
 - Add limited-range YCbCr-RGB conversion matrix enums (Robert)
 - Document matrix values (Pekka)
 - Clarify RGB709 to RGB2020 is a full-range matrix (Pekka)
 - Fix confusing "CSC preset" doc (Pekka)

v2:
 - Naming changes (Pekka)

v3:
 - Fix NC matrix enum name and string (Melissa)
 - Rebase

Co-developed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Melissa Wen <mwen@igalia.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Reviewed-by: Alex Hung <alex.hung@amd.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---
 drivers/gpu/drm/drm_atomic.c      |   4 +
 drivers/gpu/drm/drm_atomic_uapi.c |   7 ++
 drivers/gpu/drm/drm_colorop.c     | 107 +++++++++++++++++++++++++
 include/drm/drm_colorop.h         | 127 ++++++++++++++++++++++++++++++
 include/uapi/drm/drm_mode.h       |  12 +++
 5 files changed, 257 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e5c8ef06caed..45df6376f408 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -925,6 +925,10 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
 				  drm_get_colorop_lut3d_interpolation_name(state->lut3d_interpolation));
 		drm_printf_indent(p, 1, "data blob id=%d\n", state->data ? state->data->base.id : 0);
 		break;
+	case DRM_COLOROP_FIXED_MATRIX:
+		drm_printf_indent(p, 1, "fixed_matrix_type=%s\n",
+				  drm_get_colorop_fixed_matrix_type_name(state->fixed_matrix_type));
+		break;
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 1050dddadb17..d52504d823bc 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -786,6 +786,11 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
 		return drm_atomic_color_set_data_property(colorop, state,
 							  property, val,
 							  replaced);
+	} else if (property == colorop->fixed_matrix_type_property) {
+		if (state->fixed_matrix_type != val) {
+			state->fixed_matrix_type = val;
+			*replaced = true;
+		}
 	} else {
 		drm_dbg_atomic(colorop->dev,
 			       "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
@@ -818,6 +823,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
 		*val = state->lut3d_interpolation;
 	else if (property == colorop->data_property)
 		*val = (state->data) ? state->data->base.id : 0;
+	else if (property == colorop->fixed_matrix_type_property)
+		*val = state->fixed_matrix_type;
 	else
 		return -EINVAL;
 
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 682fcc651525..95da03d6c65c 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -68,6 +68,7 @@ static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
 	{ DRM_COLOROP_CTM_3X4, "3x4 Matrix"},
 	{ DRM_COLOROP_MULTIPLIER, "Multiplier"},
 	{ DRM_COLOROP_3D_LUT, "3D LUT"},
+	{ DRM_COLOROP_FIXED_MATRIX, "Fixed Matrix"},
 };
 
 static const char * const colorop_curve_1d_type_names[] = {
@@ -90,6 +91,15 @@ static const struct drm_prop_enum_list drm_colorop_lut3d_interpolation_list[] =
 	{ DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL, "Tetrahedral" },
 };
 
+static const char * const colorop_fixed_matrix_type_names[] = {
+	[DRM_COLOROP_FM_YCBCR601_FULL_RGB] = "YCbCr 601 Full to RGB",
+	[DRM_COLOROP_FM_YCBCR601_LIMITED_RGB] = "YCbCr 601 Limited to RGB",
+	[DRM_COLOROP_FM_YCBCR709_FULL_RGB] = "YCbCr 709 Full to RGB",
+	[DRM_COLOROP_FM_YCBCR709_LIMITED_RGB] = "YCbCr 709 Limited to RGB",
+	[DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB] = "YCbCr 2020 NC Full to RGB",
+	[DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB] = "YCbCr 2020 NC Limited to RGB",
+};
+
 /* Init Helpers */
 
 static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
@@ -453,6 +463,80 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
 }
 EXPORT_SYMBOL(drm_plane_colorop_3dlut_init);
 
+/**
+ * drm_plane_colorop_fixed_matrix_init - Initialize a DRM_COLOROP_FIXED_MATRIX
+ *
+ * @dev: DRM device
+ * @colorop: The drm_colorop object to initialize
+ * @plane: The associated drm_plane
+ * @funcs: control functions for the new colorop
+ * @supported_fm: A bitfield of supported drm_colorop_fixed_matrix_type enum values,
+ *               created using BIT(fixed_matrix_type) and combined with the OR '|'
+ *               operator.
+ * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
+ * @return zero on success, -E value on failure
+ */
+int drm_plane_colorop_fixed_matrix_init(struct drm_device *dev, struct drm_colorop *colorop,
+					struct drm_plane *plane,
+					const struct drm_colorop_funcs *funcs,
+					u64 supported_fm, uint32_t flags)
+{
+	struct drm_prop_enum_list enum_list[DRM_COLOROP_FM_COUNT];
+	int i, len;
+	struct drm_property *prop;
+	int ret;
+
+	if (!supported_fm) {
+		drm_err(dev,
+			"No supported FM type op for new Fixed Matrix colorop on [PLANE:%d:%s]\n",
+			plane->base.id, plane->name);
+		return -EINVAL;
+	}
+
+	if ((supported_fm & -BIT(DRM_COLOROP_FM_COUNT)) != 0) {
+		drm_err(dev, "Unknown Fixed Matrix provided on [PLANE:%d:%s]\n",
+			plane->base.id, plane->name);
+		return -EINVAL;
+	}
+
+	ret = drm_plane_colorop_init(dev, colorop, plane, funcs, DRM_COLOROP_FIXED_MATRIX, flags);
+	if (ret)
+		return ret;
+
+	len = 0;
+	for (i = 0; i < DRM_COLOROP_FM_COUNT; i++) {
+		if ((supported_fm & BIT(i)) == 0)
+			continue;
+
+		enum_list[len].type = i;
+		enum_list[len].name = colorop_fixed_matrix_type_names[i];
+		len++;
+	}
+
+	if (WARN_ON(len <= 0))
+		return -EINVAL;
+
+	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC, "FIXED_MATRIX_TYPE",
+					enum_list, len);
+
+	if (!prop)
+		return -ENOMEM;
+
+	colorop->fixed_matrix_type_property = prop;
+	/*
+	 * Default to the first supported CSC mode as provided by the driver.
+	 * Intuitively this should be something that keeps the colorop in pixel bypass
+	 * mode but that is already handled via the standard colorop bypass
+	 * property.
+	 */
+	drm_object_attach_property(&colorop->base, colorop->fixed_matrix_type_property,
+				   enum_list[0].type);
+	drm_colorop_reset(colorop);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_colorop_fixed_matrix_init);
+
 static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop,
 							struct drm_colorop_state *state)
 {
@@ -533,6 +617,13 @@ static void __drm_colorop_state_init(struct drm_colorop_state *colorop_state,
 							   &val))
 			colorop_state->lut3d_interpolation = val;
 	}
+
+	if (colorop->fixed_matrix_type_property) {
+		if (!drm_object_property_get_default_value(&colorop->base,
+							   colorop->fixed_matrix_type_property,
+							   &val))
+			colorop_state->fixed_matrix_type = val;
+	}
 }
 
 /**
@@ -596,6 +687,7 @@ static const char * const colorop_type_name[] = {
 	[DRM_COLOROP_CTM_3X4] = "3x4 Matrix",
 	[DRM_COLOROP_MULTIPLIER] = "Multiplier",
 	[DRM_COLOROP_3D_LUT] = "3D LUT",
+	[DRM_COLOROP_FIXED_MATRIX] = "Fixed Matrix",
 };
 
 static const char * const colorop_lu3d_interpolation_name[] = {
@@ -652,6 +744,21 @@ const char *drm_get_colorop_lut3d_interpolation_name(enum drm_colorop_lut3d_inte
 	return colorop_lu3d_interpolation_name[type];
 }
 
+/**
+ * drm_get_colorop_fixed_matrix_type_name: return a string for fixed matrix type
+ * @type: fixed matrix type to compute name of
+ *
+ * In contrast to the other drm_get_*_name functions this one here returns a
+ * const pointer and hence is threadsafe.
+ */
+const char *drm_get_colorop_fixed_matrix_type_name(enum drm_colorop_fixed_matrix_type type)
+{
+	if (WARN_ON(type >= ARRAY_SIZE(colorop_fixed_matrix_type_names)))
+		return "unknown";
+
+	return colorop_fixed_matrix_type_names[type];
+}
+
 /**
  * drm_colorop_set_next_property - sets the next pointer
  * @colorop: drm colorop
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 224fae40ed2b..26ef67c835e2 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -134,6 +134,114 @@ enum drm_colorop_curve_1d_type {
 	DRM_COLOROP_1D_CURVE_COUNT
 };
 
+/**
+ * enum drm_colorop_fixed_matrix_type - type of Fixed Matrix
+ *
+ * Describes a Fixed Matrix operation to be applied by the DRM_COLOROP_FIXED_MATRIX
+ */
+enum drm_colorop_fixed_matrix_type {
+	/**
+	 * @DRM_COLOROP_FM_YCBCR601_FULL_RGB:
+	 *
+	 * enum string "YCbCr 601 Full to RGB"
+	 *
+	 * Converts full-range YCbCr into full-range RGB using the BT.601
+	 * coefficients. Y is normalized to [0, 1] and Cb, Cr are centered
+	 * at 0 (the nominal 0.5 offset removed) before the matrix::
+	 *
+	 *   | R |   | 1.0   0.000000   1.402000 |   | Y  |
+	 *   | G | = | 1.0  -0.344136  -0.714136 | x | Cb |
+	 *   | B |   | 1.0   1.772000   0.000000 |   | Cr |
+	 */
+	DRM_COLOROP_FM_YCBCR601_FULL_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR601_LIMITED_RGB:
+	 *
+	 * enum string "YCbCr 601 Limited to RGB"
+	 *
+	 * Converts limited- (narrow-) range YCbCr into full-range RGB using
+	 * the BT.601 coefficients. Before the matrix Y is offset by 16/255
+	 * and Cb, Cr are centered at 0, following the studio-range
+	 * convention of ITU-R BT.601::
+	 *
+	 *   | R |   | 1.164384   0.000000   1.596027 |   | Y - 16/255 |
+	 *   | G | = | 1.164384  -0.391762  -0.812968 | x | Cb         |
+	 *   | B |   | 1.164384   2.017232   0.000000 |   | Cr         |
+	 */
+	DRM_COLOROP_FM_YCBCR601_LIMITED_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
+	 *
+	 * enum string "YCbCr 709 Full to RGB"
+	 *
+	 * Converts full-range YCbCr into full-range RGB using the BT.709
+	 * coefficients. Y is normalized to [0, 1] and Cb, Cr are centered
+	 * at 0 (the nominal 0.5 offset removed) before the matrix::
+	 *
+	 *   | R |   | 1.0   0.000000   1.574800 |   | Y  |
+	 *   | G | = | 1.0  -0.187324  -0.468124 | x | Cb |
+	 *   | B |   | 1.0   1.855600   0.000000 |   | Cr |
+	 */
+	DRM_COLOROP_FM_YCBCR709_FULL_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR709_LIMITED_RGB:
+	 *
+	 * enum string "YCbCr 709 Limited to RGB"
+	 *
+	 * Converts limited- (narrow-) range YCbCr into full-range RGB using
+	 * the BT.709 coefficients. Before the matrix Y is offset by 16/255
+	 * and Cb, Cr are centered at 0, following the studio-range
+	 * convention of ITU-R BT.709::
+	 *
+	 *   | R |   | 1.164384   0.000000   1.792741 |   | Y - 16/255 |
+	 *   | G | = | 1.164384  -0.213249  -0.532909 | x | Cb         |
+	 *   | B |   | 1.164384   2.112402   0.000000 |   | Cr         |
+	 */
+	DRM_COLOROP_FM_YCBCR709_LIMITED_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
+	 *
+	 * enum string "YCbCr 2020 NC Full to RGB"
+	 *
+	 * Converts full-range YCbCr into full-range RGB using the BT.2020
+	 * non-constant luminance coefficients. Y is normalized to [0, 1]
+	 * and Cb, Cr are centered at 0 (the nominal 0.5 offset removed)
+	 * before the matrix::
+	 *
+	 *   | R |   | 1.0   0.000000   1.474600 |   | Y  |
+	 *   | G | = | 1.0  -0.164553  -0.571353 | x | Cb |
+	 *   | B |   | 1.0   1.881400   0.000000 |   | Cr |
+	 */
+	DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB:
+	 *
+	 * enum string "YCbCr 2020 NC Limited to RGB"
+	 *
+	 * Converts limited- (narrow-) range YCbCr into full-range RGB using
+	 * the BT.2020 non-constant luminance coefficients. Before the matrix
+	 * Y is offset by 16/255 and Cb, Cr are centered at 0, following the
+	 * studio-range convention of ITU-R BT.2020::
+	 *
+	 *   | R |   | 1.164384   0.000000   1.678674 |   | Y - 16/255 |
+	 *   | G | = | 1.164384  -0.187326  -0.650424 | x | Cb         |
+	 *   | B |   | 1.164384   2.141772   0.000000 |   | Cr         |
+	 */
+	DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_COUNT:
+	 *
+	 * enum value denoting the size of the enum
+	 */
+	DRM_COLOROP_FM_COUNT
+};
+
 /**
  * struct drm_colorop_state - mutable colorop state
  */
@@ -197,6 +305,13 @@ struct drm_colorop_state {
 	 */
 	enum drm_colorop_lut3d_interpolation_type lut3d_interpolation;
 
+	/**
+	 * @fixed_matrix_type:
+	 *
+	 * Type of Fixed Matrix operation.
+	 */
+	enum drm_colorop_fixed_matrix_type fixed_matrix_type;
+
 	/** @state: backpointer to global drm_atomic_commit */
 	struct drm_atomic_commit *state;
 };
@@ -366,6 +481,13 @@ struct drm_colorop {
 	 */
 	struct drm_property *data_property;
 
+	/**
+	 * @fixed_matrix_type_property:
+	 *
+	 * Sub-type for DRM_COLOROP_FIXED_MATRIX type.
+	 */
+	struct drm_property *fixed_matrix_type_property;
+
 	/**
 	 * @next_property:
 	 *
@@ -422,6 +544,10 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
 				 uint32_t lut_size,
 				 enum drm_colorop_lut3d_interpolation_type interpolation,
 				 uint32_t flags);
+int drm_plane_colorop_fixed_matrix_init(struct drm_device *dev, struct drm_colorop *colorop,
+					struct drm_plane *plane,
+					const struct drm_colorop_funcs *funcs,
+					u64 supported_fm, uint32_t flags);
 
 struct drm_colorop_state *
 drm_atomic_helper_colorop_create_state(struct drm_colorop *colorop);
@@ -480,6 +606,7 @@ drm_get_colorop_lut1d_interpolation_name(enum drm_colorop_lut1d_interpolation_ty
 
 const char *
 drm_get_colorop_lut3d_interpolation_name(enum drm_colorop_lut3d_interpolation_type type);
+const char *drm_get_colorop_fixed_matrix_type_name(enum drm_colorop_fixed_matrix_type type);
 
 void drm_colorop_set_next_property(struct drm_colorop *colorop, struct drm_colorop *next);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index bd435effdcee..ed1a660a3dfd 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -955,6 +955,18 @@ enum drm_colorop_type {
 	 *         color = lut3d[index]
 	 */
 	DRM_COLOROP_3D_LUT,
+
+	/**
+	 * @DRM_COLOROP_FIXED_MATRIX:
+	 *
+	 * enum string "Fixed Matrix"
+	 *
+	 * A Colorop block that performs a pre-defined matrix operation selected
+	 * via the FIXED_MATRIX_TYPE enum property. The driver advertises the supported
+	 * operations through this property.
+	 */
+	DRM_COLOROP_FIXED_MATRIX,
+
 };
 
 /**
-- 
2.50.1

[v6 2/9] drm/i915/color: Add CSC on SDR plane color pipeline

[Chaitanya Kumar Borah]

Add the fixed-function CSC block to color pipeline in SDR planes
as a DRM_COLOROP_FIXED_MATRIX colorop.

v2:
- s/DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC/
	DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB
- Inline icl_is_hdr_plane() instead of storing in local variable

v3:
- In preparation of a simple pipeline
	[YUV Full/Limited -> RGB]
  Make the Fixed Matrix ColorOp support Limited Range enums
  (DRM_COLOROP_FM_YCBCRXXX_LIMITED_RGB) too.
- Therefore s/sdr_plane_pipeline/sdr_plane_yuv_pipeline

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com> #v2
---
 .../drm/i915/display/intel_color_pipeline.c   | 23 ++++++++++++++++++-
 .../drm/i915/display/intel_display_limits.h   |  1 +
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 6cf8080ee800..efd4375c4331 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -43,6 +43,18 @@ static const enum intel_color_block hdr_plane_pipeline[] = {
 	INTEL_PLANE_CB_POST_CSC_LUT,
 };
 
+static const enum intel_color_block sdr_plane_yuv_pipeline[] = {
+	INTEL_PLANE_CB_CSC_FF,
+};
+
+static const u64 intel_plane_supported_csc_ff =
+		BIT(DRM_COLOROP_FM_YCBCR601_FULL_RGB) |
+		BIT(DRM_COLOROP_FM_YCBCR601_LIMITED_RGB) |
+		BIT(DRM_COLOROP_FM_YCBCR709_FULL_RGB) |
+		BIT(DRM_COLOROP_FM_YCBCR709_LIMITED_RGB) |
+		BIT(DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB) |
+		BIT(DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB);
+
 static bool plane_has_3dlut(struct intel_display *display, enum pipe pipe,
 			    struct drm_plane *plane)
 {
@@ -92,6 +104,12 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
 							  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		break;
+	case INTEL_PLANE_CB_CSC_FF:
+		ret = drm_plane_colorop_fixed_matrix_init(dev, &colorop->base, plane,
+							  &intel_colorop_funcs,
+							  intel_plane_supported_csc_ff,
+							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		break;
 	default:
 		drm_err(plane->dev, "Invalid colorop id [%d]", id);
 		ret = -EINVAL;
@@ -126,9 +144,12 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	if (plane_has_3dlut(display, pipe, plane)) {
 		pipeline = xe3plpd_primary_plane_pipeline;
 		pipeline_len = ARRAY_SIZE(xe3plpd_primary_plane_pipeline);
-	} else {
+	} else if (icl_is_hdr_plane(display, to_intel_plane(plane)->id)) {
 		pipeline = hdr_plane_pipeline;
 		pipeline_len = ARRAY_SIZE(hdr_plane_pipeline);
+	} else {
+		pipeline = sdr_plane_yuv_pipeline;
+		pipeline_len = ARRAY_SIZE(sdr_plane_yuv_pipeline);
 	}
 
 	for (i = 0; i < pipeline_len; i++) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_limits.h b/drivers/gpu/drm/i915/display/intel_display_limits.h
index ea89473c177f..7ba7360c574e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_limits.h
+++ b/drivers/gpu/drm/i915/display/intel_display_limits.h
@@ -169,6 +169,7 @@ enum aux_ch {
 enum intel_color_block {
 	INTEL_PLANE_CB_PRE_CSC_LUT,
 	INTEL_PLANE_CB_CSC,
+	INTEL_PLANE_CB_CSC_FF,
 	INTEL_PLANE_CB_POST_CSC_LUT,
 	INTEL_PLANE_CB_3DLUT,
 
-- 
2.50.1

[v6 3/9] drm/i915/display: extract glk_plane_color_ctl_input_csc helper

[Chaitanya Kumar Borah]

Extract the input CSC and YUV range correction logic from
glk_plane_color_ctl() into a dedicated glk_plane_color_ctl_input_csc()
helper. No functional change.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 .../drm/i915/display/skl_universal_plane.c    | 32 +++++++++++--------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 07a683293352..0234492425d5 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1240,37 +1240,43 @@ static u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state)
 	return plane_color_ctl;
 }
 
-static u32 glk_plane_color_ctl(const struct intel_plane_state *plane_state)
+static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_state)
 {
 	struct intel_display *display = to_intel_display(plane_state);
 	const struct drm_framebuffer *fb = plane_state->hw.fb;
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
-	u32 plane_color_ctl = 0;
-
-	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
-	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
+	u32 ctl = 0;
 
 	if (fb->format->is_yuv && !icl_is_hdr_plane(display, plane->id)) {
 		switch (plane_state->hw.color_encoding) {
 		case DRM_COLOR_YCBCR_BT709:
-			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
+			ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
 			break;
 		case DRM_COLOR_YCBCR_BT2020:
-			plane_color_ctl |=
-				PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
+			ctl |= PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
 			break;
 		default:
-			plane_color_ctl |=
-				PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601;
+			ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601;
 		}
 		if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+			ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	} else if (fb->format->is_yuv) {
-		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
+		ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
 		if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+			ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	}
 
+	return ctl;
+}
+
+static u32 glk_plane_color_ctl(const struct intel_plane_state *plane_state)
+{
+	u32 plane_color_ctl = 0;
+
+	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
+	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
+	plane_color_ctl |= glk_plane_color_ctl_input_csc(plane_state);
+
 	if (plane_state->force_black)
 		plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE;
 
-- 
2.50.1

[v6 4/9] drm/i915/display: simplify glk_plane_color_ctl_input_csc

[Chaitanya Kumar Borah]

Add early return for non-YUV formats and hoist the duplicated
color_range check out of the if/else branches. No functional change.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 0234492425d5..73be4c6ff1d3 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1247,7 +1247,10 @@ static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_s
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	u32 ctl = 0;
 
-	if (fb->format->is_yuv && !icl_is_hdr_plane(display, plane->id)) {
+	if (!fb->format->is_yuv)
+		return 0;
+
+	if (!icl_is_hdr_plane(display, plane->id)) {
 		switch (plane_state->hw.color_encoding) {
 		case DRM_COLOR_YCBCR_BT709:
 			ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
@@ -1258,14 +1261,13 @@ static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_s
 		default:
 			ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601;
 		}
-		if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-			ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
-	} else if (fb->format->is_yuv) {
+	} else {
 		ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
-		if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-			ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	}
 
+	if (plane_state->hw.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
+		ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+
 	return ctl;
 }
 
-- 
2.50.1

[v6 5/9] drm/i915/display: Program CSC on SDR planes based on Fixed Matrix Colorop

[Chaitanya Kumar Borah]

When a color pipeline is active, program the SDR plane fixed-function
CSC based on the Fixed Matrix Colorop's state. Re-use the existing plane
state variables for color_range and color_encoding. Track the bypass state
explicitly as a boolean since bypass is managed separately from the
FIXED_MATRIX enum value in the colorop framework. Keep the programming
based on color_encoding/color_range legacy properties intact.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  1 +
 drivers/gpu/drm/i915/display/intel_plane.c    | 53 ++++++++++++++++++-
 .../drm/i915/display/skl_universal_plane.c    |  4 +-
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 20a07ea06b5e..beb0169cabef 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -683,6 +683,7 @@ struct intel_plane_state {
 		enum drm_color_range color_range;
 		enum drm_scaling_filter scaling_filter;
 		struct drm_property_blob *ctm, *degamma_lut, *gamma_lut, *lut_3d;
+		bool csc_ff_enable;
 	} hw;
 
 	struct i915_vma *ggtt_vma;
diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
index d0f99a87c42e..25ca049009ef 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -462,6 +462,42 @@ intel_plane_colorop_replace_blob(struct intel_plane_state *plane_state,
 	return false;
 }
 
+static u32
+fixedmatrix_colorop_to_encoding(enum drm_colorop_fixed_matrix_type fm_type)
+{
+	switch (fm_type) {
+	case DRM_COLOROP_FM_YCBCR709_FULL_RGB:
+	case DRM_COLOROP_FM_YCBCR709_LIMITED_RGB:
+		return DRM_COLOR_YCBCR_BT709;
+
+	case DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
+	case DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB:
+		return DRM_COLOR_YCBCR_BT2020;
+
+	case DRM_COLOROP_FM_YCBCR601_FULL_RGB:
+	case DRM_COLOROP_FM_YCBCR601_LIMITED_RGB:
+	default:
+		return DRM_COLOR_YCBCR_BT601;
+	}
+}
+
+static u32
+fixedmatrix_colorop_to_range(enum drm_colorop_fixed_matrix_type fm_type)
+{
+	switch (fm_type) {
+	case DRM_COLOROP_FM_YCBCR601_FULL_RGB:
+	case DRM_COLOROP_FM_YCBCR709_FULL_RGB:
+	case DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
+		return DRM_COLOR_YCBCR_FULL_RANGE;
+
+	case DRM_COLOROP_FM_YCBCR601_LIMITED_RGB:
+	case DRM_COLOROP_FM_YCBCR709_LIMITED_RGB:
+	case DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB:
+	default:
+		return DRM_COLOR_YCBCR_LIMITED_RANGE;
+	}
+}
+
 static void
 intel_plane_color_copy_uapi_to_hw_state(struct intel_atomic_state *state,
 					struct intel_plane_state *plane_state,
@@ -474,6 +510,7 @@ intel_plane_color_copy_uapi_to_hw_state(struct intel_atomic_state *state,
 	struct drm_property_blob *blob;
 	struct intel_crtc_state *new_crtc_state = state ?
 		intel_atomic_get_new_crtc_state(state, crtc) : NULL;
+	enum drm_colorop_fixed_matrix_type fm_type;
 	bool changed = false;
 	int i = 0;
 
@@ -485,11 +522,23 @@ intel_plane_color_copy_uapi_to_hw_state(struct intel_atomic_state *state,
 	while (iter_colorop) {
 		for_each_new_colorop_in_state(&state->base, colorop, new_colorop_state, i) {
 			if (new_colorop_state->colorop == iter_colorop) {
-				blob = new_colorop_state->bypass ? NULL : new_colorop_state->data;
 				intel_colorop = to_intel_colorop(colorop);
-				changed |= intel_plane_colorop_replace_blob(plane_state,
+				if (intel_colorop->id == INTEL_PLANE_CB_CSC_FF) {
+					fm_type = new_colorop_state->fixed_matrix_type;
+
+					plane_state->hw.csc_ff_enable =
+						!new_colorop_state->bypass;
+					plane_state->hw.color_encoding =
+						fixedmatrix_colorop_to_encoding(fm_type);
+					plane_state->hw.color_range =
+						fixedmatrix_colorop_to_range(fm_type);
+				} else {
+					blob = new_colorop_state->bypass ?
+						NULL : new_colorop_state->data;
+					changed |= intel_plane_colorop_replace_blob(plane_state,
 									    intel_colorop,
 									    blob);
+				}
 			}
 		}
 		iter_colorop = iter_colorop->next;
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 73be4c6ff1d3..222c3b6bbe46 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1245,9 +1245,11 @@ static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_s
 	struct intel_display *display = to_intel_display(plane_state);
 	const struct drm_framebuffer *fb = plane_state->hw.fb;
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
+	bool color_pipeline = !!plane_state->uapi.color_pipeline;
+	bool needs_csc = color_pipeline ? plane_state->hw.csc_ff_enable : fb->format->is_yuv;
 	u32 ctl = 0;
 
-	if (!fb->format->is_yuv)
+	if (!needs_csc)
 		return 0;
 
 	if (!icl_is_hdr_plane(display, plane->id)) {
-- 
2.50.1

[v6 6/9] drm/i915/color: Add support for 1D LUT in SDR planes

[Chaitanya Kumar Borah]

Extend the SDR plane color pipeline to post-CSC 1D LUT block.

v2:
 - In preparation of a simple pipeline
	[YUV Full/Limited -> RGB] -> [1D LUT]
   Drop pre-CSC LUT from the pipeline as it has no use in a YUV -> RGB
   pipeline. This makes the pipeline simple since the block lies between
   the YUV range correct block and Fixed function CSC. It can be added back
   when [RGB709 -> RGB2020] capability is added. Then it can be used for
   linearization.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com> #v1
---
 drivers/gpu/drm/i915/display/intel_color_pipeline.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index efd4375c4331..53e55ce0a5a3 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -45,6 +45,7 @@ static const enum intel_color_block hdr_plane_pipeline[] = {
 
 static const enum intel_color_block sdr_plane_yuv_pipeline[] = {
 	INTEL_PLANE_CB_CSC_FF,
+	INTEL_PLANE_CB_POST_CSC_LUT,
 };
 
 static const u64 intel_plane_supported_csc_ff =
-- 
2.50.1

[v6 7/9] drm/i915/color: Extract HDR post-CSC LUT programming to helper function

[Chaitanya Kumar Borah]

From: Pranay Samala <pranay.samala@intel.com>

Move HDR plane post-CSC LUT programming to improve code organization.

While at it, remove the segment 0 index register writes as it is not
currently programmed.

Signed-off-by: Pranay Samala <pranay.samala@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 35 ++++++++++++----------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 87ced9f6ff40..36c8688ed28e 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3998,25 +3998,17 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 }
 
 static void
-xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
-				 const struct intel_plane_state *plane_state)
+xelpd_load_hdr_post_csc_lut(struct intel_display *display,
+			    struct intel_dsb *dsb,
+			    enum pipe pipe,
+			    enum plane_id plane,
+			    const struct drm_color_lut32 *post_csc_lut)
 {
-	struct intel_display *display = to_intel_display(plane_state);
-	const struct drm_plane_state *state = &plane_state->uapi;
-	enum pipe pipe = to_intel_plane(state->plane)->pipe;
-	enum plane_id plane = to_intel_plane(state->plane)->id;
-	const struct drm_color_lut32 *post_csc_lut = plane_state->hw.gamma_lut->data;
 	int i, lut_size = 32;
 	u32 lut_val;
 
-	if (!icl_is_hdr_plane(display, plane))
-		return;
-
 	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0),
 			   PLANE_PAL_PREC_AUTO_INCREMENT);
-	/* TODO: Add macro */
-	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH(pipe, plane, 0),
-			   PLANE_PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < lut_size + 3; i++) {
 		if (post_csc_lut) {
@@ -4036,8 +4028,21 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
 	}
 
 	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
-	intel_de_write_dsb(display, dsb,
-			   PLANE_POST_CSC_GAMC_SEG0_INDEX_ENH(pipe, plane, 0), 0);
+}
+
+static void
+xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
+				 const struct intel_plane_state *plane_state)
+{
+	struct intel_display *display = to_intel_display(plane_state);
+	const struct drm_plane_state *state = &plane_state->uapi;
+	enum pipe pipe = to_intel_plane(state->plane)->pipe;
+	enum plane_id plane = to_intel_plane(state->plane)->id;
+	const struct drm_color_lut32 *post_csc_lut = plane_state->hw.gamma_lut ?
+		plane_state->hw.gamma_lut->data : NULL;
+
+	if (icl_is_hdr_plane(display, plane))
+		xelpd_load_hdr_post_csc_lut(display, dsb, pipe, plane, post_csc_lut);
 }
 
 static void
-- 
2.50.1

[v6 8/9] drm/i915/color: Program Plane Post CSC registers for SDR planes

[Chaitanya Kumar Borah]

From: Pranay Samala <pranay.samala@intel.com>

Implement plane post-CSC LUT support for SDR planes.

v2:
- Restructure loop to match HDR function pattern

Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Pranay Samala <pranay.samala@intel.com>
Co-developed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 36c8688ed28e..f1df0f9ba762 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -4030,6 +4030,45 @@ xelpd_load_hdr_post_csc_lut(struct intel_display *display,
 	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
 }
 
+static void
+xelpd_load_sdr_post_csc_lut(struct intel_display *display,
+			    struct intel_dsb *dsb,
+			    enum pipe pipe,
+			    enum plane_id plane,
+			    const struct drm_color_lut32 *post_csc_lut)
+{
+	int i, lut_size = 32;
+	u32 lut_val;
+
+	/*
+	 * First 3 planes are HDR, so reduce by 3 to get to the right
+	 * SDR plane offset
+	 */
+	plane = plane - 3;
+
+	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_INDEX(pipe, plane, 0),
+			   PLANE_PAL_PREC_AUTO_INCREMENT);
+
+	for (i = 0; i < lut_size + 3; i++) {
+		if (post_csc_lut) {
+			if (i < lut_size)
+				lut_val = drm_color_lut32_extract(post_csc_lut[i].green, 16);
+			/* else duplicate last lut_val */
+		} else {
+			if (i < lut_size)
+				lut_val = (i * ((1 << 16) - 1)) / (lut_size - 1);
+			else
+				lut_val = 1 << 16;
+		}
+
+		intel_de_write_dsb(display, dsb,
+				   PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+				   lut_val);
+	}
+
+	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_INDEX(pipe, plane, 0), 0);
+}
+
 static void
 xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
 				 const struct intel_plane_state *plane_state)
@@ -4043,6 +4082,8 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
 
 	if (icl_is_hdr_plane(display, plane))
 		xelpd_load_hdr_post_csc_lut(display, dsb, pipe, plane, post_csc_lut);
+	else
+		xelpd_load_sdr_post_csc_lut(display, dsb, pipe, plane, post_csc_lut);
 }
 
 static void
-- 
2.50.1

[v6 9/9] drm/i915/color: Add color pipeline support for SDR planes

[Chaitanya Kumar Borah]

Now that everything is in place expose the SDR plane color pipeline
to user-space.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color_pipeline.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 53e55ce0a5a3..38cfd6ed585d 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -177,17 +177,11 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 
 int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
 {
-	struct drm_device *dev = plane->dev;
-	struct intel_display *display = to_intel_display(dev);
 	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
 	int len = 0;
 	int ret = 0;
 	int i;
 
-	/* Currently expose pipeline only for HDR planes */
-	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id))
-		return 0;
-
 	/* Add pipeline consisting of transfer functions */
 	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);
 	if (ret)
-- 
2.50.1

Re: [v6 2/9] drm/i915/color: Add CSC on SDR plane color pipeline

[sashiko-bot]

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

New issues:
- [Medium] The newly added SDR color pipeline is entirely unreachable because its initialization is blocked by an early return guard for non-HDR planes.
- [Low] Validating a 64-bit mask (`supported_fm`) using `-BIT()` causes integer truncation on 32-bit platforms, failing to check the upper 32 bits of the bitmask.

Pre-existing issues:
- [High] Error handling in `intel_color_pipeline_plane_add_colorop()` triggers multiple NULL pointer dereferences when tearing down partially initialized colorops.
- [High] Missing `drm_mode_object_unregister()` in `drm_colorop_cleanup()` causes IDR leaks and potential Use-After-Free of drm_colorop objects.
--

--- Patch [2]: [v6 2/9] drm/i915/color: Add CSC on SDR plane color pipeline ---
commit 8f7cf1094859b74b7d23e60a46f2012f047db34f
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Subject: drm/i915/color: Add CSC on SDR plane color pipeline

This commit adds the fixed-function CSC block to the color pipeline in SDR
planes as a DRM_COLOROP_FIXED_MATRIX colorop.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c

[ ... ]

> @@ -92,6 +104,12 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
>  							  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>  							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		break;
> +	case INTEL_PLANE_CB_CSC_FF:
> +		ret = drm_plane_colorop_fixed_matrix_init(dev, &colorop->base, plane,
> +							  &intel_colorop_funcs,
> +							  intel_plane_supported_csc_ff,
> +							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
> +		break;

[Severity: Low]
Does this call expose a truncation bug on 32-bit platforms?

Looking at drm_plane_colorop_fixed_matrix_init() in drm_colorop.c:

drm_colorop.c:drm_plane_colorop_fixed_matrix_init() {
    ...
    if ((supported_fm & -BIT(DRM_COLOROP_FM_COUNT)) != 0) {
        drm_err(dev, "Unknown Fixed Matrix provided...\n");
        return -EINVAL;
    }
    ...
}

Since BIT() uses UL, on 32-bit systems -BIT() evaluates as an unsigned
32-bit value. When promoted to 64-bit to evaluate against supported_fm,
it zero-extends. This means the upper 32 bits are not checked properly,
which could silently allow invalid caller configurations where bits >= 32
are set.

[Severity: High]
This is a pre-existing issue, but does this error path cause a NULL pointer
dereference?

If drm_plane_colorop_fixed_matrix_init() fails and returns an error before
setting colorop->dev, we jump to the cleanup label in
intel_color_pipeline_plane_add_colorop():

intel_color_pipeline.c:intel_color_pipeline_plane_add_colorop() {
    ...
cleanup:
    intel_colorop_destroy(&colorop->base);
    return ERR_PTR(ret);
}

This calls drm_colorop_cleanup(), which unconditionally dereferences
colorop->dev:

drm_colorop.c:drm_colorop_cleanup() {
    struct drm_device *dev = colorop->dev;
    struct drm_mode_config *config = &dev->mode_config;
    
    list_del(&colorop->head);
    config->num_colorop--;
    ...
}

Since dev is not set yet, dev->mode_config will crash, and list_del() will
operate on a zero-initialized list head.

[Severity: High]
This is a pre-existing issue, but does drm_colorop_cleanup() leak the
mode object ID?

If a colorop init function successfully adds the mode object to the IDR via
drm_mode_object_add() but fails later, it calls drm_colorop_cleanup().

Since drm_colorop_cleanup() does not call drm_mode_object_unregister(), the
object ID remains in the DRM device's IDR. When the colorop memory is freed,
this leaves a dangling pointer in the IDR, which could lead to a Use-After-Free
if userspace queries it via DRM_IOCTL_MODE_OBJ_GETPROPERTIES.

[ ... ]

> @@ -126,9 +144,12 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  	if (plane_has_3dlut(display, pipe, plane)) {
>  		pipeline = xe3plpd_primary_plane_pipeline;
>  		pipeline_len = ARRAY_SIZE(xe3plpd_primary_plane_pipeline);
> -	} else {
> +	} else if (icl_is_hdr_plane(display, to_intel_plane(plane)->id)) {
>  		pipeline = hdr_plane_pipeline;
>  		pipeline_len = ARRAY_SIZE(hdr_plane_pipeline);
> +	} else {
> +		pipeline = sdr_plane_yuv_pipeline;
> +		pipeline_len = ARRAY_SIZE(sdr_plane_yuv_pipeline);
>  	}

[Severity: Medium]
Is this newly added SDR pipeline logic reachable?

Any initialization of an SDR plane calls intel_color_pipeline_plane_init(),
which appears to have an early return guard for non-HDR planes before it
calls _intel_color_pipeline_plane_init():

intel_color_pipeline.c:intel_color_pipeline_plane_init() {
    ...
    /* Currently expose pipeline only for HDR planes */
    if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id))
        return 0;

    return _intel_color_pipeline_plane_init(plane, list, pipe);
}

Because of this check, _intel_color_pipeline_plane_init() will never be
reached for an SDR plane, making the new sdr_plane_yuv_pipeline dead code.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804123107.2256124-1-chaitanya.kumar.borah@intel.com?part=2

Re: [v6 5/9] drm/i915/display: Program CSC on SDR planes based on Fixed Matrix Colorop

[sashiko-bot]

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

Pre-existing issues:
- [High] Plane color pipeline configuration (including `csc_ff_enable` and LUT blobs) is silently discarded if an atomic commit updates the plane but omits the color pipeline objects.
--

--- Patch [5]: [v6 5/9] drm/i915/display: Program CSC on SDR planes based on Fixed Matrix Colorop ---
commit 17128052d77ad1b002034d74a8e48d89696c0df8
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/display: Program CSC on SDR planes based on Fixed Matrix Colorop

When a color pipeline is active, program the SDR plane fixed-function
CSC based on the Fixed Matrix Colorop's state. Re-use the existing plane
state variables for color_range and color_encoding.

> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
> index d0f99a87c42e8..25ca049009efb 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c

[ ... ]

> @@ -485,11 +522,23 @@ intel_plane_color_copy_uapi_to_hw_state(struct intel_atomic_state *state,
>  	while (iter_colorop) {
>  		for_each_new_colorop_in_state(&state->base, colorop, new_colorop_state, i) {
>  			if (new_colorop_state->colorop == iter_colorop) {
> -				blob = new_colorop_state->bypass ? NULL : new_colorop_state->data;
>  				intel_colorop = to_intel_colorop(colorop);
> -				changed |= intel_plane_colorop_replace_blob(plane_state,
> +				if (intel_colorop->id == INTEL_PLANE_CB_CSC_FF) {
> +					fm_type = new_colorop_state->fixed_matrix_type;
> +
> +					plane_state->hw.csc_ff_enable =
> +						!new_colorop_state->bypass;

[Severity: High]
This isn't a bug introduced by this patch, but does this iteration macro fail
to preserve the hardware state during page flips?

If userspace performs a simple plane update, such as a page flip to update the
FB, without explicitly including the color pipeline objects in the atomic
commit, for_each_new_colorop_in_state() in
intel_plane_color_copy_uapi_to_hw_state() will skip the unmodified colorops.

Since intel_plane_copy_uapi_to_hw_state() calls intel_plane_clear_hw_state()
prior to this step, the entire hardware state is cleared. As a result, the
new csc_ff_enable flag and other LUT pointers will not be restored,
permanently losing the hardware configuration.

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 73be4c6ff1d3b..222c3b6bbe467 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c

[ ... ]

> @@ -1245,9 +1245,11 @@ static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_s
>  	struct intel_display *display = to_intel_display(plane_state);
>  	const struct drm_framebuffer *fb = plane_state->hw.fb;
>  	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> +	bool color_pipeline = !!plane_state->uapi.color_pipeline;
> +	bool needs_csc = color_pipeline ? plane_state->hw.csc_ff_enable : fb->format->is_yuv;
>  	u32 ctl = 0;
>  
> -	if (!fb->format->is_yuv)
> +	if (!needs_csc)
>  		return 0;

[Severity: High]
Because of the issue mentioned above, could needs_csc evaluate to false
incorrectly?

When the csc_ff_enable flag is silently dropped during an atomic commit that
does not modify colorops, glk_plane_color_ctl_input_csc() will evaluate
needs_csc to false. This disables the hardware CSC, which can lead to
incorrect colors or visual artifacts on the display.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804123107.2256124-1-chaitanya.kumar.borah@intel.com?part=5

Re: [v6 6/9] drm/i915/color: Add support for 1D LUT in SDR planes

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing drm_mode_object_unregister() in colorop cleanup paths leaves dangling pointers in the DRM object IDR.
- [High] Missing NULL check for kasprintf leads to crash during color pipeline property creation.
- [High] Error cleanup path in intel_color_pipeline_plane_add_colorop crashes when colorop initialization fails early.
--

--- Patch [6]: [v6 6/9] drm/i915/color: Add support for 1D LUT in SDR planes ---
commit b0190470a3e343638c9aa8e156a26be16a13657a
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/color: Add support for 1D LUT in SDR planes
    
This commit extends the SDR plane color pipeline to include a post-CSC 1D
LUT block. It simplifies the YUV to RGB pipeline by dropping the pre-CSC
LUT.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index efd4375c43318..53e55ce0a5a35 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -45,6 +45,7 @@ static const enum intel_color_block hdr_plane_pipeline[] = {
>  
>  static const enum intel_color_block sdr_plane_yuv_pipeline[] = {
>  	INTEL_PLANE_CB_CSC_FF,
> +	INTEL_PLANE_CB_POST_CSC_LUT,
>  };

[Severity: High]
This is a pre-existing issue, but I have a question about the cleanup path
when initializing these colorops. If colorop initialization fails halfway
(for example, if drm_property_create_enum fails in drm_plane_colorop_init
after drm_mode_object_add), the error path calls intel_colorop_destroy
which delegates to drm_colorop_cleanup:

void drm_colorop_cleanup(struct drm_colorop *colorop)
{
	struct drm_device *dev = colorop->dev;
	struct drm_mode_config *config = &dev->mode_config;

	list_del(&colorop->head);
	config->num_colorop--;
...

Is a call to drm_mode_object_unregister() missing here before the caller
frees the memory? It seems this could leave dangling pointers in the DRM
object IDR, which might be exploitable via DRM_IOCTL_MODE_OBJ_GETPROPERTIES.

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
intel_color_pipeline_plane_add_colorop handle early initialization failures
safely?

intel_color_pipeline_plane_add_colorop()
...
cleanup:
	intel_colorop_destroy(&colorop->base);
	return ERR_PTR(ret);

If an initialization function like drm_plane_colorop_curve_1d_lut_init
fails early inside drm_plane_colorop_init (such as when drm_mode_object_add
fails), colorop->dev is left as NULL and the head list node is uninitialized.
When drm_colorop_cleanup accesses dev->mode_config.num_colorop, could this
cause a NULL pointer dereference and an invalid list_del?

[Severity: High]
This is another pre-existing issue in the color pipeline setup code. I
noticed a potential issue in _intel_color_pipeline_plane_init where
kasprintf is used without checking the return value:

_intel_color_pipeline_plane_init()
...
	list->type = colorop[0]->base.base.id;
	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", colorop[0]->base.base.id);

	return 0;

Can this lead to a crash if kasprintf fails? If list->name is NULL, the
function still returns success. This NULL name is then passed through to
drm_plane_create_color_pipeline_property and eventually drm_property_add_enum,
where WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN) could oops when attempting
to dereference the NULL pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804123107.2256124-1-chaitanya.kumar.borah@intel.com?part=6

Re: [v6 9/9] drm/i915/color: Add color pipeline support for SDR planes

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Use-After-Free vulnerability due to missing `drm_mode_object_unregister()` in `drm_colorop_cleanup()`.
- [High] Kernel crash via NULL pointer dereference if `kasprintf()` fails to allocate the pipeline name.
- [High] NULL pointer dereference of `colorop->dev` during error cleanup in `drm_colorop_cleanup()`.
- [High] Kernel crash via NULL pointer dereference when calling `list_del()` on an uninitialized `list_head`.
--

--- Patch [9]: [v6 9/9] drm/i915/color: Add color pipeline support for SDR planes ---
commit 80efc788f141365ff51bb15bb83bcb8d636605f2
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/color: Add color pipeline support for SDR planes

This commit exposes the SDR plane color pipeline to user-space by
removing the restriction that previously limited it only to HDR planes.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 53e55ce0a5a35..38cfd6ed585d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -177,17 +177,11 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  
>  int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
>  {
[ ... ]
> -	/* Currently expose pipeline only for HDR planes */
> -	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id))
> -		return 0;
> -
>  	/* Add pipeline consisting of transfer functions */
>  	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);

[Severity: High]
This is a pre-existing issue, but does this function properly handle
memory allocation failures?

In drivers/gpu/drm/i915/display/intel_color_pipeline.c:
_intel_color_pipeline_plane_init(), the pipeline name is allocated:

	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d",
			       colorop[0]->base.base.id);

	return 0;

If kasprintf() fails, list->name is NULL, but the function still returns 0.
This NULL pointer is then passed up the stack and eventually to
drm_property_add_enum(), which calls strlen(name):

	if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN))
		return -EINVAL;

Could this cause a NULL pointer dereference when memory is tight?


[Severity: High]
This is a pre-existing issue, but could error cleanup paths triggered during
initialization cause a NULL pointer dereference?

If intel_color_pipeline_plane_add_colorop() fails early (for example, if
drm_mode_object_add() fails), the cleanup path calls intel_colorop_destroy(),
which invokes drm_colorop_cleanup().

At this point, colorop->dev might still be NULL. In drm_colorop_cleanup():

	struct drm_device *dev = colorop->dev;
	struct drm_mode_config *config = &dev->mode_config;

	list_del(&colorop->head);
	config->num_colorop--;

Will this unconditionally dereference the NULL dev pointer?


[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free risk when freeing
a colorop?

When a colorop is successfully added to the DRM IDR via drm_mode_object_add(),
it needs to be unregistered when destroyed. However, drm_colorop_cleanup()
does not appear to call drm_mode_object_unregister().

If a colorop is freed, but its mode object ID remains valid in the IDR, could
userspace trigger a use-after-free by interacting with the stale ID?


[Severity: High]
This is a pre-existing issue, but could the error handling crash when removing
an uninitialized list node?

In drm_plane_colorop_init(), if drm_mode_object_add() fails, the function
returns early before the colorop is added to the list:

	ret = drm_mode_object_add(dev, &colorop->base,
				  DRM_MODE_OBJECT_COLOROP);
	if (ret)
		return ret;

When the error path later calls drm_colorop_cleanup(), it unconditionally
executes list_del(&colorop->head). Since the head was zero-initialized via
kzalloc_obj(), __list_del() attempts to write to next->prev.

Will this result in an immediate NULL pointer dereference?

>  	if (ret)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804123107.2256124-1-chaitanya.kumar.borah@intel.com?part=9