Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Request submission to the ELSP hardware is executed with interrupts 
enabled, violating hardware constraints for tightly packed MMIO sequences and 
risking GPU hangs.
--

commit 03bdcff0829362b8c38fdcf058920884f728b511
Author: Sebastian Andrzej Siewior <[email protected]>
drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock()

This commit replaces the local_irq_disable() wrapper around
execlists_dequeue() with spin_lock_irq() inside the function, to resolve a
PREEMPT_RT incompatibility.

> 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);
>  
>       /*
>        * We can skip poking the HW if we ended up with exactly the same set
> @@ -1614,13 +1614,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) */
> -}

[Severity: High]
Does removing the local_irq_disable() wrapper leave the hardware submission
sequence exposed to interruptions?

In the original code, local_irq_disable() inside execlists_dequeue_irq()
protected the entire execlists_dequeue() function. Now that
spin_unlock_irq() is used instead, interrupts are re-enabled before the
function finishes:

        spin_unlock_irq(&sched_engine->lock);

        /* ... */
        if (submit && ...) {
                /* ... */
                execlists_submit_ports(engine);
        }

Because interrupts are now enabled prior to calling execlists_submit_ports(),
an interrupt or thread preemption event could occur during the ELSP MMIO
writes. Since the ELSP hardware requires tightly packed writes without
excessive delays, could this interruption cause a hardware state machine
timeout and result in a GPU hang?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to