Skip to content

Commit

Permalink
drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spi…
Browse files Browse the repository at this point in the history
…n_lock()

execlists_dequeue() is invoked from a function which uses
local_irq_disable() to disable interrupts so the spin_lock() behaves
like spin_lock_irq().
This breaks PREEMPT_RT because local_irq_disable() + spin_lock() is not
the same as spin_lock_irq().

execlists_dequeue_irq() and execlists_dequeue() has each one caller
only. If intel_engine_cs::active::lock is acquired and released with the
_irq suffix then it behaves almost as if execlists_dequeue() would be
invoked with disabled interrupts. The difference is the last part of the
function which is then invoked with enabled interrupts.
I can't tell if this makes a difference. From looking at it, it might
work to move the last unlock at the end of the function as I didn't find
anything that would acquire the lock again.

Reported-by: Clark Williams <williams@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
  • Loading branch information
Sebastian Andrzej Siewior committed Sep 13, 2021
1 parent e0faa56 commit 51f3722
Showing 1 changed file with 5 additions and 12 deletions.
17 changes: 5 additions & 12 deletions drivers/gpu/drm/i915/gt/intel_execlists_submission.c
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
* and context switches) submission.
*/

spin_lock(&sched_engine->lock);
spin_lock_irq(&sched_engine->lock);

/*
* If the queue is higher priority than the last
Expand Down Expand Up @@ -1383,7 +1383,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
* Even if ELSP[1] is occupied and not worthy
* of timeslices, our queue might be.
*/
spin_unlock(&sched_engine->lock);
spin_unlock_irq(&sched_engine->lock);
return;
}
}
Expand All @@ -1409,7 +1409,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)

if (last && !can_merge_rq(last, rq)) {
spin_unlock(&ve->base.sched_engine->lock);
spin_unlock(&engine->sched_engine->lock);
spin_unlock_irq(&engine->sched_engine->lock);
return; /* leave this for another sibling */
}

Expand Down Expand Up @@ -1571,7 +1571,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
*/
sched_engine->queue_priority_hint = queue_prio(sched_engine);
i915_sched_engine_reset_on_empty(sched_engine);
spin_unlock(&sched_engine->lock);
spin_unlock_irq(&sched_engine->lock);

/*
* We can skip poking the HW if we ended up with exactly the same set
Expand All @@ -1597,13 +1597,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
}
}

static void execlists_dequeue_irq(struct intel_engine_cs *engine)
{
local_irq_disable(); /* Suspend interrupts across request submission */
execlists_dequeue(engine);
local_irq_enable(); /* flush irq_work (e.g. breadcrumb enabling) */
}

static void clear_ports(struct i915_request **ports, int count)
{
memset_p((void **)ports, NULL, count);
Expand Down Expand Up @@ -2427,7 +2420,7 @@ static void execlists_submission_tasklet(struct tasklet_struct *t)
}

if (!engine->execlists.pending[0]) {
execlists_dequeue_irq(engine);
execlists_dequeue(engine);
start_timeslice(engine);
}

Expand Down

0 comments on commit 51f3722

Please sign in to comment.