Re: [PATCH] drm/i915: Only start with the fake-irq timer if interrupts are dead

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

On 17/02/2017 11:55, Chris Wilson wrote:
> As a backup to waiting on a user-interrupt from the GPU, we use a heavy
> and frequent timer to wake up the waiting process should we detect an
> inconsistency whilst waiting. After seeing a "missed interrupt", the
> next time we wait, we restart the heavy timer. This patch is more
> reluctant to restart the timer and will only do so if we have not see any
> interrupts since when we started the fake irq timer. If we are seeing
> interrupts, then the waiters are being woken normally and we had an
> incoherency that caused to miss last time - that is unlikely to reoccur
> and so taking the risk of stalling again seems pragmatic.
>
> 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 | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index 83a8b67d6427..1719f9da13b8 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -107,6 +107,23 @@ static void irq_disable(struct intel_engine_cs *engine)
>  	spin_unlock(&engine->i915->irq_lock);
>  }
>
> +static bool use_fake_irq(const struct intel_breadcrumbs *b)
> +{
> +	const struct intel_engine_cs *engine =
> +		container_of(b, struct intel_engine_cs, breadcrumbs);
> +
> +	if (!test_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings))
> +		return false;
> +
> +	/* Only start with the heavy weight fake irq timer if we have not
> +	 * seen any interrupts since enabling it the first time. If the
> +	 * interrupts are still arriving, it means we made a mistake in our
> +	 * engine->seqno_barrier(), a timing error that should be transient
> +	 * and unlikely to reoccur.
> +	 */
> +	return atomic_read(&engine->irq_count) == b->hangcheck_interrupts;
> +}
> +
>  static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
>  {
>  	struct intel_engine_cs *engine =
> @@ -145,8 +162,7 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
>  		b->irq_enabled = true;
>  	}
>
> -	if (!b->irq_enabled ||
> -	    test_bit(engine->id, &i915->gpu_error.missed_irq_rings)) {
> +	if (!b->irq_enabled || use_fake_irq(b)) {
>  		mod_timer(&b->fake_irq, jiffies + 1);
>  		i915_queue_hangcheck(i915);
>  	} else {
>

Very soothing now that I have discovered the pre-existing behaviour. :)

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