>-----Original Message----- >From: Brian Starkey [mailto:Brian.Starkey@xxxxxxx] >Sent: Tuesday, November 20, 2018 9:06 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx>; Syrjala, Ville ><ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten <maarten.lankhorst@xxxxxxxxx>; >emil.l.velikov@xxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx >Cc: nd <nd@xxxxxxx> >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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
