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

[Imre Deak]

On Wed, Apr 23, 2025 at 06:23:33PM +0530, Arun R Murthy wrote:
> 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)
> v4: Expose only drm_dp_link_symbol_cycles() (Imre)
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 51 +++++++++++++++++++++------------
>  include/drm/display/drm_dp_helper.h     |  2 ++
>  2 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 57828f2b7b5a0582ca4a6f2a9be2d5909fe8ad24..d33a46016504ce52857b7c4e63d866af9c1eff34 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4393,8 +4393,9 @@ 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)
> +static int drm_dp_link_data_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;
> @@ -4402,22 +4403,41 @@ static int drm_dp_link_symbol_cycles(int lane_count, int pixels, int bpp_x16,
>  	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)
> +/**
> + * drm_dp_link_symbol_cycles - calculate the link symbol count with/without dsc
> + * @lane_count: DP link lane count
> + * @pixels: number of pixels in a scanline
> + * @dsc_slice_count: number of slices

Nit: missed in my previous review, but could be clearer by:
@dsc_slice_count: number of slices for DSC or 0 for non-DSC

> + * @bpp_x16: bits per pixel in .4 binary fixed format
> + * @symbol_size: DP symbol size
> + * @is_mst: %true for MST and %false for SST
> + *
> + * Calculate the link symbol cycles for both DSC(@dsc_slice_count !=0) and
> + * non-DSC case(@dsc_slice_count == 0)and return the count.

Nit: keep one space before '(' and after ')'.

> + */
> +int drm_dp_link_symbol_cycles(int lane_count, int pixels, int dsc_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);
> -	int slice_eoc_cycles = is_mst ? 4 / lane_count : 1;
> +	int slice_count = dsc_slice_count ? : 1;
> +	int slice_data_cycles = drm_dp_link_data_symbol_cycles(lane_count,
> +							       pixels,
> +							       bpp_x16,
> +							       symbol_size,
> +							       is_mst);
> +	int slice_eoc_cycles = 0;
> +
> +	if (dsc_slice_count)
> +		slice_eoc_cycles = is_mst ? 4 / lane_count : 1;
>  
>  	return slice_count * (slice_data_cycles + slice_eoc_cycles);
>  }
> +EXPORT_SYMBOL(drm_dp_link_symbol_cycles);
>  
>  /**
>   * drm_dp_bw_overhead - Calculate the BW overhead of a DP link stream
>   * @lane_count: DP link lane count
>   * @hactive: pixel count of the active period in one scanline of the stream
> - * @dsc_slice_count: DSC slice count if @flags/DRM_DP_LINK_BW_OVERHEAD_DSC is set
> + * @dsc_slice_count: number of slices

Same nit as the above @dsc_slice_count one.

>   * @bpp_x16: bits per pixel in .4 binary fixed point
>   * @flags: DRM_DP_OVERHEAD_x flags

There is one more place referring to %DRM_DP_OVERHEAD_DSC in this
comment, could be changed to:

    * - compression mode (@dsc_slice_count != 0)

Regardless of the nits, patch looks ok, thanks:

Reviewed-by: Imre Deak <imre.deak at intel.com>

After this patch DRM_DP_BW_OVERHEAD_DSC will be only set by the i915
driver, but it won't be used anywhere, so in a follow patch the flag
could be removed.

>   *
> @@ -4486,15 +4506,10 @@ 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_symbol_cycles(lane_count, hactive,
> +						  dsc_slice_count,
> +						  bpp_x16, symbol_size,
> +						  is_mst);
>  
>  	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..7b19192c70313d66dce1b7ba40dd59c14f80a182 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_symbol_cycles(int lane_count, int pixels, int dsc_slice_count,
> +			      int bpp_x16, int symbol_size, bool is_mst);
>  
>  #endif /* _DRM_DP_HELPER_H_ */
> 
> -- 
> 2.25.1
>