[PATCH v3 1/3] drm/display/dp: Export fn to calculate link symbol cycles

[Kandpal, Suraj]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Arun R
> Murthy
> Sent: Thursday, April 17, 2025 4:22 PM
> To: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; intel-
> xe at lists.freedesktop.org
> Cc: Govindapillai, Vinod <vinod.govindapillai at intel.com>; Deak, Imre
> <imre.deak at intel.com>; Murthy, Arun R <arun.r.murthy at intel.com>
> Subject: [PATCH v3 1/3] drm/display/dp: Export fn to calculate link symbol
> cycles
> 
> Unify the function to calculate the link symbol cycles for both dsc and non-dsc
> case and export the function so that it can be used in the respective platform
> display drivers for other calculations.
> 
> v2: unify the fn for both dsc and non-dsc case (Imre)
> v3: rename drm_dp_link_symbol_cycles to drm_dp_link_data_symbol_cycles
>     retain slice_eoc_cycles as is (Imre)
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 53 +++++++++++++++++--------------
> --
>  include/drm/display/drm_dp_helper.h     |  2 ++
>  2 files changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index
> 57828f2b7b5a0582ca4a6f2a9be2d5909fe8ad24..5ce8ccc3310fb71b39ea5f74c4
> 022474c180f727 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4392,26 +4392,33 @@ EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
> 
>  #endif
> 
> -/* See DP Standard v2.1 2.6.4.4.1.1, 2.8.4.4, 2.8.7 */ -static int
> drm_dp_link_symbol_cycles(int lane_count, int pixels, int bpp_x16,
> -				     int symbol_size, bool is_mst)
> -{
> -	int cycles = DIV_ROUND_UP(pixels * bpp_x16, 16 * symbol_size *
> lane_count);
> -	int align = is_mst ? 4 / lane_count : 1;
> -
> -	return ALIGN(cycles, align);
> -}
> -
> -static int drm_dp_link_dsc_symbol_cycles(int lane_count, int pixels, int
> slice_count,
> -					 int bpp_x16, int symbol_size, bool
> is_mst)
> -{
> -	int slice_pixels = DIV_ROUND_UP(pixels, slice_count);
> -	int slice_data_cycles = drm_dp_link_symbol_cycles(lane_count,
> slice_pixels,
> -							  bpp_x16,
> symbol_size, is_mst);
> +/**
> + * drm_dp_link_data_symbol_cycles - calculate the link symbol count
> + * @lane_coount: DP link lane count
 
Typo "lane_count"

> + * @pixels: horizontal active pixels
> + * @bpp_x16: bits per pixel in .4 binary fixed format
> + * @symbol_size: DP symbol size
> + * @is_mst: is mst or sst
> + * @slice_count: number of slices
> + *
> + * Calculate the link symbol cycles for both dsc and non dsc case and
> + * return the count.

Lets add the DP spec to be referred seems like it was missed

> + */
> +int drm_dp_link_data_symbol_cycles(int lane_count, int pixels, int bpp_x16,
> +				   int symbol_size, bool is_mst, int slice_count)
> {
> +	int slice_pixels = slice_count ? DIV_ROUND_UP(pixels, slice_count) :
> +					 pixels;
> +	int cycles = DIV_ROUND_UP(slice_pixels * bpp_x16,
> +				  (6 * symbol_size * lane_count));

Shouldn't this be 16
Also one thing I see which was there previously to while calculating is we ignore the two ceils 
Inside the function and merge it into a single div_round_up which may bring as slight variation in calculation
For example for non dsc case
Spec says
HACT_LL_SYM_CYC_CNT
= CEIL(CEIL(HACT_WIDTH / 4) × PIX_BPP / SYMBOL_SIZE)
HACT_ML_SYM_CYC_CNT
= HACT_LL_SYM_CYC_CNT × 4 / PHY_LANE_CNT

But we do
DIV_ROUND_UP(slice_pixels * bpp_x16,	  (6 * symbol_size * lane_count));

Which translates to 
CEIL( (HACT_WIDTH* BPP*4)/(16 *SYMBOL_SIZE *LANECOUNT))

Which does not seem to match the calculation exactly as what was said in the spec
Lets have an intermediate ll_symbol_cycle variable too should make the calculations
More clearer and precise according to me.

Also for dsc case lets have chunk size instead of reusing slice pixels.

Regards,
Suraj Kandpal

> +	int slice_data_cycles = ALIGN(cycles, is_mst ? (4 / lane_count) : 1);
>  	int slice_eoc_cycles = is_mst ? 4 / lane_count : 1;
> 
> -	return slice_count * (slice_data_cycles + slice_eoc_cycles);
> +	return slice_count ? (slice_count *
> +			      (slice_data_cycles + slice_eoc_cycles)) :
> +			      slice_data_cycles;
>  }
> +EXPORT_SYMBOL(drm_dp_link_data_symbol_cycles);
> 
>  /**
>   * drm_dp_bw_overhead - Calculate the BW overhead of a DP link stream @@
> -4486,15 +4493,9 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
>  	WARN_ON((flags & DRM_DP_BW_OVERHEAD_UHBR) &&
>  		(flags & DRM_DP_BW_OVERHEAD_FEC));
> 
> -	if (flags & DRM_DP_BW_OVERHEAD_DSC)
> -		symbol_cycles = drm_dp_link_dsc_symbol_cycles(lane_count,
> hactive,
> -							      dsc_slice_count,
> -							      bpp_x16,
> symbol_size,
> -							      is_mst);
> -	else
> -		symbol_cycles = drm_dp_link_symbol_cycles(lane_count,
> hactive,
> -							  bpp_x16,
> symbol_size,
> -							  is_mst);
> +	symbol_cycles = drm_dp_link_data_symbol_cycles(lane_count,
> hactive,
> +						       bpp_x16, symbol_size,
> +						       is_mst, dsc_slice_count);
> 
>  	return DIV_ROUND_UP_ULL(mul_u32_u32(symbol_cycles *
> symbol_size * lane_count,
>  					    overhead * 16),
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index
> d9614e2c89397536f44bb7258e894628ae1dccc9..98bbbe98e5bc0ce0f9cdf513b
> 2c5ea90bb5caffb 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -971,5 +971,7 @@ int drm_dp_bw_channel_coding_efficiency(bool
> is_uhbr);  int drm_dp_max_dprx_data_rate(int max_link_rate, int max_lanes);
> 
>  ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct
> dp_sdp *sdp);
> +int drm_dp_link_data_symbol_cycles(int lane_count, int pixels, int bpp_x16,
> +				   int symbol_size, bool is_mst, int slice_count);
> 
>  #endif /* _DRM_DP_HELPER_H_ */
> 
> --
> 2.25.1