On 3/16/2026 2:27 PM, Pekka Paalanen wrote:
On Mon, 16 Mar 2026 12:46:39 +0530
"Borah, Chaitanya Kumar" <[email protected]> wrote:
Hi Pekka,
Thank you for looking into the patch.
Hi Chaitanya!
Replies inline below.
On 3/10/2026 8:02 PM, Pekka Paalanen wrote:
On Fri, 6 Mar 2026 22:22:58 +0530
Chaitanya Kumar Borah <[email protected]> wrote:
Introduce DRM_COLOROP_CSC_FF, a new colorop type representing a
fixed-function Color Space Conversion (CSC) block.
Unlike CTM-based colorops, this block does not expose programmable
coefficients. Instead, userspace selects one of the predefined
hardware modes via a new CSC_FF_TYPE enum property. Supported modes
include common YUV->RGB and RGB709->RGB2020 conversions.
Signed-off-by: Chaitanya Kumar Borah <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 4 ++
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_colorop.c | 105 ++++++++++++++++++++++++++++++
include/drm/drm_colorop.h | 72 ++++++++++++++++++++
include/uapi/drm/drm_mode.h | 13 ++++
5 files changed, 198 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 04925166df98..7296b844e3fd 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -844,6 +844,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_CSC_FF:
+ drm_printf(p, "\tcsc_ff_type=%s\n",
+
drm_get_colorop_csc_ff_type_name(state->csc_ff_type));
+ break;
default:
break;
}
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 87de41fb4459..9af73325aa93 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -757,6 +757,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->csc_ff_type_property) {
+ state->csc_ff_type = val;
} else {
drm_dbg_atomic(colorop->dev,
"[COLOROP:%d:%d] unknown property
[PROP:%d:%s]\n",
@@ -789,6 +791,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->csc_ff_type_property)
+ *val = state->csc_ff_type;
else
return -EINVAL;
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index f421c623b3f0..49422c625f4d 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_CSC_FF, "CSC Fixed-Function"},
Hi,
the fundamental idea seems fine to me, but I have a lot to say about the
nomenclature.
What would you think of a more readable name DRM_COLOROP_FIXED_MATRIX
"Fixed Matrix"?
Alternatively DRM_COLOROP_ENUM_MATRIX "Enumerated Matrix".
I was intentionally staying away from the word matrix because there was
no programmable matrix but it would make sense to name it something like
DRM_COLOROP_FIXED_MATRIX (or *_PRESET_MATRIX for that matter).
};
static const char * const colorop_curve_1d_type_names[] = {
@@ -90,6 +91,13 @@ static const struct drm_prop_enum_list
drm_colorop_lut3d_interpolation_list[] =
{ DRM_COLOROP_LUT3D_INTERPOLATION_TETRAHEDRAL, "Tetrahedral" },
};
+static const char * const colorop_csc_ff_type_names[] = {
+ [DRM_COLOROP_CSC_FF_YUV601_RGB601] = "YUV601 to RGB601",
+ [DRM_COLOROP_CSC_FF_YUV709_RGB709] = "YUV709 to RGB709",
+ [DRM_COLOROP_CSC_FF_YUV2020_RGB2020] = "YUV2020 to RGB2020",
+ [DRM_COLOROP_CSC_FF_RGB709_RGB2020] = "RGB709 to RGB2020",
I'd suggest names:
"YCbCr 601 to RGB"
"YCbCr 709 to RGB"
"YCbCr 2020 NC to RGB"
"RGB709 to RGB2020"
or something in that direction.
The relevant ITU-R BT specifications use YCbCr nomenclature IIRC. Wrt.
YCbCr-to-RGB conversion, there is no RGB601, RGB709 or RGB2020. There
is only some RGB, and which primaries it uses is not always tied to
which YCbCr conversion was used.
What I understand from this is that the BT.709(et al.) only defines the
matrix that is used for YCbCr->RGB, "what" RGB it is defined by the
primaries (which comes with metadata?).
Unfortunately, BT.601, BT.709 and BT.2020 define two separate things each:
- the YCbCr<->RGB conversion, and
- the colorspace primaries (and white point, but that is the same for
them all).
BT.601 actually has two different sets of primaries. Bt.2020 defines
two different YCbCr conversions. BT.709 uses the same primaries as
sRGB, but is different from sRGB on all other aspects.
Therefore, when you refer to any one of these, you also need to be
clear whether you are referring to the YCbCr conversion or to the
primaries.
In that case, if the HW block says that it does YCbCr to RGB conversion
using rec BT.709, the resultant RGB follows the primaries as described
by BT.709 or mathematically it does not really matter?
I will read up on why our HW names these bits as such.
Sure, but keep in mind that your hardware naming is irrelevant for the
UAPI design.
Understood, I just want to make sure that the HW does exactly what we
will advertise through the UAPI.
For YCbCr 2020 I feel it's nice to remember, that there are two
different conversions in the specification: the simple matrix one
called "non-constant luminance", and the complex one called "constant
luminance". Hence "NC".
It's also good to recall that YCbCr-RGB conversions are done in an
electrical space, while RGB709-to-RGB2020 conversion must be done in the
optical space. It is up to the userspace to arrange the neighbouring
colorops to use the fixed matrix right.
Ack on the above.
+};
+
/* Init Helpers */
static int drm_plane_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
@@ -459,6 +467,80 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev,
struct drm_colorop *col
}
EXPORT_SYMBOL(drm_plane_colorop_3dlut_init);
+/**
+ * drm_plane_colorop_csc_ff_init - Initialize a DRM_COLOROP_CSC_FF
+ *
+ * @dev: DRM device
+ * @colorop: The drm_colorop object to initialize
+ * @plane: The associated drm_plane
+ * @funcs: control functions for the new colorop
+ * @supported_csc_ff: A bitfield of supported drm_plane_colorop_csc_ff_type
enum values,
+ * created using BIT(csc_ff_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_csc_ff_init(struct drm_device *dev, struct drm_colorop
*colorop,
+ struct drm_plane *plane, const struct
drm_colorop_funcs *funcs,
+ u64 supported_csc_ff, uint32_t flags)
+{
+ struct drm_prop_enum_list enum_list[DRM_COLOROP_CSC_FF_COUNT];
+ int i, len;
+
+ struct drm_property *prop;
+ int ret;
+
+ if (!supported_csc_ff) {
+ drm_err(dev,
+ "No supported CSC op for new CSC FF colorop on
[PLANE:%d:%s]\n",
+ plane->base.id, plane->name);
+ return -EINVAL;
+ }
+
+ if ((supported_csc_ff & -BIT(DRM_COLOROP_CSC_FF_COUNT)) != 0) {
+ drm_err(dev, "Unknown CSC provided on [PLANE:%d:%s]\n",
+ plane->base.id, plane->name);
+ return -EINVAL;
+ }
+
+ ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
DRM_COLOROP_CSC_FF, flags);
+ if (ret)
+ return ret;
+
+ len = 0;
+ for (i = 0; i < DRM_COLOROP_CSC_FF_COUNT; i++) {
+ if ((supported_csc_ff & BIT(i)) == 0)
+ continue;
+
+ enum_list[len].type = i;
+ enum_list[len].name = colorop_csc_ff_type_names[i];
+ len++;
+ }
+
+ if (WARN_ON(len <= 0))
+ return -EINVAL;
+
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
"CSC_FF_TYPE",
+ enum_list, len);
The Color Space Conversion Fixed-Function type is always "fixed
matrix", right?
The name for the colorop property to choose one of the supported
matrices could be... "matrix"? "choice"?
Ack.
Does the property name need to be unique over all colorop types?
I am not sure if I understand your question. Could you please elaborate?
Let's say we have two colorop types: MATRIX implements an arbitrary
programmable matrix, and FIXED_MATRIX where you pick the matrix from an
enum.
MATRIX needs a property for the matrix data, I think there is a
colorop property named DATA that takes a blob id and is used by several
colorop types for different kinds of data - they all take a blob id
though.
Yes, I think that is what [1] does.
[1]
https://lore.kernel.org/dri-devel/20251223-mtk-ovl-pre-blend-colorops-v1-9-0cb99bd0a...@collabora.com/.
Ok, so there is no strict requirement for the property to be unique
over all colorop types. But are there any design guidelines here?
Hmm, maybe not.
Yeah I don't think there is any such design guideline.
In theory, we could have re-used the enum property "CURVE_1D_TYPE"
(after renaming it, ofcourse) and assign meaning to the property based
on the type of the colorop it is attached to, like we do with "DATA" but
I think that ship has sailed(?).
But in hindsight that would have been a better approach so as to not
bloat the colorop object with properties.
+
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->csc_ff_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->csc_ff_type_property,
+ enum_list[0].type);
+ drm_colorop_reset(colorop);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_colorop_csc_ff_init);
+
static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop
*colorop,
struct
drm_colorop_state *state)
{
@@ -513,6 +595,13 @@ static void __drm_colorop_state_reset(struct
drm_colorop_state *colorop_state,
&val);
colorop_state->curve_1d_type = val;
}
+
+ if (colorop->csc_ff_type_property) {
+ drm_object_property_get_default_value(&colorop->base,
+
colorop->csc_ff_type_property,
+ &val);
+ colorop_state->csc_ff_type = val;
+ }
}
/**
@@ -551,6 +640,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_CSC_FF] = "CSC Fixed-Function",
};
Why are there two arrays with the same DRM_COLOROP_* = name association?
drm_colorop_type_enum_list is the first one.
This array is explicitly used by drm_get_colorop_type_name(). Connectors
use an enum list for a similar purpose, so colorops could also reuse an
enum list here, provided that the enum array index remains in sync with
the corresponding enum value.
static const char * const colorop_lu3d_interpolation_name[] = {
@@ -607,6 +697,21 @@ const char *drm_get_colorop_lut3d_interpolation_name(enum
drm_colorop_lut3d_inte
return colorop_lu3d_interpolation_name[type];
}
+/**
+ * drm_get_colorop_csc_ff_type_name: return a string for interpolation type
+ * @type: csc ff 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_csc_ff_type_name(enum drm_colorop_csc_ff_type type)
+{
+ if (WARN_ON(type >= ARRAY_SIZE(colorop_csc_ff_type_names)))
+ return "unknown";
+
+ return colorop_csc_ff_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..2cd8e0779c2a 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -134,6 +134,60 @@ enum drm_colorop_curve_1d_type {
DRM_COLOROP_1D_CURVE_COUNT
};
+/**
+ * enum drm_colorop_csc_ff_type - type of CSC Fixed-Function
+ *
+ * Describes a CSC operation to be applied by the DRM_COLOROP_CSC_FF colorop.
It's a matrix operation. It seems to me that "CSC operation" is more
specific and does not fit the YCbCr-to-RGB conversion.
Yes makes sense, matrix would be a more generic term.
+ */
+enum drm_colorop_csc_ff_type {
+ /**
+ * @DRM_COLOROP_CSC_FF_YUV601_RGB601
+ *
+ * enum string "YUV601 to RGB601"
+ *
+ * Selects the fixed-function CSC preset that converts YUV
+ * (BT.601) colorimetry to RGB (BT.601).
This selects the matrix that converts YCbCr into RGB
according to the BT.601 coefficients.
+ */
+ DRM_COLOROP_CSC_FF_YUV601_RGB601,
+
+ /**
+ * @DRM_COLOROP_CSC_FF_YUV709_RGB709:
+ *
+ * enum string "YUV709 to RGB709"
+ *
+ * Selects the fixed-function CSC preset that converts YUV
+ * (BT.709) colorimetry to RGB (BT.709).
This selects the matrix that converts YCbCr into RGB
according to the BT.709 coefficients.
+ */
+ DRM_COLOROP_CSC_FF_YUV709_RGB709,
+
+ /**
+ * @DRM_COLOROP_CSC_FF_YUV2020_RGB2020:
+ *
+ * enum string "YUV2020 to RGB2020"
+ *
+ * Selects the fixed-function CSC preset that converts YUV
+ * (BT.2020) colorimetry to RGB (BT.2020).
This selects the matrix that converts YCbCr into RGB
according to the BT.2020 non-constant luminance coefficients.
+ */
+ DRM_COLOROP_CSC_FF_YUV2020_RGB2020,
+
+ /**
+ * @DRM_COLOROP_CSC_FF_RGB709_RGB2020:
+ *
+ * enum string "RGB709 to RGB2020"
+ *
+ * Selects the fixed-function CSC preset that converts RGB
+ * (BT.709) colorimetry to RGB (BT.2020).
This selects the matrix that converts optical RGB from BT.709 primaries
to BT.2020 primaries.
Ack on the documentation.
+ */
+ DRM_COLOROP_CSC_FF_RGB709_RGB2020,
+
+ /**
+ * @DRM_COLOROP_CSC_FF_COUNT:
+ *
+ * enum value denoting the size of the enum
+ */
+ DRM_COLOROP_CSC_FF_COUNT
+};
+
/**
* struct drm_colorop_state - mutable colorop state
*/
@@ -183,6 +237,13 @@ struct drm_colorop_state {
*/
struct drm_property_blob *data;
+ /**
+ * @csc_ff_type:
+ *
+ * Type of Fixed function CSC.
+ */
+ enum drm_colorop_csc_ff_type csc_ff_type;
+
/** @state: backpointer to global drm_atomic_state */
struct drm_atomic_state *state;
};
@@ -368,6 +429,13 @@ struct drm_colorop {
*/
struct drm_property *data_property;
+ /**
+ * @csc_ff_type_property:
+ *
+ * Sub-type for DRM_COLOROP_CSC_FF type.
+ */
+ struct drm_property *csc_ff_type_property;
+
/**
* @next_property:
*
@@ -424,6 +492,9 @@ 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_csc_ff_init(struct drm_device *dev, struct drm_colorop
*colorop,
+ struct drm_plane *plane, const struct
drm_colorop_funcs *funcs,
+ u64 supported_csc_ff, uint32_t flags);
struct drm_colorop_state *
drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop);
@@ -480,6 +551,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_csc_ff_type_name(enum drm_colorop_csc_ff_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 3693d82b5279..f7808e7ea984 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -968,6 +968,19 @@ enum drm_colorop_type {
* color = lut3d[index]
*/
DRM_COLOROP_3D_LUT,
+
+ /**
+ * @DRM_COLOROP_CSC_FF:
+ *
+ * enum string "CSC Fixed-Function"
+ *
+ * A fixed-function Color Space Conversion block where the coefficients
+ * are not programmable but selected from predefined hardware modes via
+ * the CSC_FF_TYPE enum property. The driver advertises the supported
+ * CSC modes through this property.
This would be a lot more obvious if it was called a "fixed matrix"
operation or such. The current wording never mentions "matrix".
Ack.
I also wanted throw this question out there. Since we have introduced
YUV to RGB conversion colorop which essentially replaces the color
encoding property, would this also be the right time to bring in
something to replace the color range property.
Yes.
I recall Harry mentioning in the cover letter of the original series
that he was working on something along those lines.
Reading Harry's latest blog post it sounds he has done similar work as
you.
Harry's blog link seems dead, but the post is available at
https://planet.freedesktop.org/
titled "Harry Wentland: Plane Color Pipeline, CSC, 3D LUT, and KWin"
with links to patches.
I will have a look at it, thank you.
==
Chaitanya
Thanks,
pq
==
Chaitanya
+ */
+ DRM_COLOROP_CSC_FF,
+
};
/**
Thanks,
pq
