Re: [PATCH v3 3/5] drm/i915: Defer enabling hangcheck to the first fake breadcrumb interrupt

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 24/02/2017 18:01, Chris Wilson wrote:
> By deferring hangcheck to the fake breadcrumb interrupt, we can simply
> the enabling procedure slightly - as by enabling the fake, we then
> enable the hangcheck. By always enabling the hangcheck from each fake
> interrupt (it will be a no-op for an already queued hangcheck), it will
> make restoring the breadcrumbs after a reset simpler in the next patch.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_breadcrumbs.c | 36 ++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index ba5f72fb4890..d7511e89c8ab 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -75,17 +75,6 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
>  	DRM_DEBUG("Hangcheck timer elapsed... %s idle\n", engine->name);
>  	set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
>  	mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
> -
> -	/* Ensure that even if the GPU hangs, we get woken up.
> -	 *
> -	 * However, note that if no one is waiting, we never notice
> -	 * a gpu hang. Eventually, we will have to wait for a resource
> -	 * held by the GPU and so trigger a hangcheck. In the most
> -	 * pathological case, this will be upon memory starvation! To
> -	 * prevent this, we also queue the hangcheck from the retire
> -	 * worker.
> -	 */
> -	i915_queue_hangcheck(engine->i915);
>  }
>
>  static void intel_breadcrumbs_fake_irq(unsigned long data)
> @@ -99,8 +88,21 @@ static void intel_breadcrumbs_fake_irq(unsigned long data)
>  	 * every jiffie in order to kick the oldest waiter to do the
>  	 * coherent seqno check.
>  	 */
> -	if (intel_engine_wakeup(engine))
> -		mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
> +	if (!intel_engine_wakeup(engine))
> +		return;
> +
> +	mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
> +
> +	/* Ensure that even if the GPU hangs, we get woken up.
> +	 *
> +	 * However, note that if no one is waiting, we never notice
> +	 * a gpu hang. Eventually, we will have to wait for a resource
> +	 * held by the GPU and so trigger a hangcheck. In the most
> +	 * pathological case, this will be upon memory starvation! To
> +	 * prevent this, we also queue the hangcheck from the retire
> +	 * worker.
> +	 */
> +	i915_queue_hangcheck(engine->i915);
>  }
>
>  static void irq_enable(struct intel_engine_cs *engine)
> @@ -179,13 +181,11 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
>  		b->irq_enabled = true;
>  	}
>
> -	if (!b->irq_enabled || use_fake_irq(b)) {
> +	/* Ensure we never sleep indefinitely */
> +	if (!b->irq_enabled || use_fake_irq(b))
>  		mod_timer(&b->fake_irq, jiffies + 1);
> -		i915_queue_hangcheck(i915);
> -	} else {
> -		/* Ensure we never sleep indefinitely */
> +	else
>  		mod_timer(&b->hangcheck, wait_timeout());
> -	}
>  }
>
>  static void __intel_breadcrumbs_disable_irq(struct intel_breadcrumbs *b)
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx