Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Re-enabling interrupts before `execlists_submit_ports()` violates the hardware ELSP submission constraint and causes premature `irq_work` flushing, breaking cross-engine request execution ordering. --
commit 2338d7dc7111f08419fa617c233f6984399c8937 Author: Sebastian Andrzej Siewior <[email protected]> drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock() This patch modifies execlists_dequeue() to use spin_lock_irq() and spin_unlock_irq() directly. It removes execlists_dequeue_irq() which previously wrapped the entire dequeue operation in local_irq_disable(). > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > index 1359fc9cb88ef..e11db81dca9c5 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c [ ... ] > @@ -1588,7 +1588,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); [Severity: High] Does re-enabling interrupts here before calling execlists_submit_ports() violate the hardware ELSP submission constraints? The original execlists_dequeue_irq() kept interrupts disabled across the entire dequeue process, including the final MMIO writes. By changing to spin_unlock_irq() here, interrupts are re-enabled before the execution of: execlists_submit_ports() { ... for (n = execlists_num_ports(execlists); n--; ) { struct i915_request *rq = execlists->pending[n]; write_desc(execlists, rq ? execlists_update_context(rq) : 0, n); } } Hardware requires these consecutive ELSP MMIO writes to be atomic without intervening accesses. If an interrupt fires during this loop, could it corrupt the submission sequence and cause a GPU hang? Additionally, can enabling interrupts this early cause premature irq_work flushing, breaking cross-engine request execution ordering? > > /* > * We can skip poking the HW if we ended up with exactly the same set [ ... ] > -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) */ > -} > - -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
