Claude review: drm/panthor: Process FW events in IRQ context

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

Replaces the threaded handler with a custom hard IRQ handler `panthor_job_irq_raw_handler()`:

```c
static irqreturn_t panthor_job_irq_raw_handler(int irq, void *data)
{
    struct panthor_irq *pirq = data;

    if (!gpu_read(pirq->iomem, INT_STAT))
        return IRQ_NONE;

    scoped_guard(spinlock_irqsave, &pirq->mask_lock) {
        if (pirq->state != PANTHOR_IRQ_STATE_ACTIVE)
            return IRQ_NONE;
        pirq->state = PANTHOR_IRQ_STATE_PROCESSING;
    }

    /* We can use INT_STAT here, because we didn't mask the IRQs. */
    panthor_job_irq_handler(pirq, gpu_read(pirq->iomem, INT_STAT));

    scoped_guard(spinlock_irqsave, &pirq->mask_lock) {
        if (pirq->state == PANTHOR_IRQ_STATE_PROCESSING)
            pirq->state = PANTHOR_IRQ_STATE_ACTIVE;
    }

    return IRQ_HANDLED;
}
```

**Key difference from the old threaded handler:** The old handler looped reading `INT_RAWSTAT & mask`, clearing and processing until no more events. This new handler reads `INT_STAT` once. The comment says "We can use INT_STAT here, because we didn't mask the IRQs." This is correct — since the mask stays enabled, `INT_STAT` reflects only newly-active, unmasked interrupts. If another event arrives during processing, it will fire a new IRQ.

**Concern — single-shot processing:** The old threaded handler looped until `INT_RAWSTAT & mask` was zero, ensuring all pending events were drained. This new handler processes a single `INT_STAT` read. If new events arrive between the first `INT_STAT` read (the check at the top) and the second `INT_STAT` read (passed to the handler), the handler sees both. But if events arrive *after* the second read, they'll trigger a new interrupt since the mask is still active. This should be fine for correctness, but could result in more IRQ entries under heavy load compared to the drain-loop approach. Given the ~5% performance improvement measured, the reduced latency clearly outweighs any overhead from additional IRQ entries.

**Concern — `INT_STAT` read without `INT_CLEAR`:** Looking at `panthor_job_irq_handler()`, the handler does call `gpu_write(pirq->iomem, INT_CLEAR, status)` at the start. The `INT_STAT` register reflects `INT_RAWSTAT & INT_MASK`. After clearing via `INT_CLEAR`, the `INT_RAWSTAT` bit drops, so `INT_STAT` for that bit also drops. New events will set new `INT_RAWSTAT` bits, generating new `INT_STAT` bits and new IRQs. This is correct.

**Minor concern — two `INT_STAT` reads:** The initial `INT_STAT` check and the second `INT_STAT` read passed to the handler could see different values if events arrive between them. The second read may include more events than the first, but that's fine — more events processed is better. If the second read returns 0 (all events cleared between reads), the handler would call `panthor_job_irq_handler(pirq, 0)` which does `gpu_write(..., INT_CLEAR, 0)` (no-op) and `panthor_sched_report_fw_events(ptdev, 0)` (which is benign — no bits set means no processing). So this edge case is harmless.

No blocking concerns.

---

---
Generated by Claude Code Patch Reviewer