>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Ville Syrjälä >Sent: Saturday, February 2, 2019 12:31 AM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; >Lankhorst, Maarten <maarten.lankhorst@xxxxxxxxx>; dri- >devel@xxxxxxxxxxxxxxxxxxxxx >Subject: Re: [Intel-gfx] [v10 3/3] drm/i915: Attach colorspace property and >enable modeset > >On Wed, Jan 30, 2019 at 06:24:26PM +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: 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 a checkpatch complaint, Addressed Maarten' review comment, >> updated the RB from Maarten and Jani's ack. >> >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> >> Acked-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> --- >> 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..a4bb906 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_conn_state->base.colorspace != >> +old_conn_state->base.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..8352d0b 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; > >Default should map to NONE. Ok, I will make it NO DATA i.e. 0 for DEFAULT. > >> + } 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; >> + } > >This code seems like something that should be in the core. > >Like said earlier maybe we could just directly map the values to these bits, and >then this should just become > >if.colorimetry = val & 0x3; >if.extended_colorimetry = (val >> 2) & 0x7; if.ace = (val >> 5) & 0xf; Ok, will modify along these lines. >Hmm, looks like hdmi.h doesn't even know about the ACE bits yet :( :) Thanks Ville for all your comments and suggestion. Regards, Uma Shankar > >> + >> 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; >> >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Ville Syrjälä >Intel >_______________________________________________ >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
