Re: [Intel-gfx] [v7 04/10] drm/i915/display: Enable BT2020 for HDR on LSPCON devices

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Uma Shankar <uma.shankar@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [v7 04/10] drm/i915/display: Enable BT2020 for HDR on LSPCON devices
Date: Thu, 8 Oct 2020 14:18:57 +0300	[thread overview]
Message-ID: <20201008111857.GM6112@intel.com> (raw)
In-Reply-To: <20201006130654.331-5-uma.shankar@intel.com>

On Tue, Oct 06, 2020 at 06:36:48PM +0530, Uma Shankar wrote:
> Enable Colorspace as BT2020 if driving HDR content.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.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_lspcon.c | 25 ++++++++++++++++-----
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index 440d2b3c2212..9ffa36797daf 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -534,12 +534,25 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>  		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>  	}
>  
> -	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> -					   conn_state->connector,
> -					   adjusted_mode,
> -					   crtc_state->limited_color_range ?
> -					   HDMI_QUANTIZATION_RANGE_LIMITED :
> -					   HDMI_QUANTIZATION_RANGE_FULL);
> +	drm_hdmi_avi_infoframe_colorspace(&frame.avi, conn_state);

That seems to be a missing part from
commit 9d1bb6f0222c ("drm/i915/dp: Attach colorspace property")

Also looks like
commit 2f146b78d5a9 ("drm/i915: Attach colorspace property and enable modeset")
added a bogus lspcon check into intel_hdmi_add_properties(). That should
be nuked.

Hmm. This whole thing seems like a total snafu. Since we use
AVI IF for lspcon it should follow the HDMI colorimetry stuff, but
now it uses some kind of mix of both HDMI and DP. We need to sort this
out somehow...

> +
> +	/* nonsense combination */
> +	drm_WARN_ON(encoder->base.dev, crtc_state->limited_color_range &&
> +		    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);
> +
> +	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) {
> +		drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +						   conn_state->connector,
> +						   adjusted_mode,
> +						   crtc_state->limited_color_range ?
> +						   HDMI_QUANTIZATION_RANGE_LIMITED :
> +						   HDMI_QUANTIZATION_RANGE_FULL);
> +	} else {
> +		frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> +		frame.avi.ycc_quantization_range = HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
> +	}

This part looks OK.

> +
> +	drm_hdmi_avi_infoframe_content_type(&frame.avi, conn_state);

I don't think we have that property attached to the connector.
Probably want a separte patch to add both the prop and this thing.

>  
>  	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
>  	if (ret < 0) {
> -- 
> 2.26.2

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx