From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/v3d: Remove dedicated fence_lock Date: Mon, 09 Mar 2026 09:20:16 +1000 Message-ID: In-Reply-To: <20260305-v3d-reset-locking-improv-v2-6-fd53c91f7726@igalia.com> References: <20260305-v3d-reset-locking-improv-v2-0-fd53c91f7726@igalia.com> <20260305-v3d-reset-locking-improv-v2-6-fd53c91f7726@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review With `queue_lock` no longer taken from the IRQ handler for stats updates (the original reason for the split), merging `fence_lock` back is a reasonable simplification: ```c dma_fence_init(&fence->base, &v3d_fence_ops, &queue->queue_lock, queue->fence_context, fence->seqno); ``` The reasoning in the commit message is sound. The `fence_lock` was introduced specifically because `v3d_job_update_stats` was taking `queue_lock` in the IRQ handler alongside DMA fence signaling, which caused PREEMPT_RT issues. That usage is now gone. One thing worth verifying: after this merge, `queue_lock` is used from process context with `spin_lock_irqsave` (e.g., in `v3d_bin_job_run`) and from IRQ context via `dma_fence_signal`. Since both paths use IRQ-safe locking, this is correct for non-RT kernels. On PREEMPT_RT, the situation is the same as the original `fence_lock` (both were `spinlock_t`), so no regression there. Clean and correct patch. --- Generated by Claude Code Patch Reviewer