[PATCH v2 04/11] drm/i915/dp_mst: Account for channel coding efficiency in the DSC DPT bpp limit

[Nautiyal, Ankit K]

On 4/17/2024 3:40 AM, Imre Deak wrote:
> The DSC DPT interface BW limit check should take into account the link
> clock's (aka DDI clock in bspec) channel coding efficiency overhead.
> Bspec suggests that the FEC overhead needs to be applied, however HW
> people claim this isn't the case, nor is any overhead applicable.
>
> However based on testing various 5k/6k modes both on the DELL U3224KBA
> monitor and the Unigraf UCD-500 CTS test device, both the channel coding
> efficiency (which includes the FEC overhead) and an additional 3%
> overhead must be accounted for to get these modes working.
>
> Bspec: 49259
>
> v2:
> - Apply an additional 3% overhead, add a commit log and code comment
>    about these overheads and the relation to the Bspec BW limit formula.
>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com> (v1)
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp_mst.c | 23 +++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 58eb6bf33c92e..0448cc343a33f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -59,11 +59,30 @@ static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp
>   	if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 14 && dsc) {
>   		int output_bpp = bpp;
>   		int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock);
> +		/*
> +		 * Bspec/49259 suggests that the FEC overhead needs to be
> +		 * applied here, though HW people claim that neither this FEC
> +		 * or any other overhead is applicable here (that is the actual
> +		 * available_bw is just symbol_clock * 72). However based on
> +		 * testing on MTL-P the
> +		 * - DELL U3224KBA display
> +		 * - Unigraf UCD-500 CTS test sink
> +		 * devices the
> +		 * - 5120x2880/995.59Mhz
> +		 * - 6016x3384/1357.23Mhz
> +		 * - 6144x3456/1413.39Mhz
> +		 * modes (all which had a DPT limit on the above devices),
nitpick : all 'of' which
> +		 * both the channel coding efficiency and an additional 3%
> +		 * overhead needs to be accounted for.
> +		 */
> +		int available_bw = mul_u32_u32(symbol_clock * 72,
> +					       drm_dp_bw_channel_coding_efficiency(true)) /
> +				   1030000;

IMHO, generally overhead of 3% would be better represented by 
multiplying available bandwidth with 97%, but since this is measured to 
be around 3%, this way seems simpler.

Patch looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>



>   
>   		if (output_bpp * adjusted_mode->crtc_clock >
> -		    symbol_clock * 72) {
> +		    available_bw) {
>   			drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n",
> -				    output_bpp * adjusted_mode->crtc_clock, symbol_clock * 72);
> +				    output_bpp * adjusted_mode->crtc_clock, available_bw);
>   			return -EINVAL;
>   		}
>   	}