Skip to content

Commit

Permalink
drm/i915: Split breadcrumbs spinlock into two
Browse files Browse the repository at this point in the history
As we now take the breadcrumbs spinlock within the interrupt handler, we
wish to minimise its hold time. During the interrupt we do not care
about the state of the full rbtree, only that of the first element, so
we can guard that with a separate lock.

v2: Rename first_wait to irq_wait to make it clearer that it is guarded
by irq_lock.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170303190824.1330-1-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Mar 3, 2017
1 parent b66255f commit 61d3dc7
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 58 deletions.
12 changes: 6 additions & 6 deletions drivers/gpu/drm/i915/i915_debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -726,14 +726,14 @@ static void i915_ring_seqno_info(struct seq_file *m,
seq_printf(m, "Current sequence (%s): %x\n",
engine->name, intel_engine_get_seqno(engine));

spin_lock_irq(&b->lock);
spin_lock_irq(&b->rb_lock);
for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
struct intel_wait *w = rb_entry(rb, typeof(*w), node);

seq_printf(m, "Waiting (%s): %s [%d] on %x\n",
engine->name, w->tsk->comm, w->tsk->pid, w->seqno);
}
spin_unlock_irq(&b->lock);
spin_unlock_irq(&b->rb_lock);
}

static int i915_gem_seqno_info(struct seq_file *m, void *data)
Expand Down Expand Up @@ -1380,14 +1380,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
&dev_priv->gpu_error.missed_irq_rings)),
yesno(engine->hangcheck.stalled));

spin_lock_irq(&b->lock);
spin_lock_irq(&b->rb_lock);
for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
struct intel_wait *w = rb_entry(rb, typeof(*w), node);

seq_printf(m, "\t%s [%d] waiting for %x\n",
w->tsk->comm, w->tsk->pid, w->seqno);
}
spin_unlock_irq(&b->lock);
spin_unlock_irq(&b->rb_lock);

seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
(long long)engine->hangcheck.acthd,
Expand Down Expand Up @@ -3385,14 +3385,14 @@ static int i915_engine_info(struct seq_file *m, void *unused)
I915_READ(RING_PP_DIR_DCLV(engine)));
}

spin_lock_irq(&b->lock);
spin_lock_irq(&b->rb_lock);
for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
struct intel_wait *w = rb_entry(rb, typeof(*w), node);

seq_printf(m, "\t%s [%d] waiting for %x\n",
w->tsk->comm, w->tsk->pid, w->seqno);
}
spin_unlock_irq(&b->lock);
spin_unlock_irq(&b->rb_lock);

seq_puts(m, "\n");
}
Expand Down
8 changes: 4 additions & 4 deletions drivers/gpu/drm/i915/i915_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -4097,16 +4097,16 @@ __i915_request_irq_complete(const struct drm_i915_gem_request *req)
* the seqno before we believe it coherent since they see
* irq_posted == false but we are still running).
*/
spin_lock_irqsave(&b->lock, flags);
if (b->first_wait && b->first_wait->tsk != current)
spin_lock_irqsave(&b->irq_lock, flags);
if (b->irq_wait && b->irq_wait->tsk != current)
/* Note that if the bottom-half is changed as we
* are sending the wake-up, the new bottom-half will
* be woken by whomever made the change. We only have
* to worry about when we steal the irq-posted for
* ourself.
*/
wake_up_process(b->first_wait->tsk);
spin_unlock_irqrestore(&b->lock, flags);
wake_up_process(b->irq_wait->tsk);
spin_unlock_irqrestore(&b->irq_lock, flags);

if (__i915_gem_request_completed(req, seqno))
return true;
Expand Down
8 changes: 4 additions & 4 deletions drivers/gpu/drm/i915/i915_gpu_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -1111,15 +1111,15 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine,
if (RB_EMPTY_ROOT(&b->waiters))
return;

if (!spin_trylock_irq(&b->lock)) {
if (!spin_trylock_irq(&b->rb_lock)) {
ee->waiters = ERR_PTR(-EDEADLK);
return;
}

count = 0;
for (rb = rb_first(&b->waiters); rb != NULL; rb = rb_next(rb))
count++;
spin_unlock_irq(&b->lock);
spin_unlock_irq(&b->rb_lock);

waiter = NULL;
if (count)
Expand All @@ -1129,7 +1129,7 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine,
if (!waiter)
return;

if (!spin_trylock_irq(&b->lock)) {
if (!spin_trylock_irq(&b->rb_lock)) {
kfree(waiter);
ee->waiters = ERR_PTR(-EDEADLK);
return;
Expand All @@ -1147,7 +1147,7 @@ static void error_record_engine_waiters(struct intel_engine_cs *engine,
if (++ee->num_waiters == count)
break;
}
spin_unlock_irq(&b->lock);
spin_unlock_irq(&b->rb_lock);
}

static void error_record_engine_registers(struct i915_gpu_state *error,
Expand Down
6 changes: 3 additions & 3 deletions drivers/gpu/drm/i915/i915_irq.c
Original file line number Diff line number Diff line change
Expand Up @@ -1042,8 +1042,8 @@ static void notify_ring(struct intel_engine_cs *engine)
atomic_inc(&engine->irq_count);
set_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted);

spin_lock(&engine->breadcrumbs.lock);
wait = engine->breadcrumbs.first_wait;
spin_lock(&engine->breadcrumbs.irq_lock);
wait = engine->breadcrumbs.irq_wait;
if (wait) {
/* We use a callback from the dma-fence to submit
* requests after waiting on our own requests. To
Expand All @@ -1064,7 +1064,7 @@ static void notify_ring(struct intel_engine_cs *engine)
} else {
__intel_engine_disarm_breadcrumbs(engine);
}
spin_unlock(&engine->breadcrumbs.lock);
spin_unlock(&engine->breadcrumbs.irq_lock);

if (rq) {
dma_fence_signal(&rq->fence);
Expand Down
Loading

0 comments on commit 61d3dc7

Please sign in to comment.