[PATCH v2 00/13] drm/i915/color: Enable SDR plane color pipeline

[PATCH v2 00/13] 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]

The series also fixes an issue in HDR pre-CSC LUT programming where the
loop condition prevented the last entries from being programmed.

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

Chaitanya Kumar Borah (8):
  drm/colorop: Add DRM_COLOROP_FIXED_MATRIX
  drm/i915/color: Add CSC on SDR plane color pipeline
  drm/i915/color: Program fixed-function CSC on SDR planes
  drm/i915/color: Add support for 1D LUT in SDR planes
  drm/i915/color: Add color pipeline support for SDR planes
  drm/i915/color: Add YCbCr limited-to-full range color block support
  drm/i915/color: Program plane YUV range correction colorop
  drm/i915/color: Add YUV range correction to SDR plane pipeline

Pranay Samala (5):
  drm/i915/color: Fix HDR pre-CSC LUT programming loop
  drm/i915/color: Extract HDR pre-CSC LUT programming to helper function
  drm/i915/color: Program Pre-CSC registers for SDR
  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             |   4 +
 drivers/gpu/drm/drm_colorop.c                 | 107 +++++++
 drivers/gpu/drm/i915/display/intel_color.c    | 288 +++++++++++++-----
 .../drm/i915/display/intel_color_pipeline.c   |  44 ++-
 .../drm/i915/display/intel_display_limits.h   |   2 +
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_plane.c    |  15 +-
 .../drm/i915/display/skl_universal_plane.c    |  33 ++
 include/drm/drm_colorop.h                     |  84 +++++
 include/uapi/drm/drm_mode.h                   |  12 +
 11 files changed, 503 insertions(+), 93 deletions(-)

-- 
2.25.1

[PATCH v2 01/13] 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.

v2:
 - Naming changes (Pekka)

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/drm_atomic.c      |   4 ++
 drivers/gpu/drm/drm_atomic_uapi.c |   4 ++
 drivers/gpu/drm/drm_colorop.c     | 107 ++++++++++++++++++++++++++++++
 include/drm/drm_colorop.h         |  84 +++++++++++++++++++++++
 include/uapi/drm/drm_mode.h       |  12 ++++
 5 files changed, 211 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 41c57063f3b4..16a2183b11bf 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -845,6 +845,10 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
 			   drm_get_colorop_lut3d_interpolation_name(colorop->lut3d_interpolation));
 		drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data->base.id : 0);
 		break;
+	case DRM_COLOROP_FIXED_MATRIX:
+		drm_printf(p, "\tfixed_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 5bd5bf6661df..d69406b08a0f 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -761,6 +761,8 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
 	} else if (property == colorop->data_property) {
 		return drm_atomic_color_set_data_property(colorop, state,
 							  property, val);
+	} else if (property == colorop->fixed_matrix_type_property) {
+		state->fixed_matrix_type = val;
 	} else {
 		drm_dbg_atomic(colorop->dev,
 			       "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
@@ -793,6 +795,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
 		*val = colorop->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 566816e3c6f0..dd385e8dacbe 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,14 @@ 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_YCBCR709_FULL_RGB]    = "YCbCr 709 Full to RGB",
+	[DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC]  = "YCbCr 2020 Full to RGB NC",
+	[DRM_COLOROP_FM_YCBCR_LIMITED_FULL]  = "YCbCr limited to full",
+	[DRM_COLOROP_FM_RGB709_RGB2020]     = "RGB709 to RGB2020",
+};
+
 /* Init Helpers */
 
 static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
@@ -455,6 +464,81 @@ 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)
 {
@@ -521,6 +605,13 @@ static void __drm_colorop_state_reset(struct drm_colorop_state *colorop_state,
 						      &val);
 		colorop_state->curve_1d_type = val;
 	}
+
+	if (colorop->fixed_matrix_type_property) {
+		drm_object_property_get_default_value(&colorop->base,
+						      colorop->fixed_matrix_type_property,
+						      &val);
+		colorop_state->fixed_matrix_type = val;
+	}
 }
 
 /**
@@ -561,6 +652,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[] = {
@@ -617,6 +709,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 bd082854ca74..c13b4b045fff 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -134,6 +134,71 @@ 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"
+	 *
+	 * This selects the matrix that converts full range YCbCr into RGB
+	 * according to the BT.601 coefficients.
+	 */
+	DRM_COLOROP_FM_YCBCR601_FULL_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
+	 *
+	 * enum string "YCbCr 709 Full to RGB"
+	 *
+	 * This selects the matrix that converts full range YCbCr into RGB
+	 * according to the BT.709 coefficients.
+	 */
+	DRM_COLOROP_FM_YCBCR709_FULL_RGB,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
+	 *
+	 * enum string "YCbCr 2020 Full to RGB NC"
+	 *
+	 * This selects the matrix that converts full range YCbCr into RGB
+	 * according to the BT.2020 non-constant luminance coefficients.
+	 */
+	DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC,
+
+	/**
+	 * @DRM_COLOROP_FM_YCBCR_LIMITED_FULL:
+	 *
+	 * enum string "YCbCr limited to full"
+	 *
+	 * This selects the matrix that converts limited range YCbCr into
+	 * full range YCbCr. Though not strictly a matrix operation but
+	 * can be represented as one.
+	 */
+	DRM_COLOROP_FM_YCBCR_LIMITED_FULL,
+
+	/**
+	 * @DRM_COLOROP_FM_RGB709_RGB2020:
+	 *
+	 * enum string "RGB709 to RGB2020"
+	 *
+	 * Selects the fixed-function CSC preset that converts RGB
+	 * (BT.709) colorimetry to RGB (BT.2020).
+	 */
+	DRM_COLOROP_FM_RGB709_RGB2020,
+
+	/**
+	 * @DRM_COLOROP_FM_COUNT:
+	 *
+	 * enum value denoting the size of the enum
+	 */
+	DRM_COLOROP_FM_COUNT
+};
+
 /**
  * struct drm_colorop_state - mutable colorop state
  */
@@ -183,6 +248,13 @@ struct drm_colorop_state {
 	 */
 	struct drm_property_blob *data;
 
+	/**
+	 * @fixed_matrix_type:
+	 *
+	 * Type of Fixed Matrix operation.
+	 */
+	enum drm_colorop_fixed_matrix_type fixed_matrix_type;
+
 	/** @state: backpointer to global drm_atomic_state */
 	struct drm_atomic_state *state;
 };
@@ -368,6 +440,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:
 	 *
@@ -424,6 +503,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_duplicate_state(struct drm_colorop *colorop);
@@ -480,6 +563,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 a4bdc4bd11bc..dc4b8566fec8 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -971,6 +971,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.25.1

[PATCH v2 02/13] 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.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 .../drm/i915/display/intel_color_pipeline.c   | 22 ++++++++++++++++++-
 .../drm/i915/display/intel_display_limits.h   |  1 +
 2 files changed, 22 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..49c621d1bf22 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -43,6 +43,16 @@ static const enum intel_color_block hdr_plane_pipeline[] = {
 	INTEL_PLANE_CB_POST_CSC_LUT,
 };
 
+static const enum intel_color_block sdr_plane_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_YCBCR709_FULL_RGB) |
+		BIT(DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC) |
+		BIT(DRM_COLOROP_FM_RGB709_RGB2020);
+
 static bool plane_has_3dlut(struct intel_display *display, enum pipe pipe,
 			    struct drm_plane *plane)
 {
@@ -92,6 +102,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;
@@ -122,13 +138,17 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
 	int pipeline_len;
 	int ret = 0;
 	int i;
+	bool is_hdr = icl_is_hdr_plane(display, to_intel_plane(plane)->id);
 
 	if (plane_has_3dlut(display, pipe, plane)) {
 		pipeline = xe3plpd_primary_plane_pipeline;
 		pipeline_len = ARRAY_SIZE(xe3plpd_primary_plane_pipeline);
-	} else {
+	} else if (is_hdr) {
 		pipeline = hdr_plane_pipeline;
 		pipeline_len = ARRAY_SIZE(hdr_plane_pipeline);
+	} else {
+		pipeline = sdr_plane_pipeline;
+		pipeline_len = ARRAY_SIZE(sdr_plane_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 453f7b720815..f4aad54472ce 100644
--- a/drivers/gpu/drm/i915/display/intel_display_limits.h
+++ b/drivers/gpu/drm/i915/display/intel_display_limits.h
@@ -167,6 +167,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.25.1

[PATCH v2 03/13] drm/i915/color: Program fixed-function CSC on SDR planes

[Chaitanya Kumar Borah]

Program the fixed-function CSC block for SDR planes based on the
DRM_COLOROP_FIXED_MATRIX state.

Track the bypass state explicitly as a boolean in the plane hw state
since bypass is managed separately from the FIXED_MATRIX enum value in
the colorop framework.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2496db1642a..3347bba598d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -679,6 +679,8 @@ 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;
+		enum drm_colorop_fixed_matrix_type csc_ff_type; /* For SDR plane */
+		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 5390ceb21ca4..3ef43db8d5d7 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -378,11 +378,19 @@ intel_plane_color_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
 	while (iter_colorop) {
 		for_each_new_colorop_in_state(state, 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) {
+					plane_state->hw.csc_ff_enable =
+						!new_colorop_state->bypass;
+					plane_state->hw.csc_ff_type =
+						new_colorop_state->fixed_matrix_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 11ba42c67e3e..2234abcd1b03 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1240,6 +1240,32 @@ static u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state)
 	return plane_color_ctl;
 }
 
+static u32 intel_csc_ff_type_to_csc_mode(enum drm_colorop_fixed_matrix_type csc_ff_type,
+					 bool enable)
+{
+	u32 csc_mode = PLANE_COLOR_CSC_MODE_BYPASS;
+
+	if (enable) {
+		switch (csc_ff_type) {
+		case DRM_COLOROP_FM_YCBCR601_FULL_RGB:
+			csc_mode = PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601;
+			break;
+		case DRM_COLOROP_FM_YCBCR709_FULL_RGB:
+			csc_mode = PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
+			break;
+		case DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC:
+			csc_mode = PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
+			break;
+		case DRM_COLOROP_FM_RGB709_RGB2020:
+			csc_mode = PLANE_COLOR_CSC_MODE_RGB709_TO_RGB2020;
+			break;
+		default:
+			csc_mode = PLANE_COLOR_CSC_MODE_BYPASS;
+		}
+	}
+	return csc_mode;
+}
+
 static u32 glk_plane_color_ctl(const struct intel_plane_state *plane_state)
 {
 	struct intel_display *display = to_intel_display(plane_state);
@@ -1271,6 +1297,10 @@ static u32 glk_plane_color_ctl(const struct intel_plane_state *plane_state)
 			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	}
 
+	if (!icl_is_hdr_plane(display, plane->id))
+		plane_color_ctl |= intel_csc_ff_type_to_csc_mode(plane_state->hw.csc_ff_type,
+								 plane_state->hw.csc_ff_enable);
+
 	if (plane_state->force_black)
 		plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE;
 
-- 
2.25.1

[PATCH v2 04/13] drm/i915/color: Add support for 1D LUT in SDR planes

[Chaitanya Kumar Borah]

Extend the SDR plane color pipeline to include pre- and post-CSC
1D LUT blocks.

SDR planes use a smaller LUT size than HDR planes and therefore
initialize the 1D LUT colorops with the appropriate hardware
capacity.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color_pipeline.c | 9 ++++++++-
 1 file changed, 8 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 49c621d1bf22..20f5e0e83496 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -15,6 +15,7 @@
 #define MAX_COLOROP 4
 #define PLANE_DEGAMMA_SIZE 128
 #define PLANE_GAMMA_SIZE 32
+#define PLANE_DEGAMMA_SIZE_SDR 32
 
 static const struct drm_colorop_funcs intel_colorop_funcs = {
 	.destroy = intel_colorop_destroy,
@@ -44,7 +45,9 @@ static const enum intel_color_block hdr_plane_pipeline[] = {
 };
 
 static const enum intel_color_block sdr_plane_pipeline[] = {
+	INTEL_PLANE_CB_PRE_CSC_LUT,
 	INTEL_PLANE_CB_CSC_FF,
+	INTEL_PLANE_CB_POST_CSC_LUT,
 };
 
 static const u64 intel_plane_supported_csc_ff =
@@ -67,8 +70,10 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
 							     enum intel_color_block id)
 {
 	struct drm_device *dev = plane->dev;
+	struct intel_display *display = to_intel_display(dev);
 	struct intel_colorop *colorop;
 	int ret;
+	bool is_hdr = icl_is_hdr_plane(display, to_intel_plane(plane)->id);
 
 	colorop = intel_colorop_create(id);
 
@@ -80,7 +85,9 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
 		ret = drm_plane_colorop_curve_1d_lut_init(dev,
 							  &colorop->base, plane,
 							  &intel_colorop_funcs,
-							  PLANE_DEGAMMA_SIZE,
+							  is_hdr ?
+							  PLANE_DEGAMMA_SIZE :
+							  PLANE_DEGAMMA_SIZE_SDR,
 							  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
 							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		break;
-- 
2.25.1

[PATCH v2 05/13] drm/i915/color: Fix HDR pre-CSC LUT programming loop

[Chaitanya Kumar Borah]

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

The integer lut programming loop never executes completely due to
incorrect condition (i++ > 130).

Fix to properly program 129th+ entries for values > 1.0.

Cc: <stable@vger.kernel.org> #v6.19
Fixes: 82caa1c8813f ("drm/i915/color: Program Pre-CSC registers")
Signed-off-by: Pranay Samala <pranay.samala@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index e7950655434b..6d1cffc6d2be 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3976,7 +3976,7 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 				intel_de_write_dsb(display, dsb,
 						   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
 						   (1 << 24));
-			} while (i++ > 130);
+			} while (i++ < 130);
 		} else {
 			for (i = 0; i < lut_size; i++) {
 				u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
-- 
2.25.1

[PATCH v2 06/13] drm/i915/color: Extract HDR pre-CSC LUT programming to helper function

[Chaitanya Kumar Borah]

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

As we prepare to add support for LUT programming in SDR planes,
refactor HDR plane pre-CSC LUT programming to a helper.

Signed-off-by: Pranay Samala <pranay.samala@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 92 ++++++++++++----------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 6d1cffc6d2be..17ab4364faea 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3943,6 +3943,55 @@ xelpd_load_plane_csc_matrix(struct intel_dsb *dsb,
 			   ctm_to_twos_complement(input[11], 0, 12));
 }
 
+static void
+xelpd_load_hdr_pre_csc_lut(struct intel_display *display,
+			   struct intel_dsb *dsb,
+			   enum pipe pipe,
+			   enum plane_id plane,
+			   const struct drm_color_lut32 *pre_csc_lut)
+{
+	u32 lut_size = 128;
+	u32 lut_val;
+	int i;
+
+	intel_de_write_dsb(display, dsb,
+			   PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0),
+			   PLANE_PAL_PREC_AUTO_INCREMENT);
+
+	if (pre_csc_lut) {
+		for (i = 0; i < lut_size; i++) {
+			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),
+					   lut_val);
+		}
+
+		/* Program the max register to clamp values > 1.0. */
+		/* TODO: Restrict to 0x7ffffff */
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					   (1 << 24));
+		} while (i++ < 130);
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = (i * ((1 << 24) - 1)) / (lut_size - 1);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					   1 << 24);
+		} while (i++ < 130);
+	}
+
+	intel_de_write_dsb(display, dsb, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
+}
+
 static void
 xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 				const struct intel_plane_state *plane_state)
@@ -3952,48 +4001,9 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 	enum pipe pipe = to_intel_plane(state->plane)->pipe;
 	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;
 
-	if (icl_is_hdr_plane(display, plane)) {
-		lut_size = 128;
-
-		intel_de_write_dsb(display, dsb,
-				   PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0),
-				   PLANE_PAL_PREC_AUTO_INCREMENT);
-
-		if (pre_csc_lut) {
-			for (i = 0; i < lut_size; i++) {
-				u32 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),
-						   lut_val);
-			}
-
-			/* Program the max register to clamp values > 1.0. */
-			/* TODO: Restrict to 0x7ffffff */
-			do {
-				intel_de_write_dsb(display, dsb,
-						   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
-						   (1 << 24));
-			} while (i++ < 130);
-		} else {
-			for (i = 0; i < lut_size; i++) {
-				u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
-
-				intel_de_write_dsb(display, dsb,
-						   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0), v);
-			}
-
-			do {
-				intel_de_write_dsb(display, dsb,
-						   PLANE_PRE_CSC_GAMC_DATA_ENH(pipe, plane, 0),
-						   1 << 24);
-			} while (i++ < 130);
-		}
-
-		intel_de_write_dsb(display, dsb, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
-	}
+	if (icl_is_hdr_plane(display, plane))
+		xelpd_load_hdr_pre_csc_lut(display, dsb, pipe, plane, pre_csc_lut);
 }
 
 static void
-- 
2.25.1

[PATCH v2 07/13] drm/i915/color: Program Pre-CSC registers for SDR

[Chaitanya Kumar Borah]

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

Implement plane pre-CSC LUT support for SDR planes.

v2:
- s/drm_color_lut_extract/drm_color_lut32_extract

Signed-off-by: Pranay Samala <pranay.samala@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 17ab4364faea..7d670ae8325f 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -3992,6 +3992,59 @@ xelpd_load_hdr_pre_csc_lut(struct intel_display *display,
 	intel_de_write_dsb(display, dsb, PLANE_PRE_CSC_GAMC_INDEX_ENH(pipe, plane, 0), 0);
 }
 
+static void
+xelpd_load_sdr_pre_csc_lut(struct intel_display *display,
+			   struct intel_dsb *dsb,
+			   enum pipe pipe,
+			   enum plane_id plane,
+			   const struct drm_color_lut32 *pre_csc_lut)
+{
+	u32 lut_size = 32;
+	u32 lut_val;
+	int i;
+
+	/*
+	 * 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_PRE_CSC_GAMC_INDEX(pipe, plane, 0),
+			   PLANE_PAL_PREC_AUTO_INCREMENT);
+
+	if (pre_csc_lut) {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = drm_color_lut32_extract(pre_csc_lut[i].green, 16);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0),
+					   lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0),
+					   (1 << 16));
+		} while (i++ < 34);
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = (i * ((1 << 16) - 1)) / (lut_size - 1);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0), lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_PRE_CSC_GAMC_DATA(pipe, plane, 0),
+					   1 << 16);
+		} while (i++ < 34);
+	}
+
+	intel_de_write_dsb(display, dsb, PLANE_PRE_CSC_GAMC_INDEX(pipe, plane, 0), 0);
+}
+
 static void
 xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 				const struct intel_plane_state *plane_state)
@@ -4004,6 +4057,8 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 
 	if (icl_is_hdr_plane(display, plane))
 		xelpd_load_hdr_pre_csc_lut(display, dsb, pipe, plane, pre_csc_lut);
+	else
+		xelpd_load_sdr_pre_csc_lut(display, dsb, pipe, plane, pre_csc_lut);
 }
 
 static void
-- 
2.25.1

[PATCH v2 08/13] 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.

Also removes 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>
---
 drivers/gpu/drm/i915/display/intel_color.c | 92 +++++++++++-----------
 1 file changed, 48 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 7d670ae8325f..55e206abe5b1 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -4061,6 +4061,52 @@ xelpd_program_plane_pre_csc_lut(struct intel_dsb *dsb,
 		xelpd_load_sdr_pre_csc_lut(display, dsb, pipe, plane, pre_csc_lut);
 }
 
+static void
+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)
+{
+	u32 lut_size = 32;
+	u32 lut_val;
+	int i;
+
+	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_INDEX_ENH(pipe, plane, 0),
+			   PLANE_PAL_PREC_AUTO_INCREMENT);
+
+	if (post_csc_lut) {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = drm_color_lut32_extract(post_csc_lut[i].green, 24);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					   lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					   (1 << 24));
+		} while (i++ < 34);
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = (i * ((1 << 24) - 1)) / (lut_size - 1);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
+					   1 << 24);
+		} while (i++ < 34);
+	}
+
+	intel_de_write_dsb(display, dsb, PLANE_POST_CSC_GAMC_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)
@@ -4070,51 +4116,9 @@ xelpd_program_plane_post_csc_lut(struct intel_dsb *dsb,
 	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;
-	u32 i, lut_size, lut_val;
-
-	if (icl_is_hdr_plane(display, plane)) {
-		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);
-		if (post_csc_lut) {
-			lut_size = 32;
-			for (i = 0; i < lut_size; i++) {
-				lut_val = drm_color_lut32_extract(post_csc_lut[i].green, 24);
-
-				intel_de_write_dsb(display, dsb,
-						   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
-						   lut_val);
-			}
-
-			/* Segment 2 */
-			do {
-				intel_de_write_dsb(display, dsb,
-						   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
-						   (1 << 24));
-			} while (i++ < 34);
-		} else {
-			/*TODO: Add for segment 0 */
-			lut_size = 32;
-			for (i = 0; i < lut_size; i++) {
-				u32 v = (i * ((1 << 24) - 1)) / (lut_size - 1);
-
-				intel_de_write_dsb(display, dsb,
-						   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0), v);
-			}
-
-			do {
-				intel_de_write_dsb(display, dsb,
-						   PLANE_POST_CSC_GAMC_DATA_ENH(pipe, plane, 0),
-						   1 << 24);
-			} while (i++ < 34);
-		}
 
-		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);
-	}
+	if (icl_is_hdr_plane(display, plane))
+		xelpd_load_hdr_post_csc_lut(display, dsb, pipe, plane, post_csc_lut);
 }
 
 static void
-- 
2.25.1

[PATCH v2 09/13] 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.

Signed-off-by: Pranay Samala <pranay.samala@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 55e206abe5b1..57f574f452b3 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -4107,6 +4107,57 @@ 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)
+{
+	u32 lut_size = 32;
+	u32 lut_val;
+	int i;
+
+	/*
+	 * 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);
+
+	if (post_csc_lut) {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = drm_color_lut32_extract(post_csc_lut[i].green, 16);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+					   lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+					   (1 << 16));
+		} while (i++ < 34);
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			lut_val = (i * ((1 << 16) - 1)) / (lut_size - 1);
+
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0), lut_val);
+		}
+
+		do {
+			intel_de_write_dsb(display, dsb,
+					   PLANE_POST_CSC_GAMC_DATA(pipe, plane, 0),
+					   1 << 16);
+		} while (i++ < 34);
+	}
+
+	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)
@@ -4119,6 +4170,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.25.1

[PATCH v2 10/13] 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>
---
 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 20f5e0e83496..9e5891338f31 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -182,17 +182,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.25.1

[PATCH v2 11/13] drm/i915/color: Add YCbCr limited-to-full range color block support

[Chaitanya Kumar Borah]

Add support for color block which performs YCbCr limited-to-full
range expansion in the plane color pipeline. The color block is
represented using DRM_COLOROP_FIXED_MATRIX colorop.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color_pipeline.c | 6 ++++++
 drivers/gpu/drm/i915/display/intel_display_limits.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
index 9e5891338f31..17cbbbc839af 100644
--- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
+++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
@@ -115,6 +115,12 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
 							  intel_plane_supported_csc_ff,
 							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
 		break;
+	case INTEL_PLANE_CB_YUV_RANGE_CORRECT:
+		ret = drm_plane_colorop_fixed_matrix_init(dev, &colorop->base, plane,
+							  &intel_colorop_funcs,
+							  BIT(DRM_COLOROP_FM_YCBCR_LIMITED_FULL),
+							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
+		break;
 	default:
 		drm_err(plane->dev, "Invalid colorop id [%d]", id);
 		ret = -EINVAL;
diff --git a/drivers/gpu/drm/i915/display/intel_display_limits.h b/drivers/gpu/drm/i915/display/intel_display_limits.h
index f4aad54472ce..85986a8b4528 100644
--- a/drivers/gpu/drm/i915/display/intel_display_limits.h
+++ b/drivers/gpu/drm/i915/display/intel_display_limits.h
@@ -170,6 +170,7 @@ enum intel_color_block {
 	INTEL_PLANE_CB_CSC_FF,
 	INTEL_PLANE_CB_POST_CSC_LUT,
 	INTEL_PLANE_CB_3DLUT,
+	INTEL_PLANE_CB_YUV_RANGE_CORRECT,
 
 	INTEL_CB_MAX
 };
-- 
2.25.1

[PATCH v2 12/13] drm/i915/color: Program plane YUV range correction colorop

[Chaitanya Kumar Borah]

Hook up the INTEL_PLANE_CB_YUV_RANGE_CORRECT colorop to plane state and
program the corresponding plane color control bits. Track the colorop
state in intel_plane_state and enable/disable YUV range correction
based on the colorop bypass state.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 3347bba598d9..8f942683fc5c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -681,6 +681,7 @@ struct intel_plane_state {
 		struct drm_property_blob *ctm, *degamma_lut, *gamma_lut, *lut_3d;
 		enum drm_colorop_fixed_matrix_type csc_ff_type; /* For SDR plane */
 		bool csc_ff_enable;
+		bool yuv_range_correct;
 	} 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 3ef43db8d5d7..0ef06a42f2ac 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -384,6 +384,9 @@ intel_plane_color_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
 						!new_colorop_state->bypass;
 					plane_state->hw.csc_ff_type =
 						new_colorop_state->fixed_matrix_type;
+				} else if (intel_colorop->id == INTEL_PLANE_CB_YUV_RANGE_CORRECT) {
+					plane_state->hw.yuv_range_correct =
+						!new_colorop_state->bypass;
 				} else {
 					blob = new_colorop_state->bypass ?
 						NULL : new_colorop_state->data;
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 2234abcd1b03..560093940ae3 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1297,9 +1297,12 @@ static u32 glk_plane_color_ctl(const struct intel_plane_state *plane_state)
 			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	}
 
-	if (!icl_is_hdr_plane(display, plane->id))
+	if (!icl_is_hdr_plane(display, plane->id)) {
 		plane_color_ctl |= intel_csc_ff_type_to_csc_mode(plane_state->hw.csc_ff_type,
 								 plane_state->hw.csc_ff_enable);
+		plane_color_ctl |= plane_state->hw.yuv_range_correct ? 0 :
+					PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+	}
 
 	if (plane_state->force_black)
 		plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE;
-- 
2.25.1

[PATCH v2 13/13] drm/i915/color: Add YUV range correction to SDR plane pipeline

[Chaitanya Kumar Borah]

Insert the INTEL_PLANE_CB_YUV_RANGE_CORRECT color block into the SDR
plane color pipeline. This enables YCbCr limited-to-full range expansion
ahead of the CSC block that expects full range pixel data. With this,
the pipeline can accept limited range framebuffers.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 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 17cbbbc839af..301ee3bdc4b0 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_pipeline[] = {
+	INTEL_PLANE_CB_YUV_RANGE_CORRECT,
 	INTEL_PLANE_CB_PRE_CSC_LUT,
 	INTEL_PLANE_CB_CSC_FF,
 	INTEL_PLANE_CB_POST_CSC_LUT,
-- 
2.25.1

✗ i915.CI.BAT: failure for drm/i915/color: Enable SDR plane color pipeline (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2520 bytes --]

== Series Details ==

Series: drm/i915/color: Enable SDR plane color pipeline (rev2)
URL   : https://patchwork.freedesktop.org/series/162788/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18292 -> Patchwork_162788v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_162788v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_162788v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162788v2/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_162788v2:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-hdmi-a-2:
    - fi-glk-j4005:       [PASS][1] -> [FAIL][2] +3 other tests fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18292/fi-glk-j4005/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-hdmi-a-2.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162788v2/fi-glk-j4005/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-hdmi-a-2.html

  
Known issues
------------

  Here are the changes found in Patchwork_162788v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_debugfs@read-all-entries:
    - bat-adlp-9:         [PASS][3] -> [DMESG-WARN][4] ([i915#15673])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18292/bat-adlp-9/igt@core_debugfs@read-all-entries.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162788v2/bat-adlp-9/igt@core_debugfs@read-all-entries.html

  
  [i915#15673]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15673


Build changes
-------------

  * Linux: CI_DRM_18292 -> Patchwork_162788v2

  CI-20190529: 20190529
  CI_DRM_18292: f074368a55893ee121ba2920497b6c25e265d190 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8850: 8850
  Patchwork_162788v2: f074368a55893ee121ba2920497b6c25e265d190 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162788v2/index.html

[-- Attachment #2: Type: text/html, Size: 3146 bytes --]

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Melissa Wen]

On 08/04/2026 02:15, Chaitanya Kumar Borah wrote:
> 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.
>
> v2:
>   - Naming changes (Pekka)
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   drivers/gpu/drm/drm_atomic.c      |   4 ++
>   drivers/gpu/drm/drm_atomic_uapi.c |   4 ++
>   drivers/gpu/drm/drm_colorop.c     | 107 ++++++++++++++++++++++++++++++
>   include/drm/drm_colorop.h         |  84 +++++++++++++++++++++++
>   include/uapi/drm/drm_mode.h       |  12 ++++
>   5 files changed, 211 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 41c57063f3b4..16a2183b11bf 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -845,6 +845,10 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
>   			   drm_get_colorop_lut3d_interpolation_name(colorop->lut3d_interpolation));
>   		drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data->base.id : 0);
>   		break;
> +	case DRM_COLOROP_FIXED_MATRIX:
> +		drm_printf(p, "\tfixed_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 5bd5bf6661df..d69406b08a0f 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -761,6 +761,8 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
>   	} else if (property == colorop->data_property) {
>   		return drm_atomic_color_set_data_property(colorop, state,
>   							  property, val);
> +	} else if (property == colorop->fixed_matrix_type_property) {
> +		state->fixed_matrix_type = val;
>   	} else {
>   		drm_dbg_atomic(colorop->dev,
>   			       "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
> @@ -793,6 +795,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
>   		*val = colorop->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 566816e3c6f0..dd385e8dacbe 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,14 @@ 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_YCBCR709_FULL_RGB]    = "YCbCr 709 Full to RGB",
> +	[DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC]  = "YCbCr 2020 Full to RGB NC",
> +	[DRM_COLOROP_FM_YCBCR_LIMITED_FULL]  = "YCbCr limited to full",
> +	[DRM_COLOROP_FM_RGB709_RGB2020]     = "RGB709 to RGB2020",
> +};
> +
>   /* Init Helpers */
>   
>   static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
> @@ -455,6 +464,81 @@ 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)
>   {
> @@ -521,6 +605,13 @@ static void __drm_colorop_state_reset(struct drm_colorop_state *colorop_state,
>   						      &val);
>   		colorop_state->curve_1d_type = val;
>   	}
> +
> +	if (colorop->fixed_matrix_type_property) {
> +		drm_object_property_get_default_value(&colorop->base,
> +						      colorop->fixed_matrix_type_property,
> +						      &val);
> +		colorop_state->fixed_matrix_type = val;
> +	}
>   }
>   
>   /**
> @@ -561,6 +652,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[] = {
> @@ -617,6 +709,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 bd082854ca74..c13b4b045fff 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -134,6 +134,71 @@ 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"
> +	 *
> +	 * This selects the matrix that converts full range YCbCr into RGB
> +	 * according to the BT.601 coefficients.
> +	 */
> +	DRM_COLOROP_FM_YCBCR601_FULL_RGB,
> +
> +	/**
> +	 * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
> +	 *
> +	 * enum string "YCbCr 709 Full to RGB"
> +	 *
> +	 * This selects the matrix that converts full range YCbCr into RGB
> +	 * according to the BT.709 coefficients.
> +	 */
> +	DRM_COLOROP_FM_YCBCR709_FULL_RGB,
> +
> +	/**
> +	 * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
> +	 *
> +	 * enum string "YCbCr 2020 Full to RGB NC"
Nit: I think you mean "YCbCr 2020 NC Full to RGB"?
> +	 *
> +	 * This selects the matrix that converts full range YCbCr into RGB
> +	 * according to the BT.2020 non-constant luminance coefficients.
> +	 */
> +	DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC,
... and here ^ DRM_COLOROP_FM_YCBCR_NC_FULL_RGB
> +
> +	/**
> +	 * @DRM_COLOROP_FM_YCBCR_LIMITED_FULL:
> +	 *
> +	 * enum string "YCbCr limited to full"
> +	 *
> +	 * This selects the matrix that converts limited range YCbCr into
> +	 * full range YCbCr. Though not strictly a matrix operation but
> +	 * can be represented as one.
> +	 */
> +	DRM_COLOROP_FM_YCBCR_LIMITED_FULL,
I didn't full understand how this is going to work.
Looks like it's merging two properties (COLOR_ENCODING and COLOR_RANGE) 
in a single colorop.
But we can only select one enum value. So how to communicate, for 
example, a "YCbCr 709 Limited to RGB" conversion?
The driver will have to define a coloro pipeline with two fixed matrix 
colorop in a row, one with only "Limited to Full" and another with 
"YCbCr Full to RGB" ?

Melissa
> +
> +	/**
> +	 * @DRM_COLOROP_FM_RGB709_RGB2020:
> +	 *
> +	 * enum string "RGB709 to RGB2020"
> +	 *
> +	 * Selects the fixed-function CSC preset that converts RGB
> +	 * (BT.709) colorimetry to RGB (BT.2020).
> +	 */
> +	DRM_COLOROP_FM_RGB709_RGB2020,
> +
> +	/**
> +	 * @DRM_COLOROP_FM_COUNT:
> +	 *
> +	 * enum value denoting the size of the enum
> +	 */
> +	DRM_COLOROP_FM_COUNT
> +};
> +
>   /**
>    * struct drm_colorop_state - mutable colorop state
>    */
> @@ -183,6 +248,13 @@ struct drm_colorop_state {
>   	 */
>   	struct drm_property_blob *data;
>   
> +	/**
> +	 * @fixed_matrix_type:
> +	 *
> +	 * Type of Fixed Matrix operation.
> +	 */
> +	enum drm_colorop_fixed_matrix_type fixed_matrix_type;
> +
>   	/** @state: backpointer to global drm_atomic_state */
>   	struct drm_atomic_state *state;
>   };
> @@ -368,6 +440,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:
>   	 *
> @@ -424,6 +503,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_duplicate_state(struct drm_colorop *colorop);
> @@ -480,6 +563,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 a4bdc4bd11bc..dc4b8566fec8 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -971,6 +971,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,
> +
>   };
>   
>   /**

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Borah, Chaitanya Kumar]

On 4/21/2026 6:40 PM, Melissa Wen wrote:
> 
> 
> On 08/04/2026 02:15, Chaitanya Kumar Borah wrote:
>> 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.
>>
>> v2:
>>   - Naming changes (Pekka)
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>   drivers/gpu/drm/drm_atomic.c      |   4 ++
>>   drivers/gpu/drm/drm_atomic_uapi.c |   4 ++
>>   drivers/gpu/drm/drm_colorop.c     | 107 ++++++++++++++++++++++++++++++
>>   include/drm/drm_colorop.h         |  84 +++++++++++++++++++++++
>>   include/uapi/drm/drm_mode.h       |  12 ++++
>>   5 files changed, 211 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 41c57063f3b4..16a2183b11bf 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -845,6 +845,10 @@ static void drm_atomic_colorop_print_state(struct 
>> drm_printer *p,
>>                  drm_get_colorop_lut3d_interpolation_name(colorop- 
>> >lut3d_interpolation));
>>           drm_printf(p, "\tdata blob id=%d\n", state->data ? state- 
>> >data->base.id : 0);
>>           break;
>> +    case DRM_COLOROP_FIXED_MATRIX:
>> +        drm_printf(p, "\tfixed_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 5bd5bf6661df..d69406b08a0f 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -761,6 +761,8 @@ static int drm_atomic_colorop_set_property(struct 
>> drm_colorop *colorop,
>>       } else if (property == colorop->data_property) {
>>           return drm_atomic_color_set_data_property(colorop, state,
>>                                 property, val);
>> +    } else if (property == colorop->fixed_matrix_type_property) {
>> +        state->fixed_matrix_type = val;
>>       } else {
>>           drm_dbg_atomic(colorop->dev,
>>                      "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
>> @@ -793,6 +795,8 @@ drm_atomic_colorop_get_property(struct drm_colorop 
>> *colorop,
>>           *val = colorop->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 566816e3c6f0..dd385e8dacbe 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,14 @@ 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_YCBCR709_FULL_RGB]    = "YCbCr 709 Full to RGB",
>> +    [DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC]  = "YCbCr 2020 Full to RGB 
>> NC",
>> +    [DRM_COLOROP_FM_YCBCR_LIMITED_FULL]  = "YCbCr limited to full",
>> +    [DRM_COLOROP_FM_RGB709_RGB2020]     = "RGB709 to RGB2020",
>> +};
>> +
>>   /* Init Helpers */
>>   static int drm_plane_colorop_init(struct drm_device *dev, struct 
>> drm_colorop *colorop,
>> @@ -455,6 +464,81 @@ 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)
>>   {
>> @@ -521,6 +605,13 @@ static void __drm_colorop_state_reset(struct 
>> drm_colorop_state *colorop_state,
>>                                 &val);
>>           colorop_state->curve_1d_type = val;
>>       }
>> +
>> +    if (colorop->fixed_matrix_type_property) {
>> +        drm_object_property_get_default_value(&colorop->base,
>> +                              colorop->fixed_matrix_type_property,
>> +                              &val);
>> +        colorop_state->fixed_matrix_type = val;
>> +    }
>>   }
>>   /**
>> @@ -561,6 +652,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[] = {
>> @@ -617,6 +709,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 bd082854ca74..c13b4b045fff 100644
>> --- a/include/drm/drm_colorop.h
>> +++ b/include/drm/drm_colorop.h
>> @@ -134,6 +134,71 @@ 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"
>> +     *
>> +     * This selects the matrix that converts full range YCbCr into RGB
>> +     * according to the BT.601 coefficients.
>> +     */
>> +    DRM_COLOROP_FM_YCBCR601_FULL_RGB,
>> +
>> +    /**
>> +     * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
>> +     *
>> +     * enum string "YCbCr 709 Full to RGB"
>> +     *
>> +     * This selects the matrix that converts full range YCbCr into RGB
>> +     * according to the BT.709 coefficients.
>> +     */
>> +    DRM_COLOROP_FM_YCBCR709_FULL_RGB,
>> +
>> +    /**
>> +     * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
>> +     *
>> +     * enum string "YCbCr 2020 Full to RGB NC"
> Nit: I think you mean "YCbCr 2020 NC Full to RGB"?
>> +     *
>> +     * This selects the matrix that converts full range YCbCr into RGB
>> +     * according to the BT.2020 non-constant luminance coefficients.
>> +     */
>> +    DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC,
> ... and here ^ DRM_COLOROP_FM_YCBCR_NC_FULL_RGB

Ack.

>> +
>> +    /**
>> +     * @DRM_COLOROP_FM_YCBCR_LIMITED_FULL:
>> +     *
>> +     * enum string "YCbCr limited to full"
>> +     *
>> +     * This selects the matrix that converts limited range YCbCr into
>> +     * full range YCbCr. Though not strictly a matrix operation but
>> +     * can be represented as one.
>> +     */
>> +    DRM_COLOROP_FM_YCBCR_LIMITED_FULL,
> I didn't full understand how this is going to work.
> Looks like it's merging two properties (COLOR_ENCODING and COLOR_RANGE) 
> in a single colorop.
> But we can only select one enum value. So how to communicate, for 
> example, a "YCbCr 709 Limited to RGB" conversion?
> The driver will have to define a coloro pipeline with two fixed matrix 
> colorop in a row, one with only "Limited to Full" and another with 
> "YCbCr Full to RGB" ?
> 

It depends on what the HW supports. Let's say a fixed function HW block 
can itself support "YCbCr Limited to RGB" then we just need a single 
colorop that exposes this enum. These are being added in the series that 
Harry published.[1]

In Intel's case, we have two separate blocks to do this. One that does 
the range correction (Limited to Full) and then a fixed function CSC 
block that does only "YCbCr Full to RGB" conversion. If they are 
adjacent in the pipeline, they can very well be represent by a single 
colorop described in the previous paragraph but we have a unique 
situation where there is a 1D LUT in between the two blocks.

[YUV Limited to Full] -> [1D LUT] -> [YCbCr XXX Full to RGB] -> [1D LUT].

==
Chaitanya

[1] 
https://lore.kernel.org/dri-devel/20260330153451.99472-3-harry.wentland@amd.com/

> Melissa
>> +
>> +    /**
>> +     * @DRM_COLOROP_FM_RGB709_RGB2020:
>> +     *
>> +     * enum string "RGB709 to RGB2020"
>> +     *
>> +     * Selects the fixed-function CSC preset that converts RGB
>> +     * (BT.709) colorimetry to RGB (BT.2020).
>> +     */
>> +    DRM_COLOROP_FM_RGB709_RGB2020,
>> +
>> +    /**
>> +     * @DRM_COLOROP_FM_COUNT:
>> +     *
>> +     * enum value denoting the size of the enum
>> +     */
>> +    DRM_COLOROP_FM_COUNT
>> +};
>> +
>>   /**
>>    * struct drm_colorop_state - mutable colorop state
>>    */
>> @@ -183,6 +248,13 @@ struct drm_colorop_state {
>>        */
>>       struct drm_property_blob *data;
>> +    /**
>> +     * @fixed_matrix_type:
>> +     *
>> +     * Type of Fixed Matrix operation.
>> +     */
>> +    enum drm_colorop_fixed_matrix_type fixed_matrix_type;
>> +
>>       /** @state: backpointer to global drm_atomic_state */
>>       struct drm_atomic_state *state;
>>   };
>> @@ -368,6 +440,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:
>>        *
>> @@ -424,6 +503,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_duplicate_state(struct drm_colorop *colorop);
>> @@ -480,6 +563,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 a4bdc4bd11bc..dc4b8566fec8 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -971,6 +971,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,
>> +
>>   };
>>   /**
> 

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Melissa Wen]

On 21/04/2026 11:57, Borah, Chaitanya Kumar wrote:
>
>
> On 4/21/2026 6:40 PM, Melissa Wen wrote:
>>
>>
>> On 08/04/2026 02:15, Chaitanya Kumar Borah wrote:
>>> 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.
>>>
>>> v2:
>>>   - Naming changes (Pekka)
>>>
>>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>> ---
>>>   drivers/gpu/drm/drm_atomic.c      |   4 ++
>>>   drivers/gpu/drm/drm_atomic_uapi.c |   4 ++
>>>   drivers/gpu/drm/drm_colorop.c     | 107 
>>> ++++++++++++++++++++++++++++++
>>>   include/drm/drm_colorop.h         |  84 +++++++++++++++++++++++
>>>   include/uapi/drm/drm_mode.h       |  12 ++++
>>>   5 files changed, 211 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic.c 
>>> b/drivers/gpu/drm/drm_atomic.c
>>> index 41c57063f3b4..16a2183b11bf 100644
>>> --- a/drivers/gpu/drm/drm_atomic.c
>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>> @@ -845,6 +845,10 @@ static void 
>>> drm_atomic_colorop_print_state(struct drm_printer *p,
>>> drm_get_colorop_lut3d_interpolation_name(colorop- 
>>> >lut3d_interpolation));
>>>           drm_printf(p, "\tdata blob id=%d\n", state->data ? state- 
>>> >data->base.id : 0);
>>>           break;
>>> +    case DRM_COLOROP_FIXED_MATRIX:
>>> +        drm_printf(p, "\tfixed_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 5bd5bf6661df..d69406b08a0f 100644
>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>> @@ -761,6 +761,8 @@ static int 
>>> drm_atomic_colorop_set_property(struct drm_colorop *colorop,
>>>       } else if (property == colorop->data_property) {
>>>           return drm_atomic_color_set_data_property(colorop, state,
>>>                                 property, val);
>>> +    } else if (property == colorop->fixed_matrix_type_property) {
>>> +        state->fixed_matrix_type = val;
>>>       } else {
>>>           drm_dbg_atomic(colorop->dev,
>>>                      "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
>>> @@ -793,6 +795,8 @@ drm_atomic_colorop_get_property(struct 
>>> drm_colorop *colorop,
>>>           *val = colorop->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 566816e3c6f0..dd385e8dacbe 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,14 @@ 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_YCBCR709_FULL_RGB]    = "YCbCr 709 Full to RGB",
>>> +    [DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC]  = "YCbCr 2020 Full to 
>>> RGB NC",
>>> +    [DRM_COLOROP_FM_YCBCR_LIMITED_FULL]  = "YCbCr limited to full",
>>> +    [DRM_COLOROP_FM_RGB709_RGB2020]     = "RGB709 to RGB2020",
>>> +};
>>> +
>>>   /* Init Helpers */
>>>   static int drm_plane_colorop_init(struct drm_device *dev, struct 
>>> drm_colorop *colorop,
>>> @@ -455,6 +464,81 @@ 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)
>>>   {
>>> @@ -521,6 +605,13 @@ static void __drm_colorop_state_reset(struct 
>>> drm_colorop_state *colorop_state,
>>>                                 &val);
>>>           colorop_state->curve_1d_type = val;
>>>       }
>>> +
>>> +    if (colorop->fixed_matrix_type_property) {
>>> + drm_object_property_get_default_value(&colorop->base,
>>> + colorop->fixed_matrix_type_property,
>>> +                              &val);
>>> +        colorop_state->fixed_matrix_type = val;
>>> +    }
>>>   }
>>>   /**
>>> @@ -561,6 +652,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[] = {
>>> @@ -617,6 +709,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 bd082854ca74..c13b4b045fff 100644
>>> --- a/include/drm/drm_colorop.h
>>> +++ b/include/drm/drm_colorop.h
>>> @@ -134,6 +134,71 @@ 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"
>>> +     *
>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>> +     * according to the BT.601 coefficients.
>>> +     */
>>> +    DRM_COLOROP_FM_YCBCR601_FULL_RGB,
>>> +
>>> +    /**
>>> +     * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
>>> +     *
>>> +     * enum string "YCbCr 709 Full to RGB"
>>> +     *
>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>> +     * according to the BT.709 coefficients.
>>> +     */
>>> +    DRM_COLOROP_FM_YCBCR709_FULL_RGB,
>>> +
>>> +    /**
>>> +     * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
>>> +     *
>>> +     * enum string "YCbCr 2020 Full to RGB NC"
>> Nit: I think you mean "YCbCr 2020 NC Full to RGB"?
>>> +     *
>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>> +     * according to the BT.2020 non-constant luminance coefficients.
>>> +     */
>>> +    DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC,
>> ... and here ^ DRM_COLOROP_FM_YCBCR_NC_FULL_RGB
>
> Ack.
>
>>> +
>>> +    /**
>>> +     * @DRM_COLOROP_FM_YCBCR_LIMITED_FULL:
>>> +     *
>>> +     * enum string "YCbCr limited to full"
>>> +     *
>>> +     * This selects the matrix that converts limited range YCbCr into
>>> +     * full range YCbCr. Though not strictly a matrix operation but
>>> +     * can be represented as one.
>>> +     */
>>> +    DRM_COLOROP_FM_YCBCR_LIMITED_FULL,
>> I didn't full understand how this is going to work.
>> Looks like it's merging two properties (COLOR_ENCODING and 
>> COLOR_RANGE) in a single colorop.
>> But we can only select one enum value. So how to communicate, for 
>> example, a "YCbCr 709 Limited to RGB" conversion?
>> The driver will have to define a coloro pipeline with two fixed 
>> matrix colorop in a row, one with only "Limited to Full" and another 
>> with "YCbCr Full to RGB" ?
>>
>
> It depends on what the HW supports. Let's say a fixed function HW 
> block can itself support "YCbCr Limited to RGB" then we just need a 
> single colorop that exposes this enum. These are being added in the 
> series that Harry published.[1]
>
> In Intel's case, we have two separate blocks to do this. One that does 
> the range correction (Limited to Full) and then a fixed function CSC 
> block that does only "YCbCr Full to RGB" conversion. If they are 
> adjacent in the pipeline, they can very well be represent by a single 
> colorop described in the previous paragraph but we have a unique 
> situation where there is a 1D LUT in between the two blocks.
>
> [YUV Limited to Full] -> [1D LUT] -> [YCbCr XXX Full to RGB] -> [1D LUT].

I see. Thanks for clarifying.

Apart from the DRM_COLOROP_FM_YCBCR_NC_FULL_RGB fix above-mentioned, 
this patch LGTM.
It was missing IGT tests, but I see Harry provided it in the series you 
linked.

Reviewed-by: Melissa Wen <mwen@igalia.com>

>
> ==
> Chaitanya
>
> [1] 
> https://lore.kernel.org/dri-devel/20260330153451.99472-3-harry.wentland@amd.com/
>
>> Melissa
>>> +
>>> +    /**
>>> +     * @DRM_COLOROP_FM_RGB709_RGB2020:
>>> +     *
>>> +     * enum string "RGB709 to RGB2020"
>>> +     *
>>> +     * Selects the fixed-function CSC preset that converts RGB
>>> +     * (BT.709) colorimetry to RGB (BT.2020).
>>> +     */
>>> +    DRM_COLOROP_FM_RGB709_RGB2020,
>>> +
>>> +    /**
>>> +     * @DRM_COLOROP_FM_COUNT:
>>> +     *
>>> +     * enum value denoting the size of the enum
>>> +     */
>>> +    DRM_COLOROP_FM_COUNT
>>> +};
>>> +
>>>   /**
>>>    * struct drm_colorop_state - mutable colorop state
>>>    */
>>> @@ -183,6 +248,13 @@ struct drm_colorop_state {
>>>        */
>>>       struct drm_property_blob *data;
>>> +    /**
>>> +     * @fixed_matrix_type:
>>> +     *
>>> +     * Type of Fixed Matrix operation.
>>> +     */
>>> +    enum drm_colorop_fixed_matrix_type fixed_matrix_type;
>>> +
>>>       /** @state: backpointer to global drm_atomic_state */
>>>       struct drm_atomic_state *state;
>>>   };
>>> @@ -368,6 +440,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:
>>>        *
>>> @@ -424,6 +503,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_duplicate_state(struct drm_colorop 
>>> *colorop);
>>> @@ -480,6 +563,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 a4bdc4bd11bc..dc4b8566fec8 100644
>>> --- a/include/uapi/drm/drm_mode.h
>>> +++ b/include/uapi/drm/drm_mode.h
>>> @@ -971,6 +971,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,
>>> +
>>>   };
>>>   /**
>>
>

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Borah, Chaitanya Kumar]

On 4/23/2026 3:00 AM, Melissa Wen wrote:
> 
> 
> On 21/04/2026 11:57, Borah, Chaitanya Kumar wrote:
>>
>>
>> On 4/21/2026 6:40 PM, Melissa Wen wrote:
>>>
>>>
>>> On 08/04/2026 02:15, Chaitanya Kumar Borah wrote:
>>>> 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.
>>>>
>>>> v2:
>>>>   - Naming changes (Pekka)
>>>>
>>>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/drm_atomic.c      |   4 ++
>>>>   drivers/gpu/drm/drm_atomic_uapi.c |   4 ++
>>>>   drivers/gpu/drm/drm_colorop.c     | 107 ++++++++++++++++++++++++++ 
>>>> ++++
>>>>   include/drm/drm_colorop.h         |  84 +++++++++++++++++++++++
>>>>   include/uapi/drm/drm_mode.h       |  12 ++++
>>>>   5 files changed, 211 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/ 
>>>> drm_atomic.c
>>>> index 41c57063f3b4..16a2183b11bf 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -845,6 +845,10 @@ static void 
>>>> drm_atomic_colorop_print_state(struct drm_printer *p,
>>>> drm_get_colorop_lut3d_interpolation_name(colorop- 
>>>> >lut3d_interpolation));
>>>>           drm_printf(p, "\tdata blob id=%d\n", state->data ? state- 
>>>> >data->base.id : 0);
>>>>           break;
>>>> +    case DRM_COLOROP_FIXED_MATRIX:
>>>> +        drm_printf(p, "\tfixed_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 5bd5bf6661df..d69406b08a0f 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>>> @@ -761,6 +761,8 @@ static int 
>>>> drm_atomic_colorop_set_property(struct drm_colorop *colorop,
>>>>       } else if (property == colorop->data_property) {
>>>>           return drm_atomic_color_set_data_property(colorop, state,
>>>>                                 property, val);
>>>> +    } else if (property == colorop->fixed_matrix_type_property) {
>>>> +        state->fixed_matrix_type = val;
>>>>       } else {
>>>>           drm_dbg_atomic(colorop->dev,
>>>>                      "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
>>>> @@ -793,6 +795,8 @@ drm_atomic_colorop_get_property(struct 
>>>> drm_colorop *colorop,
>>>>           *val = colorop->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 566816e3c6f0..dd385e8dacbe 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,14 @@ 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_YCBCR709_FULL_RGB]    = "YCbCr 709 Full to RGB",
>>>> +    [DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC]  = "YCbCr 2020 Full to 
>>>> RGB NC",
>>>> +    [DRM_COLOROP_FM_YCBCR_LIMITED_FULL]  = "YCbCr limited to full",
>>>> +    [DRM_COLOROP_FM_RGB709_RGB2020]     = "RGB709 to RGB2020",
>>>> +};
>>>> +
>>>>   /* Init Helpers */
>>>>   static int drm_plane_colorop_init(struct drm_device *dev, struct 
>>>> drm_colorop *colorop,
>>>> @@ -455,6 +464,81 @@ 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)
>>>>   {
>>>> @@ -521,6 +605,13 @@ static void __drm_colorop_state_reset(struct 
>>>> drm_colorop_state *colorop_state,
>>>>                                 &val);
>>>>           colorop_state->curve_1d_type = val;
>>>>       }
>>>> +
>>>> +    if (colorop->fixed_matrix_type_property) {
>>>> + drm_object_property_get_default_value(&colorop->base,
>>>> + colorop->fixed_matrix_type_property,
>>>> +                              &val);
>>>> +        colorop_state->fixed_matrix_type = val;
>>>> +    }
>>>>   }
>>>>   /**
>>>> @@ -561,6 +652,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[] = {
>>>> @@ -617,6 +709,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 bd082854ca74..c13b4b045fff 100644
>>>> --- a/include/drm/drm_colorop.h
>>>> +++ b/include/drm/drm_colorop.h
>>>> @@ -134,6 +134,71 @@ 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"
>>>> +     *
>>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>>> +     * according to the BT.601 coefficients.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR601_FULL_RGB,
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
>>>> +     *
>>>> +     * enum string "YCbCr 709 Full to RGB"
>>>> +     *
>>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>>> +     * according to the BT.709 coefficients.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR709_FULL_RGB,
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
>>>> +     *
>>>> +     * enum string "YCbCr 2020 Full to RGB NC"
>>> Nit: I think you mean "YCbCr 2020 NC Full to RGB"?
>>>> +     *
>>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>>> +     * according to the BT.2020 non-constant luminance coefficients.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC,
>>> ... and here ^ DRM_COLOROP_FM_YCBCR_NC_FULL_RGB
>>
>> Ack.
>>
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_YCBCR_LIMITED_FULL:
>>>> +     *
>>>> +     * enum string "YCbCr limited to full"
>>>> +     *
>>>> +     * This selects the matrix that converts limited range YCbCr into
>>>> +     * full range YCbCr. Though not strictly a matrix operation but
>>>> +     * can be represented as one.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR_LIMITED_FULL,
>>> I didn't full understand how this is going to work.
>>> Looks like it's merging two properties (COLOR_ENCODING and 
>>> COLOR_RANGE) in a single colorop.
>>> But we can only select one enum value. So how to communicate, for 
>>> example, a "YCbCr 709 Limited to RGB" conversion?
>>> The driver will have to define a coloro pipeline with two fixed 
>>> matrix colorop in a row, one with only "Limited to Full" and another 
>>> with "YCbCr Full to RGB" ?
>>>
>>
>> It depends on what the HW supports. Let's say a fixed function HW 
>> block can itself support "YCbCr Limited to RGB" then we just need a 
>> single colorop that exposes this enum. These are being added in the 
>> series that Harry published.[1]
>>
>> In Intel's case, we have two separate blocks to do this. One that does 
>> the range correction (Limited to Full) and then a fixed function CSC 
>> block that does only "YCbCr Full to RGB" conversion. If they are 
>> adjacent in the pipeline, they can very well be represent by a single 
>> colorop described in the previous paragraph but we have a unique 
>> situation where there is a 1D LUT in between the two blocks.
>>
>> [YUV Limited to Full] -> [1D LUT] -> [YCbCr XXX Full to RGB] -> [1D LUT].
> 
> I see. Thanks for clarifying.
> 
> Apart from the DRM_COLOROP_FM_YCBCR_NC_FULL_RGB fix above-mentioned, 
> this patch LGTM.
> It was missing IGT tests, but I see Harry provided it in the series you 
> linked.
> 
> Reviewed-by: Melissa Wen <mwen@igalia.com>
> 

Thank you, Melissa, for the review.

Regarding the IGTs they are yet to be upstreamed with this version of 
the series as we are still working out how to use CRC matching for pass 
criteria. (As i915/xe does not have writeback hooked up yet)

==
Chaitanya

>>
>> ==
>> Chaitanya
>>
>> [1] https://lore.kernel.org/dri-devel/20260330153451.99472-3- 
>> harry.wentland@amd.com/
>>
>>> Melissa
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_RGB709_RGB2020:
>>>> +     *
>>>> +     * enum string "RGB709 to RGB2020"
>>>> +     *
>>>> +     * Selects the fixed-function CSC preset that converts RGB
>>>> +     * (BT.709) colorimetry to RGB (BT.2020).
>>>> +     */
>>>> +    DRM_COLOROP_FM_RGB709_RGB2020,
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_COUNT:
>>>> +     *
>>>> +     * enum value denoting the size of the enum
>>>> +     */
>>>> +    DRM_COLOROP_FM_COUNT
>>>> +};
>>>> +
>>>>   /**
>>>>    * struct drm_colorop_state - mutable colorop state
>>>>    */
>>>> @@ -183,6 +248,13 @@ struct drm_colorop_state {
>>>>        */
>>>>       struct drm_property_blob *data;
>>>> +    /**
>>>> +     * @fixed_matrix_type:
>>>> +     *
>>>> +     * Type of Fixed Matrix operation.
>>>> +     */
>>>> +    enum drm_colorop_fixed_matrix_type fixed_matrix_type;
>>>> +
>>>>       /** @state: backpointer to global drm_atomic_state */
>>>>       struct drm_atomic_state *state;
>>>>   };
>>>> @@ -368,6 +440,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:
>>>>        *
>>>> @@ -424,6 +503,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_duplicate_state(struct drm_colorop 
>>>> *colorop);
>>>> @@ -480,6 +563,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 a4bdc4bd11bc..dc4b8566fec8 100644
>>>> --- a/include/uapi/drm/drm_mode.h
>>>> +++ b/include/uapi/drm/drm_mode.h
>>>> @@ -971,6 +971,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,
>>>> +
>>>>   };
>>>>   /**
>>>
>>
> 

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Melissa Wen]

On 23/04/2026 06:38, Borah, Chaitanya Kumar wrote:
> Regarding the IGTs they are yet to be upstreamed with this version of 
> the series as we are still working out how to use CRC matching for 
> pass criteria. (As i915/xe does not have writeback hooked up yet)

Out of curiousity, are you working on adding writeback support to them?

Melissa

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Borah, Chaitanya Kumar]

On 4/24/2026 8:19 PM, Melissa Wen wrote:
> On 23/04/2026 06:38, Borah, Chaitanya Kumar wrote:
>> Regarding the IGTs they are yet to be upstreamed with this version of 
>> the series as we are still working out how to use CRC matching for 
>> pass criteria. (As i915/xe does not have writeback hooked up yet)
> 
> Out of curiousity, are you working on adding writeback support to them?


It is currently a work in progress.

https://lore.kernel.org/dri-devel/20260316083008.87466-1-suraj.kandpal@intel.com/

https://lore.kernel.org/intel-gfx/20260325110744.1096786-1-suraj.kandpal@intel.com/

==
Chaitanya

Re: [PATCH v2 01/13] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[Harry Wentland]

On 2026-04-22 17:30, Melissa Wen wrote:
> 
> 
> On 21/04/2026 11:57, Borah, Chaitanya Kumar wrote:
>>
>>
>> On 4/21/2026 6:40 PM, Melissa Wen wrote:
>>>
>>>
>>> On 08/04/2026 02:15, Chaitanya Kumar Borah wrote:
>>>> 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.
>>>>
>>>> v2:
>>>>   - Naming changes (Pekka)
>>>>
>>>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/drm_atomic.c      |   4 ++
>>>>   drivers/gpu/drm/drm_atomic_uapi.c |   4 ++
>>>>   drivers/gpu/drm/drm_colorop.c     | 107 ++++++++++++++++++++++++++++++
>>>>   include/drm/drm_colorop.h         |  84 +++++++++++++++++++++++
>>>>   include/uapi/drm/drm_mode.h       |  12 ++++
>>>>   5 files changed, 211 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>>> index 41c57063f3b4..16a2183b11bf 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -845,6 +845,10 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
>>>> drm_get_colorop_lut3d_interpolation_name(colorop- >lut3d_interpolation));
>>>>           drm_printf(p, "\tdata blob id=%d\n", state->data ? state- >data->base.id : 0);
>>>>           break;
>>>> +    case DRM_COLOROP_FIXED_MATRIX:
>>>> +        drm_printf(p, "\tfixed_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 5bd5bf6661df..d69406b08a0f 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>>> @@ -761,6 +761,8 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
>>>>       } else if (property == colorop->data_property) {
>>>>           return drm_atomic_color_set_data_property(colorop, state,
>>>>                                 property, val);
>>>> +    } else if (property == colorop->fixed_matrix_type_property) {
>>>> +        state->fixed_matrix_type = val;
>>>>       } else {
>>>>           drm_dbg_atomic(colorop->dev,
>>>>                      "[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
>>>> @@ -793,6 +795,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
>>>>           *val = colorop->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 566816e3c6f0..dd385e8dacbe 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,14 @@ 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_YCBCR709_FULL_RGB]    = "YCbCr 709 Full to RGB",
>>>> +    [DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC]  = "YCbCr 2020 Full to RGB NC",
>>>> +    [DRM_COLOROP_FM_YCBCR_LIMITED_FULL]  = "YCbCr limited to full",
>>>> +    [DRM_COLOROP_FM_RGB709_RGB2020]     = "RGB709 to RGB2020",
>>>> +};
>>>> +
>>>>   /* Init Helpers */
>>>>   static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
>>>> @@ -455,6 +464,81 @@ 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)
>>>>   {
>>>> @@ -521,6 +605,13 @@ static void __drm_colorop_state_reset(struct drm_colorop_state *colorop_state,
>>>>                                 &val);
>>>>           colorop_state->curve_1d_type = val;
>>>>       }
>>>> +
>>>> +    if (colorop->fixed_matrix_type_property) {
>>>> + drm_object_property_get_default_value(&colorop->base,
>>>> + colorop->fixed_matrix_type_property,
>>>> +                              &val);
>>>> +        colorop_state->fixed_matrix_type = val;
>>>> +    }
>>>>   }
>>>>   /**
>>>> @@ -561,6 +652,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[] = {
>>>> @@ -617,6 +709,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 bd082854ca74..c13b4b045fff 100644
>>>> --- a/include/drm/drm_colorop.h
>>>> +++ b/include/drm/drm_colorop.h
>>>> @@ -134,6 +134,71 @@ 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"
>>>> +     *
>>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>>> +     * according to the BT.601 coefficients.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR601_FULL_RGB,
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_YCBCR709_FULL_RGB:
>>>> +     *
>>>> +     * enum string "YCbCr 709 Full to RGB"
>>>> +     *
>>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>>> +     * according to the BT.709 coefficients.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR709_FULL_RGB,
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
>>>> +     *
>>>> +     * enum string "YCbCr 2020 Full to RGB NC"
>>> Nit: I think you mean "YCbCr 2020 NC Full to RGB"?
>>>> +     *
>>>> +     * This selects the matrix that converts full range YCbCr into RGB
>>>> +     * according to the BT.2020 non-constant luminance coefficients.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR2020_FULL_RGB_NC,
>>> ... and here ^ DRM_COLOROP_FM_YCBCR_NC_FULL_RGB
>>
>> Ack.
>>
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_YCBCR_LIMITED_FULL:
>>>> +     *
>>>> +     * enum string "YCbCr limited to full"
>>>> +     *
>>>> +     * This selects the matrix that converts limited range YCbCr into
>>>> +     * full range YCbCr. Though not strictly a matrix operation but
>>>> +     * can be represented as one.
>>>> +     */
>>>> +    DRM_COLOROP_FM_YCBCR_LIMITED_FULL,
>>> I didn't full understand how this is going to work.
>>> Looks like it's merging two properties (COLOR_ENCODING and COLOR_RANGE) in a single colorop.
>>> But we can only select one enum value. So how to communicate, for example, a "YCbCr 709 Limited to RGB" conversion?
>>> The driver will have to define a coloro pipeline with two fixed matrix colorop in a row, one with only "Limited to Full" and another with "YCbCr Full to RGB" ?
>>>
>>
>> It depends on what the HW supports. Let's say a fixed function HW block can itself support "YCbCr Limited to RGB" then we just need a single colorop that exposes this enum. These are being added in the series that Harry published.[1]
>>
>> In Intel's case, we have two separate blocks to do this. One that does the range correction (Limited to Full) and then a fixed function CSC block that does only "YCbCr Full to RGB" conversion. If they are adjacent in the pipeline, they can very well be represent by a single colorop described in the previous paragraph but we have a unique situation where there is a 1D LUT in between the two blocks.
>>
>> [YUV Limited to Full] -> [1D LUT] -> [YCbCr XXX Full to RGB] -> [1D LUT].
> 
> I see. Thanks for clarifying.
> 
> Apart from the DRM_COLOROP_FM_YCBCR_NC_FULL_RGB fix above-mentioned, this patch LGTM.
> It was missing IGT tests, but I see Harry provided it in the series you linked.
> 
> Reviewed-by: Melissa Wen <mwen@igalia.com>

Feel free to also add my
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> 
>>
>> ==
>> Chaitanya
>>
>> [1] https://lore.kernel.org/dri-devel/20260330153451.99472-3-harry.wentland@amd.com/
>>
>>> Melissa
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_RGB709_RGB2020:
>>>> +     *
>>>> +     * enum string "RGB709 to RGB2020"
>>>> +     *
>>>> +     * Selects the fixed-function CSC preset that converts RGB
>>>> +     * (BT.709) colorimetry to RGB (BT.2020).
>>>> +     */
>>>> +    DRM_COLOROP_FM_RGB709_RGB2020,
>>>> +
>>>> +    /**
>>>> +     * @DRM_COLOROP_FM_COUNT:
>>>> +     *
>>>> +     * enum value denoting the size of the enum
>>>> +     */
>>>> +    DRM_COLOROP_FM_COUNT
>>>> +};
>>>> +
>>>>   /**
>>>>    * struct drm_colorop_state - mutable colorop state
>>>>    */
>>>> @@ -183,6 +248,13 @@ struct drm_colorop_state {
>>>>        */
>>>>       struct drm_property_blob *data;
>>>> +    /**
>>>> +     * @fixed_matrix_type:
>>>> +     *
>>>> +     * Type of Fixed Matrix operation.
>>>> +     */
>>>> +    enum drm_colorop_fixed_matrix_type fixed_matrix_type;
>>>> +
>>>>       /** @state: backpointer to global drm_atomic_state */
>>>>       struct drm_atomic_state *state;
>>>>   };
>>>> @@ -368,6 +440,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:
>>>>        *
>>>> @@ -424,6 +503,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_duplicate_state(struct drm_colorop *colorop);
>>>> @@ -480,6 +563,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 a4bdc4bd11bc..dc4b8566fec8 100644
>>>> --- a/include/uapi/drm/drm_mode.h
>>>> +++ b/include/uapi/drm/drm_mode.h
>>>> @@ -971,6 +971,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,
>>>> +
>>>>   };
>>>>   /**
>>>
>>
>