[Intel-gfx] [RFC 1/3] drm: Add colorspace property

[Maarten Lankhorst]

Op 01-08-18 om 16:01 schreef Shankar, Uma:
>
>> -----Original Message-----
>> From: Adam Jackson [mailto:ajax at redhat.com]
>> Sent: Wednesday, August 1, 2018 1:24 AM
>> To: Shankar, Uma <uma.shankar at intel.com>; intel-gfx at lists.freedesktop.org;
>> dri-devel at lists.freedesktop.org
>> Cc: Syrjala, Ville <ville.syrjala at intel.com>; Lankhorst, Maarten
>> <maarten.lankhorst at intel.com>
>> Subject: Re: [RFC 1/3] drm: Add colorspace property
>>
>> On Tue, 2018-07-24 at 21:15 +0530, Uma Shankar wrote:
>>
>>> --- a/include/uapi/drm/drm_mode.h
>>> +++ b/include/uapi/drm/drm_mode.h
>>> @@ -209,6 +209,17 @@
>>>  #define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
>>>  #define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
>>>
>>> +enum extended_colorimetry {
>>> +	EXTENDED_COLORIMETRY_XV_YCC_601 = 0,
>>> +	EXTENDED_COLORIMETRY_XV_YCC_709,
>>> +	EXTENDED_COLORIMETRY_S_YCC_601,
>>> +	EXTENDED_COLORIMETRY_ADOBE_YCC_601,
>>> +	EXTENDED_COLORIMETRY_ADOBE_RGB,
>>> +	EXTENDED_COLORIMETRY_BT2020_RGB,
>>> +	EXTENDED_COLORIMETRY_BT2020_YCC,
>>> +	EXTENDED_COLORIMETRY_BT2020_CYCC,
>>> +};
>> This doesn't give any way to distinguish "not set" from BT.601, which I'm not sure
>> I like.
> This enum gives a list of all possible colorspace which can be set on the sink device.
> The compositors/userspace can choose one of them, based on the capabilities of sink
> as well as based on rendering/blending policies which are designed to take advantage
> of hardware resources available.
>
> If you suggest to add something like NO_COLORSPACE_SET = -1, I can add that to this
> enum list.
I would add a default, but not sure I would introduce a new enum.
hdmi_extended_colorimetry is already available.

I would argue to keep it as it is, Or really keep it as it is, since we have to set it
to *something* when sending the infoframe. And we currently send this value.

Hm maybe for DP it could be useful, if it's optional to put it in the SDP packet there.

>> Is this enum simply built to match the values you're injecting into the InfoFrame?
> Yes they are as per HDMI SPEC defined Infoframe. This can directly be assigned to create
> the equivalent infoframe.
Yes, in fact I would take the one from hdmi.h instead of redefining it. :)
We already do for the other props.
>> Would we need a different enum for DisplayPort?
> DP will define a SDP packet to pass this info. From userspace, we can still pass this
> enum value, as part of SDP packet creation DP driver can pick equivalent value as per DP spec
> (which may be different from this enum value). But driver will still know as to what
> colorspace is requested by userspace. 
Sounds good. :)

~Maarten