On Fri, Feb 08, 2019 at 11:54: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: 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. > > v8: Moved colorspace AVI Infoframe programming to drm core and > removed from driver as per Ville's suggestion. > > v9: Added a check to allow only RGB colorspaces to be set in > infoframe through the colorspace property. Since there is no output > csc property to control planar formats and it will be added later. > Changes for RGB->YUV conversion inside driver without userspace > knowledge is still supported. This is as per Ville's suggestion. > > v10: Fixed an error in if check for rgb colorspace. > > 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 | 24 ++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_connector.c | 8 ++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++++++++++ > 4 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 7cf9290..ba60d51 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -102,6 +102,20 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, > return -EINVAL; > } > > +static inline bool check_colorspace_is_rgb(u32 colorspace) > +{ > + if (colorspace | (DRM_MODE_COLORIMETRY_OPRGB | > + DRM_MODE_COLORIMETRY_BT2020_RGB | > + DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65 | > + DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER | > + DRM_MODE_DP_COLORIMETRY_SRGB | > + DRM_MODE_DP_COLORIMETRY_RGB_WIDE_GAMUT | > + DRM_MODE_DP_COLORIMETRY_SCRGB)) > + return true; That's the same as bool func(...) { return true; } > + > + return false; > +} > + > int intel_digital_connector_atomic_check(struct drm_connector *conn, > struct drm_connector_state *new_state) > { > @@ -118,6 +132,15 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn, > if (!new_state->crtc) > return 0; > > + /* > + * Reject any planar formats, as currently not enabled. > + * ToDo: Add a output CSC property interface to control planar > + * formats. > + */ > + if ((new_conn_state->base.colorspace != DRM_MODE_COLORIMETRY_DEFAULT) || > + !check_colorspace_is_rgb(new_conn_state->base.colorspace)) > + return 0; Not sure what this stuff has to do with planar vs. not. Either way, the code doesn't make sense. > + > crtc_state = drm_atomic_get_new_crtc_state(new_state->state, new_state->crtc); > > /* > @@ -126,6 +149,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 5eb0b66..4af574f 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1803,6 +1803,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 f125a62..765718b 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -498,6 +498,8 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder, > else > frame.avi.colorspace = HDMI_COLORSPACE_RGB; > > + drm_hdmi_avi_infoframe_colorspace(&frame.avi, conn_state); > + > drm_hdmi_avi_infoframe_quant_range(&frame.avi, > conn_state->connector, > adjusted_mode, > @@ -2143,10 +2145,21 @@ 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 > + * ToDo: This needs to be extended for LSPCON implementation > + * as well. Will be implemented separately. > + */ > + 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