diff mbox series

[2/4] drm/i915/dp: Fix UHBR link M/N values

Message ID 20231113201110.510724-2-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/i915/dp: Account for channel coding efficiency on UHBR links | expand

Commit Message

Imre Deak Nov. 13, 2023, 8:11 p.m. UTC
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@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(-)

Comments

Murthy, Arun R Nov. 14, 2023, 3:29 a.m. UTC | #1
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre
> Deak
> Sent: Tuesday, November 14, 2023 1:41 AM
> To: intel-gfx@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@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
Imre Deak Nov. 14, 2023, 7:43 a.m. UTC | #2
On Tue, Nov 14, 2023 at 05:29:35AM +0200, Murthy, Arun R wrote:
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre
> > Deak
> > Sent: Tuesday, November 14, 2023 1:41 AM
> > To: intel-gfx@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@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

The function is also used by non-DP outputs, so not sure. In any case
it would need to be a separate change.

> Also per the spec the constant N values is 0x800000

For the link M/N values I can't see this in the spec. It's mentioned in
the context of calculating data M/N. Changing that - if it makes sense -
should be in a separate patch.

> 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

On DP1.4 before the 8b/10b conversion the symbol size is 8 bits, after
the conversion (which is what @rate describes and for which the symbol
size is returned for) it's 10 bits.

> 
> 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
>
Murthy, Arun R Nov. 15, 2023, 1:29 p.m. UTC | #3
> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Tuesday, November 14, 2023 1:13 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/4] drm/i915/dp: Fix UHBR link M/N values
> 
> On Tue, Nov 14, 2023 at 05:29:35AM +0200, Murthy, Arun R wrote:
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > > Of Imre Deak
> > > Sent: Tuesday, November 14, 2023 1:41 AM
> > > To: intel-gfx@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@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
> 
> The function is also used by non-DP outputs, so not sure. In any case it would
> need to be a separate change.
> 
> > Also per the spec the constant N values is 0x800000
> 
> For the link M/N values I can't see this in the spec. It's mentioned in the context
> of calculating data M/N. Changing that - if it makes sense - should be in a
> separate patch.

Agree!

Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------
> 
> > 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
> 
> On DP1.4 before the 8b/10b conversion the symbol size is 8 bits, after the
> conversion (which is what @rate describes and for which the symbol size is
> returned for) it's 10 bits.
> 
> >
> > 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
> >
diff mbox series

Patch

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);
 }
 
@@ -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;
+}
+
+/**
+ * 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);