> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Imre
> Deak
> Sent: Tuesday, November 14, 2023 1:41 AM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [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@xxxxxxxxx>
> ---
> 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
