Re: [PATCH V9 00/43] Color Pipeline API w/ VKMS

[Harry Wentland <harry.wentland@amd.com>]

On 2025-05-22 03:57, Pekka Paalanen wrote:
> On Wed, 21 May 2025 15:48:00 -0400
> Harry Wentland <harry.wentland@amd.com> wrote:
> 
>> On 2025-05-17 07:51, Xaver Hugl wrote:
>>> Am Do., 15. Mai 2025 um 22:00 Uhr schrieb Leandro Ribeiro
>>> <leandro.ribeiro@collabora.com>:  
>>>>
>>>>
>>>>
>>>> On 5/15/25 15:39, Daniel Stone wrote:  
>>>>> Hi,
>>>>>
>>>>> On Thu, 15 May 2025 at 19:02, Harry Wentland <harry.wentland@amd.com> wrote:  
>>>>>> On 2025-05-15 13:19, Daniel Stone wrote:  
>>>>>>> Yeah, the Weston patches are marching on. We've still been doing a
>>>>>>> little bit of cleanup and prep work in the background to land them,
>>>>>>> but we also can't land them until the kernel lands. None of that work
>>>>>>> is material to the uAPI though: as said previously, the uAPI looks
>>>>>>> completely solid and it's something we can definitely beneficially use
>>>>>>> in Weston. (Even if we do need the obvious follow-ons for
>>>>>>> post-blending as well ...)  
>>>>>>
>>>>>> We can't merge kernel uAPI without canonical userspace that uses it.
>>>>>> To move forward we'll need a userspace to at least publish a branch
>>>>>> that shows the use of this new uAPI.
>>>>>>
>>>>>> Do you have a public branch for the Weston work for this?  
>>>>>
>>>>> Yeah, https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1702
>>>>> has been around for a little while now. There are some driver bugs
>>>>> that Leandro commented on, but they don't seem material to the uAPI as
>>>>> such?  
>>>>
>>>> Hello,
>>>>
>>>> Yes, there's nothing related to the API that is blocking us. It seemed
>>>> very flexible and easy to use. The bugs that I've spotted are probably
>>>> internal to AMD driver.
>>>>
>>>> I'd say that the Weston patches are converging nicely, we just need time
>>>> to get them fully reviewed. We had a few preparation MR's to land
>>>> before !1702, and now there's only one left (!1617).  
>>>
>>> I also updated the KWin MR
>>> (https://invent.kde.org/plasma/kwin/-/merge_requests/6600), it can now
>>> use all the available properties and I think it's ready. I found two
>>> issues with the kernel patches though:
>>> - while attempting to set COLOR_ENCODING and COLOR_RANGE results in
>>> the atomic commit being rejected, the existing values still get
>>> applied if you use YCbCr-type buffers. I would've expected the color
>>> pipeline to operate on the YUV values in that case - and leave
>>> conversion to RGB up to the compositor adding the relevant matrix to
>>> the pipeline  
>>
>> AMD HW always operates on RGB values, so there'll always be an
>> implicit conversion of YCbCr-type buffers to RGB.
> 
> Surely the transformation from YCbCr to RGB can be one that maps each
> of Y, Cb and Cr channels to G, B and R ranges [0.0, 1.0]?
> 

Probably, but that's not a HW config that's been explored, while the
one performing a color-space conversion has been used for years.

> Of course, that's not identity transformation (Cb and Cr have domain
> [-0.5, 0.5] or something like that) and R, B may not handle negative
> values.
> 
> But if R and B channels can be negative too, then identity mapping
> would be usable.
> 
> There will need to be a definition of how YCbCr enters the color
> pipeline: the order of the channels and their domains. The rest can be
> just more colorops.
> 
> We should be careful to allow limited range YUV formats to be
> interpreted as full range to avoid clipping the sub-black and
> super-white signal ranges. The color pipeline can be configured to deal
> with those ranges as necessary.
> 
>> What we should
>> do is reject YCbCr-type buffers with the color pipeline until we
>> implement support for COLOR_ENCODING and COLOR_RANGE as a new
>> CSC colorop.
> 
> Rejecting is fine, but is implementing COLOR_ENCODING and COLOR_RANGE
> really a good idea instead of making the color pipelines handle them?
> 
> Wasn't the original plan to hide all such legacy plane properties when
> userspace signals color pipeline support?
> 

It still is. But handling the color-space conversion via a new
colorop with two properties: COLOR_ENCODING and COLOR_RANGE
seemed to be the most straight-forward way of dealing with this.

I'm open to other suggestions and we can explore them.

Harry

> 
> Thanks,
> pq