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.
>+ * 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.
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
