Re: [v10 2/3] drm: Add DP colorspace property

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jan 30, 2019 at 06:24:25PM +0530, Uma Shankar wrote:
> This patch adds a DP 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.
> 
> v3: Split the changes to have separate colorspace property for
> DP and HDMI.
> 
> v4: Addressed Chris and Ville's review comments, and created a
> common colorspace property for DP and HDMI, filtered the list
> based on the colorspaces supported by the respective protocol
> standard.
> 
> v5: Merged the DP handling along with platform colorspace
> handling as per Shashank's comments.
> 
> v6: Reverted to old design of exposing all colorspaces to
> userspace as per Ville's review comment
> 
> v7: Fixed sparse warnings, updated the RB from Maarten and Jani's ack.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ed10dd9..b331be8 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -850,6 +850,29 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
>  	{ DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
>  };
>  
> +static const struct drm_prop_enum_list dp_colorspaces[] = {
> +	/* For Default case, driver will set the colorspace */
> +	{ DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
> +	/* Standard Definition Colorimetry based on CEA 861 */
> +	{ DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" },
> +	{ DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" },

What's with the duplicated 601/709 values? I think in the HDMI verison
you had only these ones here. Maybe we want to actually state explicitly
that they are for YCbCr, if only to be consistent with the
BT2020 values.

> +	/* Standard Definition Colorimetry based on IEC 61966-2-4 */
> +	{ DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
> +	/* High Definition Colorimetry based on IEC 61966-2-4 */
> +	{ DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
> +	/* Colorimetry based on IEC 61966-2-5 */
> +	{ DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
> +	/* DP MSA Colorimetry */
> +	{ DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" },
> +	{ DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" },
> +	{ DRM_MODE_DP_COLORIMETRY_SRGB, "sRGB" },
> +	{ DRM_MODE_DP_COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" },
> +	{ DRM_MODE_DP_COLORIMETRY_SCRGB, "scRGB" },
> +	{ DRM_MODE_DP_COLORIMETRY_DCI_P3, "DCI-P3" },
> +	{ DRM_MODE_DP_COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Profile" },

I don't think we want this last one since we don't implement anything
that could transmit the custom profile.

The MSA bits are have "CEA RGB" which we probably want to keep hidden
since it seems to be just a poorly specced limited range vs. full range
knob, and we already have a mechanism for that. The Y-only and RAW I
guess we can skip. Not sure anyone would ever have use for those.

> +};
> +
> +
>  /**
>   * DOC: standard connector properties
>   *
> @@ -1611,6 +1634,14 @@ int drm_mode_create_colorspace_property(struct drm_connector *connector)
>  						ARRAY_SIZE(hdmi_colorspaces));
>  		if (!prop)
>  			return -ENOMEM;
> +	} else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +		   connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> +		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> +						"Colorspace", dp_colorspaces,
> +						ARRAY_SIZE(dp_colorspaces));
> +
> +		if (!prop)
> +			return -ENOMEM;
>  	} else {
>  		DRM_DEBUG_KMS("Colorspace property not supported\n");
>  		return 0;
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx