[Intel-gfx] [PATCH v2] drm/i915/cx0: Check and increase msgbus timeout threshold

[Kahola, Mika]

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa at intel.com>
> Sent: Wednesday, August 30, 2023 3:15 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>; Kahola, Mika <mika.kahola at intel.com>; Nikula, Jani
> <jani.nikula at intel.com>
> Subject: [PATCH v2] drm/i915/cx0: Check and increase msgbus timeout threshold
> 
> We have experienced timeout issues when going through the sequence to access C20 SRAM registers. Experimentation showed
> that bumping the message bus timer threshold helped on getting display Type-C connection on the C20 PHY to work.
> 
> While the timeout is still under investigation with the HW team, having logic to allow forward progress (with the proper warnings)
> seems useful.
> Thus, let's bump the threshold when a timeout is detected.
> 
> The bumped value of 0x200 pclk cycles was somewhat arbitrary - 2x the default value. That value was successfully tested on real
> hardware that was displaying timeouts otherwise.
> 
> v2:
>   - Reword commit message to indicate that access to C20 SRAM registers
>     is not direct. (Radhakrishna)
>   - Prefer not to use REG_FIELD_PREP() in intel_cx0_phy.c.
>     (Radhakrishna)
>   - Simplify intel_cx0_bus_check_and_bump_timer() to use a fixed bumped
>     value instead of progressively increasing the threshold. (Mika)
> 
> BSpec: 65156
> Cc: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> Cc: Mika Kahola <mika.kahola at intel.com>

Looks ok to me.

Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 39 +++++++++++++++++++  .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 13
> +++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index dd489b50ad60..ffc6b54be12b 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -29,6 +29,8 @@
>  #define INTEL_CX0_LANE1		BIT(1)
>  #define INTEL_CX0_BOTH_LANES	(INTEL_CX0_LANE1 | INTEL_CX0_LANE0)
> 
> +#define INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL	0x200
> +
>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)  {
>  	if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C) @@ -119,6 +121,42 @@ static void
> intel_cx0_bus_reset(struct drm_i915_private *i915, enum port port, i
>  	intel_clear_response_ready_flag(i915, port, lane);  }
> 
> +/*
> + * Check if there was a timeout detected by the hardware in the message
> +bus
> + * and bump the threshold if so.
> + */
> +static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private *i915,
> +					       enum port port, int lane)
> +{
> +	enum phy phy = intel_port_to_phy(i915, port);
> +	i915_reg_t reg;
> +	u32 val;
> +	u32 timer_val;
> +
> +	reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
> +	val = intel_de_read(i915, reg);
> +
> +	if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
> +		drm_warn(&i915->drm,
> +			 "PHY %c lane %d: hardware did not detect a timeout\n",
> +			 phy_name(phy), lane);
> +		return;
> +	}
> +
> +	timer_val = REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
> +
> +	if (timer_val == INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL)
> +		return;
> +
> +	val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
> +	val |=
> +XELPDP_PORT_MSGBUS_TIMER_VAL(INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL);
> +
> +	drm_dbg_kms(&i915->drm,
> +		    "PHY %c lane %d: increasing msgbus timer threshold to %#x\n",
> +		    phy_name(phy), lane, INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL);
> +	intel_de_write(i915, reg, val);
> +}
> +
>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
>  				  int command, int lane, u32 *val)
>  {
> @@ -132,6 +170,7 @@ static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
>  					 XELPDP_MSGBUS_TIMEOUT_SLOW, val)) {
>  		drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for message ACK. Status: 0x%x\n",
>  			    phy_name(phy), *val);
> +		intel_cx0_bus_check_and_bump_timer(i915, port, lane);
>  		intel_cx0_bus_reset(i915, port, lane);
>  		return -ETIMEDOUT;
>  	}
> 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 cb5d1be2ba19..b2db4cc366d6 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> @@ -110,6 +110,19 @@
>  #define   CX0_P4PG_STATE_DISABLE			0xC
>  #define   CX0_P2_STATE_RESET				0x2
> 
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A			0x640d8
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B			0x641d8
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1		0x16f258
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2		0x16f458
> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)		_MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
> +										 _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
> +										 _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
> +
> _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
> +
> _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT		REG_BIT(31)
> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK		REG_GENMASK(23, 0)
> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL(val)		REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
> val)
> +
>  #define _XELPDP_PORT_CLOCK_CTL_A			0x640E0
>  #define _XELPDP_PORT_CLOCK_CTL_B			0x641E0
>  #define _XELPDP_PORT_CLOCK_CTL_USBC1			0x16F260
> --
> 2.41.0