* [PATCH v3 1/2] drm/v3d: Refactor perfmon locking
2026-07-04 12:15 [PATCH v3 0/2] drm/v3d: Fix perfmon locking and cross-queue isolation Maíra Canal
@ 2026-07-04 12:15 ` Maíra Canal
2026-07-04 12:31 ` sashiko-bot
2026-07-04 12:15 ` [PATCH v3 2/2] drm/v3d: Serialize jobs across queues when a perfmon is attached Maíra Canal
1 sibling, 1 reply; 5+ messages in thread
From: Maíra Canal @ 2026-07-04 12:15 UTC (permalink / raw)
To: Melissa Wen, Iago Toral, Tvrtko Ursulin, David Airlie,
Simona Vetter
Cc: kernel-dev, dri-devel, Maíra Canal
v3d exposes a single set of performance counters per core, so at any
moment at most one performance monitor can be programmed in HW. In
software, this singleton is represented by v3d_dev->active_perfmon, but
until now nothing actually serialized access to it: scheduler callbacks,
the GPU-reset path, and perfmon ioctls all read and wrote that field
lock-free.
The existence of v3d_perfmon->lock mutex did not close the gap. It
serialized start/stop of *one* perfmon object against itself, but the
invariant that needs protection is device-wide: there can be exactly one
active perfmon at any moment in HW. Two threads acting on different
perfmon objects could race through v3d_dev->active_perfmon and the
counter registers, leaving software and HW out of sync.
This commit moves the locking to where the invariant actually lives. Group
the active perfmon pointer with a device-wide spinlock and route every
state transition (job start, job completion, set global, reset,
suspend/resume, destruction) through a small set of locked entry points
that are the only mutators of the HW counters.
Some design improvements needed to be made for the refactor:
1. Stop the perfmon from the IRQ handler at job-completion time (the
natural boundary for "active perfmon follows the active job"). This
required a change from a mutex to a spinlock. This solves another
issue of the existing design: perfmon start/stop was exclusively
attached to run_job() callbacks, which means that if nothing was
further queued up, a perfmon would never actually be stopped.
2. Pause/resume the HW counters across runtime-PM transitions without
dropping the software reference. This preserves the perfmon state
while the device is idle.
3. Move the global perfmon lifecycle management to the set_global
IOCTL. This simplifies the logic in v3d_perfmon_start() and
v3d_perfmon_stop(), as there is no need to always check if the
global perfmon is enabled.
4. v3d_perfmon_get_values_ioctl() doesn't stop the perfmon when
capturing the values. All lifecycle management is handled by the
job (for per-job perfmons) or the set_global IOCTL (for global
perfmons).
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/v3d_drv.h | 17 ++--
drivers/gpu/drm/v3d/v3d_gem.c | 4 +-
drivers/gpu/drm/v3d/v3d_irq.c | 7 +-
drivers/gpu/drm/v3d/v3d_perfmon.c | 183 +++++++++++++++++++++++++++-----------
drivers/gpu/drm/v3d/v3d_power.c | 4 +
drivers/gpu/drm/v3d/v3d_sched.c | 26 ++----
drivers/gpu/drm/v3d/v3d_submit.c | 6 +-
7 files changed, 165 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 2e298d7545c0..cf836c4f13ad 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -87,9 +87,6 @@ struct v3d_perfmon {
*/
refcount_t refcnt;
- /* Protects perfmon stop, as it can be invoked from multiple places. */
- struct mutex lock;
-
/* Number of counters activated in this perfmon instance
* (should be less than DRM_V3D_MAX_PERF_COUNTERS).
*/
@@ -171,8 +168,14 @@ struct v3d_dev {
struct v3d_queue_state queue[V3D_MAX_QUEUES];
- /* Used to track the active perfmon if any. */
- struct v3d_perfmon *active_perfmon;
+ /* Tracks the performance monitor state. */
+ struct {
+ /* Protects @active. */
+ spinlock_t lock;
+
+ /* Perfmon currently programmed in HW (or NULL if none). */
+ struct v3d_perfmon *active;
+ } perfmon_state;
/* Protects bo_stats */
struct mutex bo_lock;
@@ -667,6 +670,10 @@ void v3d_perfmon_put(struct v3d_perfmon *perfmon);
void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon);
void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
bool capture);
+void v3d_perfmon_stop_locked(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
+ bool capture);
+void v3d_perfmon_suspend(struct v3d_dev *v3d);
+void v3d_perfmon_resume(struct v3d_dev *v3d);
struct v3d_perfmon *v3d_perfmon_find(struct v3d_file_priv *v3d_priv, int id);
void v3d_perfmon_open_file(struct v3d_file_priv *v3d_priv);
void v3d_perfmon_close_file(struct v3d_file_priv *v3d_priv);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index c43d9af41374..7ad81b146f02 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -137,7 +137,8 @@ v3d_reset(struct v3d_dev *v3d)
v3d_mmu_set_page_table(v3d);
v3d_irq_reset(v3d);
- v3d_perfmon_stop(v3d, v3d->active_perfmon, false);
+ /* Re-arm the global perfmon HW counters that the reset zeroed. */
+ v3d_perfmon_resume(v3d);
trace_v3d_reset_end(dev);
}
@@ -307,6 +308,7 @@ v3d_gem_init(struct drm_device *dev)
}
spin_lock_init(&v3d->mm_lock);
+ spin_lock_init(&v3d->perfmon_state.lock);
ret = drmm_mutex_init(dev, &v3d->bo_lock);
if (ret)
goto err_stats;
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 86efaef2722c..963d711dc16b 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -90,9 +90,12 @@ v3d_irq_signal_fence(struct v3d_dev *v3d, enum v3d_queue q,
void (*trace_irq)(struct drm_device *, uint64_t))
{
struct v3d_queue_state *queue = &v3d->queue[q];
- struct v3d_fence *fence = to_v3d_fence(queue->active_job->irq_fence);
+ struct v3d_job *job = queue->active_job;
+ struct v3d_fence *fence = to_v3d_fence(job->irq_fence);
- v3d_job_update_stats(queue->active_job);
+ v3d_perfmon_stop(v3d, job->perfmon, true);
+
+ v3d_job_update_stats(job);
trace_irq(&v3d->drm, fence->seqno);
queue->active_job = NULL;
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 48ae748247be..3ad0f022753c 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -217,26 +217,15 @@ void v3d_perfmon_get(struct v3d_perfmon *perfmon)
void v3d_perfmon_put(struct v3d_perfmon *perfmon)
{
- if (perfmon && refcount_dec_and_test(&perfmon->refcnt)) {
- mutex_destroy(&perfmon->lock);
+ if (perfmon && refcount_dec_and_test(&perfmon->refcnt))
kfree(perfmon);
- }
}
-void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
+static void v3d_perfmon_hw_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
{
+ u8 ncounters = perfmon->ncounters;
+ u32 mask = GENMASK(ncounters - 1, 0);
unsigned int i;
- u32 mask;
- u8 ncounters;
-
- if (WARN_ON_ONCE(!perfmon || v3d->active_perfmon))
- return;
-
- if (!pm_runtime_get_if_active(v3d->drm.dev))
- return;
-
- ncounters = perfmon->ncounters;
- mask = GENMASK(ncounters - 1, 0);
for (i = 0; i < ncounters; i++) {
u32 source = i / 4;
@@ -258,39 +247,106 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
V3D_CORE_WRITE(0, V3D_V4_PCTR_0_EN, mask);
V3D_CORE_WRITE(0, V3D_V4_PCTR_0_CLR, mask);
V3D_CORE_WRITE(0, V3D_PCTR_0_OVERFLOW, mask);
+}
- v3d->active_perfmon = perfmon;
+static void v3d_perfmon_hw_capture(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
+{
+ u32 mask = GENMASK(perfmon->ncounters - 1, 0);
+ for (int i = 0; i < perfmon->ncounters; i++)
+ perfmon->values[i] += V3D_CORE_READ(0, V3D_PCTR_0_PCTRX(i));
+
+ V3D_CORE_WRITE(0, V3D_V4_PCTR_0_CLR, mask);
+}
+
+static void v3d_perfmon_hw_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
+ bool capture)
+{
+ if (capture)
+ v3d_perfmon_hw_capture(v3d, perfmon);
+
+ V3D_CORE_WRITE(0, V3D_V4_PCTR_0_EN, 0);
+}
+
+void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
+{
+ guard(spinlock_irqsave)(&v3d->perfmon_state.lock);
+
+ if (!perfmon || v3d->global_perfmon)
+ return;
+
+ if (!pm_runtime_get_if_active(v3d->drm.dev))
+ return;
+
+ v3d_perfmon_hw_start(v3d, perfmon);
+ v3d->perfmon_state.active = perfmon;
+
+ v3d_pm_runtime_put(v3d);
+}
+
+static void v3d_perfmon_capture_locked(struct v3d_dev *v3d,
+ struct v3d_perfmon *perfmon)
+{
+ lockdep_assert_held(&v3d->perfmon_state.lock);
+
+ if (!perfmon || perfmon != v3d->perfmon_state.active)
+ return;
+
+ if (!pm_runtime_get_if_active(v3d->drm.dev))
+ return;
+
+ v3d_perfmon_hw_capture(v3d, perfmon);
+ v3d_pm_runtime_put(v3d);
+}
+
+void v3d_perfmon_stop_locked(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
+ bool capture)
+{
+ lockdep_assert_held(&v3d->perfmon_state.lock);
+
+ if (!perfmon || perfmon != v3d->perfmon_state.active)
+ return;
+
+ v3d->perfmon_state.active = NULL;
+
+ /* If the device is suspended, the HW has already stopped counting. */
+ if (!pm_runtime_get_if_active(v3d->drm.dev))
+ return;
+
+ v3d_perfmon_hw_stop(v3d, perfmon, capture);
v3d_pm_runtime_put(v3d);
}
void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
bool capture)
{
- unsigned int i;
-
- if (!perfmon || !v3d->active_perfmon)
+ if (!perfmon)
return;
- mutex_lock(&perfmon->lock);
- if (perfmon != v3d->active_perfmon)
- goto out;
+ guard(spinlock_irqsave)(&v3d->perfmon_state.lock);
+ v3d_perfmon_stop_locked(v3d, perfmon, capture);
+}
- if (!pm_runtime_get_if_active(v3d->drm.dev))
- goto out_clear;
+void
+v3d_perfmon_suspend(struct v3d_dev *v3d)
+{
+ guard(spinlock_irqsave)(&v3d->perfmon_state.lock);
- if (capture)
- for (i = 0; i < perfmon->ncounters; i++)
- perfmon->values[i] += V3D_CORE_READ(0, V3D_PCTR_0_PCTRX(i));
+ if (!v3d->perfmon_state.active)
+ return;
- V3D_CORE_WRITE(0, V3D_V4_PCTR_0_EN, 0);
+ v3d_perfmon_hw_stop(v3d, v3d->perfmon_state.active, true);
+}
- v3d_pm_runtime_put(v3d);
+void
+v3d_perfmon_resume(struct v3d_dev *v3d)
+{
+ guard(spinlock_irqsave)(&v3d->perfmon_state.lock);
-out_clear:
- v3d->active_perfmon = NULL;
-out:
- mutex_unlock(&perfmon->lock);
+ if (!v3d->perfmon_state.active)
+ return;
+
+ v3d_perfmon_hw_start(v3d, v3d->perfmon_state.active);
}
struct v3d_perfmon *v3d_perfmon_find(struct v3d_file_priv *v3d_priv, int id)
@@ -316,14 +372,17 @@ static void v3d_perfmon_delete(struct v3d_file_priv *v3d_priv,
struct v3d_dev *v3d = v3d_priv->v3d;
/* If the active perfmon is being destroyed, stop it first */
- if (perfmon == v3d->active_perfmon)
- v3d_perfmon_stop(v3d, perfmon, false);
+ scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock) {
+ v3d_perfmon_stop_locked(v3d, perfmon, false);
- /* If the global perfmon is being destroyed, clean it and release
- * the reference stashed in v3d_perfmon_set_global_ioctl().
- */
- if (cmpxchg(&v3d->global_perfmon, perfmon, NULL) == perfmon)
- v3d_perfmon_put(perfmon);
+ /* If the global perfmon is being destroyed, clean it and release
+ * the reference stashed in v3d_perfmon_set_global_ioctl().
+ */
+ if (v3d->global_perfmon == perfmon) {
+ v3d_perfmon_put(v3d->global_perfmon);
+ v3d->global_perfmon = NULL;
+ }
+ }
v3d_perfmon_put(perfmon);
}
@@ -371,12 +430,10 @@ int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
perfmon->ncounters = req->ncounters;
refcount_set(&perfmon->refcnt, 1);
- mutex_init(&perfmon->lock);
ret = xa_alloc(&v3d_priv->perfmons, &id, perfmon, xa_limit_32b,
GFP_KERNEL);
if (ret < 0) {
- mutex_destroy(&perfmon->lock);
kfree(perfmon);
return ret;
}
@@ -408,7 +465,9 @@ int v3d_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
struct v3d_dev *v3d = to_v3d_dev(dev);
struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
struct drm_v3d_perfmon_get_values *req = data;
+ u64 values[DRM_V3D_MAX_PERF_COUNTERS];
struct v3d_perfmon *perfmon;
+ size_t size;
int ret = 0;
if (req->pad != 0)
@@ -418,10 +477,14 @@ int v3d_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
if (!perfmon)
return -EINVAL;
- v3d_perfmon_stop(v3d, perfmon, true);
+ size = perfmon->ncounters * sizeof(u64);
- if (copy_to_user(u64_to_user_ptr(req->values_ptr), perfmon->values,
- perfmon->ncounters * sizeof(u64)))
+ scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock) {
+ v3d_perfmon_capture_locked(v3d, perfmon);
+ memcpy(values, perfmon->values, size);
+ }
+
+ if (copy_to_user(u64_to_user_ptr(req->values_ptr), values, size))
ret = -EFAULT;
v3d_perfmon_put(perfmon);
@@ -482,18 +545,36 @@ int v3d_perfmon_set_global_ioctl(struct drm_device *dev, void *data,
*/
v3d_perfmon_put(perfmon);
- old = xchg(&v3d->global_perfmon, NULL);
- if (!old)
- return -EINVAL;
+ scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock) {
+ old = v3d->global_perfmon;
+ if (!old)
+ return -EINVAL;
+
+ v3d_perfmon_stop_locked(v3d, old, true);
+ v3d->global_perfmon = NULL;
+ }
v3d_perfmon_put(old);
return 0;
}
- if (cmpxchg(&v3d->global_perfmon, NULL, perfmon)) {
- v3d_perfmon_put(perfmon);
- return -EBUSY;
+ scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock) {
+ if (v3d->perfmon_state.active || v3d->global_perfmon) {
+ v3d_perfmon_put(perfmon);
+ return -EBUSY;
+ }
+
+ v3d->global_perfmon = perfmon;
+ v3d->perfmon_state.active = perfmon;
+
+ /* If the device is suspended, v3d_perfmon_resume() will
+ * program the HW on the next resume.
+ */
+ if (pm_runtime_get_if_active(v3d->drm.dev)) {
+ v3d_perfmon_hw_start(v3d, perfmon);
+ v3d_pm_runtime_put(v3d);
+ }
}
return 0;
diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/v3d_power.c
index f7df6393d38f..ade8e932fb9c 100644
--- a/drivers/gpu/drm/v3d/v3d_power.c
+++ b/drivers/gpu/drm/v3d/v3d_power.c
@@ -50,6 +50,8 @@ int v3d_power_suspend(struct device *dev)
struct v3d_dev *v3d = to_v3d_dev(drm);
int ret;
+ v3d_perfmon_suspend(v3d);
+
v3d_irq_disable(v3d);
v3d_clean_caches(v3d);
@@ -85,5 +87,7 @@ int v3d_power_resume(struct device *dev)
v3d_mmu_set_page_table(v3d);
v3d_irq_enable(v3d);
+ v3d_perfmon_resume(v3d);
+
return 0;
}
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index e950cee31bcb..5f33d3794598 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -125,24 +125,6 @@ v3d_performance_query_info_free(struct v3d_performance_query_info *query_info,
}
}
-static void
-v3d_switch_perfmon(struct v3d_dev *v3d, struct v3d_job *job)
-{
- struct v3d_perfmon *perfmon = v3d->global_perfmon;
-
- if (!perfmon)
- perfmon = job->perfmon;
-
- if (perfmon == v3d->active_perfmon)
- return;
-
- if (perfmon != v3d->active_perfmon)
- v3d_perfmon_stop(v3d, v3d->active_perfmon, true);
-
- if (perfmon && v3d->active_perfmon != perfmon)
- v3d_perfmon_start(v3d, perfmon);
-}
-
static void
v3d_stats_start(struct v3d_stats *stats, u64 now)
{
@@ -220,7 +202,7 @@ static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)
job->start, job->end);
v3d_job_start_stats(&job->base);
- v3d_switch_perfmon(v3d, &job->base);
+ v3d_perfmon_start(v3d, job->base.perfmon);
/* Set the current and end address of the control list.
* Writing the end register is what starts the job.
@@ -278,7 +260,7 @@ static struct dma_fence *v3d_render_job_run(struct drm_sched_job *sched_job)
job->start, job->end);
v3d_job_start_stats(&job->base);
- v3d_switch_perfmon(v3d, &job->base);
+ v3d_perfmon_start(v3d, job->base.perfmon);
/* XXX: Set the QCFG */
@@ -381,7 +363,7 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
trace_v3d_submit_csd(dev, to_v3d_fence(fence)->seqno);
v3d_job_start_stats(&job->base);
- v3d_switch_perfmon(v3d, &job->base);
+ v3d_perfmon_start(v3d, job->base.perfmon);
csd_cfg0_reg = V3D_CSD_QUEUED_CFG0(v3d->ver);
for (i = 1; i <= 6; i++)
@@ -723,6 +705,8 @@ v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job *sched_job,
if (sched_job)
drm_sched_increase_karma(sched_job);
+ v3d_perfmon_stop(v3d, job->perfmon, false);
+
/* get the GPU back into the init state */
v3d_reset(v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index ee2ac2540ed5..27cc98770b37 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -302,8 +302,10 @@ v3d_attach_perfmon_to_jobs(struct v3d_submit *submit, u32 perfmon_id)
if (!perfmon_id)
return 0;
- if (v3d->global_perfmon)
- return -EAGAIN;
+ scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock) {
+ if (v3d->global_perfmon)
+ return -EAGAIN;
+ }
perfmon = v3d_perfmon_find(v3d_priv, perfmon_id);
if (!perfmon)
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3 2/2] drm/v3d: Serialize jobs across queues when a perfmon is attached
2026-07-04 12:15 [PATCH v3 0/2] drm/v3d: Fix perfmon locking and cross-queue isolation Maíra Canal
2026-07-04 12:15 ` [PATCH v3 1/2] drm/v3d: Refactor perfmon locking Maíra Canal
@ 2026-07-04 12:15 ` Maíra Canal
2026-07-04 12:29 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Maíra Canal @ 2026-07-04 12:15 UTC (permalink / raw)
To: Melissa Wen, Iago Toral, Tvrtko Ursulin, David Airlie,
Simona Vetter
Cc: kernel-dev, dri-devel, Maíra Canal
A non-global perfmon is meant to count events generated by a specific
submission, but the scheduler can run jobs from different queues
concurrently on the same V3D core. Without explicit serialization, an
unrelated job running in parallel with a perfmon-carrying job pollutes
the counters and generates unusable results.
To address such issue, we must enforce cross-queue serialization when we
detect a perfmon-carrying submission. It's possible to implement
serialization by enforcing two rules:
1. A job that carries a non-global perfmon must wait for every job
currently in-flight across all HW queues to finish.
2. While a perfmon-carrying job is still in-flight, all subsequently
submitted jobs must wait for it.
Note that serialization is not needed in the global perfmon case, as the
global perfmon tracks activity from all jobs, so concurrency is desirable.
Therefore, check if serialization is needed during job submission and if
so, attach fence dependences to enforce cross-queue serialization.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/v3d_drv.h | 32 ++++++++++++++++----
drivers/gpu/drm/v3d/v3d_gem.c | 3 ++
drivers/gpu/drm/v3d/v3d_perfmon.c | 6 ++++
drivers/gpu/drm/v3d/v3d_submit.c | 62 +++++++++++++++++++++++++++++++++++++++
4 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index cf836c4f13ad..1b88b3dff757 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -75,11 +75,13 @@ struct v3d_queue_state {
spinlock_t queue_lock;
};
-/* Performance monitor object. The perform lifetime is controlled by userspace
- * using perfmon related ioctls. A perfmon can be attached to a submit_cl
- * request, and when this is the case, HW perf counters will be activated just
- * before the submit_cl is submitted to the GPU and disabled when the job is
- * done. This way, only events related to a specific job will be counted.
+/* Performance monitor object
+ *
+ * The performance monitor (perfmon) lifetime is controlled by userspace using
+ * perfmon related ioctls. A perfmon can be attached to a CL or CSD submission
+ * request, and when it is, HW performance counters will be activated just
+ * before the job is submitted to the GPU and disabled when the job is done.
+ * This way, only events related to a specific submission will be counted.
*/
struct v3d_perfmon {
/* Tracks the number of users of the perfmon, when this counter reaches
@@ -168,13 +170,31 @@ struct v3d_dev {
struct v3d_queue_state queue[V3D_MAX_QUEUES];
- /* Tracks the performance monitor state. */
+ /*
+ * Tracks the performance monitor state and consistency.
+ *
+ * When a non-global perfmon is attached to a job, the scheduler must
+ * not run any other job on the HW concurrently (otherwise, the
+ * counters would be polluted by unrelated work).
+ */
struct {
/* Protects @active. */
spinlock_t lock;
/* Perfmon currently programmed in HW (or NULL if none). */
struct v3d_perfmon *active;
+
+ /* Finished fence of the most recently submitted job that
+ * opened a serialization window (i.e. a job with a non-global
+ * perfmon attached).
+ */
+ struct dma_fence *fence;
+
+ /* Finished fence of the most recently submitted job on each HW
+ * queue. Used so that a new perfmon-carrying job can depend on
+ * every job currently in-flight across all queues.
+ */
+ struct dma_fence *last_hw_fence[V3D_MAX_QUEUES];
} perfmon_state;
/* Protects bo_stats */
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 7ad81b146f02..18c941f15e2e 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -371,8 +371,11 @@ v3d_gem_destroy(struct drm_device *dev)
for (q = 0; q < V3D_MAX_QUEUES; q++) {
WARN_ON(v3d->queue[q].active_job);
v3d_stats_put(v3d->queue[q].stats);
+ dma_fence_put(v3d->perfmon_state.last_hw_fence[q]);
}
+ dma_fence_put(v3d->perfmon_state.fence);
+
drm_mm_takedown(&v3d->mm);
dma_free_coherent(v3d->drm.dev, 4096 * 1024, (void *)v3d->pt,
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 3ad0f022753c..07dab7fb3060 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -275,6 +275,12 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
if (!perfmon || v3d->global_perfmon)
return;
+ /* Cross-queue serialization should have drained any previous perfmon
+ * job before this one runs.
+ */
+ if (WARN_ON_ONCE(v3d->perfmon_state.active))
+ return;
+
if (!pm_runtime_get_if_active(v3d->drm.dev))
return;
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 27cc98770b37..2bb955a9ed5e 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -320,6 +320,62 @@ v3d_attach_perfmon_to_jobs(struct v3d_submit *submit, u32 perfmon_id)
return 0;
}
+/*
+ * Prepare fences to enforce job serialization when a perfmon is active. A job
+ * that carries a non-global perfmon must wait for every job currently in-flight
+ * across all HW queues to finish, otherwise concurrent unrelated work on the
+ * same core would pollute the performance counters. Symmetrically, while such a
+ * job is still in-flight, all subsequently submitted jobs must wait for it.
+ *
+ * We don't serialize the jobs when using a global perfmon as it's expected to
+ * track concurrent activity from all jobs.
+ */
+static int
+v3d_serialize_for_perfmon(struct v3d_job *job)
+{
+ struct v3d_dev *v3d = job->v3d;
+ bool is_global_perfmon;
+ int ret;
+
+ lockdep_assert_held(&v3d->sched_lock);
+
+ scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock)
+ is_global_perfmon = !!v3d->global_perfmon;
+
+ if (is_global_perfmon)
+ goto publish;
+
+ if (job->perfmon) {
+ for (enum v3d_queue q = 0; q < V3D_MAX_QUEUES; q++) {
+ struct dma_fence *f = v3d->perfmon_state.last_hw_fence[q];
+
+ if (!f || dma_fence_is_signaled(f))
+ continue;
+
+ ret = drm_sched_job_add_dependency(&job->base, dma_fence_get(f));
+ if (ret)
+ return ret;
+ }
+ } else if (v3d->perfmon_state.fence &&
+ !dma_fence_is_signaled(v3d->perfmon_state.fence)) {
+ ret = drm_sched_job_add_dependency(&job->base,
+ dma_fence_get(v3d->perfmon_state.fence));
+ if (ret)
+ return ret;
+ }
+
+publish:
+ dma_fence_put(v3d->perfmon_state.last_hw_fence[job->queue]);
+ v3d->perfmon_state.last_hw_fence[job->queue] = dma_fence_get(job->done_fence);
+
+ if (job->perfmon && !is_global_perfmon) {
+ dma_fence_put(v3d->perfmon_state.fence);
+ v3d->perfmon_state.fence = dma_fence_get(job->done_fence);
+ }
+
+ return 0;
+}
+
static void
v3d_submit_attach_object_fences(struct v3d_submit *submit)
{
@@ -396,6 +452,12 @@ v3d_submit_jobs(struct v3d_submit *submit, struct drm_syncobj *sync_out,
goto err;
}
+ for (int i = 0; i < submit->job_count; i++) {
+ ret = v3d_serialize_for_perfmon(submit->jobs[i]);
+ if (ret)
+ goto err;
+ }
+
for (int i = 0; i < submit->job_count; i++)
drm_sched_entity_push_job(&submit->jobs[i]->base);
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread