Op 31-10-18 om 13:05 schreef Uma Shankar:
> 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.shan...@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" },
2 definitions that share the same name?
All in all, I think the enum strings need to be more descriptive and 
self-consistent.
> +     /* 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" },
sRGB
> +     { COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" },
> +     { COLORIMETRY_SCRGB, "SCRGB" },
scRGB?
> +     { COLORIMETRY_DCI_P3, "DCIP3" },
DCI-P3?
> +     { COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Proflie" },
^Typo
> +     /* FOR HD 720p+, Default Colorimetry is based on ITU-R BT.709 */
> +     { COLORIMETRY_DEFAULT, "Default: ITU_709" },
This enum together with the code below is broken.

+       COLORIMETRY_DEFAULT = COLORIMETRY_ITU_709,

I would just make it COLORIMETRY_UNSET = "Unset".

To set the default colorimetry for all drivers, just make the default value 0 
(preferred),
or add it to __drm_atomic_helper_connector_reset().

> +};
> +
>  /**
>   * 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:
> + *   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));
> +     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;


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to