From 6b1a56b908e702c06f55c63b19b695a47f607456 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Tue, 25 Aug 2020 11:54:14 -0700 Subject: [PATCH] iris: Drop stale syncobj references in fence_server_sync MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 and Kefei Yao for their excellent work in tracking down this issue! Fixes: f459c56be6b ("iris: Add fence support using drm_syncobj") Reviewed-by: Lionel Landwerlin Tested-by: Tapani Pälli Tested-by: Yang A Shi Part-of: --- src/gallium/drivers/iris/iris_fence.c | 52 +++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/gallium/drivers/iris/iris_fence.c b/src/gallium/drivers/iris/iris_fence.c index 22acc317c0c..d9bdf9bba7b 100644 --- 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); } } -- 2.30.2