Re: [Intel-gfx] [PATCH v2] drm/i915: Reset the breadcrumbs IRQ more carefully

[Mika Kuoppala]

Chris Wilson  writes:

> Along with the interrupt, we want to restore the fake-irq and
> wait-timeout detection. If we use the breadcrumbs interface to setup the
> interrupt as it wants, the auxiliary timers will also be restored.
>
> v2: Cancel both timers as well, sanitize the IMR.
>
> Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 

Reviewed-by: Mika Kuoppala 

> ---
>  drivers/gpu/drm/i915/intel_breadcrumbs.c | 31 +--
>  drivers/gpu/drm/i915/intel_engine_cs.c   | 15 ---
>  drivers/gpu/drm/i915/intel_lrc.c |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c  |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.h  |  2 +-
>  5 files changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c 
> b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index 9bad14d22c95..3dd23c16bea1 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -578,6 +578,34 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs 
> *engine)
>   return 0;
>  }
>  
> +static void cancel_fake_irq(struct intel_engine_cs *engine)
> +{
> + struct intel_breadcrumbs *b = >breadcrumbs;
> +
> + del_timer_sync(>hangcheck);
> + del_timer_sync(>fake_irq);
> + clear_bit(engine->id, >i915->gpu_error.missed_irq_rings);
> +}
> +
> +void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
> +{
> + struct intel_breadcrumbs *b = >breadcrumbs;
> +
> + cancel_fake_irq(engine);
> + spin_lock(>lock);
> +
> + __intel_breadcrumbs_disable_irq(b);
> + if (intel_engine_has_waiter(engine)) {
> + b->timeout = wait_timeout();
> + __intel_breadcrumbs_enable_irq(b);
> + } else {
> + /* sanitize the IMR and unmask any auxiliary interrupts */
> + irq_disable(engine);
> + }
> +
> + spin_unlock(>lock);
> +}
> +
>  void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
>  {
>   struct intel_breadcrumbs *b = >breadcrumbs;
> @@ -585,8 +613,7 @@ void intel_engine_fini_breadcrumbs(struct intel_engine_cs 
> *engine)
>   if (!IS_ERR_OR_NULL(b->signaler))
>   kthread_stop(b->signaler);
>  
> - del_timer_sync(>hangcheck);
> - del_timer_sync(>fake_irq);
> + cancel_fake_irq(engine);
>  }
>  
>  unsigned int intel_kick_waiters(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/intel_engine_cs.c
> index d00ec805f93d..480584c09306 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -210,9 +210,6 @@ void intel_engine_init_seqno(struct intel_engine_cs 
> *engine, u32 seqno)
>  void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
>  {
>   memset(>hangcheck, 0, sizeof(engine->hangcheck));
> - clear_bit(engine->id, >i915->gpu_error.missed_irq_rings);
> - if (intel_engine_has_waiter(engine))
> - i915_queue_hangcheck(engine->i915);
>  }
>  
>  static void intel_engine_init_requests(struct intel_engine_cs *engine)
> @@ -307,18 +304,6 @@ int intel_engine_init_common(struct intel_engine_cs 
> *engine)
>   return 0;
>  }
>  
> -void intel_engine_reset_irq(struct intel_engine_cs *engine)
> -{
> - struct drm_i915_private *dev_priv = engine->i915;
> -
> - spin_lock_irq(_priv->irq_lock);
> - if (intel_engine_has_waiter(engine))
> - engine->irq_enable(engine);
> - else
> - engine->irq_disable(engine);
> - spin_unlock_irq(_priv->irq_lock);
> -}
> -
>  /**
>   * intel_engines_cleanup_common - cleans up the engine state created by
>   *the common initiailizers.
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 936f6f63f626..44904e298bfc 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1230,7 +1230,7 @@ static int gen8_init_common_ring(struct intel_engine_cs 
> *engine)
>  
>   lrc_init_hws(engine);
>  
> - intel_engine_reset_irq(engine);
> + intel_engine_reset_breadcrumbs(engine);
>  
>   I915_WRITE(RING_HWSTAM(engine->mmio_base), 0x);
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 35f359e38f4d..729f373782e2 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -548,7 +548,7 @@ static int init_ring_common(struct intel_engine_cs 
> *engine)
>   else
>   intel_ring_setup_status_page(engine);
>  
> - intel_engine_reset_irq(engine);
> + intel_engine_reset_breadcrumbs(engine);
>  
>   /* Enforce ordering by reading HEAD register back */
>   

[Intel-gfx] [PATCH v2] drm/i915: Reset the breadcrumbs IRQ more carefully

[Chris Wilson]

Along with the interrupt, we want to restore the fake-irq and
wait-timeout detection. If we use the breadcrumbs interface to setup the
interrupt as it wants, the auxiliary timers will also be restored.

v2: Cancel both timers as well, sanitize the IMR.

Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 31 +--
 drivers/gpu/drm/i915/intel_engine_cs.c   | 15 ---
 drivers/gpu/drm/i915/intel_lrc.c |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c  |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h  |  2 +-
 5 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c 
b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 9bad14d22c95..3dd23c16bea1 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -578,6 +578,34 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs 
*engine)
return 0;
 }
 
+static void cancel_fake_irq(struct intel_engine_cs *engine)
+{
+   struct intel_breadcrumbs *b = >breadcrumbs;
+
+   del_timer_sync(>hangcheck);
+   del_timer_sync(>fake_irq);
+   clear_bit(engine->id, >i915->gpu_error.missed_irq_rings);
+}
+
+void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
+{
+   struct intel_breadcrumbs *b = >breadcrumbs;
+
+   cancel_fake_irq(engine);
+   spin_lock(>lock);
+
+   __intel_breadcrumbs_disable_irq(b);
+   if (intel_engine_has_waiter(engine)) {
+   b->timeout = wait_timeout();
+   __intel_breadcrumbs_enable_irq(b);
+   } else {
+   /* sanitize the IMR and unmask any auxiliary interrupts */
+   irq_disable(engine);
+   }
+
+   spin_unlock(>lock);
+}
+
 void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
 {
struct intel_breadcrumbs *b = >breadcrumbs;
@@ -585,8 +613,7 @@ void intel_engine_fini_breadcrumbs(struct intel_engine_cs 
*engine)
if (!IS_ERR_OR_NULL(b->signaler))
kthread_stop(b->signaler);
 
-   del_timer_sync(>hangcheck);
-   del_timer_sync(>fake_irq);
+   cancel_fake_irq(engine);
 }
 
 unsigned int intel_kick_waiters(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index d00ec805f93d..480584c09306 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -210,9 +210,6 @@ void intel_engine_init_seqno(struct intel_engine_cs 
*engine, u32 seqno)
 void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
 {
memset(>hangcheck, 0, sizeof(engine->hangcheck));
-   clear_bit(engine->id, >i915->gpu_error.missed_irq_rings);
-   if (intel_engine_has_waiter(engine))
-   i915_queue_hangcheck(engine->i915);
 }
 
 static void intel_engine_init_requests(struct intel_engine_cs *engine)
@@ -307,18 +304,6 @@ int intel_engine_init_common(struct intel_engine_cs 
*engine)
return 0;
 }
 
-void intel_engine_reset_irq(struct intel_engine_cs *engine)
-{
-   struct drm_i915_private *dev_priv = engine->i915;
-
-   spin_lock_irq(_priv->irq_lock);
-   if (intel_engine_has_waiter(engine))
-   engine->irq_enable(engine);
-   else
-   engine->irq_disable(engine);
-   spin_unlock_irq(_priv->irq_lock);
-}
-
 /**
  * intel_engines_cleanup_common - cleans up the engine state created by
  *the common initiailizers.
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 936f6f63f626..44904e298bfc 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1230,7 +1230,7 @@ static int gen8_init_common_ring(struct intel_engine_cs 
*engine)
 
lrc_init_hws(engine);
 
-   intel_engine_reset_irq(engine);
+   intel_engine_reset_breadcrumbs(engine);
 
I915_WRITE(RING_HWSTAM(engine->mmio_base), 0x);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 35f359e38f4d..729f373782e2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -548,7 +548,7 @@ static int init_ring_common(struct intel_engine_cs *engine)
else
intel_ring_setup_status_page(engine);
 
-   intel_engine_reset_irq(engine);
+   intel_engine_reset_breadcrumbs(engine);
 
/* Enforce ordering by reading HEAD register back */
I915_READ_HEAD(engine);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 34954ca03a4a..124f4646958d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -520,7 +520,6 @@ int