On Mon, Apr 15, 2024 at 01:44:04PM +0530, Balasubramani Vivekanandan wrote:
> From: Mitul Golani <[email protected]>
>
> 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 <[email protected]>
> CC: Matt Roper <[email protected]>
> CC: Jani Nikula <[email protected]>
> Signed-off-by: Mitul Golani <[email protected]>
> Signed-off-by: Balasubramani Vivekanandan
> <[email protected]>
> ---
> 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
