[v8 2/2] drm/i915: Attach colorspace property and enable modeset
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Jan 29 15:43:56 UTC 2019
On Tue, Jan 29, 2019 at 09:22:56PM +0530, Uma Shankar wrote:
> This patch attaches the colorspace connector property to the
> hdmi connector. Based on colorspace change, modeset will be
> triggered to switch to new colorspace.
>
> Based on colorspace property value create an infoframe
> with appropriate colorspace. This can be used to send an
> infoframe packet with proper colorspace value set which
> will help to enable wider color gamut like BT2020 on sink.
>
> This patch attaches and enables HDMI colorspace, DP will be
> taken care separately.
>
> v2: Merged the changes of creating infoframe as well to this
> patch as per Maarten's suggestion.
>
> v3: Addressed review comments from Shashank. Separated HDMI
> and DP colorspaces as suggested by Ville and Maarten.
>
> 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. Handle the default case properly.
>
> v5: Added Platform specific colorspace enums and called the
> property creation helper using the same.
>
> v6: Addressed Shashank's review comments.
>
> v7: Rebase
>
> v8: Addressed Maarten's review comments.
>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> Reviewed-by: Shashank Sharma <shashank.sharma at intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 1 +
> drivers/gpu/drm/i915/intel_connector.c | 63 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++
> 4 files changed, 89 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 16263ad..76b7114 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
> */
> if (new_conn_state->force_audio != old_conn_state->force_audio ||
> new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
> + new_state->colorspace != old_state->colorspace ||
> new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
> new_conn_state->base.content_type != old_conn_state->base.content_type ||
> new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
> diff --git a/drivers/gpu/drm/i915/intel_connector.c b/drivers/gpu/drm/i915/intel_connector.c
> index ee16758..9a12d5e 100644
> --- a/drivers/gpu/drm/i915/intel_connector.c
> +++ b/drivers/gpu/drm/i915/intel_connector.c
> @@ -30,6 +30,48 @@
> #include "intel_drv.h"
> #include "i915_drv.h"
>
> +static const struct drm_prop_enum_list gen10_hdmi_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" },
> + /* 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-1/Amendment 1 */
> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 [33] */
> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 */
> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> +};
> +
> +static const struct drm_prop_enum_list legacy_hdmi_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" },
> + /* 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-1/Amendment 1 */
> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 [33] */
> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 */
> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
> +};
Why are we duplicating these in the driver code? And why do we have
different sets for gen10 and older?
> +
> int intel_connector_init(struct intel_connector *connector)
> {
> struct intel_digital_connector_state *conn_state;
> @@ -265,3 +307,24 @@ int intel_ddc_get_modes(struct drm_connector *connector,
> connector->dev->mode_config.aspect_ratio_property,
> DRM_MODE_PICTURE_ASPECT_NONE);
> }
> +
> +void
> +intel_attach_colorspace_property(struct drm_connector *connector)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (INTEL_GEN(dev_priv) >= 10) {
> + if (!drm_mode_create_colorspace_property(connector,
> + gen10_hdmi_colorspaces,
> + ARRAY_SIZE(gen10_hdmi_colorspaces)))
> + drm_object_attach_property(&connector->base,
> + connector->colorspace_property, 0);
> + } else {
> + if (!drm_mode_create_colorspace_property(connector,
> + legacy_hdmi_colorspaces,
> + ARRAY_SIZE(legacy_hdmi_colorspaces)))
> + drm_object_attach_property(&connector->base,
> + connector->colorspace_property, 0);
> + }
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 85b913e..5178a9a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1783,6 +1783,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
> void intel_attach_force_audio_property(struct drm_connector *connector);
> void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> +void intel_attach_colorspace_property(struct drm_connector *connector);
>
> /* intel_csr.c */
> void intel_csr_ucode_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 97a98e1..9ed00e3 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -498,6 +498,24 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
> else
> frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>
> + if (conn_state->colorspace == DRM_MODE_COLORIMETRY_DEFAULT) {
> + /* Set ITU 709 as default for HDMI */
> + frame.avi.colorimetry = DRM_MODE_COLORIMETRY_ITU_709;
> + } else if (conn_state->colorspace < DRM_MODE_COLORIMETRY_XV_YCC_601) {
> + frame.avi.colorimetry = conn_state->colorspace;
> + } else {
> + frame.avi.colorimetry = HDMI_COLORIMETRY_EXTENDED;
> + /*
> + * Starting from extended list where COLORIMETRY_XV_YCC_601
> + * is the first extended mode and its value is 0 as per HDMI
> + * specification.
> + * TODO: This needs to be extended for LSPCON implementation
> + * as well. Will be implemented separately.
> + */
> + frame.avi.extended_colorimetry = conn_state->colorspace -
> + DRM_MODE_COLORIMETRY_XV_YCC_601;
> + }
> +
> drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> conn_state->connector,
> adjusted_mode,
> @@ -2143,10 +2161,16 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
> intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct intel_digital_port *intel_dig_port =
> + hdmi_to_dig_port(intel_hdmi);
>
> intel_attach_force_audio_property(connector);
> intel_attach_broadcast_rgb_property(connector);
> intel_attach_aspect_ratio_property(connector);
> + /* Attach Colorspace property for Non LSPCON based device */
> + if (!intel_dig_port->lspcon.active)
> + intel_attach_colorspace_property(connector);
> +
> drm_connector_attach_content_type_property(connector);
> connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
More information about the dri-devel
mailing list