On 2026-07-03 06:52, Pekka Paalanen wrote:
> On Tue, 23 Jun 2026 12:48:02 -0400
> Harry Wentland <[email protected]> wrote:
>
>> From: Chaitanya Kumar Borah <[email protected]>
>>
>> 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)
>>
>> v3:
>> - Fix NC matrix enum name and string (Melissa)
>> - Rebase
>>
>> Signed-off-by: Chaitanya Kumar Borah <[email protected]>
>> Reviewed-by: Melissa Wen <[email protected]>
>> Reviewed-by: Harry Wentland <[email protected]>
>> ---
>> drivers/gpu/drm/drm_atomic.c | 4 ++
>> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
>> drivers/gpu/drm/drm_colorop.c | 106 ++++++++++++++++++++++++++++++
>> include/drm/drm_colorop.h | 84 +++++++++++++++++++++++
>> include/uapi/drm/drm_mode.h | 12 ++++
>> 5 files changed, 210 insertions(+)
>>
>
> Hi,
>
> the UAPI looks good.
>
> Acked-by: Pekka Paalanen <[email protected]>
>
> It's a little bit unfortunate that the colorop UAPI documentation is
> embedded into kernel-private enum types' documentation.
>
Agreed. This goes beyond this patchset. I'll have a look at a separate
series to see if we can fix that.
> Is RGB always full-range? The documentation could be more explicit
> about that. Maybe the documentation should spell out the matrix
> contents just to be clear? There are more than one form of limited
> range IIRC (HDMI, JPEG, SDI). Or maybe JPEG was a different type of
> full-range?
>
It's always full-range. Agreed about spelling out the matrix contents.
>> + * enum string "RGB709 to RGB2020"
>> + *
>> + * Selects the fixed-function CSC preset that converts RGB
>> + * (BT.709) colorimetry to RGB (BT.2020).
>
> Why is this called a "CSC preset" rather than a matrix? This confuses
> me.
>
Will fix that up.
I was taking Chaitanya's patch as-is and adding my changes in Patch 2
but with all these changes I'll end up squashing the two commits
together.
> Was there a conclusion from discussions around compatible pixel formats
> for a specific choice of a fixed matrix? How will userspace discover
> that, or will it be based on documentation and enforced as a common
> requirement over all drivers?
>
> ISTR some hardware requiring an YUV pixel format to be able to use the
> YCbCr conversion matrices, or vice versa. Otherwise there is no reason
> to have that limitation.
>
We talked about this at the Hackfest (offline most likely) and the
consensus is that there's no reason to limit this to certain pixel formats.
We should treat it as a simple matrix that operates on the 3 input channels.
If userspace decides to select a CSC matrix for an RGB buffer it's assumed
that that's intentional and userspace knows what it's doing.
Harry
>
> Thanks,
> pq
>
>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 3af1b9cc9a06..ced591c4a0bd 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -925,6 +925,10 @@ static void drm_atomic_colorop_print_state(struct
>> drm_printer *p,
>>
>> drm_get_colorop_lut3d_interpolation_name(colorop->lut3d_interpolation));
>> drm_printf_indent(p, 1, "data blob id=%d\n", state->data ?
>> state->data->base.id : 0);
>> break;
>> + case DRM_COLOROP_FIXED_MATRIX:
>> + drm_printf_indent(p, 1, "fixed_matrix_type=%s\n",
>> +
>> drm_get_colorop_fixed_matrix_type_name(state->fixed_matrix_type));
>> + break;
>> default:
>> break;
>> }
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index c7f80d90794c..cee4550ffdbd 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 c0eecde8c176..c11c3012fcc5 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_NC_FULL_RGB] = "YCbCr 2020 NC Full to RGB",
>> + [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,80 @@ int drm_plane_colorop_3dlut_init(struct drm_device
>> *dev, struct drm_colorop *col
>> }
>> EXPORT_SYMBOL(drm_plane_colorop_3dlut_init);
>>
>> +/**
>> + * drm_plane_colorop_fixed_matrix_init - Initialize a
>> DRM_COLOROP_FIXED_MATRIX
>> + *
>> + * @dev: DRM device
>> + * @colorop: The drm_colorop object to initialize
>> + * @plane: The associated drm_plane
>> + * @funcs: control functions for the new colorop
>> + * @supported_fm: A bitfield of supported drm_colorop_fixed_matrix_type
>> enum values,
>> + * created using BIT(fixed_matrix_type) and combined with the
>> OR '|'
>> + * operator.
>> + * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
>> + * @return zero on success, -E value on failure
>> + */
>> +int drm_plane_colorop_fixed_matrix_init(struct drm_device *dev, struct
>> drm_colorop *colorop,
>> + struct drm_plane *plane,
>> + const struct drm_colorop_funcs *funcs,
>> + u64 supported_fm, uint32_t flags)
>> +{
>> + struct drm_prop_enum_list enum_list[DRM_COLOROP_FM_COUNT];
>> + int i, len;
>> + struct drm_property *prop;
>> + int ret;
>> +
>> + if (!supported_fm) {
>> + drm_err(dev,
>> + "No supported FM type op for new Fixed Matrix colorop
>> on [PLANE:%d:%s]\n",
>> + plane->base.id, plane->name);
>> + return -EINVAL;
>> + }
>> +
>> + if ((supported_fm & -BIT(DRM_COLOROP_FM_COUNT)) != 0) {
>> + drm_err(dev, "Unknown Fixed Matrix provided on [PLANE:%d:%s]\n",
>> + plane->base.id, plane->name);
>> + return -EINVAL;
>> + }
>> +
>> + ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
>> DRM_COLOROP_FIXED_MATRIX, flags);
>> + if (ret)
>> + return ret;
>> +
>> + len = 0;
>> + for (i = 0; i < DRM_COLOROP_FM_COUNT; i++) {
>> + if ((supported_fm & BIT(i)) == 0)
>> + continue;
>> +
>> + enum_list[len].type = i;
>> + enum_list[len].name = colorop_fixed_matrix_type_names[i];
>> + len++;
>> + }
>> +
>> + if (WARN_ON(len <= 0))
>> + return -EINVAL;
>> +
>> + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
>> "FIXED_MATRIX_TYPE",
>> + enum_list, len);
>> +
>> + if (!prop)
>> + return -ENOMEM;
>> +
>> + colorop->fixed_matrix_type_property = prop;
>> + /*
>> + * Default to the first supported CSC mode as provided by the driver.
>> + * Intuitively this should be something that keeps the colorop in pixel
>> bypass
>> + * mode but that is already handled via the standard colorop bypass
>> + * property.
>> + */
>> + drm_object_attach_property(&colorop->base,
>> colorop->fixed_matrix_type_property,
>> + enum_list[0].type);
>> + drm_colorop_reset(colorop);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(drm_plane_colorop_fixed_matrix_init);
>> +
>> static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop
>> *colorop,
>> struct
>> drm_colorop_state *state)
>> {
>> @@ -521,6 +604,13 @@ static void __drm_colorop_state_init(struct
>> drm_colorop_state *colorop_state,
>> &val))
>> colorop_state->curve_1d_type = val;
>> }
>> +
>> + if (colorop->fixed_matrix_type_property) {
>> + if (!drm_object_property_get_default_value(&colorop->base,
>> +
>> colorop->fixed_matrix_type_property,
>> + &val))
>> + colorop_state->fixed_matrix_type = val;
>> + }
>> }
>>
>> /**
>> @@ -584,6 +674,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[] = {
>> @@ -640,6 +731,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 b4b9e4f558ab..88933d5b4d8b 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 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_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,
>> +
>> + /**
>> + * @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_commit */
>> struct drm_atomic_commit *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_create_state(struct drm_colorop *colorop);
>> @@ -482,6 +565,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 381a3e857d4e..bff6dba08d50 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -954,6 +954,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,
>> +
>> };
>>
>> /**
>
