diff mbox series

[v9,06/12] drm/i915/display: Enable BT2020 for HDR on LSPCON devices

Message ID 20201103152834.12727-7-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series [v9,01/12] drm/i915/display: Add HDR Capability detection for LSPCON | expand

Commit Message

Shankar, Uma Nov. 3, 2020, 3:28 p.m. UTC
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 | 31 +++++++++++++++++----
 1 file changed, 25 insertions(+), 6 deletions(-)

Comments

Ville Syrjala Nov. 25, 2020, 4:40 p.m. UTC | #1
On Tue, Nov 03, 2020 at 08:58:28PM +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 | 31 +++++++++++++++++----
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index 336494b60d11..19831f5e51bf 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -524,12 +524,31 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>  	else
>  		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);
> +	/*
> +	 * Set the HDMI Colorspace which are supported by DP as well.
> +	 * For all others (3 combinations which are exclusive for DP),
> +	 * Let the colorspace be set to default i.e No Data.
> +	 * Fixme: Expose HDMI colorspaces for instead of DP counterparts
> +	 */
> +	drm_hdmi_avi_infoframe_colorspace(&frame.avi, conn_state);

I would suggest taking care of that FIXME + adding this in the same
patch.

> +
> +	/* 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 like it should be a separate patch.

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

And that one looks like it should be part of the patch that adds the
corresponding property.

>  
>  	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
>  	if (ret < 0) {
> -- 
> 2.26.2
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 336494b60d11..19831f5e51bf 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -524,12 +524,31 @@  void lspcon_set_infoframes(struct intel_encoder *encoder,
 	else
 		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);
+	/*
+	 * Set the HDMI Colorspace which are supported by DP as well.
+	 * For all others (3 combinations which are exclusive for DP),
+	 * Let the colorspace be set to default i.e No Data.
+	 * Fixme: Expose HDMI colorspaces for instead of DP counterparts
+	 */
+	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);
+
+	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;
+	}
+
+	drm_hdmi_avi_infoframe_content_type(&frame.avi, conn_state);
 
 	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
 	if (ret < 0) {