Every job merges its finished fence into the per-queue accumulator so
that a job carrying a perfmon can later depend on everything still in
flight. This is a (relatively high) cost that every job pays before
it is submitted.

Perfmons only exist while userspace runs a performance query, so the
common case is a client paying that on every job for a dependency that is
never actually added, increasing the submission latency.

Address this situation by counting the number of perfmons alive on the
device and returning early while that count is zero. This means that if
no perfmon exists, v3d_serialize_for_perfmon() will bail out immediately.

Jobs submitted while no perfmon is alive stay unaccounted and may overlap
the first measured job. The count returns to zero whenever the last
perfmon is destroyed, so that window reopens on every measurement cycle.
Closing it would mean merging a fence on every submission for the
lifetime of the device, so it is a reasonable compromise for the average
use case.

Reviewed-by: Iago Toral Quiroga <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
---
 drivers/gpu/drm/v3d/v3d_drv.h     | 13 +++++++++++--
 drivers/gpu/drm/v3d/v3d_perfmon.c |  8 ++++++--
 drivers/gpu/drm/v3d/v3d_submit.c  |  7 +++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 26f8c3f635f9..02b6ab7dc72b 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -84,6 +84,8 @@ struct v3d_queue_state {
  * This way, only events related to a specific submission will be counted.
  */
 struct v3d_perfmon {
+       struct v3d_dev *v3d;
+
        /* Tracks the number of users of the perfmon, when this counter reaches
         * zero the perfmon is destroyed.
         */
@@ -184,6 +186,11 @@ struct v3d_dev {
                /* Perfmon currently programmed in HW (or NULL if none). */
                struct v3d_perfmon *active;
 
+               /* Number of perfmons alive on this device. Jobs are not
+                * serialized if the number is zero.
+                */
+               atomic_t nperfmons;
+
                /* Finished fence of the most recently submitted job that
                 * opened a serialization window (i.e. a job with a non-global
                 * perfmon attached).
@@ -191,8 +198,10 @@ struct v3d_dev {
                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.
+                * queue, which is used so that a new perfmon-carrying job can
+                * depend on every job currently in-flight across all queues.
+                *
+                * Finished fences are only tracked if @nperfmons > 0.
                 */
                struct dma_fence *last_hw_fence[V3D_MAX_QUEUES];
        } perfmon_state;
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c 
b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 4cadc3e17280..c00f18d4a425 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -217,8 +217,10 @@ void v3d_perfmon_get(struct v3d_perfmon *perfmon)
 
 void v3d_perfmon_put(struct v3d_perfmon *perfmon)
 {
-       if (perfmon && refcount_dec_and_test(&perfmon->refcnt))
+       if (perfmon && refcount_dec_and_test(&perfmon->refcnt)) {
+               atomic_dec(&perfmon->v3d->perfmon_state.nperfmons);
                kfree(perfmon);
+       }
 }
 
 static void v3d_perfmon_hw_start(struct v3d_dev *v3d, struct v3d_perfmon 
*perfmon)
@@ -435,13 +437,15 @@ int v3d_perfmon_create_ioctl(struct drm_device *dev, void 
*data,
                perfmon->counters[i] = req->counters[i];
 
        perfmon->ncounters = req->ncounters;
+       perfmon->v3d = v3d;
 
        refcount_set(&perfmon->refcnt, 1);
+       atomic_inc(&v3d->perfmon_state.nperfmons);
 
        ret = xa_alloc(&v3d_priv->perfmons, &id, perfmon, xa_limit_32b,
                       GFP_KERNEL);
        if (ret < 0) {
-               kfree(perfmon);
+               v3d_perfmon_put(perfmon);
                return ret;
        }
 
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 834d52030979..bc3c43fd4fd9 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -357,6 +357,10 @@ v3d_attach_perfmon_to_jobs(struct v3d_submit *submit, u32 
perfmon_id)
  *
  * We don't serialize the jobs when using a global perfmon as it's expected to
  * track concurrent activity from all jobs.
+ *
+ * Keeping track of the in-flight jobs costs a fence merge per job, so it is
+ * only done while at least one perfmon is alive. Jobs submitted while no
+ * perfmon exists go untracked and may overlap the first measured job.
  */
 static int
 v3d_serialize_for_perfmon(struct v3d_job *job)
@@ -368,6 +372,9 @@ v3d_serialize_for_perfmon(struct v3d_job *job)
 
        lockdep_assert_held(&v3d->sched_lock);
 
+       if (!atomic_read(&v3d->perfmon_state.nperfmons))
+               return 0;
+
        scoped_guard(spinlock_irqsave, &v3d->perfmon_state.lock)
                is_global_perfmon = !!v3d->global_perfmon;
 

-- 
2.55.0

Reply via email to