On 10/05/2026 23:12, Maíra Canal wrote:
queue->queue_lock is only acquired from v3d_overflow_mem_work() and
v3d_bin_job_run(), both of which run from workqueue context. The hard
IRQ handler does not take this lock, so disabling interrupts while
holding it is unnecessary.
Drop the spin_lock_irqsave() and use plain spin_(un)lock() instead.
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/v3d/v3d_irq.c | 17 +++++++----------
drivers/gpu/drm/v3d/v3d_sched.c | 21 ++++++++++-----------
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 86efaef2722c..754a969b862b 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -47,7 +47,6 @@ v3d_overflow_mem_work(struct work_struct *work)
struct v3d_queue_state *queue = &v3d->queue[V3D_BIN];
struct v3d_bin_job *bin_job;
struct drm_gem_object *obj;
- unsigned long irqflags;
if (IS_ERR(bo)) {
drm_err(dev, "Couldn't allocate binner overflow mem\n");
@@ -64,18 +63,16 @@ v3d_overflow_mem_work(struct work_struct *work)
* bin job got scheduled, that's fine. We'll just give them
* some binner pool anyway.
*/
- spin_lock_irqsave(&queue->queue_lock, irqflags);
- bin_job = (struct v3d_bin_job *)queue->active_job;
+ scoped_guard(spinlock, &queue->queue_lock) {
+ bin_job = (struct v3d_bin_job *)queue->active_job;
- if (!bin_job) {
- spin_unlock_irqrestore(&queue->queue_lock, irqflags);
- goto out;
+ if (!bin_job)
+ goto out;
+
+ drm_gem_object_get(obj);
+ list_add_tail(&bo->unref_head, &bin_job->render->unref_list);
}
- drm_gem_object_get(obj);
- list_add_tail(&bo->unref_head, &bin_job->render->unref_list);
- spin_unlock_irqrestore(&queue->queue_lock, irqflags);
-
v3d_mmu_flush_all(v3d);
V3D_CORE_WRITE(0, V3D_PTB_BPOA, bo->node.start << V3D_MMU_PAGE_SHIFT);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 66569b538e4e..cc3212e2cb5d 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -204,7 +204,6 @@ static struct dma_fence *v3d_bin_job_run(struct
drm_sched_job *sched_job)
struct v3d_queue_state *queue = &v3d->queue[V3D_BIN];
struct drm_device *dev = &v3d->drm;
struct dma_fence *fence;
- unsigned long irqflags;
if (unlikely(job->base.base.s_fence->finished.error))
goto out_clean_job;
@@ -212,13 +211,13 @@ static struct dma_fence *v3d_bin_job_run(struct
drm_sched_job *sched_job)
/* Lock required around bin_job update vs
* v3d_overflow_mem_work().
*/
- spin_lock_irqsave(&queue->queue_lock, irqflags);
- queue->active_job = &job->base;
- /* Clear out the overflow allocation, so we don't
- * reuse the overflow attached to a previous job.
- */
- V3D_CORE_WRITE(0, V3D_PTB_BPOS, 0);
- spin_unlock_irqrestore(&queue->queue_lock, irqflags);
+ scoped_guard(spinlock, &queue->queue_lock) {
+ queue->active_job = &job->base;
+ /* Clear out the overflow allocation, so we don't
+ * reuse the overflow attached to a previous job.
+ */
+ V3D_CORE_WRITE(0, V3D_PTB_BPOS, 0);
Transformation is like-for-like and after the previous patch irq safety
is indeed not needed, so:
Reviewed-by: Tvrtko Ursulin <[email protected]>
The only thing I am not sure of is whether this write to V3D_PTB_BPOS
needs to under the lock. Bin job run callback is the only place which
touches it so maybe not? And if it doesn't, then of course do that
movement in a separate patch.
Regards,
Tvrtko
+ }
v3d_invalidate_caches(v3d);
@@ -254,9 +253,9 @@ static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)
return fence;
out_clean_job:
- spin_lock_irqsave(&queue->queue_lock, irqflags);
- queue->active_job = NULL;
- spin_unlock_irqrestore(&queue->queue_lock, irqflags);
+ scoped_guard(spinlock, &queue->queue_lock) {
+ queue->active_job = NULL;
+ }
return NULL;
}
