Claude review: drm/panthor: Prepare the scheduler logic for FW events in IRQ context

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

Patch Review

This is the most substantial and critical patch in the series. Key changes:

**1. New `events_lock` spinlock:** Replaces the `sched->lock` mutex for the event processing path. This is necessary because mutex can't be held in IRQ context.

**2. Event processing inlined into `panthor_sched_report_fw_events()`:** The old `fw_events_work` work item is eliminated. Events are now processed directly under `events_lock`:
```c
guard(spinlock_irqsave)(&ptdev->scheduler->events_lock);
// process events inline
```

**3. Fence signalling moved inline:** `group_sync_upd_work` (work item) replaced by `group_check_job_completion()` called directly from `csg_slot_sync_update_locked()`. This is the key change enabling fence signalling from IRQ context. The `dma_fence_signal_locked()` call is correctly wrapped with `dma_fence_begin_signalling()` / `dma_fence_end_signalling()`.

**4. Lock upgrades:** Multiple `spin_lock()` converted to `spin_lock_irqsave()` variants for `fence_ctx.lock` and `fdinfo.lock`, since these are now taken under `events_lock` (IRQ context).

**Concern — `events_lock` scope in `tick_ctx_apply` and `panthor_sched_suspend`:**

In `tick_ctx_apply`, the patch uses `guard(spinlock_irqsave)` inside an `if` block:
```c
if (group->csg_id >= 0) {
    guard(spinlock_irqsave)(&sched->events_lock);
    sched_process_csg_irq_locked(ptdev, group->csg_id);
}

group_unbind_locked(group);
```

The `guard()` acquires at declaration and releases at scope exit. Since it's declared inside the `if` block, the lock is released when the `if` block exits, **before** `group_unbind_locked()` is called. However, `group_unbind_locked()` itself takes `events_lock` internally (for the slot reassignment). So the lock is released between `sched_process_csg_irq_locked()` and the unbind's `events_lock` section. There's a theoretical window where a new IRQ could arrive and process events for this CSG slot between the two. This is addressed in patch 7 by moving the `sched_process_csg_irq_locked()` call into `group_unbind_locked()` itself, making it atomic. So while this patch has a subtle window, patch 7 fixes it. This is acceptable as a series but worth noting the ordering dependency.

**Concern — `sched_queue_delayed_work` from IRQ context:**

`sched_queue_delayed_work` calls `mod_delayed_work()`, which is documented as IRQ-safe, so this is fine.

**Concern — `cancel_delayed_work` from IRQ context:**

`queue_suspend_timeout_locked()` calls `cancel_delayed_work()` (not `_sync`), which is IRQ-safe. Good.

**Concern — double `dma_fence_begin_signalling`:**

In `queue_check_job_completion()`:
```c
cookie = dma_fence_begin_signalling();
scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock) { ... }
dma_fence_end_signalling(cookie);
```

And the caller `group_check_job_completion()` also does:
```c
cookie = dma_fence_begin_signalling();
for (...) queue_check_job_completion(queue);
dma_fence_end_signalling(cookie);
```

This results in nested `dma_fence_begin_signalling()` calls. The kernel's fence signalling annotation framework supports nesting (it's a counter), so this is correct but redundant. The inner `dma_fence_begin_signalling` in `queue_check_job_completion` could be removed since the caller already establishes the signalling context, but it's not a bug.

Overall this patch is the riskiest in the series but the approach is sound.

---

---
Generated by Claude Code Patch Reviewer