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

Pekka Paalanen
2026-03-16T16:03:55Z

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

On Mon, 16 Mar 2026 10:36:44 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2026-03-16 07:53, Pekka Paalanen wrote:
> > On Mon, 16 Mar 2026 16:04:32 +0530
> > "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com> wrote:
> >
> >> On 3/16/2026 2:27 PM, Pekka Paalanen wrote:
> >>> On Mon, 16 Mar 2026 12:46:39 +0530
> >>> "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com> 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 <chaitanya.kumar.borah@intel.com> 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>
> >
> > ...
> >
> >>>>>> 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).
> >>>
>
> Would it make sense to treat these as two separate things in terms
> of colorops?

Hi Harry,

no, if your hardware not care. There are no semantics for the numbers
in the UAPI, it's just whatever numbers, and mathematical operations on
them.

I suspect that your hardware does care, though, and actually has
separate hardware elements for the YCbCr conversion and the colorspace
conversion matrix. After all, one has to be able to put a LUT or a
curve between the two to make sense.

IOW, two different colorops, yes. But different colorop types? Maybe
that depends on whether they would have the same colorop properties or
not.

> I have done some work on a CSC colorop and intend to send out the
> patches in the next couple of days.
>
> https://gitlab.freedesktop.org/hwentland/linux/-/commits/csc-colorop
>
> It follows the drm_plane's COLOR_RANGE and COLOR_ENCODING semantic
> and is only intended for YCbCr-to-RGB conversion, like the original
> properties on the plane.
>
> For the colorspace conversion within RGB (e.g., BT709 to BT2020)
> it might make sense then to have its own colorop if HW works on
> pre-defined transformations, or use the CTM 3x3 or 3x4 matrix ops
> if HW provides a flexible matrix.
>
> We might need to think about naming, since colorspace conversion (CSC)
> right now seems to refer to both YCbCr conversion and primaries
> conversion.

Indeed.

YCbCr conversion is usually a matrix operation. H.273 calls it
MatrixCoefficients, but it also lists cases where you need the EOTF in
the mix. I could go with "YCbCr coefficients". The pure matrix forms
are used on electrical pixel values.

The color space conversion matrices that are based on (Normalized)
Primary Matrices (NPM) must be used on optical pixel values. NPM is the
matrix that converts optical RGB values to CIE 1931 XYZ. For an
RGB-to-RGB conversion you need one NPM and another inverse NPM chained.

I guess using CSC for the latter is not obvious enough because it has
been used for the former (color model conversion) as well?

How about "color-primary conversion"?

I stole it from
https://onlinelibrary.wiley.com/doi/pdf/10.1002/9780470994375.app8

Thanks,
pq

> >>> 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?
> >
> > Hi,
> >
> > the resultant RGB may or may not follow BT.709 primaries, and knowing
> > the primaries is important for further processing in general.
> >
> > YCbCr<->RGB conversion does not change the primaries. What went in,
> > will come out.
> >
> >>>> 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.
> >
> > Precisely.
> >
> >
> > Thanks,
> > pq
> >
> >>>>> 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.
>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]