Op 01-08-18 om 16:01 schreef Shankar, Uma:
-----Original Message----- From: Adam Jackson [mailto:ajax@redhat.com] Sent: Wednesday, August 1, 2018 1:24 AM To: Shankar, Uma uma.shankar@intel.com; intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Syrjala, Ville ville.syrjala@intel.com; Lankhorst, Maarten maarten.lankhorst@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