Re: [PATCH 2/5] drm/i915/dp: Calculate DSC slice count based on per-slice peak throughput

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

On Fri, Sep 19, 2025 at 12:12:20AM +0300, Imre Deak wrote:
> Use the branch device's actual per-slice peak throughput to calculate
> the minimum number of required DSC slices, falling back to the
> hard-coded throughput values (as suggested by the DP Standard) if the
> device's reported throughput value is 0.
> 
> For now use the minimum of the two throughput values, which is ok,
> potentially resulting in a higher than required minimum slice count.
> This doesn't change the current way of using the same minimum throughput
> value regardless of the RGB/YUV output format used.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 47 +++++++++++++++++++++----
>  1 file changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2eab591a8ef56..a963a58c87372 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1011,6 +1011,26 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct intel_display *display,
>  	return bits_per_pixel;
>  }
>  
> +static int dsc_per_slice_throughput(struct intel_display *display, int mode_clock, int bw_code)

s/bw_code/throughput_mode/ or something maybe to make it match the spec
a bit better? "bw_code" is I think what we've been calling the
LINK_BW_SET value.

> +{
> +	switch (bw_code) {
> +	case 0:
> +		if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> +			return DP_DSC_MAX_ENC_THROUGHPUT_0;
> +		else
> +			return DP_DSC_MAX_ENC_THROUGHPUT_1;

These look like some unhelpful single use defines. I'd get
rid of them and just stick the actual numbers here.

> +	case 1:
> +		return 340000;
> +	case 2 ... 14:
> +		return 400000 + 50000 * (bw_code - 2);
> +	case 15:
> +		return 170000;
> +	default:
> +		drm_err(display->drm, "Invalid DSC peak throughput code\n");

This whole thing looks like it belongs in some drm helper.
Probably best to pass in struct drm_device from the start
so that moving it over will later be easier.

> +		return 340000;
> +	}
> +}
> +
>  u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
>  				int mode_clock, int mode_hdisplay,
>  				int num_joined_pipes)
> @@ -1018,13 +1038,28 @@ u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
>  	struct intel_display *display = to_intel_display(connector);
>  	u8 min_slice_count, i;
>  	int max_slice_width;
> +	int tp_rgb_yuv444;
> +	int tp_yuv422_420;
> +	u8 val;
>  
> -	if (mode_clock <= DP_DSC_PEAK_PIXEL_RATE)
> -		min_slice_count = DIV_ROUND_UP(mode_clock,
> -					       DP_DSC_MAX_ENC_THROUGHPUT_0);
> -	else
> -		min_slice_count = DIV_ROUND_UP(mode_clock,
> -					       DP_DSC_MAX_ENC_THROUGHPUT_1);
> +	val = connector->dp.dsc_dpcd[DP_DSC_PEAK_THROUGHPUT - DP_DSC_SUPPORT];
> +	tp_rgb_yuv444 = dsc_per_slice_throughput(display, mode_clock,
> +						 REG_FIELD_GET8(DP_DSC_THROUGHPUT_MODE_0_MASK,
> +								val));
> +	tp_yuv422_420 = dsc_per_slice_throughput(display, mode_clock,
> +						 REG_FIELD_GET8(DP_DSC_THROUGHPUT_MODE_1_MASK,
> +								val));
> +
> +	/*
> +	 * TODO: Use the throughput value specific to the actual RGB/YUV
> +	 * format of the output.
> +	 * For now use the smaller of these, which is ok, potentially
> +	 * resulting in a higher than required minimum slice count.
> +	 * The RGB/YUV444 throughput value should be always either equal
> +	 * or smaller than the YUV422/420 value, but let's not depend on
> +	 * this assumption.
> +	 */
> +	min_slice_count = DIV_ROUND_UP(mode_clock, min(tp_rgb_yuv444, tp_yuv422_420));
>  
>  	/*
>  	 * Due to some DSC engine BW limitations, we need to enable second
> -- 
> 2.49.1

-- 
Ville Syrjälä
Intel