iris: Drop stale syncobj references in fence_server_sync

When calling glWaitSync (fence_server_sync), we added dependencies
in all batches (render and compute) on existing work.  Even if
applications don't use compute at all, they theoretically could,
so we record that the compute batch depends on the render batch.

But if the application truly doesn't use compute, or rarely uses
it, we ended up recording dependencies on _all_ previous render
batches, racking up a massive list of syncobjs.  Not only is this
pointless, it also meant that we never allowed the kernel to free
the underlying i915_request objects.

There are a number of solutions to this problem, but for now, we
take a simple one: when recording a new syncobj dependency, we
walk the list and see if any of them have already passed.  If so,
that dependency has been fulfilled.  We no longer need to track it,
and can simply drop it from the list, unreferencing the syncobj.

Android's SurfaceFlinger in particular was hitting this issue,
as it uses glWaitSync, doesn't typically use compute shaders,
and runs for long durations.

Thanks to Yang A Shi <yang.a.shi@intel.com> and
Kefei Yao <kefei.yao@intel.com> for their excellent work in
tracking down this issue!

Fixes: f459c56be6b ("iris: Add fence support using drm_syncobj")
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Tested-by: Tapani Pälli <tapani.palli@intel.com>
Tested-by: Yang A Shi <yang.a.shi@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6463>

diff --git a/src/gallium/drivers/iris/iris_fence.c b/src/gallium/drivers/iris/iris_fence.c
index 22acc317c0c0b072ea7a22c73f6cac4eb2917978..d9bdf9bba7b744cc0765a67d2965370de8ab5c4a 100644 (file)
--- a/src/gallium/drivers/iris/iris_fence.c
+++ b/src/gallium/drivers/iris/iris_fence.c
@@ -110,6 +110,55 @@ iris_batch_add_syncobj(struct iris_batch *batch,
    iris_syncobj_reference(batch->screen, store, syncobj);
 }
 
+/**
+ * Walk through a batch's dependencies (any I915_EXEC_FENCE_WAIT syncobjs)
+ * and unreference any which have already passed.
+ *
+ * Sometimes the compute batch is seldom used, and accumulates references
+ * to stale render batches that are no longer of interest, so we can free
+ * those up.
+ */
+static void
+clear_stale_syncobjs(struct iris_batch *batch)
+{
+   struct iris_screen *screen = batch->screen;
+
+   int n = util_dynarray_num_elements(&batch->syncobjs, struct iris_syncobj *);
+
+   assert(n == util_dynarray_num_elements(&batch->exec_fences,
+                                          struct drm_i915_gem_exec_fence));
+
+   /* Skip the first syncobj, as it's the signalling one. */
+   for (int i = n - 1; i > 1; i--) {
+      struct iris_syncobj **syncobj =
+         util_dynarray_element(&batch->syncobjs, struct iris_syncobj *, i);
+      struct drm_i915_gem_exec_fence *fence =
+         util_dynarray_element(&batch->exec_fences,
+                               struct drm_i915_gem_exec_fence, i);
+      assert(fence->flags & I915_EXEC_FENCE_WAIT);
+
+      if (iris_wait_syncobj(&screen->base, *syncobj, 0))
+         continue;
+
+      /* This sync object has already passed, there's no need to continue
+       * marking it as a dependency; we can stop holding on to the reference.
+       */
+      iris_syncobj_reference(screen, syncobj, NULL);
+
+      /* Remove it from the lists; move the last element here. */
+      struct iris_syncobj **nth_syncobj =
+         util_dynarray_pop_ptr(&batch->syncobjs, struct iris_syncobj *);
+      struct drm_i915_gem_exec_fence *nth_fence =
+         util_dynarray_pop_ptr(&batch->exec_fences,
+                               struct drm_i915_gem_exec_fence);
+
+      if (syncobj != nth_syncobj) {
+         *syncobj = *nth_syncobj;
+         memcpy(nth_fence, fence, sizeof(*fence));
+      }
+   }
+}
+
 /* ------------------------------------------------------------------- */
 
 struct pipe_fence_handle {
@@ -271,6 +320,9 @@ iris_fence_await(struct pipe_context *ctx,
           */
          iris_batch_flush(batch);
 
+         /* Before adding a new reference, clean out any stale ones. */
+         clear_stale_syncobjs(batch);
+
          iris_batch_add_syncobj(batch, fine->syncobj, I915_EXEC_FENCE_WAIT);
       }
    }