Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Rather than have multiple locked instructions inside the notify_ring()
> irq handler, move them inside the spinlock and reduce their intrinsic
> locking.
>
Less is better, could note in commit message that we omit the non
wait ones.
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> drivers/gpu/drm/i915/i915_request.c | 4 ++--
> drivers/gpu/drm/i915/intel_breadcrumbs.c | 11 +++++++----
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
> 4 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 53dad48f92ce..6730c1a7f135 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1153,8 +1153,6 @@ static void notify_ring(struct intel_engine_cs *engine)
> if (unlikely(!engine->breadcrumbs.irq_armed))
> return;
>
> - atomic_inc(&engine->irq_count);
> -
> rcu_read_lock();
>
> spin_lock(&engine->breadcrumbs.irq_lock);
> @@ -1189,6 +1187,8 @@ static void notify_ring(struct intel_engine_cs *engine)
> tsk = wait->tsk;
> }
> }
> +
> + engine->breadcrumbs.irq_count++;
> } else {
> if (engine->breadcrumbs.irq_armed)
> __intel_engine_disarm_breadcrumbs(engine);
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 11f175554da8..696125663105 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1161,7 +1161,7 @@ static bool __i915_spin_request(const struct i915_request *rq,
> * takes to sleep on a request, on the order of a microsecond.
> */
>
> - irq = atomic_read(&engine->irq_count);
> + irq = READ_ONCE(engine->breadcrumbs.irq_count);
> timeout_us += local_clock_us(&cpu);
> do {
> if (i915_seqno_passed(intel_engine_get_seqno(engine), seqno))
> @@ -1173,7 +1173,7 @@ static bool __i915_spin_request(const struct i915_request *rq,
> * assume we won't see one in the near future but require
> * the engine->seqno_barrier() to fixup coherency.
> */
> - if (atomic_read(&engine->irq_count) != irq)
> + if (READ_ONCE(engine->breadcrumbs.irq_count) != irq)
> break;
>
> if (signal_pending_state(state, current))
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index 86a987b8ac66..1db6ba7d926e 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -98,12 +98,14 @@ static void intel_breadcrumbs_hangcheck(struct timer_list *t)
> struct intel_engine_cs *engine =
> from_timer(engine, t, breadcrumbs.hangcheck);
> struct intel_breadcrumbs *b = &engine->breadcrumbs;
> + unsigned int irq_count;
>
> if (!b->irq_armed)
> return;
>
> - if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) {
> - b->hangcheck_interrupts = atomic_read(&engine->irq_count);
> + irq_count = READ_ONCE(b->irq_count);
> + if (b->hangcheck_interrupts != irq_count) {
> + b->hangcheck_interrupts = irq_count;
> mod_timer(&b->hangcheck, wait_timeout());
> return;
> }
> @@ -272,13 +274,14 @@ static bool use_fake_irq(const struct intel_breadcrumbs *b)
> 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
> + /*
> + * 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;
> + return READ_ONCE(b->irq_count) == b->hangcheck_interrupts;
> }
>
> static void enable_fake_irq(struct intel_breadcrumbs *b)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 8dd34b9dc18a..33602eb1c77f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -356,7 +356,6 @@ struct intel_engine_cs {
> struct drm_i915_gem_object *default_state;
> void *pinned_default_state;
>
> - atomic_t irq_count;
> unsigned long irq_posted;
> #define ENGINE_IRQ_BREADCRUMB 0
>
> @@ -390,6 +389,7 @@ struct intel_engine_cs {
>
> unsigned int hangcheck_interrupts;
> unsigned int irq_enabled;
> + unsigned int irq_count;
>
> bool irq_armed : 1;
> I915_SELFTEST_DECLARE(bool mock : 1);
> --
> 2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
