Re: [PATCH] drm/i915/display: UHBR rates for Thunderbolt

[Imre Deak <imre.deak@intel.com>]

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@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.

> ---
>  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;

> +		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().

>  
>  	/* 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?

>  #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
>