Hi Melissa,
On 25/08/25 16:57, Melissa Wen wrote:
On 15/08/2025 16:58, Maíra Canal wrote:
Each V3D queue works independently and all the dependencies between the
jobs are handled through the DRM scheduler. Therefore, there is no need
to use one single lock for all queues. Using it, creates unnecessary
contention between different queues that can operate independently.
Replace the global spinlock with per-queue locks to improve parallelism
and reduce contention between different V3D queues (BIN, RENDER, TFU,
CSD). This allows independent queues to operate concurrently while
maintaining proper synchronization within each queue.
Signed-off-by: Maíra Canal <mca...@igalia.com>
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
---
drivers/gpu/drm/v3d/v3d_drv.h | 8 ++------
drivers/gpu/drm/v3d/v3d_fence.c | 11 ++++++-----
drivers/gpu/drm/v3d/v3d_gem.c | 3 ++-
drivers/gpu/drm/v3d/v3d_irq.c | 6 +++---
drivers/gpu/drm/v3d/v3d_sched.c | 13 +++++++------
5 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/
v3d_drv.h
index
d557caca5c4b8a7d7dcd35067208c5405de9df3c..cfc2f9c123aa99f6f1875b297eaf6c226b03d4ec 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -61,6 +61,7 @@ struct v3d_queue_state {
/* Currently active job for this queue */
struct v3d_job *active_job;
+ spinlock_t queue_lock;
};
/* Performance monitor object. The perform lifetime is controlled by
userspace
@@ -164,11 +165,6 @@ struct v3d_dev {
struct v3d_queue_state queue[V3D_MAX_QUEUES];
- /* Spinlock used to synchronize the overflow memory
- * management against bin job submission.
- */
- spinlock_t job_lock;
-
/* Used to track the active perfmon if any. */
struct v3d_perfmon *active_perfmon;
@@ -568,7 +564,7 @@ void v3d_get_stats(const struct v3d_stats *stats,
u64 timestamp,
/* v3d_fence.c */
extern const struct dma_fence_ops v3d_fence_ops;
-struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, enum
v3d_queue queue);
+struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, enum
v3d_queue q);
nit: Why rename queue -> q?
[...]
+struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, enum
v3d_queue q)
{
+ struct v3d_queue_state *queue = &v3d->queue[q];
I'm trying to establish a convention inside the driver in which q is
`enum v3d_queue` and queue is `struct `v3d_queue_state`, which was
created in PATCH 2/6.
Thanks for the review!
Best Regards,
- Maíra
