On Fri, Nov 02, 2018 at 04:44:10PM +0100, Maarten Lankhorst wrote: > Op 02-11-18 om 16:41 schreef Ville Syrjälä: > > On Fri, Nov 02, 2018 at 02:18:42PM +0000, Shankar, Uma wrote: > >> > >>> -----Original Message----- > >>> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > >>> Sent: Friday, November 2, 2018 2:53 PM > >>> To: Shankar, Uma <uma.shankar@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > >>> intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >>> Cc: Syrjala, Ville <ville.syrjala@xxxxxxxxx>; ajax@xxxxxxxxxx; Lankhorst, Maarten > >>> <maarten.lankhorst@xxxxxxxxx> > >>> Subject: Re: [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@xxxxxxxxx> > >>>> --- > >>>> 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. > > > What about passive DP to HDMI convertors? By passive you mean DP++? Those are HDMI for us. External DP->HDMI protocol converters are perhaps more tricky, but since we treat those as pure DP now and don't send any infoframes I think we should do the same when it comes to the exposed valeus for the property. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
