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

[Pekka Paalanen <pekka.paalanen@collabora.com>]

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

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).
> > 
> > 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 --]