[v2, 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane

[Andrzej Hajda]

On 30.11.2018 15:48, Hans Verkuil wrote:
> On 11/30/18 15:29, Ville Syrjälä wrote:
>> On Fri, Nov 30, 2018 at 03:20:59PM +0100, Andrzej Hajda wrote:
>>> Hi Ville,
>>>
>>> As Christoph cannot respond till middle next week I can try to respond
>>> in his absence, as I am familiar with the subject.
>>>
>>> On 30.11.2018 14:25, Ville Syrjälä wrote:
>>>> On Fri, Nov 30, 2018 at 02:08:11PM +0100, Christoph Manszewski wrote:
>>>>> Hi,
>>>>>
>>>>> I am looking for a way to export the color encoding and range selection
>>>>> to user space. I came across those properties and am wondering, why
>>>>> they are meant only for non RGB color encodings. Would it be okay, to
>>>>> modify them and use with RGB formats as well?
>>>> What you trying to do? Input limited range RGB data and expand to full
>>>> range?
>>>
>>> For example. But there are two more general questions, which
>>> surprisingly we have not found answer for.
>>>
>>> 1. What color encoding and range drm should expect on its input RGB
>>> buffers by default?
>> RGB is just RGB. There is no encoding. 


OK, Now I see I have confused encoding with colorspace, Hans thanks for
the documentation.

As I understand drm by default should expect sRGB colorspace, also for
Y'CbCr buffers, and currently there are no standard ways to specify
buffer colorspace.

Looking at current users of drm_plane_create_color_properties for Y'CbCr
buffers it seems default range should be LIMITED.

But default encoding is different: mali, armada, nouveau use
DRM_COLOR_YCBCR_BT601, i915 uses DRM_COLOR_YCBCR_BT709 - shouldn't this
be somehow standardized?


What I still do not understand is colorspace conversion(not encoding!) -
between pipeline input (framebuffers) and output (HDMI connector for
example):

1. Since sRGB, SMPTE170M/BT.601, BT.709 colorspaces are 'almost
identical in coverage' (according to wikipedia[1]) no colorspace
conversion is performed at all.

2. It is performed somewhere by HW, just my IP documentation is not
clear about it.


Another thing which still bothers me is RGB range in case of HDMI -
CEA-861 expects than on CEA modes limited RGB range should be sent, but
the pipeline on the particular hardware I have do not perform any
conversion of input buffers. So only limited range RGB buffers are
displayed correctly. In such case property describing plane with limited
RGB would be useful.


[1]: https://en.wikipedia.org/wiki/Rec._709#Primary_chromaticities


> It's assumed to be full range 
>> because no one really uses anything else.
> For simple desktop usage that's true. When dealing with video inputs,
> this becomes much more complicated.


As I said earlier there are cases where output stream should be limited
RGB, and no conversion in pipeline - thus framebuffers also should be
'limited RGB'.


Regards

Andrzej


>
>>> 2. How userspace should inform drm that given buffer has specified
>>> non-default color encoding and range?
>>>
>>>
>>> Hopefully this patch introduces such properties but only for YCbCr
>>> formats, the question is what should be the best way to expand it to RGB
>>> formats:
>>>
>>> A. Add another enums: DRM_COLOR_RGB_BT601 and friends.
>> BT.601 specifies how to encoder RGB data as YCbCr. So without
>> YCbCr BT.601 does not mean anything. Well, the standard does
>> contain other things as well I suppose, but for the purposes
>> of the color encoding prop only that one part is relevant.
> Ah, I misunderstood the meaning of DRM_COLOR_RGB_BT601.
> This is the equivalent of V4L2_YCBCR_ENC_601, and that's indeed
> only defined for Y'CbCr. But it is often (ab)used as an alias for
> the SMPTE170M colorspace (used by SDTV).
>
> V4L2 has the following defines for colorspaces, transfer functions,
> Y'CbCr (and HSV) encodings and quantization ranges:
>
> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html
>
> Missing in this list is the color encoding (RGB vs YCbCr vs HSV) which
> is set through the pixelformat fourcc.
>
> And indeed, we don't have an RGB encoding define since RGB is just RGB :-)
>
> Regards,
>
> 	Hans
>
>>> B. Reuse current enums, but remove format information from them:
>>> DRM_COLOR_YCBCR_BT601 => DRM_COLOR_BT601.
>>>
>>>
>>> Regards
>>>
>>> Andrzej
>>>
...