| Message ID | 1548777176-16783-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Colorspace connector property interface | expand |
On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the > property creation helper using the same. > > v6: Addressed Shashank's review comments. > > v7: Rebase > > v8: Addressed Maarten's review comments. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/intel_atomic.c | 1 + > drivers/gpu/drm/i915/intel_connector.c | 63 ++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ > 4 files changed, 89 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 || > 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..9a12d5e 100644 > --- a/drivers/gpu/drm/i915/intel_connector.c > +++ b/drivers/gpu/drm/i915/intel_connector.c > @@ -30,6 +30,48 @@ > #include "intel_drv.h" > #include "i915_drv.h" > > +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { > + /* For Default case, driver will set the colorspace */ > + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > + /* Standard Definition Colorimetry based on CEA 861 */ > + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > + /* High Definition Colorimetry based on IEC 61966-2-4 */ > + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ > + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, > + /* Colorimetry based on IEC 61966-2-5 [33] */ > + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, > + /* Colorimetry based on IEC 61966-2-5 */ > + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, > + /* Colorimetry based on ITU-R BT.2020 */ > + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, > + /* Colorimetry based on ITU-R BT.2020 */ > + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, > + /* Colorimetry based on ITU-R BT.2020 */ > + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, > +}; > + > +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { > + /* For Default case, driver will set the colorspace */ > + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > + /* Standard Definition Colorimetry based on CEA 861 */ > + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > + /* High Definition Colorimetry based on IEC 61966-2-4 */ > + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ > + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, > + /* Colorimetry based on IEC 61966-2-5 [33] */ > + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, > + /* Colorimetry based on IEC 61966-2-5 */ > + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, > +}; Why are we duplicating these in the driver code? And why do we have different sets for gen10 and older? > + > int intel_connector_init(struct intel_connector *connector) > { > struct intel_digital_connector_state *conn_state; > @@ -265,3 +307,24 @@ 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) > +{ > + struct drm_device *dev = connector->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + > + if (INTEL_GEN(dev_priv) >= 10) { > + if (!drm_mode_create_colorspace_property(connector, > + gen10_hdmi_colorspaces, > + ARRAY_SIZE(gen10_hdmi_colorspaces))) > + drm_object_attach_property(&connector->base, > + connector->colorspace_property, 0); > + } else { > + if (!drm_mode_create_colorspace_property(connector, > + legacy_hdmi_colorspaces, > + ARRAY_SIZE(legacy_hdmi_colorspaces))) > + 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..9ed00e3 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,16 @@ 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 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Ville Syrjälä >Sent: Tuesday, January 29, 2019 9:14 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; Syrjala, Ville ><ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>; >dri-devel@lists.freedesktop.org >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable modeset > >On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the property >> creation helper using the same. >> >> v6: Addressed Shashank's review comments. >> >> v7: Rebase >> >> v8: Addressed Maarten's review comments. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> drivers/gpu/drm/i915/intel_connector.c | 63 >++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ >> 4 files changed, 89 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 || >> 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..9a12d5e 100644 >> --- a/drivers/gpu/drm/i915/intel_connector.c >> +++ b/drivers/gpu/drm/i915/intel_connector.c >> @@ -30,6 +30,48 @@ >> #include "intel_drv.h" >> #include "i915_drv.h" >> >> +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { >> + /* For Default case, driver will set the colorspace */ >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> + /* Standard Definition Colorimetry based on CEA 861 */ >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >> + /* Colorimetry based on IEC 61966-2-5 [33] */ >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >> + /* Colorimetry based on IEC 61966-2-5 */ >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, >> + /* Colorimetry based on ITU-R BT.2020 */ >> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, >> + /* Colorimetry based on ITU-R BT.2020 */ >> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, >> + /* Colorimetry based on ITU-R BT.2020 */ >> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; >> + >> +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { >> + /* For Default case, driver will set the colorspace */ >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> + /* Standard Definition Colorimetry based on CEA 861 */ >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >> + /* Colorimetry based on IEC 61966-2-5 [33] */ >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >> + /* Colorimetry based on IEC 61966-2-5 */ >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, }; > >Why are we duplicating these in the driver code? And why do we have different >sets for gen10 and older? This is to expose colorspaces which a particular platforms supports instead of giving full HDMI protocol supported list as some platforms may not support all of it. Here, BT2020 support is added for Gen10 onwards, while older platforms support all the rest apart from BT2020. For sink supported colorspaces, userspace will retrieve it from EDID and set the colorspace which is supported by both platform as well as sink. Regards, Uma Shankar >> + >> int intel_connector_init(struct intel_connector *connector) { >> struct intel_digital_connector_state *conn_state; @@ -265,3 +307,24 >> @@ 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) { >> + struct drm_device *dev = connector->dev; >> + struct drm_i915_private *dev_priv = to_i915(dev); >> + >> + if (INTEL_GEN(dev_priv) >= 10) { >> + if (!drm_mode_create_colorspace_property(connector, >> + gen10_hdmi_colorspaces, >> + ARRAY_SIZE(gen10_hdmi_colorspaces))) >> + drm_object_attach_property(&connector->base, >> + connector->colorspace_property, 0); >> + } else { >> + if (!drm_mode_create_colorspace_property(connector, >> + legacy_hdmi_colorspaces, >> + ARRAY_SIZE(legacy_hdmi_colorspaces))) >> + 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..9ed00e3 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,16 @@ 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 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >-- >Ville Syrjälä >Intel >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Jan 29, 2019 at 03:57:29PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of > >Ville Syrjälä > >Sent: Tuesday, January 29, 2019 9:14 PM > >To: Shankar, Uma <uma.shankar@intel.com> > >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; Syrjala, Ville > ><ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>; > >dri-devel@lists.freedesktop.org > >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable modeset > > > >On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the property > >> creation helper using the same. > >> > >> v6: Addressed Shashank's review comments. > >> > >> v7: Rebase > >> > >> v8: Addressed Maarten's review comments. > >> > >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_atomic.c | 1 + > >> drivers/gpu/drm/i915/intel_connector.c | 63 > >++++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_drv.h | 1 + > >> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ > >> 4 files changed, 89 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 || > >> 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..9a12d5e 100644 > >> --- a/drivers/gpu/drm/i915/intel_connector.c > >> +++ b/drivers/gpu/drm/i915/intel_connector.c > >> @@ -30,6 +30,48 @@ > >> #include "intel_drv.h" > >> #include "i915_drv.h" > >> > >> +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { > >> + /* For Default case, driver will set the colorspace */ > >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > >> + /* Standard Definition Colorimetry based on CEA 861 */ > >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ > >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ > >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, > >> + /* Colorimetry based on IEC 61966-2-5 [33] */ > >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, > >> + /* Colorimetry based on IEC 61966-2-5 */ > >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, > >> + /* Colorimetry based on ITU-R BT.2020 */ > >> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, > >> + /* Colorimetry based on ITU-R BT.2020 */ > >> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, > >> + /* Colorimetry based on ITU-R BT.2020 */ > >> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; > >> + > >> +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { > >> + /* For Default case, driver will set the colorspace */ > >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > >> + /* Standard Definition Colorimetry based on CEA 861 */ > >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ > >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ > >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, > >> + /* Colorimetry based on IEC 61966-2-5 [33] */ > >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, > >> + /* Colorimetry based on IEC 61966-2-5 */ > >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, }; > > > >Why are we duplicating these in the driver code? And why do we have different > >sets for gen10 and older? > > This is to expose colorspaces which a particular platforms supports instead of giving > full HDMI protocol supported list as some platforms may not support all of it. > > Here, BT2020 support is added for Gen10 onwards, while older platforms support > all the rest apart from BT2020. This is just about the infoframe. I don't think we have any platform limitations there. > > For sink supported colorspaces, userspace will retrieve it from EDID and set the colorspace > which is supported by both platform as well as sink. > > Regards, > Uma Shankar > > >> + > >> int intel_connector_init(struct intel_connector *connector) { > >> struct intel_digital_connector_state *conn_state; @@ -265,3 +307,24 > >> @@ 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) { > >> + struct drm_device *dev = connector->dev; > >> + struct drm_i915_private *dev_priv = to_i915(dev); > >> + > >> + if (INTEL_GEN(dev_priv) >= 10) { > >> + if (!drm_mode_create_colorspace_property(connector, > >> + gen10_hdmi_colorspaces, > >> + ARRAY_SIZE(gen10_hdmi_colorspaces))) > >> + drm_object_attach_property(&connector->base, > >> + connector->colorspace_property, 0); > >> + } else { > >> + if (!drm_mode_create_colorspace_property(connector, > >> + legacy_hdmi_colorspaces, > >> + ARRAY_SIZE(legacy_hdmi_colorspaces))) > >> + 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..9ed00e3 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,16 @@ 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 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > >-- > >Ville Syrjälä > >Intel > >_______________________________________________ > >dri-devel mailing list > >dri-devel@lists.freedesktop.org > >https://lists.freedesktop.org/mailman/listinfo/dri-devel
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Tuesday, January 29, 2019 9:34 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; Syrjala, Ville ><ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>; >dri-devel@lists.freedesktop.org >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable modeset > >On Tue, Jan 29, 2019 at 03:57:29PM +0000, Shankar, Uma wrote: >> >> >> >-----Original Message----- >> >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On >> >Behalf Of Ville Syrjälä >> >Sent: Tuesday, January 29, 2019 9:14 PM >> >To: Shankar, Uma <uma.shankar@intel.com> >> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; >> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten >> ><maarten.lankhorst@intel.com>; dri-devel@lists.freedesktop.org >> >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable >> >modeset >> > >> >On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the >> >> property creation helper using the same. >> >> >> >> v6: Addressed Shashank's review comments. >> >> >> >> v7: Rebase >> >> >> >> v8: Addressed Maarten's review comments. >> >> >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> >> drivers/gpu/drm/i915/intel_connector.c | 63 >> >++++++++++++++++++++++++++++++++++ >> >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> >> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ >> >> 4 files changed, 89 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 || >> >> 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..9a12d5e 100644 >> >> --- a/drivers/gpu/drm/i915/intel_connector.c >> >> +++ b/drivers/gpu/drm/i915/intel_connector.c >> >> @@ -30,6 +30,48 @@ >> >> #include "intel_drv.h" >> >> #include "i915_drv.h" >> >> >> >> +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { >> >> + /* For Default case, driver will set the colorspace */ >> >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> >> + /* Standard Definition Colorimetry based on CEA 861 */ >> >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >> >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >> >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >> >> + /* Colorimetry based on IEC 61966-2-5 [33] */ >> >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >> >> + /* Colorimetry based on IEC 61966-2-5 */ >> >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, >> >> + /* Colorimetry based on ITU-R BT.2020 */ >> >> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, >> >> + /* Colorimetry based on ITU-R BT.2020 */ >> >> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, >> >> + /* Colorimetry based on ITU-R BT.2020 */ >> >> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; >> >> + >> >> +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { >> >> + /* For Default case, driver will set the colorspace */ >> >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> >> + /* Standard Definition Colorimetry based on CEA 861 */ >> >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >> >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >> >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >> >> + /* Colorimetry based on IEC 61966-2-5 [33] */ >> >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >> >> + /* Colorimetry based on IEC 61966-2-5 */ >> >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, }; >> > >> >Why are we duplicating these in the driver code? And why do we have >> >different sets for gen10 and older? >> >> This is to expose colorspaces which a particular platforms supports >> instead of giving full HDMI protocol supported list as some platforms may not >support all of it. >> >> Here, BT2020 support is added for Gen10 onwards, while older platforms >> support all the rest apart from BT2020. > >This is just about the infoframe. I don't think we have any platform limitations >there. BT2020 is not supported on legacy platforms so we thought to not expose this here as well, else it may lead to confusion. Not sure what's the best approach here. Earlier it was done without any platform checks in v4. If you suggest to expose all HDMI supported colorspaces, I will make the change and update this series. What do you recommend ? Regards, Uma Shankar >> >> For sink supported colorspaces, userspace will retrieve it from EDID >> and set the colorspace which is supported by both platform as well as sink. >> >> Regards, >> Uma Shankar >> >> >> + >> >> int intel_connector_init(struct intel_connector *connector) { >> >> struct intel_digital_connector_state *conn_state; @@ -265,3 >> >> +307,24 @@ 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) { >> >> + struct drm_device *dev = connector->dev; >> >> + struct drm_i915_private *dev_priv = to_i915(dev); >> >> + >> >> + if (INTEL_GEN(dev_priv) >= 10) { >> >> + if (!drm_mode_create_colorspace_property(connector, >> >> + gen10_hdmi_colorspaces, >> >> + ARRAY_SIZE(gen10_hdmi_colorspaces))) >> >> + drm_object_attach_property(&connector->base, >> >> + connector->colorspace_property, 0); >> >> + } else { >> >> + if (!drm_mode_create_colorspace_property(connector, >> >> + legacy_hdmi_colorspaces, >> >> + ARRAY_SIZE(legacy_hdmi_colorspaces))) >> >> + 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..9ed00e3 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,16 @@ 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 >> >> >> >> _______________________________________________ >> >> dri-devel mailing list >> >> dri-devel@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > >> >-- >> >Ville Syrjälä >> >Intel >> >_______________________________________________ >> >dri-devel mailing list >> >dri-devel@lists.freedesktop.org >> >https://lists.freedesktop.org/mailman/listinfo/dri-devel > >-- >Ville Syrjälä >Intel
On Tue, Jan 29, 2019 at 04:20:39PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Tuesday, January 29, 2019 9:34 PM > >To: Shankar, Uma <uma.shankar@intel.com> > >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; Syrjala, Ville > ><ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>; > >dri-devel@lists.freedesktop.org > >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable modeset > > > >On Tue, Jan 29, 2019 at 03:57:29PM +0000, Shankar, Uma wrote: > >> > >> > >> >-----Original Message----- > >> >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On > >> >Behalf Of Ville Syrjälä > >> >Sent: Tuesday, January 29, 2019 9:14 PM > >> >To: Shankar, Uma <uma.shankar@intel.com> > >> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; > >> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten > >> ><maarten.lankhorst@intel.com>; dri-devel@lists.freedesktop.org > >> >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable > >> >modeset > >> > > >> >On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the > >> >> property creation helper using the same. > >> >> > >> >> v6: Addressed Shashank's review comments. > >> >> > >> >> v7: Rebase > >> >> > >> >> v8: Addressed Maarten's review comments. > >> >> > >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/intel_atomic.c | 1 + > >> >> drivers/gpu/drm/i915/intel_connector.c | 63 > >> >++++++++++++++++++++++++++++++++++ > >> >> drivers/gpu/drm/i915/intel_drv.h | 1 + > >> >> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ > >> >> 4 files changed, 89 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 || > >> >> 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..9a12d5e 100644 > >> >> --- a/drivers/gpu/drm/i915/intel_connector.c > >> >> +++ b/drivers/gpu/drm/i915/intel_connector.c > >> >> @@ -30,6 +30,48 @@ > >> >> #include "intel_drv.h" > >> >> #include "i915_drv.h" > >> >> > >> >> +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { > >> >> + /* For Default case, driver will set the colorspace */ > >> >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > >> >> + /* Standard Definition Colorimetry based on CEA 861 */ > >> >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > >> >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > >> >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > >> >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ > >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > >> >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ > >> >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, > >> >> + /* Colorimetry based on IEC 61966-2-5 [33] */ > >> >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, > >> >> + /* Colorimetry based on IEC 61966-2-5 */ > >> >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, > >> >> + /* Colorimetry based on ITU-R BT.2020 */ > >> >> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, > >> >> + /* Colorimetry based on ITU-R BT.2020 */ > >> >> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, > >> >> + /* Colorimetry based on ITU-R BT.2020 */ > >> >> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; > >> >> + > >> >> +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { > >> >> + /* For Default case, driver will set the colorspace */ > >> >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > >> >> + /* Standard Definition Colorimetry based on CEA 861 */ > >> >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > >> >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > >> >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > >> >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ > >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > >> >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ > >> >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, > >> >> + /* Colorimetry based on IEC 61966-2-5 [33] */ > >> >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, > >> >> + /* Colorimetry based on IEC 61966-2-5 */ > >> >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, }; > >> > > >> >Why are we duplicating these in the driver code? And why do we have > >> >different sets for gen10 and older? > >> > >> This is to expose colorspaces which a particular platforms supports > >> instead of giving full HDMI protocol supported list as some platforms may not > >support all of it. > >> > >> Here, BT2020 support is added for Gen10 onwards, while older platforms > >> support all the rest apart from BT2020. > > > >This is just about the infoframe. I don't think we have any platform limitations > >there. > > BT2020 is not supported on legacy platforms so we thought to not expose this > here as well, else it may lead to confusion. Nothing prevents you from scanning out BT.2020 content even on a gen2. > Not sure what's the best approach here. > Earlier it was done without any platform checks in v4. > > If you suggest to expose all HDMI supported colorspaces, I will make the change and > update this series. What do you recommend ? > > Regards, > Uma Shankar > > >> > >> For sink supported colorspaces, userspace will retrieve it from EDID > >> and set the colorspace which is supported by both platform as well as sink. > >> > >> Regards, > >> Uma Shankar > >> > >> >> + > >> >> int intel_connector_init(struct intel_connector *connector) { > >> >> struct intel_digital_connector_state *conn_state; @@ -265,3 > >> >> +307,24 @@ 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) { > >> >> + struct drm_device *dev = connector->dev; > >> >> + struct drm_i915_private *dev_priv = to_i915(dev); > >> >> + > >> >> + if (INTEL_GEN(dev_priv) >= 10) { > >> >> + if (!drm_mode_create_colorspace_property(connector, > >> >> + gen10_hdmi_colorspaces, > >> >> + ARRAY_SIZE(gen10_hdmi_colorspaces))) > >> >> + drm_object_attach_property(&connector->base, > >> >> + connector->colorspace_property, 0); > >> >> + } else { > >> >> + if (!drm_mode_create_colorspace_property(connector, > >> >> + legacy_hdmi_colorspaces, > >> >> + ARRAY_SIZE(legacy_hdmi_colorspaces))) > >> >> + 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..9ed00e3 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,16 @@ 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 > >> >> > >> >> _______________________________________________ > >> >> dri-devel mailing list > >> >> dri-devel@lists.freedesktop.org > >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > > >> >-- > >> >Ville Syrjälä > >> >Intel > >> >_______________________________________________ > >> >dri-devel mailing list > >> >dri-devel@lists.freedesktop.org > >> >https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > >-- > >Ville Syrjälä > >Intel
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Ville Syrjälä >Sent: Tuesday, January 29, 2019 10:09 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: Syrjala, Ville <ville.syrjala@intel.com>; intel-gfx@lists.freedesktop.org; >emil.l.velikov@gmail.com; dri-devel@lists.freedesktop.org; Lankhorst, Maarten ><maarten.lankhorst@intel.com> >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable modeset > >On Tue, Jan 29, 2019 at 04:20:39PM +0000, Shankar, Uma wrote: >> >> >> >-----Original Message----- >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >> >Sent: Tuesday, January 29, 2019 9:34 PM >> >To: Shankar, Uma <uma.shankar@intel.com> >> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; >> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten >> ><maarten.lankhorst@intel.com>; dri-devel@lists.freedesktop.org >> >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable >> >modeset >> > >> >On Tue, Jan 29, 2019 at 03:57:29PM +0000, Shankar, Uma wrote: >> >> >> >> >> >> >-----Original Message----- >> >> >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] >> >> >On Behalf Of Ville Syrjälä >> >> >Sent: Tuesday, January 29, 2019 9:14 PM >> >> >To: Shankar, Uma <uma.shankar@intel.com> >> >> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; >> >> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten >> >> ><maarten.lankhorst@intel.com>; dri-devel@lists.freedesktop.org >> >> >Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and >> >> >enable modeset >> >> > >> >> >On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the >> >> >> property creation helper using the same. >> >> >> >> >> >> v6: Addressed Shashank's review comments. >> >> >> >> >> >> v7: Rebase >> >> >> >> >> >> v8: Addressed Maarten's review comments. >> >> >> >> >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> >> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> >> >> >> --- >> >> >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> >> >> drivers/gpu/drm/i915/intel_connector.c | 63 >> >> >++++++++++++++++++++++++++++++++++ >> >> >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> >> >> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ >> >> >> 4 files changed, 89 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 || >> >> >> 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..9a12d5e 100644 >> >> >> --- a/drivers/gpu/drm/i915/intel_connector.c >> >> >> +++ b/drivers/gpu/drm/i915/intel_connector.c >> >> >> @@ -30,6 +30,48 @@ >> >> >> #include "intel_drv.h" >> >> >> #include "i915_drv.h" >> >> >> >> >> >> +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { >> >> >> + /* For Default case, driver will set the colorspace */ >> >> >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> >> >> + /* Standard Definition Colorimetry based on CEA 861 */ >> >> >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> >> >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >> >> >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> >> >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> >> >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >> >> >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >> >> >> + /* Colorimetry based on IEC 61966-2-5 [33] */ >> >> >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >> >> >> + /* Colorimetry based on IEC 61966-2-5 */ >> >> >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, >> >> >> + /* Colorimetry based on ITU-R BT.2020 */ >> >> >> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, >> >> >> + /* Colorimetry based on ITU-R BT.2020 */ >> >> >> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, >> >> >> + /* Colorimetry based on ITU-R BT.2020 */ >> >> >> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, >}; >> >> >> + >> >> >> +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { >> >> >> + /* For Default case, driver will set the colorspace */ >> >> >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> >> >> + /* Standard Definition Colorimetry based on CEA 861 */ >> >> >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> >> >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >> >> >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> >> >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> >> >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> >> >> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >> >> >> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >> >> >> + /* Colorimetry based on IEC 61966-2-5 [33] */ >> >> >> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >> >> >> + /* Colorimetry based on IEC 61966-2-5 */ >> >> >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, }; >> >> > >> >> >Why are we duplicating these in the driver code? And why do we >> >> >have different sets for gen10 and older? >> >> >> >> This is to expose colorspaces which a particular platforms supports >> >> instead of giving full HDMI protocol supported list as some >> >> platforms may not >> >support all of it. >> >> >> >> Here, BT2020 support is added for Gen10 onwards, while older >> >> platforms support all the rest apart from BT2020. >> > >> >This is just about the infoframe. I don't think we have any platform >> >limitations there. >> >> BT2020 is not supported on legacy platforms so we thought to not >> expose this here as well, else it may lead to confusion. > >Nothing prevents you from scanning out BT.2020 content even on a gen2. Ok, I will remove this platform specific implementation and create a generic HDMI colorspace property. Will resend the next version with this updated. Thanks Ville. Regards, Uma Shankar >> Not sure what's the best approach here. >> Earlier it was done without any platform checks in v4. >> >> If you suggest to expose all HDMI supported colorspaces, I will make >> the change and update this series. What do you recommend ? >> >> Regards, >> Uma Shankar >> >> >> >> >> For sink supported colorspaces, userspace will retrieve it from >> >> EDID and set the colorspace which is supported by both platform as well as >sink. >> >> >> >> Regards, >> >> Uma Shankar >> >> >> >> >> + >> >> >> int intel_connector_init(struct intel_connector *connector) { >> >> >> struct intel_digital_connector_state *conn_state; @@ -265,3 >> >> >> +307,24 @@ 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) { >> >> >> + struct drm_device *dev = connector->dev; >> >> >> + struct drm_i915_private *dev_priv = to_i915(dev); >> >> >> + >> >> >> + if (INTEL_GEN(dev_priv) >= 10) { >> >> >> + if (!drm_mode_create_colorspace_property(connector, >> >> >> + gen10_hdmi_colorspaces, >> >> >> + ARRAY_SIZE(gen10_hdmi_colorspaces))) >> >> >> + drm_object_attach_property(&connector- >>base, >> >> >> + connector->colorspace_property, 0); >> >> >> + } else { >> >> >> + if (!drm_mode_create_colorspace_property(connector, >> >> >> + legacy_hdmi_colorspaces, >> >> >> + ARRAY_SIZE(legacy_hdmi_colorspaces))) >> >> >> + 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..9ed00e3 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,16 @@ 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 >> >> >> >> >> >> _______________________________________________ >> >> >> dri-devel mailing list >> >> >> dri-devel@lists.freedesktop.org >> >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> > >> >> >-- >> >> >Ville Syrjälä >> >> >Intel >> >> >_______________________________________________ >> >> >dri-devel mailing list >> >> >dri-devel@lists.freedesktop.org >> >> >https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > >> >-- >> >Ville Syrjälä >> >Intel > >-- >Ville Syrjälä >Intel >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hello Ville, On 1/29/2019 9:33 PM, Ville Syrjälä wrote: > On Tue, Jan 29, 2019 at 03:57:29PM +0000, Shankar, Uma wrote: >> >>> -----Original Message----- >>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >>> Ville Syrjälä >>> Sent: Tuesday, January 29, 2019 9:14 PM >>> To: Shankar, Uma <uma.shankar@intel.com> >>> Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; Syrjala, Ville >>> <ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>; >>> dri-devel@lists.freedesktop.org >>> Subject: Re: [v8 2/2] drm/i915: Attach colorspace property and enable modeset >>> >>> On Tue, Jan 29, 2019 at 09:22: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: Added Platform specific colorspace enums and called the property >>>> creation helper using the same. >>>> >>>> v6: Addressed Shashank's review comments. >>>> >>>> v7: Rebase >>>> >>>> v8: Addressed Maarten's review comments. >>>> >>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/intel_atomic.c | 1 + >>>> drivers/gpu/drm/i915/intel_connector.c | 63 >>> ++++++++++++++++++++++++++++++++++ >>>> drivers/gpu/drm/i915/intel_drv.h | 1 + >>>> drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++ >>>> 4 files changed, 89 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 || >>>> 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..9a12d5e 100644 >>>> --- a/drivers/gpu/drm/i915/intel_connector.c >>>> +++ b/drivers/gpu/drm/i915/intel_connector.c >>>> @@ -30,6 +30,48 @@ >>>> #include "intel_drv.h" >>>> #include "i915_drv.h" >>>> >>>> +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { >>>> + /* For Default case, driver will set the colorspace */ >>>> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >>>> + /* Standard Definition Colorimetry based on CEA 861 */ >>>> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >>>> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >>>> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >>>> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >>>> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >>>> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >>>> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >>>> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >>>> + /* Colorimetry based on IEC 61966-2-5 [33] */ >>>> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >>>> + /* Colorimetry based on IEC 61966-2-5 */ >>>> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, >>>> + /* Colorimetry based on ITU-R BT.2020 */ >>>> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, >>>> + /* Colorimetry based on ITU-R BT.2020 */ >>>> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, >>>> + /* Colorimetry based on ITU-R BT.2020 */ >>>> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; >>>> + >>>> +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { >>>> + /* For Default case, driver will set the colorspace */ >>>> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >>>> + /* Standard Definition Colorimetry based on CEA 861 */ >>>> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >>>> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, >>>> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >>>> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >>>> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >>>> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >>>> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ >>>> + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, >>>> + /* Colorimetry based on IEC 61966-2-5 [33] */ >>>> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, >>>> + /* Colorimetry based on IEC 61966-2-5 */ >>>> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, }; >>> Why are we duplicating these in the driver code? And why do we have different >>> sets for gen10 and older? >> This is to expose colorspaces which a particular platforms supports instead of giving >> full HDMI protocol supported list as some platforms may not support all of it. >> >> Here, BT2020 support is added for Gen10 onwards, while older platforms support >> all the rest apart from BT2020. > This is just about the infoframe. I don't think we have any platform > limitations there. I am not sure about this, reasons: - this property not only prepares infoframe, but indicates userspace that the platforms has support for following colorspaces. Many userspaces might be taking a call of BT2020/HDR support based on this property. - Also, can we send CTA-861-G infoframes using a HDMI-1.4b devices ? Even if we can, is that the right thing to do ? - Shashank >> For sink supported colorspaces, userspace will retrieve it from EDID and set the colorspace >> which is supported by both platform as well as sink. >> >> Regards, >> Uma Shankar >> >>>> + >>>> int intel_connector_init(struct intel_connector *connector) { >>>> struct intel_digital_connector_state *conn_state; @@ -265,3 +307,24 >>>> @@ 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) { >>>> + struct drm_device *dev = connector->dev; >>>> + struct drm_i915_private *dev_priv = to_i915(dev); >>>> + >>>> + if (INTEL_GEN(dev_priv) >= 10) { >>>> + if (!drm_mode_create_colorspace_property(connector, >>>> + gen10_hdmi_colorspaces, >>>> + ARRAY_SIZE(gen10_hdmi_colorspaces))) >>>> + drm_object_attach_property(&connector->base, >>>> + connector->colorspace_property, 0); >>>> + } else { >>>> + if (!drm_mode_create_colorspace_property(connector, >>>> + legacy_hdmi_colorspaces, >>>> + ARRAY_SIZE(legacy_hdmi_colorspaces))) >>>> + 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..9ed00e3 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,16 @@ 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 >>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> -- >>> Ville Syrjälä >>> Intel >>> _______________________________________________ >>> 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..9a12d5e 100644 --- a/drivers/gpu/drm/i915/intel_connector.c +++ b/drivers/gpu/drm/i915/intel_connector.c @@ -30,6 +30,48 @@ #include "intel_drv.h" #include "i915_drv.h" +static const struct drm_prop_enum_list gen10_hdmi_colorspaces[] = { + /* For Default case, driver will set the colorspace */ + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, + /* Standard Definition Colorimetry based on CEA 861 */ + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, + /* High Definition Colorimetry based on IEC 61966-2-4 */ + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, + /* Colorimetry based on IEC 61966-2-5 [33] */ + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, + /* Colorimetry based on IEC 61966-2-5 */ + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, + /* Colorimetry based on ITU-R BT.2020 */ + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, + /* Colorimetry based on ITU-R BT.2020 */ + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, + /* Colorimetry based on ITU-R BT.2020 */ + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, +}; + +static const struct drm_prop_enum_list legacy_hdmi_colorspaces[] = { + /* For Default case, driver will set the colorspace */ + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, + /* Standard Definition Colorimetry based on CEA 861 */ + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, + /* High Definition Colorimetry based on IEC 61966-2-4 */ + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ + { DRM_MODE_COLORIMETRY_S_YCC_601, "S_YCC_601" }, + /* Colorimetry based on IEC 61966-2-5 [33] */ + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, + /* Colorimetry based on IEC 61966-2-5 */ + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, +}; + int intel_connector_init(struct intel_connector *connector) { struct intel_digital_connector_state *conn_state; @@ -265,3 +307,24 @@ 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) +{ + struct drm_device *dev = connector->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + + if (INTEL_GEN(dev_priv) >= 10) { + if (!drm_mode_create_colorspace_property(connector, + gen10_hdmi_colorspaces, + ARRAY_SIZE(gen10_hdmi_colorspaces))) + drm_object_attach_property(&connector->base, + connector->colorspace_property, 0); + } else { + if (!drm_mode_create_colorspace_property(connector, + legacy_hdmi_colorspaces, + ARRAY_SIZE(legacy_hdmi_colorspaces))) + 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..9ed00e3 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,16 @@ 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;