Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Hardware MMIO submission sequence in `execlists_submit_ports()` is executed with interrupts and preemption enabled, violating the requirement for atomic hardware submission. --
commit a1c2160ec1a5e4791ddd349b07f9fe856a1bb2e5 Author: Sebastian Andrzej Siewior <[email protected]> drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock() This commit replaces local_irq_disable() and spin_lock() with spin_lock_irq() to improve PREEMPT_RT compatibility, and removes the execlists_dequeue_irq() wrapper. > 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] Is it safe to unconditionally re-enable interrupts and preemption here before the hardware submission? Looking at execlists_dequeue(), shortly after this unlock the code calls execlists_submit_ports(). The Execlist Submit Port (ELSP) mechanism typically requires consecutive MMIO writes to submit hardware contexts. Previously, the entire function was wrapped in local_irq_disable() via execlists_dequeue_irq(). With spin_unlock_irq() completely enabling interrupts (and preemption on PREEMPT_RT) prior to the submission, could a hard interrupt or task preemption delay the sequence of MMIO writes? This seems like it might lead to partial submissions, hardware timeouts, or GPU hangs if the hardware submission sequence is no longer atomic. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
