From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Date: Wed, 8 Sep 2021 19:03:41 +0200 Subject: [PATCH 09/10] drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_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> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) Index: linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c =================================================================== @ linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c:1306 @ static void execlists_dequeue(struct int * and context switches) submission. */ - spin_lock(&sched_engine->lock); + spin_lock_irq(&sched_engine->lock); /* * If the queue is higher priority than the last @ linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c:1406 @ static void execlists_dequeue(struct int * 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; } } @ linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c:1432 @ static void execlists_dequeue(struct int 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 */ } @ linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c:1594 @ done: */ 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 @ linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c:1620 @ done: } } -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); @ linux-6.8.2-rt10/drivers/gpu/drm/i915/gt/intel_execlists_submission.c:2474 @ static void execlists_submission_tasklet } if (!engine->execlists.pending[0]) { - execlists_dequeue_irq(engine); + execlists_dequeue(engine); start_timeslice(engine); }