Re: [PATCH 01/10] drm/colorop: Add DRM_COLOROP_CSC_FF

Borah, Chaitanya Kumar
2026-03-17T12:29:43Z

Hi Harry,

On 3/17/2026 2:15 AM, Harry Wentland wrote:
>
>
> On 2026-03-06 11:52, Chaitanya Kumar Borah 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 <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 | 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"},
>> };
>>
>> 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",
>
> If these are intended to be used for YCbCr to RGB conversions
> will they convert from limited to full range? Or full range
> to full range? Or something else?
>

AFAIU, the block I wanted to represent is Full Range YCbCr -> Full Range
RGB. We have the following configuration in our plane.

[YUV range correction Block] -> [Degamma LUT] -> [PRESET CSC] -> [Gamma LUT]

With the legacy "COLOR RANGE" property, selecting limited range enabled
the YUV range correction block.

I still need to figure out what use-case does the degamma LUT in between
the Range correction block and the Preset CSC serve since YCbCr to RGB
conversion will take place in non-linear domain. But it makes me wonder
if we can have "COLOR ENCODING" and "COLOR RANGE" property within the
same colorop like you have been implementing in [1] or should they be
represented by separate colorops.

Uma please weigh in if you have something to add or disagree with.

==
Chaitanya

[1] https://gitlab.freedesktop.org/hwentland/linux/-/commits/csc-colorop

> Harry
>
>> + [DRM_COLOROP_CSC_FF_RGB709_RGB2020] = "RGB709 to RGB2020",
>> +};
>> +
>> /* 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);
>> +
>> + 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",
>> };
>>
>> 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.
>> + */
>> +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).
>> + */
>> + 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).
>> + */
>> + 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).
>> + */
>> + 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).
>> + */
>> + 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.
>> + */
>> + DRM_COLOROP_CSC_FF,
>> +
>> };
>>
>> /**
>