[PATCH v3 02/21] drm/i915/display: Enable RM timeout detection

[Matt Roper]

On Mon, Apr 15, 2024 at 01:44:04PM +0530, Balasubramani Vivekanandan wrote:
> From: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>
> 
> Enable RM timeout interrupt to detect any hang during display engine
> register access.
> Current default timeout is 2ms.
> 
> v2:
> * Modified the IP version check to apply on all versions starting from
>   14
> * Improved the print log
> 
> Bspec: 50110
> CC: Suraj Kandpal <suraj.kandpal at intel.com>
> CC: Matt Roper <matthew.d.roper at intel.com>
> CC: Jani Nikula <jani.nikula at linux.intel.com>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>
> Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_irq.c | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_reg.h                  |  3 +++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index c337e0597541..9c65e9e32fca 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -852,6 +852,13 @@ gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
>  {
>  	bool found = false;
>  
> +	if (iir & GEN8_DE_RM_TIMEOUT) {
> +		u32 val = intel_uncore_read(&dev_priv->uncore,
> +				RMTIMEOUTREG_CAPTURE);
> +		drm_warn(&dev_priv->drm, "Register access timeout for offset = 0x%x\n", val);

Have we ever actually encountered an error that caused a register read
to timeout?  If so, was it a one-off error or did it cause a stream of
errors?  This seems like the kind of thing that might trigger repeatedly
once hardware goes off the rails, so we might want to use either the
rate-limited or once form of dmesg logging.


> +		found = true;
> +	}
> +
>  	if (DISPLAY_VER(dev_priv) >= 14) {
>  		if (iir & (XELPDP_PMDEMAND_RSP |
>  			   XELPDP_PMDEMAND_RSPTOUT_ERR)) {
> @@ -1667,6 +1674,9 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  			de_port_masked |= DSI0_TE | DSI1_TE;
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 14)
> +		de_misc_masked |= GEN8_DE_RM_TIMEOUT;
> +
>  	de_pipe_enables = de_pipe_masked |
>  		GEN8_PIPE_VBLANK |
>  		gen8_de_pipe_underrun_mask(dev_priv) |
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3f34efcd7d6c..a8cdabd07b04 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4225,6 +4225,8 @@
>  #define RM_TIMEOUT		_MMIO(0x42060)
>  #define  MMIO_TIMEOUT_US(us)	((us) << 0)
>  
> +#define RMTIMEOUTREG_CAPTURE	_MMIO(0x420e0)
> +
>  /* interrupts */
>  #define DE_MASTER_IRQ_CONTROL   (1 << 31)
>  #define DE_SPRITEB_FLIP_DONE    (1 << 29)
> @@ -4411,6 +4413,7 @@
>  #define GEN8_DE_MISC_IMR _MMIO(0x44464)
>  #define GEN8_DE_MISC_IIR _MMIO(0x44468)
>  #define GEN8_DE_MISC_IER _MMIO(0x4446c)
> +#define  GEN8_DE_RM_TIMEOUT		REG_BIT(29)

The "GEN8" prefix here isn't appropriate.  This bit didn't show up in
hardware until Xe_LPD+.


Actually this whole patch should probably be titled "drm/i915/xelpdp:"
to make it clear that this change applies to Xe_LPD+.  This patch
probably should have been sent a long time ago and not mixed into the
BMG work...


Matt

>  #define  XELPDP_PMDEMAND_RSPTOUT_ERR	REG_BIT(27)
>  #define  GEN8_DE_MISC_GSE		REG_BIT(27)
>  #define  GEN8_DE_EDP_PSR		REG_BIT(19)
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation