[Intel-gfx] [PATCH 2/4] drm/i915/dp: Fix UHBR link M/N values

[Murthy, Arun R]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Imre
> Deak
> Sent: Tuesday, November 14, 2023 1:41 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 2/4] drm/i915/dp: Fix UHBR link M/N values
> 
> The link M/N ratio is the data rate / link symbol clock rate, fix things up
> accordingly. On DP 1.4 this ratio was correct as the link symbol clock rate in
> that case matched the link data rate (in bytes/sec units, the symbol size being 8
> bits), however it wasn't correct for UHBR rates where the symbol size is 32 bits.
> 
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 16 ++++++++-----
>  drivers/gpu/drm/i915/display/intel_dp.c      | 24 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.h      |  2 ++
>  3 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 24aebdb715e7d..c059eb0170a5b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2411,6 +2411,7 @@ intel_link_compute_m_n(u16 bits_per_pixel, int
> nlanes,
>  		       struct intel_link_m_n *m_n)
>  {
>  	u32 data_clock = bits_per_pixel * pixel_clock;
> +	u32 link_symbol_clock = intel_dp_link_symbol_clock(link_clock);
>  	u32 data_m;
>  	u32 data_n;
> 
> @@ -2431,7 +2432,7 @@ intel_link_compute_m_n(u16 bits_per_pixel, int
> nlanes,
>  		    0x8000000);
> 
>  	compute_m_n(&m_n->link_m, &m_n->link_n,
> -		    pixel_clock, link_clock,
> +		    pixel_clock, link_symbol_clock,
>  		    0x80000);
>  }
Better if this can be moved to intel_dp.c
Also per the spec the constant N values is 0x800000
The calculation of data M has dependency with DP symbol
> 
> @@ -3943,20 +3944,23 @@ int intel_dotclock_calculate(int link_freq,
>  			     const struct intel_link_m_n *m_n)  {
>  	/*
> -	 * The calculation for the data clock is:
> +	 * The calculation for the data clock -> pixel clock is:
>  	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
>  	 * But we want to avoid losing precison if possible, so:
>  	 * pixel_clock = ((m * link_clock * nr_lanes)/(n*bpp))
>  	 *
> -	 * and the link clock is simpler:
> -	 * link_clock = (m * link_clock) / n
> +	 * and for link freq (10kbs units) -> pixel clock it is:
> +	 * link_symbol_clock = link_freq * 10 / link_symbol_size
> +	 * pixel_clock = (m * link_symbol_clock) / n
> +	 *    or for more precision:
> +	 * pixel_clock = (m * link_freq * 10) / (n * link_symbol_size)
>  	 */
> 
>  	if (!m_n->link_n)
>  		return 0;
> 
> -	return DIV_ROUND_UP_ULL(mul_u32_u32(m_n->link_m, link_freq),
> -				m_n->link_n);
> +	return DIV_ROUND_UP_ULL(mul_u32_u32(m_n->link_m, link_freq *
> 10),
> +				m_n->link_n *
> intel_dp_link_symbol_size(link_freq));
>  }
> 
>  int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config) diff --git
> a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index f662d1ce5f72c..80e1e887432fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -132,6 +132,30 @@ bool intel_dp_is_uhbr(const struct intel_crtc_state
> *crtc_state)
>  	return intel_dp_is_uhbr_rate(crtc_state->port_clock);
>  }
> 
> +/**
> + * intel_dp_link_symbol_size - get the link symbol size for a given
> +link rate
> + * @rate: link rate in 10kbit/s units
> + *
> + * Returns the link symbol size in bits/symbol units depending on the
> +link
> + * rate -> channel coding.
> + */
> +int intel_dp_link_symbol_size(int rate) {
> +	return intel_dp_is_uhbr_rate(rate) ? 32 : 10; }
As per the spec this DP symbol is 32 for DP2.0 and 8 for DP1.4

Thanks and Regards,
Arun R Murthy
--------------------
> +
> +/**
> + * intel_dp_link_symbol_clock - convert link rate to link symbol clock
> + * @rate: link rate in 10kbit/s units
> + *
> + * Returns the link symbol clock frequency in kHz units depending on
> +the
> + * link rate and channel coding.
> + */
> +int intel_dp_link_symbol_clock(int rate) {
> +	return DIV_ROUND_CLOSEST(rate * 10,
> intel_dp_link_symbol_size(rate));
> +}
> +
>  static void intel_dp_set_default_sink_rates(struct intel_dp *intel_dp)  {
>  	intel_dp->sink_rates[0] = 162000;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index e80da67554196..64dbf8f192708 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -82,6 +82,8 @@ bool intel_dp_has_hdmi_sink(struct intel_dp *intel_dp);
> bool intel_dp_is_edp(struct intel_dp *intel_dp);  bool intel_dp_is_uhbr_rate(int
> rate);  bool intel_dp_is_uhbr(const struct intel_crtc_state *crtc_state);
> +int intel_dp_link_symbol_size(int rate); int
> +intel_dp_link_symbol_clock(int rate);
>  bool intel_dp_is_port_edp(struct drm_i915_private *dev_priv, enum port
> port);  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *dig_port,
>  				  bool long_hpd);
> --
> 2.39.2