| Message ID | 1548787812-13523-4-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Colorspace connector property interface | expand |
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@intel.com> > --- > 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. Otherwise series looks good, so Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> 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; >
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Maarten Lankhorst >Sent: Wednesday, January 30, 2019 3:27 PM >To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; >dri-devel@lists.freedesktop.org >Cc: emil.l.velikov@gmail.com; Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, >Maarten <maarten.lankhorst@intel.com> >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@intel.com> >> --- >> 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@linux.intel.com> 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@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 || 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;
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@intel.com> --- 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(+)