On 2026-03-31 13:31, Borah, Chaitanya Kumar wrote:
> 
> 
> On 3/25/2026 10:28 PM, Harry Wentland wrote:
>>
>>
>> On 2026-03-24 13:01, Borah, Chaitanya Kumar wrote:
>>> Hello Harry,
>>>
>>> Few thoughts on the series from Intel perspective.
>>>
>>> On 3/17/2026 9:33 PM, Harry Wentland wrote:
>>>> When we merged the drm_plane color pipeline API the major gap
>>>> that existed was the lack of a color-space conversion colorop.
>>>> We deprecated any legacy drm_plane color properties, which
>>>> means that the COLOR_RANGE and COLOR_ENCODING properties can't
>>>> be used with the COLOR_PIPELINE property on a drm_plane. In
>>>> practice this means that we can't use a COLOR_PIPELINE on
>>>> YCbCr encoded framebuffers.
>>>>
>>>> This patchset adds a CSC colorop with the COLOR_RANGE and
>>>> COLOR_ENCODING properties and implements support in VKMS and
>>>> amdgpu.
>>>>
>>>
>>> AFAIU, while COLOR_RANGE and COLOR_ENCODING were plane properties, they 
>>> were more representative of how the framebuffer provided to the plane 
>>> should be interpreted, rather than selecting a transformation. So using 
>>> them to define CSC behavior is bit of a semantic drift.
>>>
>>
>> I guess CSC is misleading for this colorop. YUV conversion would
>> describe it better.
>>
>>> From, Intel's HW perspective we could re-use this CSC colorop but it would 
>>> be
>>> preferable to introduce new enums like "YCbCr709 to RGB", "YCbCr601 to RGB" 
>>> as discussed in [1]. That way we can still represent the "RGB709 to 
>>> RGB2020" conversion that Intel's fixed matrix CSC supports (instead of 
>>> inventing a new colorop). We might need to change the name of colorop to 
>>> something like Fixed Matrix to be inclusive of both YCbCr to RGB conversion 
>>> and Primary conversion.
>>>
>>
>> At the core your CSC FF colorop and the one I'm trying to introduce are
>> both backed by a fixed matrix. We could even express the range via the
>> CSC FF colorop by introducing full and limited matrix variants for
>> YCbCr to RGB conversion, like "YCbCr709 limited to RGB" (which is
>> probably the norm for SDR/sRGB) and "YCbCr709 full to RGB" for full
>> range YCbCr content.
>>
>> For BT.601, BT.709, BT.2020 and full and limited range that would give
>> us 6 enum entries, which is quite manageable.
>>
>> Intel would then only advertise the full-range enums, plus the RGB-to-RGB
>> CSC enums (like RGB709 to RGB2020) while AMD would advertise full and
>> limited range YCbCr to RGB enums only, no RGB-to-RGB variants.
>>
>> If this makes sense to you I'll be happy to rework my YUV conversion
>> patches based on that. I think that'll work fine.
>>
> 
> I see you already have floated a new version of the series with this change. 
> It should work in principle, I will have a look. Thank you for the changes.
> 

Thanks. Looking forward to feedback.

>>> Regarding the range property, we could re-use the COLOR_RANGE property as 
>>> you have done. In the case of Intel, we would only expose 
>>> DRM_COLOR_YCBCR_FULL_RANGE as supported for this CSC, and use a separate 
>>> colorop to perform YUV range correction. This allows userspace to still 
>>> pass limited-range framebuffers. I am assuming here that it matters for 
>>> user-space if the conversion was done in limited or full range.
>>>
>>
>> It sounds like you'd need another colorop for range conversion. I wonder if
>> it makes sense to also use the CSC FF block for that and introduce a
>> "YUV limited to YUV full" range conversion enum. In that case naming the
>> op named_matrix, might work better, as it can express range conversion,
>> YUV conversion, and color space conversions.
>>
> 
> Even though limited to full conversion is not strictly a matrix operation, I 
> guess it can be re-presented as one.
> 

True. If you have a proposal for enabling range conversion
on Intel HW I'll be happy to have a look. At this point I
don't have a strong opinion on it.

Harry

> ==
> Chaitanya
> 
>> Harry
>>
>>> [1] 
>>> https://lore.kernel.org/dri-devel/[email protected]/
>>>
>>> ==
>>> Chaitanya
>>>
>>>> An alternate way of possibly representing this has been proposed
>>>> here:
>>>> https://patchwork.freedesktop.org/patch/709860
>>>>
>>>> This code has been tested with IGT and an experimental KWin branch.
>>>>
>>>> IGT branch:
>>>> https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/csc-colorop
>>>>
>>>> KWin branch:
>>>> https://invent.kde.org/hwentlan/kwin/-/tree/csc-3dlut
>>>>
>>>> The kernel branch containing these changes, based on drm-misc-next
>>>> can be found at:
>>>> https://gitlab.freedesktop.org/hwentland/linux/-/tree/csc-colorop
>>>>
>>>> In order to successfully use this branch you might need a few
>>>> bugfixes. The kernel tree containing those fixes plus these patches
>>>> can be found at:
>>>> https://gitlab.freedesktop.org/hwentland/linux/-/tree/csc-colorop-all
>>>>
>>>> Further background on this work can be found at:
>>>> https://hwentland.github.io/2026/03/10/plane-color-pipeline-csc-3d-lut-kwin.html
>>>>
>>>> Cc: Alex Hung <[email protected]>
>>>> Cc: Daniel Stone <[email protected]>
>>>> Cc: Chaitanya Kumar Borah <[email protected]>
>>>> Cc: Uma Shankar <[email protected]>
>>>> Cc: Louis Chauvet <[email protected]>
>>>> Cc: Melissa Wen <[email protected]>
>>>> Cc: Simon Ser <[email protected]>
>>>>
>>>> Harry Wentland (10):
>>>>     drm/colorop: Add CSC colorop type
>>>>     drm/colorop: Add CSC colorop initialization helper
>>>>     drm/atomic: Add CSC colorop state handling
>>>>     drm/vkms: Add CSC colorop support
>>>>     drm/vkms: Add atomic check and matrix handling for CSC colorop
>>>>     drm/amd/display: Implement CSC colorop support
>>>>     drm/amd/display: Use GAMCOR for first TF if CSC is used
>>>>     drm/amd/display: Check CSC colorop bypass before programming
>>>>     drm/amd/display: Check actual state during commit_tail
>>>>     drm/amd/display: Set color_space to plane_infos
>>>>
>>>>    .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  14 ++-
>>>>    .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 115 +++++++++++++++++-
>>>>    .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c |  25 +++-
>>>>    drivers/gpu/drm/drm_atomic.c                  |   6 +
>>>>    drivers/gpu/drm/drm_atomic_uapi.c             |   8 ++
>>>>    drivers/gpu/drm/drm_colorop.c                 |  91 ++++++++++++++
>>>>    drivers/gpu/drm/vkms/vkms_colorop.c           |  64 +++++++---
>>>>    drivers/gpu/drm/vkms/vkms_composer.c          |   5 +
>>>>    drivers/gpu/drm/vkms/vkms_plane.c             |  50 +++++++-
>>>>    include/drm/drm_colorop.h                     |  39 ++++++
>>>>    include/uapi/drm/drm_mode.h                   |   1 +
>>>>    11 files changed, 388 insertions(+), 30 deletions(-)
>>>>
>>>> -- 
>>>> 2.53.0
>>>>
>>>
>>
> 

Reply via email to