diff mbox series

[v13,08/15] drm/i915/display: Enable colorspace programming for LSPCON devices

Message ID 20201127142820.8507-1-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Shankar, Uma Nov. 27, 2020, 2:28 p.m. UTC
Enable HDMI Colorspace for LSPCON based devices. Sending Colorimetry
data for HDR using AVI infoframe. LSPCON firmware expects this and though
SOC drives DP, for HDMI panel AVI infoframe is sent to the LSPCON device
which transfers the same to HDMI sink.

v2: Dropped state managed in drm core as per Jani Nikula's suggestion.

v3: Aligned colorimetry handling for lspcon as per compute_avi_infoframes,
as suggested by Ville.

v4: Finally fixed this with Ville's help, re-phrased the commit header
and description.

v5: Register HDMI colorspace for lspcon and move this to
intel_dp_add_properties as we can't create property at late_register.

Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 9 ++++++---
 drivers/gpu/drm/i915/display/intel_lspcon.c | 3 +++
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Ville Syrjälä Nov. 30, 2020, 7:51 p.m. UTC | #1
On Fri, Nov 27, 2020 at 07:58:20PM +0530, Uma Shankar wrote:
> Enable HDMI Colorspace for LSPCON based devices. Sending Colorimetry
> data for HDR using AVI infoframe. LSPCON firmware expects this and though
> SOC drives DP, for HDMI panel AVI infoframe is sent to the LSPCON device
> which transfers the same to HDMI sink.
> 
> v2: Dropped state managed in drm core as per Jani Nikula's suggestion.
> 
> v3: Aligned colorimetry handling for lspcon as per compute_avi_infoframes,
> as suggested by Ville.
> 
> v4: Finally fixed this with Ville's help, re-phrased the commit header
> and description.
> 
> v5: Register HDMI colorspace for lspcon and move this to
> intel_dp_add_properties as we can't create property at late_register.
> 
> Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 9 ++++++---
>  drivers/gpu/drm/i915/display/intel_lspcon.c | 3 +++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index f066031af162..21a0ca6ae2a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7193,10 +7193,13 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  	else if (INTEL_GEN(dev_priv) >= 5)
>  		drm_connector_attach_max_bpc_property(connector, 6, 12);
>  
> -	intel_attach_dp_colorspace_property(connector);
> -
> -	if (intel_bios_is_lspcon_present(dev_priv, port))
> +	/* Register HDMI colorspace for case of lspcon */
> +	if (intel_bios_is_lspcon_present(dev_priv, port)) {
>  		drm_connector_attach_content_type_property(connector);
> +		intel_attach_hdmi_colorspace_property(connector);
> +	} else {
> +		intel_attach_dp_colorspace_property(connector);
> +	}
>  
>  	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
>  		drm_object_attach_property(&connector->base,
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index 0a4c05d67108..cb768a1ae4c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -523,6 +523,9 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>  	else
>  		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>  
> +	/* Set the Colorspace as per the HDMI spec */
> +	drm_hdmi_avi_infoframe_colorspace(&frame.avi, conn_state);
> +
>  	/* nonsense combination */
>  	drm_WARN_ON(encoder->base.dev, crtc_state->limited_color_range &&
>  		    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);
> -- 
> 2.26.2
Shankar, Uma Nov. 30, 2020, 8:17 p.m. UTC | #2
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Tuesday, December 1, 2020 1:21 AM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [v13 08/15] drm/i915/display: Enable colorspace programming for
> LSPCON devices
> 
> On Fri, Nov 27, 2020 at 07:58:20PM +0530, Uma Shankar wrote:
> > Enable HDMI Colorspace for LSPCON based devices. Sending Colorimetry
> > data for HDR using AVI infoframe. LSPCON firmware expects this and
> > though SOC drives DP, for HDMI panel AVI infoframe is sent to the
> > LSPCON device which transfers the same to HDMI sink.
> >
> > v2: Dropped state managed in drm core as per Jani Nikula's suggestion.
> >
> > v3: Aligned colorimetry handling for lspcon as per
> > compute_avi_infoframes, as suggested by Ville.
> >
> > v4: Finally fixed this with Ville's help, re-phrased the commit header
> > and description.
> >
> > v5: Register HDMI colorspace for lspcon and move this to
> > intel_dp_add_properties as we can't create property at late_register.
> >
> > Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks a lot Ville for all your support, review and useful suggestions.
I have sent out a final version with all your RB's and conditional comments
addressed. 

This series is now ready for merge.

Thanks & Regards,
Uma Shankar

> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c     | 9 ++++++---
> >  drivers/gpu/drm/i915/display/intel_lspcon.c | 3 +++
> >  2 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index f066031af162..21a0ca6ae2a6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -7193,10 +7193,13 @@ intel_dp_add_properties(struct intel_dp *intel_dp,
> struct drm_connector *connect
> >  	else if (INTEL_GEN(dev_priv) >= 5)
> >  		drm_connector_attach_max_bpc_property(connector, 6, 12);
> >
> > -	intel_attach_dp_colorspace_property(connector);
> > -
> > -	if (intel_bios_is_lspcon_present(dev_priv, port))
> > +	/* Register HDMI colorspace for case of lspcon */
> > +	if (intel_bios_is_lspcon_present(dev_priv, port)) {
> >  		drm_connector_attach_content_type_property(connector);
> > +		intel_attach_hdmi_colorspace_property(connector);
> > +	} else {
> > +		intel_attach_dp_colorspace_property(connector);
> > +	}
> >
> >  	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
> >  		drm_object_attach_property(&connector->base,
> > diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c
> > b/drivers/gpu/drm/i915/display/intel_lspcon.c
> > index 0a4c05d67108..cb768a1ae4c9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> > +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> > @@ -523,6 +523,9 @@ void lspcon_set_infoframes(struct intel_encoder
> *encoder,
> >  	else
> >  		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >
> > +	/* Set the Colorspace as per the HDMI spec */
> > +	drm_hdmi_avi_infoframe_colorspace(&frame.avi, conn_state);
> > +
> >  	/* nonsense combination */
> >  	drm_WARN_ON(encoder->base.dev, crtc_state->limited_color_range &&
> >  		    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);
> > --
> > 2.26.2
> 
> --
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f066031af162..21a0ca6ae2a6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7193,10 +7193,13 @@  intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	else if (INTEL_GEN(dev_priv) >= 5)
 		drm_connector_attach_max_bpc_property(connector, 6, 12);
 
-	intel_attach_dp_colorspace_property(connector);
-
-	if (intel_bios_is_lspcon_present(dev_priv, port))
+	/* Register HDMI colorspace for case of lspcon */
+	if (intel_bios_is_lspcon_present(dev_priv, port)) {
 		drm_connector_attach_content_type_property(connector);
+		intel_attach_hdmi_colorspace_property(connector);
+	} else {
+		intel_attach_dp_colorspace_property(connector);
+	}
 
 	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
 		drm_object_attach_property(&connector->base,
diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index 0a4c05d67108..cb768a1ae4c9 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -523,6 +523,9 @@  void lspcon_set_infoframes(struct intel_encoder *encoder,
 	else
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 
+	/* Set the Colorspace as per the HDMI spec */
+	drm_hdmi_avi_infoframe_colorspace(&frame.avi, conn_state);
+
 	/* nonsense combination */
 	drm_WARN_ON(encoder->base.dev, crtc_state->limited_color_range &&
 		    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);