>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Maarten Lankhorst >Sent: Wednesday, January 30, 2019 3:27 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; >dri-devel@xxxxxxxxxxxxxxxxxxxxx >Cc: emil.l.velikov@xxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst, >Maarten <maarten.lankhorst@xxxxxxxxx> >Subject: Re: [v9 3/3] drm/i915: Attach colorspace property and enable modeset > >Op 29-01-2019 om 19:50 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. >> >> 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: Merged the DP handling along with platform colorspace handling as >> per Shashank's comments. >> >> v6: Addressed Maarten's review comment and limited this currently to >> non lspcon based devices. >> >> v7: Reverted to old design of exposing all colorspaces to userspace as >> per Ville's review comment >> >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> drivers/gpu/drm/i915/intel_connector.c | 8 ++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_hdmi.c | 25 +++++++++++++++++++++++++ >> 4 files changed, 35 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 || > >I only care if there's a new version, but can you use xxx_conn_state->base ? > >Can fixup if patch is ready otherwise. Sure, I can fix this. Will send out the next version along with your RB and Jani's ack. Thanks for the review. Regards, Uma Shankar > >Otherwise series looks good, so > >Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> for the >whole series. :) > >> 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..ac2aed7 100644 >> --- a/drivers/gpu/drm/i915/intel_connector.c >> +++ b/drivers/gpu/drm/i915/intel_connector.c >> @@ -265,3 +265,11 @@ 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) { >> + if (!drm_mode_create_colorspace_property(connector)) >> + 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..5c5009d 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,17 @@ 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; >> > > >_______________________________________________ >dri-devel mailing list >dri-devel@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
