diff mbox series

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

Message ID 20201126210314.7882-9-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable HDR on MCA LSPCON based Gen9 devices | expand

Commit Message

Shankar, Uma Nov. 26, 2020, 9:03 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.

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_lspcon.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Ville Syrjälä Nov. 27, 2020, 1:45 p.m. UTC | #1
On Fri, Nov 27, 2020 at 02:33:07AM +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.

Still missing the "expose the hdmi variant of the prop on lspcon"
part. I didn't include that in my patch since we didn't use
drm_hdmi_avi_infoframe_colorspace() yes on lspcon. Although
maybe I should have just done that change anyway since we were
already registering the prop anyway even if we didn't actually
put the data into the infoframe.

> 
> 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_lspcon.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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. 27, 2020, 2:03 p.m. UTC | #2
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Friday, November 27, 2020 7:15 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [v12 08/15] drm/i915/display: Enable colorspace programming for
> LSPCON devices
> 
> On Fri, Nov 27, 2020 at 02:33:07AM +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.
> 
> Still missing the "expose the hdmi variant of the prop on lspcon"
> part. I didn't include that in my patch since we didn't use
> drm_hdmi_avi_infoframe_colorspace() yes on lspcon. Although maybe I should
> have just done that change anyway since we were already registering the prop
> anyway even if we didn't actually put the data into the infoframe.

Yeah just sent the fix for that Ville. We had to create the properties early so moved the
checks to intel_dp_add_properties. Just see if the new version looks ok

Regards,
Uma Shankar

> >
> > 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_lspcon.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > 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_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);