public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Harry Wentland <harry.wentland@amd.com>
To: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>,
	dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org, louis.chauvet@bootlin.com,
	mwen@igalia.com, contact@emersion.fr, alex.hung@amd.com,
	daniels@collabora.com, uma.shankar@intel.com,
	maarten.lankhorst@intel.com, pranay.samala@intel.com,
	swati2.sharma@intel.com
Subject: Re: [PATCH 01/10] drm/colorop: Add DRM_COLOROP_CSC_FF
Date: Mon, 30 Mar 2026 11:37:44 -0400	[thread overview]
Message-ID: <0d438de9-cb79-4db4-8f33-dfef47a9a7bd@amd.com> (raw)
In-Reply-To: <cd9e1aa2-85b8-4add-a3b9-ba7acbd29f60@amd.com>



On 2026-03-16 13:59, Harry Wentland wrote:
> 
> 
> On 2026-03-16 12:03, Pekka Paalanen wrote:
>> 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.
>>
> 
> If I understand you correctly you're saying we can have a single
> colorop type to represent either type. A client of the API needs
> to understand what it's doing with the colorop and can use it
> as either a YCbCr conversion matrix in the case of YCbCr-to-RGB
> conversion, or an NPM for conversion of linear, normalized data
> from one set of primaries to another.
> 
> Is that understanding correct?
> 
>>> 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.
>>
> 
> This could be expressed in a single matrix, right?
> 
> I intend to send out my patches as an RFC either way, but I think I
> could just as well work with the CSC_FF colorop. I'll have a look at
> basing my work on this.
> 

I sent out a new version of my series, reworked to use the CSC_FF
colorop.

https://lore.kernel.org/dri-devel/20260330153451.99472-1-harry.wentland@amd.com/

Harry

> Harry
> 
>> 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.  
>>>
>>
> 


  parent reply	other threads:[~2026-03-30 15:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-06 16:52 [PATCH 00/10] drm/i915/color: Enable SDR plane color pipeline Chaitanya Kumar Borah
2026-03-06 16:52 ` [PATCH 01/10] drm/colorop: Add DRM_COLOROP_CSC_FF Chaitanya Kumar Borah
2026-03-10 14:32   ` Pekka Paalanen
2026-03-16  7:16     ` Borah, Chaitanya Kumar
2026-03-16  8:57       ` Pekka Paalanen
2026-03-16 10:34         ` Borah, Chaitanya Kumar
2026-03-16 11:53           ` Pekka Paalanen
2026-03-16 14:36             ` Harry Wentland
2026-03-16 16:03               ` Pekka Paalanen
2026-03-16 17:59                 ` Harry Wentland
2026-03-17  8:23                   ` Pekka Paalanen
2026-03-30 15:37                   ` Harry Wentland [this message]
2026-03-11  8:49   ` Jani Nikula
2026-03-16 20:45   ` Harry Wentland
2026-03-17 12:29     ` Borah, Chaitanya Kumar
2026-03-17 14:09       ` Pekka Paalanen
2026-03-06 16:52 ` [PATCH 02/10] drm/i915/color: Add CSC on SDR plane color pipeline Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 03/10] drm/i915/color: Program fixed-function CSC on SDR planes Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 04/10] drm/i915/color: Add support for 1D LUT in " Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 05/10] drm/i915/color: Fix HDR pre-CSC LUT programming loop Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 06/10] drm/i915/color: Extract HDR pre-CSC LUT programming to helper function Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 07/10] drm/i915/color: Program Pre-CSC registers for SDR Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 08/10] drm/i915/color: Extract HDR post-CSC LUT programming to helper function Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 09/10] drm/i915/color: Program Plane Post CSC registers for SDR planes Chaitanya Kumar Borah
2026-03-06 16:53 ` [PATCH 10/10] drm/i915/color: Add color pipeline support " Chaitanya Kumar Borah
2026-03-06 18:10 ` ✓ i915.CI.BAT: success for drm/i915/color: Enable SDR plane color pipeline Patchwork
2026-03-07 21:42 ` ✗ i915.CI.Full: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0d438de9-cb79-4db4-8f33-dfef47a9a7bd@amd.com \
    --to=harry.wentland@amd.com \
    --cc=alex.hung@amd.com \
    --cc=chaitanya.kumar.borah@intel.com \
    --cc=contact@emersion.fr \
    --cc=daniels@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=louis.chauvet@bootlin.com \
    --cc=maarten.lankhorst@intel.com \
    --cc=mwen@igalia.com \
    --cc=pekka.paalanen@collabora.com \
    --cc=pranay.samala@intel.com \
    --cc=swati2.sharma@intel.com \
    --cc=uma.shankar@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox