[Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable modeset

[Ville Syrjälä]

On Fri, Nov 02, 2018 at 02:18:42PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Maarten Lankhorst [mailto:maarten.lankhorst at linux.intel.com]
> >Sent: Friday, November 2, 2018 2:53 PM
> >To: Shankar, Uma <uma.shankar at intel.com>; dri-devel at lists.freedesktop.org;
> >intel-gfx at lists.freedesktop.org
> >Cc: Syrjala, Ville <ville.syrjala at intel.com>; ajax at redhat.com; Lankhorst, Maarten
> ><maarten.lankhorst at intel.com>
> >Subject: Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable
> >modeset
> >
> >Op 31-10-18 om 13:05 schreef Uma Shankar:
> >> 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.
> >>
> >> v2: Merged the changes of creating infoframe as well to this patch as
> >> per Maarten's suggestion.
> >>
> >> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_atomic.c | 1 +
> >>  drivers/gpu/drm/i915/intel_hdmi.c   | 5 +++++
> >>  2 files changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> >> b/drivers/gpu/drm/i915/intel_atomic.c
> >> index a5a2c8f..35ef70a 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -125,6 +125,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_hdmi.c
> >> b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index 129b880..8a41fb3 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -486,6 +486,8 @@ static void intel_hdmi_set_avi_infoframe(struct
> >intel_encoder *encoder,
> >>  	else
> >>  		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >>
> >> +	frame.avi.extended_colorimetry = conn_state->colorspace;
> >> +
> >>  	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
> >>  					   crtc_state->limited_color_range ?
> >>
> >HDMI_QUANTIZATION_RANGE_LIMITED :
> >> @@ -2125,6 +2127,9 @@ static void intel_hdmi_destroy(struct drm_connector
> >*connector)
> >>  	intel_attach_broadcast_rgb_property(connector);
> >>  	intel_attach_aspect_ratio_property(connector);
> >>  	drm_connector_attach_content_type_property(connector);
> >> +	drm_object_attach_property(&connector->base,
> >> +			connector->dev->mode_config.colorspace_property,
> >> +			COLORIMETRY_ITU_709);
> >Just put 0 here..
> >If you want to init the default colorspace, put it in the first patch.
> 
> Ok, will update this.
> 
> >We should perhaps hide color spaces that are not supported on HDMI?
> 
> Currently the supported colorspaces will be picked from edid by userspace and
> they should use the current property interface to set the one which is supported.
> Even on HDMI, some connectors may not support certain colorspace, so keeping
> it on userspace to set the one which is supported by the particular connector. Hope
> this approach is fine ?

I think we want to trim the list to whatever the infoframe vs. MSA/VSC
SDP can carry. So HDMI will have one list, DP another. And I guess for
lspcon we want to go with the HDMI definition since we populate the
infoframe by hand.

-- 
Ville Syrjälä
Intel