Re: [PATCH 08/50] drm/i915/dp: Use the effective data rate for DP BW calculation

[Luca Coelho <luca@coelho.fi>]

On Thu, 2025-11-27 at 19:49 +0200, Imre Deak wrote:
> Use intel_dp_effective_data_rate() to calculate the required link BW for
> eDP, DP-SST and MST links. This ensures that the BW is calculated the
> same way for all DP output types, during mode validation as well as
> during state computation. This approach also allows for accounting with
> BW overheads due to the SSC, DSC, FEC being enabled on a link, as well
> as due to the MST symbol alignment on the link. Accounting for these
> overheads will be added by follow-up changes.
> 
> This way also computes the stream BW on a UHBR link correctly, using the
> corresponding symbol size to effective data size ratio (i.e. ~97% link
> BW utilization for UHBR vs. only ~80% for non-UHBR).
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c       | 40 +++++++++++--------
>  drivers/gpu/drm/i915/display/intel_dp.h       |  4 +-
>  .../drm/i915/display/intel_dp_link_training.c |  4 +-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 +-
>  4 files changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4556a57db7c02..aa55a81a9a9bf 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -453,15 +453,15 @@ int intel_dp_link_bw_overhead(int link_clock, int lane_count, int hdisplay,
>  /*
>   * The required data bandwidth for a mode with given pixel clock and bpp. This
>   * is the required net bandwidth independent of the data bandwidth efficiency.
> - *
> - * TODO: check if callers of this functions should use
> - * intel_dp_effective_data_rate() instead.
>   */
> -int
> -intel_dp_link_required(int pixel_clock, int bpp)
> +int intel_dp_link_required(int link_clock, int lane_count,
> +			   int mode_clock, int mode_hdisplay,
> +			   int link_bpp_x16, unsigned long bw_overhead_flags)
>  {
> -	/* pixel_clock is in kHz, divide bpp by 8 for bit to Byte conversion */
> -	return DIV_ROUND_UP(pixel_clock * bpp, 8);
> +	int bw_overhead = intel_dp_link_bw_overhead(link_clock, lane_count, mode_hdisplay,
> +						    0, link_bpp_x16, bw_overhead_flags);
> +
> +	return intel_dp_effective_data_rate(mode_clock, link_bpp_x16, bw_overhead);
>  }
>  
>  /**
> @@ -1531,7 +1531,9 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  	max_rate = intel_dp_max_link_data_rate(intel_dp, max_link_clock, max_lanes);
>  
>  	link_bpp_x16 = intel_dp_mode_min_link_bpp_x16(connector, mode);
> -	mode_rate = intel_dp_link_required(target_clock, fxp_q4_to_int_roundup(link_bpp_x16));
> +	mode_rate = intel_dp_link_required(max_link_clock, max_lanes,
> +					   target_clock, mode->hdisplay,
> +					   link_bpp_x16, 0);
>  
>  	if (intel_dp_has_dsc(connector)) {
>  		int pipe_bpp;
> @@ -1838,7 +1840,7 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  				  const struct link_config_limits *limits)
>  {
>  	int bpp, i, lane_count, clock = intel_dp_mode_clock(pipe_config, conn_state);
> -	int mode_rate, link_rate, link_avail;
> +	int link_rate, link_avail;
>  
>  	for (bpp = fxp_q4_to_int(limits->link.max_bpp_x16);
>  	     bpp >= fxp_q4_to_int(limits->link.min_bpp_x16);
> @@ -1846,8 +1848,6 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  		int link_bpp_x16 =
>  			intel_dp_output_format_link_bpp_x16(pipe_config->output_format, bpp);
>  
> -		mode_rate = intel_dp_link_required(clock, fxp_q4_to_int_roundup(link_bpp_x16));
> -
>  		for (i = 0; i < intel_dp->num_common_rates; i++) {
>  			link_rate = intel_dp_common_rate(intel_dp, i);
>  			if (link_rate < limits->min_rate ||
> @@ -1857,11 +1857,17 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  			for (lane_count = limits->min_lane_count;
>  			     lane_count <= limits->max_lane_count;
>  			     lane_count <<= 1) {
> +				const struct drm_display_mode *adjusted_mode =
> +					&pipe_config->hw.adjusted_mode;
> +				int mode_rate =
> +					intel_dp_link_required(link_rate, lane_count,
> +							       clock, adjusted_mode->hdisplay,
> +							       link_bpp_x16, 0);
> +
>  				link_avail = intel_dp_max_link_data_rate(intel_dp,
>  									 link_rate,
>  									 lane_count);
>  
> -
>  				if (mode_rate <= link_avail) {
>  					pipe_config->lane_count = lane_count;
>  					pipe_config->pipe_bpp = bpp;
> @@ -2724,11 +2730,13 @@ int intel_dp_config_required_rate(const struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> -	int bpp = crtc_state->dsc.compression_enable ?
> -		fxp_q4_to_int_roundup(crtc_state->dsc.compressed_bpp_x16) :
> -		crtc_state->pipe_bpp;
> +	int link_bpp_x16 = crtc_state->dsc.compression_enable ?
> +		crtc_state->dsc.compressed_bpp_x16 :
> +		fxp_q4_from_int(crtc_state->pipe_bpp);
>  
> -	return intel_dp_link_required(adjusted_mode->crtc_clock, bpp);
> +	return intel_dp_link_required(crtc_state->port_clock, crtc_state->lane_count,
> +				      adjusted_mode->crtc_clock, adjusted_mode->hdisplay,
> +				      link_bpp_x16, 0);
>  }
>  
>  bool intel_dp_joiner_needs_dsc(struct intel_display *display,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index d7f9410129f49..30eebb8cad6d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -119,7 +119,9 @@ bool intel_dp_source_supports_tps4(struct intel_display *display);
>  
>  int intel_dp_link_bw_overhead(int link_clock, int lane_count, int hdisplay,
>  			      int dsc_slice_count, int bpp_x16, unsigned long flags);
> -int intel_dp_link_required(int pixel_clock, int bpp);
> +int intel_dp_link_required(int link_clock, int lane_count,
> +			   int mode_clock, int mode_hdisplay,
> +			   int link_bpp_x16, unsigned long bw_overhead_flags);
>  int intel_dp_effective_data_rate(int pixel_clock, int bpp_x16,
>  				 int bw_overhead);
>  int intel_dp_max_link_data_rate(struct intel_dp *intel_dp,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index aad5fe14962f9..54c585c59b900 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1195,7 +1195,9 @@ static bool intel_dp_can_link_train_fallback_for_edp(struct intel_dp *intel_dp,
>  		intel_panel_preferred_fixed_mode(intel_dp->attached_connector);
>  	int mode_rate, max_rate;
>  
> -	mode_rate = intel_dp_link_required(fixed_mode->clock, 18);
> +	mode_rate = intel_dp_link_required(link_rate, lane_count,
> +					   fixed_mode->clock, fixed_mode->hdisplay,
> +					   fxp_q4_from_int(18), 0);
>  	max_rate = intel_dp_max_link_data_rate(intel_dp, link_rate, lane_count);
>  	if (mode_rate > max_rate)
>  		return false;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index c1058b4a85d02..e4dd6b4ca0512 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1489,7 +1489,9 @@ mst_connector_mode_valid_ctx(struct drm_connector *_connector,
>  
>  	max_rate = intel_dp_max_link_data_rate(intel_dp,
>  					       max_link_clock, max_lanes);
> -	mode_rate = intel_dp_link_required(mode->clock, min_bpp);
> +	mode_rate = intel_dp_link_required(max_link_clock, max_lanes,
> +					   mode->clock, mode->hdisplay,
> +					   fxp_q4_from_int(min_bpp), 0);
>  
>  	/*
>  	 * TODO:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.