The binner slot pool, tracked by bin_alloc_used and bin_alloc_overflow, is serialized by job_lock. The pool has a lifecycle of its own: slots are claimed by bin_slot_try_alloc(), which the submit path retries from a wait loop, and released from the render done handler, from vc4_release_exec() and from the overflow worker. Each of those takes the device-wide job lock to update the binner slot pool, contending the job queues.
Give the pool its own spinlock. Note that, as vc4_irq_finish_render_job() frees the finished job's slots and vc4_overflow_mem_work() hand slots over to a queued job, they hold both locks, nesting bin_alloc_lock inside job_lock. However, nothing takes them in the other order. That leaves vc4_overflow_mem_work() holding job_lock over the binner overflow registers, which it never needed, as the only other writers of V3D_BPOA and V3D_BPOS are the bind and unbind paths, which run with the interrupt uninstalled. Write to them once the lock is dropped. Signed-off-by: Maíra Canal <[email protected]> --- drivers/gpu/drm/vc4/vc4_drv.h | 3 +++ drivers/gpu/drm/vc4/vc4_gem.c | 7 +++---- drivers/gpu/drm/vc4/vc4_irq.c | 46 ++++++++++++++++++++++--------------------- drivers/gpu/drm/vc4/vc4_v3d.c | 2 +- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index c6ae54d2e8b8..e2cc5bb78791 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -204,6 +204,9 @@ struct vc4_dev { /* Size of blocks allocated within bin_bo. */ uint32_t bin_alloc_size; + /* Protects @bin_alloc_used and @bin_alloc_overflow. */ + spinlock_t bin_alloc_lock; + /* Bitmask of the bin_alloc_size chunks in bin_bo that are * used. */ diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 230558b3f684..09aaa1b796ad 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -846,7 +846,6 @@ vc4_release_exec(struct kref *ref) struct vc4_exec_info *exec = container_of(ref, struct vc4_exec_info, refcount); struct vc4_dev *vc4 = exec->dev; - unsigned long irqflags; unsigned i; /* The render done handler signals the fence, so only drop the @@ -875,9 +874,8 @@ vc4_release_exec(struct kref *ref) * completion had their slots released in vc4_irq_finish_render_job(). * Only jobs that never completed still have slots to be released here. */ - spin_lock_irqsave(&vc4->job_lock, irqflags); - vc4->bin_alloc_used &= ~exec->bin_slots; - spin_unlock_irqrestore(&vc4->job_lock, irqflags); + scoped_guard(spinlock_irqsave, &vc4->bin_alloc_lock) + vc4->bin_alloc_used &= ~exec->bin_slots; /* Let anyone waiting on the binner pool retry. */ wake_up_all(&vc4->job_wait_queue); @@ -1176,6 +1174,7 @@ int vc4_gem_init(struct drm_device *dev) INIT_LIST_HEAD(&vc4->render_job_list); INIT_LIST_HEAD(&vc4->job_done_list); spin_lock_init(&vc4->job_lock); + spin_lock_init(&vc4->bin_alloc_lock); spin_lock_init(&vc4->hang_state_lock); spin_lock_init(&vc4->perfmon_state.lock); diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c index f8787d4169ef..2f3f6fbcedf3 100644 --- a/drivers/gpu/drm/vc4/vc4_irq.c +++ b/drivers/gpu/drm/vc4/vc4_irq.c @@ -64,8 +64,6 @@ vc4_overflow_mem_work(struct work_struct *work) container_of(work, struct vc4_dev, overflow_mem_work); struct vc4_bo *bo; int bin_bo_slot; - struct vc4_exec_info *exec; - unsigned long irqflags; mutex_lock(&vc4->bin_bo_lock); @@ -80,34 +78,37 @@ vc4_overflow_mem_work(struct work_struct *work) goto complete; } - spin_lock_irqsave(&vc4->job_lock, irqflags); + scoped_guard(spinlock_irqsave, &vc4->job_lock) { + struct vc4_exec_info *exec; - if (vc4->bin_alloc_overflow) { - /* If we had overflow memory allocated previously, - * then that chunk will free when the current bin job - * is done. If we don't have a bin job running, then - * the chunk will be done whenever the list of render - * jobs has drained. - */ - exec = vc4_first_bin_job(vc4); - if (!exec) - exec = vc4_last_render_job(vc4); - if (exec) { - exec->bin_slots |= vc4->bin_alloc_overflow; - } else { - /* There's nothing queued in the hardware, so - * the old slot is free immediately. + guard(spinlock)(&vc4->bin_alloc_lock); + + if (vc4->bin_alloc_overflow) { + /* If we had overflow memory allocated previously, + * then that chunk will free when the current bin job + * is done. If we don't have a bin job running, then + * the chunk will be done whenever the list of render + * jobs has drained. */ - vc4->bin_alloc_used &= ~vc4->bin_alloc_overflow; + exec = vc4_first_bin_job(vc4); + if (!exec) + exec = vc4_last_render_job(vc4); + if (exec) { + exec->bin_slots |= vc4->bin_alloc_overflow; + } else { + /* There's nothing queued in the hardware, so + * the old slot is free immediately. + */ + vc4->bin_alloc_used &= ~vc4->bin_alloc_overflow; + } } + vc4->bin_alloc_overflow = BIT(bin_bo_slot); } - vc4->bin_alloc_overflow = BIT(bin_bo_slot); V3D_WRITE(V3D_BPOA, bo->base.dma_addr + bin_bo_slot * vc4->bin_alloc_size); V3D_WRITE(V3D_BPOS, vc4->bin_alloc_size); V3D_WRITE(V3D_INTCTL, V3D_INT_OUTOMEM); V3D_WRITE(V3D_INTENA, V3D_INT_OUTOMEM); - spin_unlock_irqrestore(&vc4->job_lock, irqflags); complete: mutex_unlock(&vc4->bin_bo_lock); @@ -165,7 +166,8 @@ vc4_irq_finish_render_job(struct drm_device *dev) vc4->finished_seqno++; - vc4->bin_alloc_used &= ~exec->bin_slots; + scoped_guard(spinlock, &vc4->bin_alloc_lock) + vc4->bin_alloc_used &= ~exec->bin_slots; exec->bin_slots = 0; list_move_tail(&exec->head, &vc4->job_done_list); diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index 8a3c90136506..b4bb90765795 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -177,7 +177,7 @@ static int bin_slot_try_alloc(struct vc4_dev *vc4) { int slot; - guard(spinlock_irqsave)(&vc4->job_lock); + guard(spinlock_irqsave)(&vc4->bin_alloc_lock); slot = ffs(~vc4->bin_alloc_used); if (slot != 0) { -- 2.55.0
