> -----Original Message-----
> From: Deak, Imre <imre.deak@xxxxxxxxx>
> Sent: Wednesday, 18 December 2024 16.57
> To: Kahola, Mika <mika.kahola@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2] drm/i915/display: UHBR rates for Thunderbolt
>
> On Tue, Dec 17, 2024 at 04:34:40PM +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.
> >
> > v2: Add drm_WARN_ON() to check if port clock is not supported by
> > the platform (Imre)
> > Combine forward ungate with mask parameter (Imre)
> > Rename XE3LPDP_* to XE3D_* (Imre)
>
> I highly disagree with the usage of the XE[23]{D,LPD,LPDP} etc. ciphers in the
> driver in general, instead of the human readable MTL, LNL, PTL etc., the human
> readable versions actually serving the intended purpose of reminding a reader
> what exact platform the code they read is about.
>
> I still don't know and haven't managed to figure out from the spec what either
> XE3LPDP or XE3D is about, looks like this same clock select field width is used by
> other XE3 platforms as well. So imo let's stick with the XE3 prefix used already
> elsewhere in the driver (no need to resend the patch just for that). With that:
Thanks for the review! Patch is now merged with the change of naming to XE3_*. This is used elsewhere in the driver so maybe it's ok to use here as well.
-Mika-
>
> Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
>
> > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 39
> > +++++++++++++++++-- .../gpu/drm/i915/display/intel_cx0_phy_regs.h |
> > 4 ++
> > 2 files changed, 39 insertions(+), 4 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..a8e0450c0378 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(XE3D_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,13 +3088,18 @@ 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;
> > }
> > }
> >
> > -static int intel_mtl_tbt_clock_select(int clock)
> > +static int intel_mtl_tbt_clock_select(struct intel_display *display,
> > + int clock)
> > {
> > switch (clock) {
> > case 162000:
> > @@ -3102,6 +3110,18 @@ 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:
> > + if (DISPLAY_VER(display) < 30) {
> > + drm_WARN_ON(display->drm, "UHBR10 not supported
> for the platform\n");
> > + return XELPDP_DDI_CLOCK_SELECT_TBT_162;
> > + }
> > + return XELPDP_DDI_CLOCK_SELECT_TBT_312_5;
> > + case 2000000:
> > + if (DISPLAY_VER(display) < 30) {
> > + drm_WARN_ON(display->drm, "UHBR20 not supported
> for the platform\n");
> > + return XELPDP_DDI_CLOCK_SELECT_TBT_162;
> > + }
> > + return XELPDP_DDI_CLOCK_SELECT_TBT_625;
> > default:
> > MISSING_CASE(clock);
> > return XELPDP_DDI_CLOCK_SELECT_TBT_162; @@ -3114,15
> +3134,26 @@
> > 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 = XE3D_DDI_CLOCK_SELECT_MASK;
> > + val |=
> XE3D_DDI_CLOCK_SELECT(intel_mtl_tbt_clock_select(display, crtc_state-
> >port_clock));
> > + } else {
> > + mask = XELPDP_DDI_CLOCK_SELECT_MASK;
> > + val |=
> XELPDP_DDI_CLOCK_SELECT(intel_mtl_tbt_clock_select(display, crtc_state-
> >port_clock));
> > + }
> > +
> > + 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, val);
> >
> > /* 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..bf95ac234b2b 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 XE3D_DDI_CLOCK_SELECT_MASK
> REG_GENMASK(16, 12)
> > #define XELPDP_DDI_CLOCK_SELECT(val)
> REG_FIELD_PREP(XELPDP_DDI_CLOCK_SELECT_MASK, val)
> > +#define XE3D_DDI_CLOCK_SELECT(val)
> REG_FIELD_PREP(XE3D_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
> >
