Re: [PATCH 17/24] drm/i915/icl: Add 10-bit support for hdmi

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, May 21, 2018 at 05:25:51PM -0700, Paulo Zanoni wrote:
> From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>
> 
> Starting Icelake silicon supports 10-bpc hdmi to support certain
> media workloads. Currently hdmi supports 8 and 12 bpc. Plumbed
> in support for 10 bit hdmi.
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

Still looks reasoanble to me so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 64 +++++++++++++++++++++++++++++----------
>  1 file changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 0ca4cc877520..53ac8bb85218 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1561,14 +1561,23 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	/* check if we can do 8bpc */
>  	status = hdmi_port_clock_valid(hdmi, clock, true, force_dvi);
>  
> -	/* if we can't do 8bpc we may still be able to do 12bpc */
> -	if (!HAS_GMCH_DISPLAY(dev_priv) && status != MODE_OK && hdmi->has_hdmi_sink && !force_dvi)
> -		status = hdmi_port_clock_valid(hdmi, clock * 3 / 2, true, force_dvi);
> +	if (hdmi->has_hdmi_sink && !force_dvi) {
> +		/* if we can't do 8bpc we may still be able to do 12bpc */
> +		if (status != MODE_OK && !HAS_GMCH_DISPLAY(dev_priv))
> +			status = hdmi_port_clock_valid(hdmi, clock * 3 / 2,
> +						       true, force_dvi);
> +
> +		/* if we can't do 8,12bpc we may still be able to do 10bpc */
> +		if (status != MODE_OK && INTEL_GEN(dev_priv) >= 11)
> +			status = hdmi_port_clock_valid(hdmi, clock * 5 / 4,
> +						       true, force_dvi);
> +	}
>  
>  	return status;
>  }
>  
> -static bool hdmi_12bpc_possible(const struct intel_crtc_state *crtc_state)
> +static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
> +				     int bpc)
>  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(crtc_state->base.crtc->dev);
> @@ -1580,6 +1589,9 @@ static bool hdmi_12bpc_possible(const struct intel_crtc_state *crtc_state)
>  	if (HAS_GMCH_DISPLAY(dev_priv))
>  		return false;
>  
> +	if (bpc == 10 && INTEL_GEN(dev_priv) < 11)
> +		return false;
> +
>  	if (crtc_state->pipe_bpp <= 8*3)
>  		return false;
>  
> @@ -1587,7 +1599,7 @@ static bool hdmi_12bpc_possible(const struct intel_crtc_state *crtc_state)
>  		return false;
>  
>  	/*
> -	 * HDMI 12bpc affects the clocks, so it's only possible
> +	 * HDMI deep color affects the clocks, so it's only possible
>  	 * when not cloning with other encoder types.
>  	 */
>  	if (crtc_state->output_types != 1 << INTEL_OUTPUT_HDMI)
> @@ -1602,16 +1614,24 @@ static bool hdmi_12bpc_possible(const struct intel_crtc_state *crtc_state)
>  		if (crtc_state->ycbcr420) {
>  			const struct drm_hdmi_info *hdmi = &info->hdmi;
>  
> -			if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36))
> +			if (bpc == 12 && !(hdmi->y420_dc_modes &
> +					   DRM_EDID_YCBCR420_DC_36))
> +				return false;
> +			else if (bpc == 10 && !(hdmi->y420_dc_modes &
> +						DRM_EDID_YCBCR420_DC_30))
>  				return false;
>  		} else {
> -			if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36))
> +			if (bpc == 12 && !(info->edid_hdmi_dc_modes &
> +					   DRM_EDID_HDMI_DC_36))
> +				return false;
> +			else if (bpc == 10 && !(info->edid_hdmi_dc_modes &
> +						DRM_EDID_HDMI_DC_30))
>  				return false;
>  		}
>  	}
>  
>  	/* Display WA #1139: glk */
> -	if (IS_GLK_REVID(dev_priv, 0, GLK_REVID_A1) &&
> +	if (bpc == 12 && IS_GLK_REVID(dev_priv, 0, GLK_REVID_A1) &&
>  	    crtc_state->base.adjusted_mode.htotal > 5460)
>  		return false;
>  
> @@ -1621,7 +1641,8 @@ static bool hdmi_12bpc_possible(const struct intel_crtc_state *crtc_state)
>  static bool
>  intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>  			   struct intel_crtc_state *config,
> -			   int *clock_12bpc, int *clock_8bpc)
> +			   int *clock_12bpc, int *clock_10bpc,
> +			   int *clock_8bpc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc);
>  
> @@ -1633,6 +1654,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>  	/* YCBCR420 TMDS rate requirement is half the pixel clock */
>  	config->port_clock /= 2;
>  	*clock_12bpc /= 2;
> +	*clock_10bpc /= 2;
>  	*clock_8bpc /= 2;
>  	config->ycbcr420 = true;
>  
> @@ -1660,6 +1682,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_digital_connector_state *intel_conn_state =
>  		to_intel_digital_connector_state(conn_state);
>  	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
> +	int clock_10bpc = clock_8bpc * 5 / 4;
>  	int clock_12bpc = clock_8bpc * 3 / 2;
>  	int desired_bpp;
>  	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
> @@ -1683,12 +1706,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) {
>  		pipe_config->pixel_multiplier = 2;
>  		clock_8bpc *= 2;
> +		clock_10bpc *= 2;
>  		clock_12bpc *= 2;
>  	}
>  
>  	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
>  		if (!intel_hdmi_ycbcr420_config(connector, pipe_config,
> -						&clock_12bpc, &clock_8bpc)) {
> +						&clock_12bpc, &clock_10bpc,
> +						&clock_8bpc)) {
>  			DRM_ERROR("Can't support YCBCR420 output\n");
>  			return false;
>  		}
> @@ -1706,18 +1731,25 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	}
>  
>  	/*
> -	 * HDMI is either 12 or 8, so if the display lets 10bpc sneak
> -	 * through, clamp it down. Note that g4x/vlv don't support 12bpc hdmi
> -	 * outputs. We also need to check that the higher clock still fits
> -	 * within limits.
> +	 * Note that g4x/vlv don't support 12bpc hdmi outputs. We also need
> +	 * to check that the higher clock still fits within limits.
>  	 */
> -	if (hdmi_12bpc_possible(pipe_config) &&
> -	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true, force_dvi) == MODE_OK) {
> +	if (hdmi_deep_color_possible(pipe_config, 12) &&
> +	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc,
> +				  true, force_dvi) == MODE_OK) {
>  		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
>  		desired_bpp = 12*3;
>  
>  		/* Need to adjust the port link by 1.5x for 12bpc. */
>  		pipe_config->port_clock = clock_12bpc;
> +	} else if (hdmi_deep_color_possible(pipe_config, 10) &&
> +		   hdmi_port_clock_valid(intel_hdmi, clock_10bpc,
> +					 true, force_dvi) == MODE_OK) {
> +		DRM_DEBUG_KMS("picking bpc to 10 for HDMI output\n");
> +		desired_bpp = 10 * 3;
> +
> +		/* Need to adjust the port link by 1.25x for 10bpc. */
> +		pipe_config->port_clock = clock_10bpc;
>  	} else {
>  		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
>  		desired_bpp = 8*3;
> -- 
> 2.14.3

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