[PATCH] drm/i915/display: UHBR rates for Thunderbolt
Kahola, Mika
mika.kahola at intel.com
Tue Dec 17 10:52:03 UTC 2024
> -----Original Message-----
> From: Deak, Imre <imre.deak at intel.com>
> Sent: Monday, 16 December 2024 18.06
> To: Kahola, Mika <mika.kahola at intel.com>
> Cc: intel-xe at lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/display: UHBR rates for Thunderbolt
>
> On Mon, Dec 16, 2024 at 02:44:22PM +0200, Mika Kahola wrote:
> > tbt-alt mode is missing uhbr rates 10G and 20G. This requires requires
> > pll clock rates 312.5 MHz and 625 MHz to be added, respectively. The
> > uhbr rates are supported only form PTL+ platforms.
> >
> > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
>
> I presume all display related changes should be sent to the intel-gfx ML as well for
> review, even though the change only affects platforms supported by xe only.
Ok, I will float next version to intel-gfx mailing list as well.
>
> > ---
> > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 27
> > ++++++++++++++++--- .../gpu/drm/i915/display/intel_cx0_phy_regs.h |
> > 4 +++
> > 2 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index cc734079c3b8..e0bd47e684c5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -3070,7 +3070,10 @@ int intel_mtl_tbt_calc_port_clock(struct
> > intel_encoder *encoder)
> >
> > val = intel_de_read(display, XELPDP_PORT_CLOCK_CTL(display,
> > encoder->port));
> >
> > - clock = REG_FIELD_GET(XELPDP_DDI_CLOCK_SELECT_MASK, val);
> > + if (DISPLAY_VER(display) >= 30)
> > + clock = REG_FIELD_GET(XE3LPDP_DDI_CLOCK_SELECT_MASK,
> val);
> > + else
> > + clock = REG_FIELD_GET(XELPDP_DDI_CLOCK_SELECT_MASK,
> val);
> >
> > drm_WARN_ON(display->drm, !(val &
> XELPDP_FORWARD_CLOCK_UNGATE));
> > drm_WARN_ON(display->drm, !(val & XELPDP_TBT_CLOCK_REQUEST));
> @@
> > -3085,6 +3088,10 @@ int intel_mtl_tbt_calc_port_clock(struct intel_encoder
> *encoder)
> > return 540000;
> > case XELPDP_DDI_CLOCK_SELECT_TBT_810:
> > return 810000;
> > + case XELPDP_DDI_CLOCK_SELECT_TBT_312_5:
> > + return 1000000;
> > + case XELPDP_DDI_CLOCK_SELECT_TBT_625:
> > + return 2000000;
> > default:
> > MISSING_CASE(clock);
> > return 162000;
> > @@ -3102,6 +3109,10 @@ static int intel_mtl_tbt_clock_select(int clock)
> > return XELPDP_DDI_CLOCK_SELECT_TBT_540;
> > case 810000:
> > return XELPDP_DDI_CLOCK_SELECT_TBT_810;
> > + case 1000000:
>
> On platforms not supporting this rate and the one below, the returned value
> should be still valid, so something like if WARN(DISPLAY_VER < 30) return
> XELPDP_DDI_CLOCK_SELECT_TBT_162;
Right. I will add this check for clock rate selections.
>
> > + return XELPDP_DDI_CLOCK_SELECT_TBT_312_5;
> > + case 2000000:
> > + return XELPDP_DDI_CLOCK_SELECT_TBT_625;
> > default:
> > MISSING_CASE(clock);
> > return XELPDP_DDI_CLOCK_SELECT_TBT_162; @@ -3114,15
> +3125,25 @@
> > static void intel_mtl_tbt_pll_enable(struct intel_encoder *encoder,
> > struct intel_display *display = to_intel_display(encoder);
> > enum phy phy = intel_encoder_to_phy(encoder);
> > u32 val = 0;
> > + u32 mask;
> >
> > /*
> > * 1. Program PORT_CLOCK_CTL REGISTER to configure
> > * clock muxes, gating and SSC
> > */
> > - val |=
> XELPDP_DDI_CLOCK_SELECT(intel_mtl_tbt_clock_select(crtc_state-
> >port_clock));
> > +
> > + if (DISPLAY_VER(display) >= 30) {
> > + mask = XE3LPDP_DDI_CLOCK_SELECT_MASK;
> > + val |=
> XE3LPDP_DDI_CLOCK_SELECT(intel_mtl_tbt_clock_select(crtc_state-
> >port_clock));
> > + } else {
> > + mask = XELPDP_DDI_CLOCK_SELECT_MASK;
> > + val |=
> XELPDP_DDI_CLOCK_SELECT(intel_mtl_tbt_clock_select(crtc_state-
> >port_clock));
> > + }
> > +
>
> To handle the mask for both fields the same way:
> mask |= XELPDP_FORWARD_CLOCK_UNGATE;
>
> > val |= XELPDP_FORWARD_CLOCK_UNGATE;
> > +
> > intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder-
> >port),
> > - XELPDP_DDI_CLOCK_SELECT_MASK |
> XELPDP_FORWARD_CLOCK_UNGATE, val);
> > + mask | XELPDP_FORWARD_CLOCK_UNGATE, val);
>
> passing just mask here to intel_de_rmw().
Ok, I will do that.
>
> >
> > /* 2. Read back PORT_CLOCK_CTL REGISTER */
> > val = intel_de_read(display, XELPDP_PORT_CLOCK_CTL(display,
> > encoder->port)); diff --git
> > a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > index c685479c9756..9870d61ca493 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > @@ -187,7 +187,9 @@
> > #define XELPDP_TBT_CLOCK_REQUEST REG_BIT(19)
> > #define XELPDP_TBT_CLOCK_ACK REG_BIT(18)
> > #define XELPDP_DDI_CLOCK_SELECT_MASK
> REG_GENMASK(15, 12)
> > +#define XE3LPDP_DDI_CLOCK_SELECT_MASK
> REG_GENMASK(16, 12)
>
> Not sure about the XE3 terms, but the driver atm only uses the XE3LPD and XE3
> prefixes. I see the XE3_D and XE3P_D tags in the spec used for the above field
> width. Which platform does X3LPDP correspond to?
This naming is a bit difficult. I wouldn't like to use "PTL" as this is not just for PTL. This change is for XE3_D and XE3P_D. I thought this X3LPDP would differ from XELPDP as XE3 points to XE3 platforms. Maybe this could be renamed as XE3_D_DDI_CLOCK_SELECT_MASK?
Thanks for the comments and a review?
-Mika-
>
> > #define XELPDP_DDI_CLOCK_SELECT(val)
> REG_FIELD_PREP(XELPDP_DDI_CLOCK_SELECT_MASK, val)
> > +#define XE3LPDP_DDI_CLOCK_SELECT(val)
> REG_FIELD_PREP(XE3LPDP_DDI_CLOCK_SELECT_MASK, val)
> > #define XELPDP_DDI_CLOCK_SELECT_NONE 0x0
> > #define XELPDP_DDI_CLOCK_SELECT_MAXPCLK 0x8
> > #define XELPDP_DDI_CLOCK_SELECT_DIV18CLK 0x9
> > @@ -195,6 +197,8 @@
> > #define XELPDP_DDI_CLOCK_SELECT_TBT_270 0xd
> > #define XELPDP_DDI_CLOCK_SELECT_TBT_540 0xe
> > #define XELPDP_DDI_CLOCK_SELECT_TBT_810 0xf
> > +#define XELPDP_DDI_CLOCK_SELECT_TBT_312_5 0x18
> > +#define XELPDP_DDI_CLOCK_SELECT_TBT_625 0x19
> > #define XELPDP_FORWARD_CLOCK_UNGATE REG_BIT(10)
> > #define XELPDP_LANE1_PHY_CLOCK_SELECT REG_BIT(8)
> > #define XELPDP_SSC_ENABLE_PLLA REG_BIT(1)
> > --
> > 2.43.0
> >
More information about the Intel-xe
mailing list