Re: [PATCH v5 13/28] drm/i915/dp: Compute DSC pipe config in atomic check

[Manasi Navare <manasi.d.navare@intel.com>]

Hi Jani,

This patch has a verbal ACK from you when we went over the patch together,
This is rebased on top of edp fast/narrow optimized config like we discussed.
could you please review this?

Regards
Manasi

On Fri, Oct 05, 2018 at 04:22:51PM -0700, Manasi Navare wrote:
> DSC params like the enable, compressed bpp, slice ocunt and
> dsc_split are added to the intel_crtc_state. These parameters
> are set based on the requested mode and available link parameters
> during the pipe configuration in atomic check phase.
> These values are then later used to populate the remaining DSC
> and RC parameters before enbaling DSC in atomic commit.
> 
> v9:
> * Rebase on top of drm-tip that now uses fast_narrow config
> for edp (Manasi)
> v8:
> * Check for DSC bpc not 0 (manasi)
> 
> v7:
> * Fix indentation in compute_m_n (Manasi)
> 
> v6 (From Gaurav):
> * Remove function call of intel_dp_compute_dsc_params() and
> invoke intel_dp_compute_dsc_params() in the patch where
> it is defined to fix compilation warning (Gaurav)
> 
> v5:
> Add drm_dsc_cfg in intel_crtc_state (Manasi)
> 
> v4:
> * Rebase on refactoring of intel_dp_compute_config on tip (Manasi)
> * Add a comment why we need to check PSR while enabling DSC (Gaurav)
> 
> v3:
> * Check PPR > max_cdclock to use 2 VDSC instances (Ville)
> 
> v2:
> * Add if-else for eDP/DP (Gaurav)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  20 +++-
>  drivers/gpu/drm/i915/intel_display.h |   3 +-
>  drivers/gpu/drm/i915/intel_dp.c      | 170 ++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_dp_mst.c  |   2 +-
>  4 files changed, 155 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 36434c5359b1..4ebf7c83085c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6532,7 +6532,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  
>  	pipe_config->fdi_lanes = lane;
>  
> -	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> +	intel_link_compute_m_n(pipe_config->pipe_bpp, 0, lane, fdi_dotclock,
>  			       link_bw, &pipe_config->fdi_m_n, false);
>  
>  	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
> @@ -6767,17 +6767,25 @@ static void compute_m_n(unsigned int m, unsigned int n,
>  }
>  
>  void
> -intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> +intel_link_compute_m_n(int bits_per_pixel, uint16_t compressed_bpp,
> +		       int nlanes,
>  		       int pixel_clock, int link_clock,
>  		       struct intel_link_m_n *m_n,
>  		       bool constant_n)
>  {
>  	m_n->tu = 64;
>  
> -	compute_m_n(bits_per_pixel * pixel_clock,
> -		    link_clock * nlanes * 8,
> -		    &m_n->gmch_m, &m_n->gmch_n,
> -		    constant_n);
> +	/* For DSC, Data M/N calculation uses compressed BPP */
> +	if (compressed_bpp)
> +		compute_m_n(compressed_bpp * pixel_clock,
> +			    link_clock * nlanes * 8,
> +			    &m_n->gmch_m, &m_n->gmch_n,
> +			    constant_n);
> +	else
> +		compute_m_n(bits_per_pixel * pixel_clock,
> +			    link_clock * nlanes * 8,
> +			    &m_n->gmch_m, &m_n->gmch_n,
> +			    constant_n);
>  
>  	compute_m_n(pixel_clock, link_clock,
>  		    &m_n->link_m, &m_n->link_n,
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 9fac67e31205..9eaba1bccae8 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -402,7 +402,8 @@ struct intel_link_m_n {
>  	     (__i)++) \
>  		for_each_if(plane)
>  
> -void intel_link_compute_m_n(int bpp, int nlanes,
> +void intel_link_compute_m_n(int bpp, uint16_t compressed_bpp,
> +			    int nlanes,
>  			    int pixel_clock, int link_clock,
>  			    struct intel_link_m_n *m_n,
>  			    bool constant_n);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8e6891356d5b..05300d82b202 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -47,6 +47,8 @@
>  
>  /* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */
>  #define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER	61440
> +#define DP_DSC_MIN_SUPPORTED_BPC		8
> +#define DP_DSC_MAX_SUPPORTED_BPC		10
>  
>  /* DP DSC throughput values used for slice count calculations KPixels/s */
>  #define DP_DSC_PEAK_PIXEL_RATE			2720000
> @@ -1894,6 +1896,16 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
>  		}
>  	}
>  
> +	/* If DSC is supported, use the max value reported by panel */
> +	if (INTEL_GEN(dev_priv) >= 10 &&
> +	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
> +		bpc = min_t(u8,
> +			    drm_dp_dsc_sink_max_color_depth(intel_dp->dsc_dpcd),
> +			    DP_DSC_MAX_SUPPORTED_BPC);
> +		if (bpc)
> +			bpp = 3 * bpc;
> +	}
> +
>  	return bpp;
>  }
>  
> @@ -1954,14 +1966,11 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  				link_clock = intel_dp->common_rates[clock];
>  				link_avail = intel_dp_max_data_rate(link_clock,
>  								    lane_count);
> -
> -				if (mode_rate <= link_avail) {
> -					pipe_config->lane_count = lane_count;
> -					pipe_config->pipe_bpp = bpp;
> -					pipe_config->port_clock = link_clock;
> -
> +				pipe_config->lane_count = lane_count;
> +				pipe_config->pipe_bpp = bpp;
> +				pipe_config->port_clock = link_clock;
> +				if (mode_rate <= link_avail)
>  					return true;
> -				}
>  			}
>  		}
>  	}
> @@ -1990,14 +1999,11 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
>  				link_clock = intel_dp->common_rates[clock];
>  				link_avail = intel_dp_max_data_rate(link_clock,
>  								    lane_count);
> -
> -				if (mode_rate <= link_avail) {
> -					pipe_config->lane_count = lane_count;
> -					pipe_config->pipe_bpp = bpp;
> -					pipe_config->port_clock = link_clock;
> -
> +				pipe_config->lane_count = lane_count;
> +				pipe_config->pipe_bpp = bpp;
> +				pipe_config->port_clock = link_clock;
> +				if (mode_rate <= link_avail)
>  					return true;
> -				}
>  			}
>  		}
>  	}
> @@ -2005,14 +2011,88 @@ intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
>  	return false;
>  }
>  
> +static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> +					struct intel_crtc_state *pipe_config,
> +					struct link_config_limits *limits)
> +{
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> +	enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe;
> +	u16 dsc_max_output_bpp = 0;
> +	u8 dsc_dp_slice_count = 0;
> +
> +	if (INTEL_GEN(dev_priv) < 10 ||
> +	    !drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd))
> +		return false;
> +
> +	/* DP DSC only supported on Pipe B and C */
> +	if (pipe == PIPE_A && !intel_dp_is_edp(intel_dp))
> +		return false;
> +
> +	/* DSC not supported for DSC sink BPC < 8 */
> +	if (limits->max_bpp < 3 * DP_DSC_MIN_SUPPORTED_BPC) {
> +		DRM_DEBUG_KMS("No DSC support for less than 8bpc\n");
> +		return false;
> +	}
> +
> +	if (intel_dp_is_edp(intel_dp)) {
> +		pipe_config->dsc_params.compressed_bpp =
> +			drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4;
> +		pipe_config->dsc_params.slice_count =
> +			drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> +							true);
> +	} else {
> +		dsc_max_output_bpp =
> +			intel_dp_dsc_get_output_bpp(pipe_config->port_clock,
> +						    pipe_config->lane_count,
> +						    adjusted_mode->crtc_clock,
> +						    adjusted_mode->crtc_hdisplay);
> +		dsc_dp_slice_count =
> +			intel_dp_dsc_get_slice_count(intel_dp,
> +						     adjusted_mode->crtc_clock,
> +						     adjusted_mode->crtc_hdisplay);
> +		if (!(dsc_max_output_bpp && dsc_dp_slice_count)) {
> +			DRM_DEBUG_KMS("Compressed BPP/Slice Count not supported\n");
> +			return false;
> +		}
> +		pipe_config->dsc_params.compressed_bpp = dsc_max_output_bpp >> 4;
> +		pipe_config->dsc_params.slice_count = dsc_dp_slice_count;
> +	}
> +	/*
> +	 * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate
> +	 * is greater than the maximum Cdclock and if slice count is even
> +	 * then we need to use 2 VDSC instances.
> +	 */
> +	pipe_config->dsc_params.dsc_split = false;
> +	if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq) {
> +		if (pipe_config->dsc_params.slice_count > 1) {
> +			pipe_config->dsc_params.dsc_split = true;
> +		} else {
> +			DRM_DEBUG_KMS("Cannot split stream to use 2 VDSC instances\n");
> +			return false;
> +		}
> +	}
> +	pipe_config->dsc_params.compression_enable = true;
> +	DRM_DEBUG_KMS("DP DSC computed with Input Bpp = %d "
> +		      "Compressed Bpp = %d Slice Count = %d\n",
> +		      pipe_config->pipe_bpp,
> +		      pipe_config->dsc_params.compressed_bpp,
> +		      pipe_config->dsc_params.slice_count);
> +
> +	return true;
> +}
> +
>  static bool
>  intel_dp_compute_link_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	struct link_config_limits limits;
>  	int common_len;
> +	bool ret = false;
>  
>  	common_len = intel_dp_common_len_rate_limit(intel_dp,
>  						    intel_dp->max_link_rate);
> @@ -2026,7 +2106,9 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	limits.min_lane_count = 1;
>  	limits.max_lane_count = intel_dp_max_lane_count(intel_dp);
>  
> -	limits.min_bpp = 6 * 3;
> +	limits.min_bpp = (INTEL_GEN(dev_priv) >= 10 &&
> +			  drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) ?
> +		DP_DSC_MIN_SUPPORTED_BPC * 3 : 6 * 3;
>  	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
>  
>  	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> @@ -2051,7 +2133,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  		      intel_dp->common_rates[limits.max_clock],
>  		      limits.max_bpp, adjusted_mode->crtc_clock);
>  
> -	if (intel_dp_is_edp(intel_dp)) {
> +	if (intel_dp_is_edp(intel_dp))
>  		/*
>  		 * Optimize for fast and narrow. eDP 1.3 section 3.3 and eDP 1.4
>  		 * section A.1: "It is recommended that the minimum number of
> @@ -2061,26 +2143,48 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  		 * Note that we use the max clock and lane count for eDP 1.3 and
>  		 * earlier, and fast vs. wide is irrelevant.
>  		 */
> -		if (!intel_dp_compute_link_config_fast(intel_dp, pipe_config,
> -						       &limits))
> -			return false;
> -	} else {
> +		ret = intel_dp_compute_link_config_fast(intel_dp, pipe_config,
> +							&limits);
> +	else
>  		/* Optimize for slow and wide. */
> -		if (!intel_dp_compute_link_config_wide(intel_dp, pipe_config,
> -						       &limits))
> +		ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config,
> +							&limits);
> +
> +	/* enable compression if the mode doesn't fit available BW */
> +	if (!ret) {
> +		DRM_DEBUG_KMS("DP required Link rate %i does not fit available %i\n",
> +			      intel_dp_link_required(adjusted_mode->crtc_clock,
> +						     pipe_config->pipe_bpp),
> +			      intel_dp_max_data_rate(pipe_config->port_clock,
> +						     pipe_config->lane_count));
> +
> +		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
> +						 &limits))
>  			return false;
>  	}
>  
> -	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
> -		      pipe_config->lane_count, pipe_config->port_clock,
> -		      pipe_config->pipe_bpp);
> +	if (pipe_config->dsc_params.compression_enable) {
> +		DRM_DEBUG_KMS("DP lane count %d clock %d Input bpp %d Compressed bpp %d\n",
> +			      pipe_config->lane_count, pipe_config->port_clock,
> +			      pipe_config->pipe_bpp,
> +			      pipe_config->dsc_params.compressed_bpp);
>  
> -	DRM_DEBUG_KMS("DP link rate required %i available %i\n",
> -		      intel_dp_link_required(adjusted_mode->crtc_clock,
> -					     pipe_config->pipe_bpp),
> -		      intel_dp_max_data_rate(pipe_config->port_clock,
> -					     pipe_config->lane_count));
> +		DRM_DEBUG_KMS("DP link rate required %i available %i\n",
> +			      intel_dp_link_required(adjusted_mode->crtc_clock,
> +						     pipe_config->dsc_params.compressed_bpp),
> +			      intel_dp_max_data_rate(pipe_config->port_clock,
> +						     pipe_config->lane_count));
> +	} else {
> +		DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
> +			      pipe_config->lane_count, pipe_config->port_clock,
> +			      pipe_config->pipe_bpp);
>  
> +		DRM_DEBUG_KMS("DP link rate required %i available %i\n",
> +			      intel_dp_link_required(adjusted_mode->crtc_clock,
> +						     pipe_config->pipe_bpp),
> +			      intel_dp_max_data_rate(pipe_config->port_clock,
> +						     pipe_config->lane_count));
> +	}
>  	return true;
>  }
>  
> @@ -2159,7 +2263,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
>  	}
>  
> -	intel_link_compute_m_n(pipe_config->pipe_bpp, pipe_config->lane_count,
> +	intel_link_compute_m_n(pipe_config->pipe_bpp,
> +			       pipe_config->dsc_params.compressed_bpp,
> +			       pipe_config->lane_count,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> @@ -2168,7 +2274,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (intel_connector->panel.downclock_mode != NULL &&
>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
>  			pipe_config->has_drrs = true;
> -			intel_link_compute_m_n(pipe_config->pipe_bpp,
> +			intel_link_compute_m_n(pipe_config->pipe_bpp, 0,
>  					       pipe_config->lane_count,
>  					       intel_connector->panel.downclock_mode->clock,
>  					       pipe_config->port_clock,
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 43db2e9ac575..52a3217f13c5 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -83,7 +83,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  		return false;
>  	}
>  
> -	intel_link_compute_m_n(bpp, lane_count,
> +	intel_link_compute_m_n(bpp, 0, lane_count,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n,
> -- 
> 2.18.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel