>-----Original Message-----
>From: Brian Starkey [mailto:brian.star...@arm.com]
>Sent: Tuesday, November 20, 2018 9:06 PM
>To: Shankar, Uma <uma.shan...@intel.com>; Syrjala, Ville
><ville.syrj...@intel.com>; Lankhorst, Maarten <maarten.lankho...@intel.com>;
>emil.l.veli...@gmail.com; dri-devel@lists.freedesktop.org
>Cc: nd <n...@arm.com>
>Subject: Re: [v2 1/2] drm: Add colorspace property
>
>Hi Uma,
>
>On Wed, Oct 31, 2018 at 05:35:45PM +0530, Uma Shankar wrote:
>>This patch adds a colorspace property enabling userspace to switch to
>>various supported colorspaces.
>>This will help enable BT2020 along with other colorspaces.
>>
>>v2: Addressed Maarten and Ville's review comments. Enhanced the
>>colorspace enum to incorporate both HDMI and DP supported colorspaces.
>>Also, added a default option for colorspace.
>>
>>Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>>---
>> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++
>> drivers/gpu/drm/drm_connector.c | 44
>+++++++++++++++++++++++++++++++++++++++
>> include/drm/drm_connector.h | 7 +++++++
>> include/drm/drm_mode_config.h | 6 ++++++
>> include/uapi/drm/drm_mode.h | 24 +++++++++++++++++++++
>> 5 files changed, 85 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>>b/drivers/gpu/drm/drm_atomic_uapi.c
>>index d5b7f31..9e1d46b 100644
>>--- a/drivers/gpu/drm/drm_atomic_uapi.c
>>+++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>@@ -721,6 +721,8 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>> state->picture_aspect_ratio = val;
>> } else if (property == config->content_type_property) {
>> state->content_type = val;
>>+ } else if (property == config->colorspace_property) {
>>+ state->colorspace = val;
>> } else if (property == connector->scaling_mode_property) {
>> state->scaling_mode = val;
>> } else if (property == connector->content_protection_property) { @@
>>-795,6 +797,8 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>> *val = state->picture_aspect_ratio;
>> } else if (property == config->content_type_property) {
>> *val = state->content_type;
>>+ } else if (property == config->colorspace_property) {
>>+ *val = state->colorspace;
>> } else if (property == connector->scaling_mode_property) {
>> *val = state->scaling_mode;
>> } else if (property == connector->content_protection_property) { diff
>>--git a/drivers/gpu/drm/drm_connector.c
>>b/drivers/gpu/drm/drm_connector.c index aa18b1d..5ad52a0 100644
>>--- a/drivers/gpu/drm/drm_connector.c
>>+++ b/drivers/gpu/drm/drm_connector.c
>>@@ -826,6 +826,38 @@ int drm_display_info_set_bus_formats(struct
>>drm_display_info *info, };
>>DRM_ENUM_NAME_FN(drm_get_content_protection_name,
>drm_cp_enum_list)
>>
>>+static const struct drm_prop_enum_list colorspace[] = {
>>+ /* Standard Definition Colorimetry based on CEA 861 */
>>+ { COLORIMETRY_ITU_601, "601" },
>>+ { COLORIMETRY_ITU_709, "709" },
>>+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
>>+ { COLORIMETRY_XV_YCC_601, "601" },
>>+ /* High Definition Colorimetry based on IEC 61966-2-4 */
>>+ { COLORIMETRY_XV_YCC_709, "709" },
>>+ /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
>>+ { COLORIMETRY_S_YCC_601, "s601" },
>>+ /* Colorimetry based on IEC 61966-2-5 [33] */
>>+ { COLORIMETRY_ADOBE_YCC_601, "adobe601" },
>>+ /* Colorimetry based on IEC 61966-2-5 */
>>+ { COLORIMETRY_ADOBE_RGB, "adobe_rgb" },
>>+ /* Colorimetry based on ITU-R BT.2020 */
>>+ { COLORIMETRY_BT2020_RGB, "BT2020_rgb" },
>>+ /* Colorimetry based on ITU-R BT.2020 */
>>+ { COLORIMETRY_BT2020_YCC, "BT2020_ycc" },
>>+ /* Colorimetry based on ITU-R BT.2020 */
>>+ { COLORIMETRY_BT2020_CYCC, "BT2020_cycc" },
>>+ /* DP MSA Colorimetry */
>>+ { COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" },
>>+ { COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" },
>>+ { COLORIMETRY_SRGB, "SRGB" },
>>+ { COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" },
>>+ { COLORIMETRY_SCRGB, "SCRGB" },
>>+ { COLORIMETRY_DCI_P3, "DCIP3" },
>>+ { COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Proflie" },
>>+ /* FOR HD 720p+, Default Colorimetry is based on ITU-R BT.709 */
>>+ { COLORIMETRY_DEFAULT, "Default: ITU_709" }, };
>>+
>> /**
>> * DOC: standard connector properties
>> *
>>@@ -972,6 +1004,12 @@ int drm_display_info_set_bus_formats(struct
>drm_display_info *info,
>> * can also expose this property to external outputs, in which case they
>> * must support "None", which should be the default (since external screens
>> * have a built-in scaler).
>>+ *
>>+ * colorspace:
>
>Is it "colorspace" or "Colorspace"? The code says capital-C, the doc (and
>whatever little convention there is) says lower-case.
I will update this. It should be "Colorspace".
>>+ * This property helps select a suitable colorspace based on the sink
>>+ * capability. Modern sink devices support wider gamut like BT2020.
>>+ * This helps switch to BT2020 mode if the BT2020 encoded video stream
>>+ * is being played by the user, same for any other colorspace.
>> */
>>
>> int drm_connector_create_standard_properties(struct drm_device *dev)
>>@@ -1020,6 +1058,12 @@ int
>drm_connector_create_standard_properties(struct drm_device *dev)
>> return -ENOMEM;
>> dev->mode_config.non_desktop_property = prop;
>>
>>+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
>"Colorspace",
>>+ colorspace, ARRAY_SIZE(colorspace));
>
>Similar to what others have said: I'm not sure it's a great idea to expose the
>full
>set by default. At a minimum, actually applying this property requires some
>control over the HDMI/DP transmitter doesn't it? And I believe they vary in
>capabilities?
>
>Further, I'd expect some of them to require more extensive changes further
>through the pipe (e.g. scRGB), though I admit I don't know much about how Sinks
>handle this stuff.
I am planning to separate out HDMI and DP as separate properties (same name for
userspace), but with connector specific colorspaces in the list. Will float the
v4 soon, please
have a look and share your feedback.
Regards,
Uma Shankar
>Do you have a subset which you have a real user and use-case for which we might
>settle on before throwing them all in and making it UAPI?
>
>Cheers,
>-Brian
>
>
>>+ if (!prop)
>>+ return -ENOMEM;
>>+ dev->mode_config.colorspace_property = prop;
>>+
>> return 0;
>> }
>>
>>diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>>index dd0552c..b7f5373 100644
>>--- a/include/drm/drm_connector.h
>>+++ b/include/drm/drm_connector.h
>>@@ -497,6 +497,13 @@ struct drm_connector_state {
>> unsigned int content_protection;
>>
>> /**
>>+ * @colorspace: Connector property to request colorspace
>>+ * change. This is most commonly used to switch to wider color
>>+ * gamuts like BT2020.
>>+ */
>>+ enum encoder_colorimetry colorspace;
>>+
>>+ /**
>> * @writeback_job: Writeback job for writeback connectors
>> *
>> * Holds the framebuffer and out-fence for a writeback connector. As
>>diff --git a/include/drm/drm_mode_config.h
>>b/include/drm/drm_mode_config.h index d643d26..a6eb0aa 100644
>>--- a/include/drm/drm_mode_config.h
>>+++ b/include/drm/drm_mode_config.h
>>@@ -863,6 +863,12 @@ struct drm_mode_config {
>> uint32_t cursor_width, cursor_height;
>>
>> /**
>>+ * @colorspace_property: Connector property to set the suitable
>>+ * colorspace supported by the sink.
>>+ */
>>+ struct drm_property *colorspace_property;
>>+
>>+ /**
>> * @suspend_state:
>> *
>> * Atomic state when suspended.
>>diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>index d3e0fe3..831cc77 100644
>>--- a/include/uapi/drm/drm_mode.h
>>+++ b/include/uapi/drm/drm_mode.h
>>@@ -210,6 +210,30 @@
>> #define DRM_MODE_CONTENT_PROTECTION_DESIRED 1
>> #define DRM_MODE_CONTENT_PROTECTION_ENABLED 2
>>
>>+enum encoder_colorimetry {
>>+ /* CEA 861 Normal Colorimetry options */
>>+ COLORIMETRY_ITU_601 = 0,
>>+ COLORIMETRY_ITU_709,
>>+ /* CEA 861 Extended Colorimetry Options */
>>+ COLORIMETRY_XV_YCC_601,
>>+ COLORIMETRY_XV_YCC_709,
>>+ COLORIMETRY_S_YCC_601,
>>+ COLORIMETRY_ADOBE_YCC_601,
>>+ COLORIMETRY_ADOBE_RGB,
>>+ COLORIMETRY_BT2020_RGB,
>>+ COLORIMETRY_BT2020_YCC,
>>+ COLORIMETRY_BT2020_CYCC,
>>+ /* DP MSA Colorimetry Options */
>>+ COLORIMETRY_Y_CBCR_ITU_601,
>>+ COLORIMETRY_Y_CBCR_ITU_709,
>>+ COLORIMETRY_SRGB,
>>+ COLORIMETRY_RGB_WIDE_GAMUT,
>>+ COLORIMETRY_SCRGB,
>>+ COLORIMETRY_DCI_P3,
>>+ COLORIMETRY_CUSTOM_COLOR_PROFILE,
>>+ COLORIMETRY_DEFAULT = COLORIMETRY_ITU_709, };
>>+
>> struct drm_mode_modeinfo {
>> __u32 clock;
>> __u16 hdisplay;
>>--
>>1.9.1
>>
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
