On Fri, Mar 21, 2025 at 04:56:47PM +0530, Ankit Nautiyal wrote:
> LINK_N register has bits 31:24 for extended link N value used for
> HDMI2.1 and for an alternate mode of operation of DP TG DDA
> (Bspec:50488).
>
> Add support for these extra bits.
>
> For displays with version 14 or higher, the `PIPE_LINK_N1_EXTENDED_MASK`
> (bits 31:24) is used to handle the extended link N bits.
> For older platforms, the `DATA_LINK_M_N_MASK` (bits 23:0) is used to
> handle the standard link N bits. This distinction ensures clarity and
> maintains the semantics for platforms that support the extended bits.
> In subsequent changes the logic is updated to conditionally apply the
> extended link N bits.
>
> v2: Drop extra link_n_ext member. (Jani)
> v3: Avoid link_n_ext in set_m_n helper. (Jani)
> v4: Rebase, and update commit message.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++++++--
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3afb85fe8536..8fb0df388571 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2583,14 +2583,22 @@ void intel_set_m_n(struct intel_display *display,
> i915_reg_t data_m_reg, i915_reg_t data_n_reg,
> i915_reg_t link_m_reg, i915_reg_t link_n_reg)
> {
> + u32 link_n = m_n->link_n;
> +
> intel_de_write(display, data_m_reg, TU_SIZE(m_n->tu) | m_n->data_m);
> intel_de_write(display, data_n_reg, m_n->data_n);
> intel_de_write(display, link_m_reg, m_n->link_m);
> +
> + if (DISPLAY_VER(display) >= 14)
> + link_n &= ~PIPE_LINK_N1_EXTENDED_MASK;
> + else
> + link_n &= DATA_LINK_M_N_MASK;
There should never be anything in those bits or we've screwed up
somewhere.
The actual w/a for the M/N > 10.0 looks like a pile of annoying
hacks in the hardware. I wonder if we could just live without it
to avoid complicating the code?
As for the 10.0 limit, I suspect there's nothing platform specific
about it and it's always been there. It's just not possible to hit
it with 8b/10b encoding. The correct place to handle this would seem
to be the link rate calculation, ie. just bump up the link rate until
the limit is no longer an issue (or error out if we can't increase the
link rate sufficiently).
> +
> /*
> * On BDW+ writing LINK_N arms the double buffered update
> * of all the M/N registers, so it must be written last.
> */
> - intel_de_write(display, link_n_reg, m_n->link_n);
> + intel_de_write(display, link_n_reg, link_n);
> }
>
> bool intel_cpu_transcoder_has_m2_n2(struct intel_display *display,
> @@ -3279,7 +3287,13 @@ void intel_get_m_n(struct intel_display *display,
> i915_reg_t link_m_reg, i915_reg_t link_n_reg)
> {
> m_n->link_m = intel_de_read(display, link_m_reg) & DATA_LINK_M_N_MASK;
> - m_n->link_n = intel_de_read(display, link_n_reg) & DATA_LINK_M_N_MASK;
> + m_n->link_n = intel_de_read(display, link_n_reg);
> +
> + if (DISPLAY_VER(display) >= 14)
> + m_n->link_n &= ~PIPE_LINK_N1_EXTENDED_MASK;
> + else
> + m_n->link_n &= DATA_LINK_M_N_MASK;
> +
> m_n->data_m = intel_de_read(display, data_m_reg) & DATA_LINK_M_N_MASK;
> m_n->data_n = intel_de_read(display, data_n_reg) & DATA_LINK_M_N_MASK;
> m_n->tu = REG_FIELD_GET(TU_SIZE_MASK, intel_de_read(display, data_m_reg)) + 1;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c5064eebe063..a2054aced4f8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1869,6 +1869,8 @@
>
> #define _PIPEA_LINK_N1 0x60044
> #define _PIPEB_LINK_N1 0x61044
> +#define PIPE_LINK_N1_EXTENDED_MASK REG_GENMASK(31, 24)
> +#define PIPE_LINK_N1_EXTENDED(val) REG_FIELD_PREP(PIPE_LINK_N1_EXTENDED_MASK, (val))
> #define PIPE_LINK_N1(dev_priv, tran) _MMIO_TRANS2(dev_priv, tran, _PIPEA_LINK_N1)
>
> #define _PIPEA_LINK_M2 0x60048
> --
> 2.45.2
--
Ville Syrjälä
Intel
