From aafcd8aacb8feda179017ba9064272ff6573a78e Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Mon, 19 Apr 2021 13:03:21 -0700 Subject: [PATCH] freedreno: Re-work fd_submit fence interface Move everything into a struct assocated with the pipe_fence_handle, so that the drm layer can fill in the seqn/fd fences directly. This will give us a comvenient place to insert a util_queue_fence in the next commit. While we're at it, extract the uint32_t fence (previously called 'timestamp' in place, a kgsl legacy) into a struct that encapsulates both the kernel fence and the userspace fence. Signed-off-by: Rob Clark Part-of: --- src/freedreno/computerator/main.c | 2 +- src/freedreno/drm/freedreno_drmif.h | 20 +++++- src/freedreno/drm/freedreno_pipe.c | 11 +-- src/freedreno/drm/freedreno_priv.h | 7 +- src/freedreno/drm/freedreno_ringbuffer.c | 6 +- src/freedreno/drm/freedreno_ringbuffer.h | 19 +++++- src/freedreno/drm/msm_pipe.c | 4 +- src/freedreno/drm/msm_ringbuffer.c | 16 ++--- src/freedreno/drm/msm_ringbuffer_sp.c | 14 ++-- src/freedreno/perfcntrs/fdperf.c | 2 +- .../drivers/freedreno/freedreno_batch.c | 2 +- .../drivers/freedreno/freedreno_batch.h | 1 - .../drivers/freedreno/freedreno_context.c | 2 +- .../drivers/freedreno/freedreno_fence.c | 67 ++++++++++--------- .../drivers/freedreno/freedreno_fence.h | 18 ++--- .../drivers/freedreno/freedreno_gmem.c | 8 +-- 16 files changed, 114 insertions(+), 85 deletions(-) diff --git a/src/freedreno/computerator/main.c b/src/freedreno/computerator/main.c index 8ec9fab1d82..82b2d55de9e 100644 --- a/src/freedreno/computerator/main.c +++ b/src/freedreno/computerator/main.c @@ -284,7 +284,7 @@ main(int argc, char **argv) backend->emit_grid(kernel, grid, submit); - fd_submit_flush(submit, -1, NULL, NULL); + fd_submit_flush(submit, -1, NULL); for (int i = 0; i < kernel->num_bufs; i++) { fd_bo_cpu_prep(kernel->bufs[i], pipe, FD_BO_PREP_READ); diff --git a/src/freedreno/drm/freedreno_drmif.h b/src/freedreno/drm/freedreno_drmif.h index 3dd3609ee15..5d963e898d6 100644 --- a/src/freedreno/drm/freedreno_drmif.h +++ b/src/freedreno/drm/freedreno_drmif.h @@ -79,6 +79,22 @@ fd_fence_after(uint32_t a, uint32_t b) return (int32_t)(a - b) > 0; } +/** + * Per submit, there are actually two fences: + * 1) The userspace maintained fence, which is used to optimistically + * avoid kernel ioctls to query if specific rendering is completed + * 2) The kernel maintained fence, which we cannot directly do anything + * with, other than pass it back to the kernel + * + * The userspace fence is mostly internal to the drm layer, but we want + * the gallium layer to be able to pass it back to us for things like + * fd_pipe_wait(). So this struct encapsulates the two. + */ +struct fd_fence { + uint32_t kfence; /* kernel fence */ + uint32_t ufence; /* userspace fence */ +}; + /* bo flags: */ #define FD_BO_GPUREADONLY BITSET_BIT(1) #define FD_BO_SCANOUT BITSET_BIT(2) @@ -126,9 +142,9 @@ struct fd_pipe *fd_pipe_ref_locked(struct fd_pipe *pipe); void fd_pipe_del(struct fd_pipe *pipe); int fd_pipe_get_param(struct fd_pipe *pipe, enum fd_param_id param, uint64_t *value); -int fd_pipe_wait(struct fd_pipe *pipe, uint32_t timestamp); +int fd_pipe_wait(struct fd_pipe *pipe, const struct fd_fence *fence); /* timeout in nanosec */ -int fd_pipe_wait_timeout(struct fd_pipe *pipe, uint32_t timestamp, +int fd_pipe_wait_timeout(struct fd_pipe *pipe, const struct fd_fence *fence, uint64_t timeout); /* buffer-object functions: diff --git a/src/freedreno/drm/freedreno_pipe.c b/src/freedreno/drm/freedreno_pipe.c index 9078c29fe11..442acf153a8 100644 --- a/src/freedreno/drm/freedreno_pipe.c +++ b/src/freedreno/drm/freedreno_pipe.c @@ -124,16 +124,19 @@ fd_pipe_get_param(struct fd_pipe *pipe, enum fd_param_id param, uint64_t *value) } int -fd_pipe_wait(struct fd_pipe *pipe, uint32_t timestamp) +fd_pipe_wait(struct fd_pipe *pipe, const struct fd_fence *fence) { - return fd_pipe_wait_timeout(pipe, timestamp, ~0); + return fd_pipe_wait_timeout(pipe, fence, ~0); } int -fd_pipe_wait_timeout(struct fd_pipe *pipe, uint32_t timestamp, uint64_t timeout) +fd_pipe_wait_timeout(struct fd_pipe *pipe, const struct fd_fence *fence, + uint64_t timeout) { + if (!fd_fence_after(fence->ufence, pipe->control->fence)) + return 0; - return pipe->funcs->wait(pipe, timestamp, timeout); + return pipe->funcs->wait(pipe, fence, timeout); } uint32_t diff --git a/src/freedreno/drm/freedreno_priv.h b/src/freedreno/drm/freedreno_priv.h index 545006f822c..d24c7b67e1d 100644 --- a/src/freedreno/drm/freedreno_priv.h +++ b/src/freedreno/drm/freedreno_priv.h @@ -147,7 +147,8 @@ struct fd_pipe_funcs { struct fd_submit *(*submit_new)(struct fd_pipe *pipe); int (*get_param)(struct fd_pipe *pipe, enum fd_param_id param, uint64_t *value); - int (*wait)(struct fd_pipe *pipe, uint32_t timestamp, uint64_t timeout); + int (*wait)(struct fd_pipe *pipe, const struct fd_fence *fence, + uint64_t timeout); void (*destroy)(struct fd_pipe *pipe); }; @@ -188,8 +189,8 @@ struct fd_submit_funcs { struct fd_ringbuffer *(*new_ringbuffer)(struct fd_submit *submit, uint32_t size, enum fd_ringbuffer_flags flags); - int (*flush)(struct fd_submit *submit, int in_fence_fd, int *out_fence_fd, - uint32_t *out_fence); + int (*flush)(struct fd_submit *submit, int in_fence_fd, + struct fd_submit_fence *out_fence); void (*destroy)(struct fd_submit *submit); }; diff --git a/src/freedreno/drm/freedreno_ringbuffer.c b/src/freedreno/drm/freedreno_ringbuffer.c index 49f9a6df3ff..700ba42a6a9 100644 --- a/src/freedreno/drm/freedreno_ringbuffer.c +++ b/src/freedreno/drm/freedreno_ringbuffer.c @@ -58,11 +58,11 @@ fd_submit_ref(struct fd_submit *submit) } int -fd_submit_flush(struct fd_submit *submit, int in_fence_fd, int *out_fence_fd, - uint32_t *out_fence) +fd_submit_flush(struct fd_submit *submit, int in_fence_fd, + struct fd_submit_fence *out_fence) { submit->fence = fd_pipe_emit_fence(submit->pipe, submit->primary); - return submit->funcs->flush(submit, in_fence_fd, out_fence_fd, out_fence); + return submit->funcs->flush(submit, in_fence_fd, out_fence); } struct fd_ringbuffer * diff --git a/src/freedreno/drm/freedreno_ringbuffer.h b/src/freedreno/drm/freedreno_ringbuffer.h index d4a84729ec1..3cf1343b937 100644 --- a/src/freedreno/drm/freedreno_ringbuffer.h +++ b/src/freedreno/drm/freedreno_ringbuffer.h @@ -92,11 +92,26 @@ struct fd_ringbuffer *fd_submit_new_ringbuffer(struct fd_submit *submit, uint32_t size, enum fd_ringbuffer_flags flags); +/** + * Encapsulates submit out-fence(s), which consist of a 'timestamp' (per- + * pipe (submitqueue) sequence number) and optionally, if requested, an + * out-fence-fd + */ +struct fd_submit_fence { + struct fd_fence fence; + + /** + * Optional dma_fence fd, returned by submit if use_fence_fd is true + */ + int fence_fd; + bool use_fence_fd; +}; + /* in_fence_fd: -1 for no in-fence, else fence fd - * out_fence_fd: NULL for no output-fence requested, else ptr to return out-fence + * out_fence can be NULL if no output fence is required */ int fd_submit_flush(struct fd_submit *submit, int in_fence_fd, - int *out_fence_fd, uint32_t *out_fence); + struct fd_submit_fence *out_fence); struct fd_ringbuffer; struct fd_reloc; diff --git a/src/freedreno/drm/msm_pipe.c b/src/freedreno/drm/msm_pipe.c index c940ead937e..8de3c5c5c29 100644 --- a/src/freedreno/drm/msm_pipe.c +++ b/src/freedreno/drm/msm_pipe.c @@ -106,11 +106,11 @@ msm_pipe_get_param(struct fd_pipe *pipe, enum fd_param_id param, } static int -msm_pipe_wait(struct fd_pipe *pipe, uint32_t timestamp, uint64_t timeout) +msm_pipe_wait(struct fd_pipe *pipe, const struct fd_fence *fence, uint64_t timeout) { struct fd_device *dev = pipe->dev; struct drm_msm_wait_fence req = { - .fence = timestamp, + .fence = fence->kfence, .queueid = to_msm_pipe(pipe)->queue_id, }; int ret; diff --git a/src/freedreno/drm/msm_ringbuffer.c b/src/freedreno/drm/msm_ringbuffer.c index 5ce4c0f4185..50ec8788c56 100644 --- a/src/freedreno/drm/msm_ringbuffer.c +++ b/src/freedreno/drm/msm_ringbuffer.c @@ -267,8 +267,8 @@ handle_stateobj_relocs(struct msm_submit *submit, struct msm_ringbuffer *ring) } static int -msm_submit_flush(struct fd_submit *submit, int in_fence_fd, int *out_fence_fd, - uint32_t *out_fence) +msm_submit_flush(struct fd_submit *submit, int in_fence_fd, + struct fd_submit_fence *out_fence) { struct msm_submit *msm_submit = to_msm_submit(submit); struct msm_pipe *msm_pipe = to_msm_pipe(submit->pipe); @@ -354,7 +354,7 @@ msm_submit_flush(struct fd_submit *submit, int in_fence_fd, int *out_fence_fd, req.fence_fd = in_fence_fd; } - if (out_fence_fd) { + if (out_fence && out_fence->use_fence_fd) { req.flags |= MSM_SUBMIT_FENCE_FD_OUT; } @@ -370,12 +370,10 @@ msm_submit_flush(struct fd_submit *submit, int in_fence_fd, int *out_fence_fd, if (ret) { ERROR_MSG("submit failed: %d (%s)", ret, strerror(errno)); msm_dump_submit(&req); - } else if (!ret) { - if (out_fence) - *out_fence = req.fence; - - if (out_fence_fd) - *out_fence_fd = req.fence_fd; + } else if (!ret && out_fence) { + out_fence->fence.kfence = req.fence; + out_fence->fence.ufence = submit->fence; + out_fence->fence_fd = req.fence_fd; } for (unsigned o = 0; o < nr_objs; o++) diff --git a/src/freedreno/drm/msm_ringbuffer_sp.c b/src/freedreno/drm/msm_ringbuffer_sp.c index a2a00ee7022..4d2a304bc4c 100644 --- a/src/freedreno/drm/msm_ringbuffer_sp.c +++ b/src/freedreno/drm/msm_ringbuffer_sp.c @@ -206,7 +206,7 @@ msm_submit_sp_new_ringbuffer(struct fd_submit *submit, uint32_t size, static int msm_submit_sp_flush(struct fd_submit *submit, int in_fence_fd, - int *out_fence_fd, uint32_t *out_fence) + struct fd_submit_fence *out_fence) { struct msm_submit_sp *msm_submit = to_msm_submit_sp(submit); struct msm_pipe *msm_pipe = to_msm_pipe(submit->pipe); @@ -237,7 +237,7 @@ msm_submit_sp_flush(struct fd_submit *submit, int in_fence_fd, req.fence_fd = in_fence_fd; } - if (out_fence_fd) { + if (out_fence && out_fence->use_fence_fd) { req.flags |= MSM_SUBMIT_FENCE_FD_OUT; } @@ -276,12 +276,10 @@ msm_submit_sp_flush(struct fd_submit *submit, int in_fence_fd, if (ret) { ERROR_MSG("submit failed: %d (%s)", ret, strerror(errno)); msm_dump_submit(&req); - } else if (!ret) { - if (out_fence) - *out_fence = req.fence; - - if (out_fence_fd) - *out_fence_fd = req.fence_fd; + } else if (!ret && out_fence) { + out_fence->fence.kfence = req.fence; + out_fence->fence.ufence = submit->fence; + out_fence->fence_fd = req.fence_fd; } if (!bos_on_stack) diff --git a/src/freedreno/perfcntrs/fdperf.c b/src/freedreno/perfcntrs/fdperf.c index 59b6f0644bc..2789cc6bd87 100644 --- a/src/freedreno/perfcntrs/fdperf.c +++ b/src/freedreno/perfcntrs/fdperf.c @@ -173,7 +173,7 @@ flush_ring(void) if (!dev.submit) return; - ret = fd_submit_flush(dev.submit, -1, NULL, NULL); + ret = fd_submit_flush(dev.submit, -1, NULL); if (ret) errx(1, "submit failed: %d", ret); fd_ringbuffer_del(dev.ring); diff --git a/src/gallium/drivers/freedreno/freedreno_batch.c b/src/gallium/drivers/freedreno/freedreno_batch.c index a82b7df6d0b..3780e1e892d 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.c +++ b/src/gallium/drivers/freedreno/freedreno_batch.c @@ -205,7 +205,7 @@ batch_fini(struct fd_batch *batch) /* in case batch wasn't flushed but fence was created: */ if (batch->fence) - fd_fence_populate(batch->fence, 0, -1); + fd_fence_set_batch(batch->fence, NULL); fd_fence_ref(&batch->fence, NULL); diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h b/src/gallium/drivers/freedreno/freedreno_batch.h index b79ffc9f418..2abe1d48247 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.h +++ b/src/gallium/drivers/freedreno/freedreno_batch.h @@ -61,7 +61,6 @@ struct fd_batch { uint32_t *last_timestamp_cmd; int in_fence_fd; - bool needs_out_fence_fd; struct pipe_fence_handle *fence; struct fd_context *ctx; diff --git a/src/gallium/drivers/freedreno/freedreno_context.c b/src/gallium/drivers/freedreno/freedreno_context.c index 1c7dbfc5d6d..3cb7eadf00d 100644 --- a/src/gallium/drivers/freedreno/freedreno_context.c +++ b/src/gallium/drivers/freedreno/freedreno_context.c @@ -117,7 +117,7 @@ fd_context_flush(struct pipe_context *pctx, struct pipe_fence_handle **fencep, fd_fence_ref(&fence, batch->fence); if (flags & PIPE_FLUSH_FENCE_FD) - batch->needs_out_fence_fd = true; + fence->submit_fence.use_fence_fd = true; fd_bc_dump(ctx->screen, "%p: flushing %p<%u>, flags=0x%x, pending:\n", ctx, batch, batch->seqno, flags); diff --git a/src/gallium/drivers/freedreno/freedreno_fence.c b/src/gallium/drivers/freedreno/freedreno_fence.c index 89eda70043a..a4d4be997bf 100644 --- a/src/gallium/drivers/freedreno/freedreno_fence.c +++ b/src/gallium/drivers/freedreno/freedreno_fence.c @@ -59,8 +59,8 @@ fence_flush(struct pipe_context *pctx, struct pipe_fence_handle *fence, } } - /* We've already waited for batch to be flushed and fd_fence_populate() - * called: + /* We've already waited for batch to be flushed and fence->batch + * to be cleared: */ assert(!fence->batch); return true; @@ -74,35 +74,23 @@ fence_flush(struct pipe_context *pctx, struct pipe_fence_handle *fence, return true; } -void -fd_fence_populate(struct pipe_fence_handle *fence, uint32_t timestamp, - int fence_fd) -{ - if (!fence->batch) - return; - fence->timestamp = timestamp; - fence->fence_fd = fence_fd; - fence->batch = NULL; - - if (fence->needs_signal) { - util_queue_fence_signal(&fence->ready); - fence->needs_signal = false; - } -} - void fd_fence_repopulate(struct pipe_fence_handle *fence, struct pipe_fence_handle *last_fence) { - int fence_fd = (last_fence->fence_fd == -1) ? -1 : dup(last_fence->fence_fd); - fd_fence_populate(fence, last_fence->timestamp, fence_fd); + /* The fence we are re-populating must not be an fd-fence (but last_fince + * might have been) + */ + assert(!fence->submit_fence.use_fence_fd); + + fence->submit_fence.fence = last_fence->submit_fence.fence; } static void fd_fence_destroy(struct pipe_fence_handle *fence) { tc_unflushed_batch_token_reference(&fence->tc_token, NULL); - if (fence->fence_fd != -1) - close(fence->fence_fd); + if (fence->submit_fence.use_fence_fd) + close(fence->submit_fence.fence_fd); if (fence->syncobj) drmSyncobjDestroy(fd_device_fd(fence->screen->dev), fence->syncobj); fd_pipe_del(fence->pipe); @@ -125,12 +113,12 @@ fd_fence_finish(struct pipe_screen *pscreen, struct pipe_context *pctx, if (!fence_flush(pctx, fence, timeout)) return false; - if (fence->fence_fd != -1) { - int ret = sync_wait(fence->fence_fd, timeout / 1000000); + if (fence->submit_fence.use_fence_fd) { + int ret = sync_wait(fence->submit_fence.fence_fd, timeout / 1000000); return ret == 0; } - if (fd_pipe_wait_timeout(fence->pipe, fence->timestamp, timeout)) + if (fd_pipe_wait_timeout(fence->pipe, &fence->submit_fence.fence, timeout)) return false; return true; @@ -153,7 +141,8 @@ fence_create(struct fd_context *ctx, struct fd_batch *batch, int fence_fd, fence->batch = batch; fence->pipe = fd_pipe_ref(ctx->pipe); fence->screen = ctx->screen; - fence->fence_fd = fence_fd; + fence->submit_fence.fence_fd = fence_fd; + fence->submit_fence.use_fence_fd = (fence_fd != -1); fence->syncobj = syncobj; return fence; @@ -198,10 +187,10 @@ fd_fence_server_sync(struct pipe_context *pctx, struct pipe_fence_handle *fence) fence_flush(pctx, fence, 0); /* if not an external fence, then nothing more to do without preemption: */ - if (fence->fence_fd == -1) + if (!fence->submit_fence.use_fence_fd) return; - if (sync_accumulate("freedreno", &ctx->in_fence_fd, fence->fence_fd)) { + if (sync_accumulate("freedreno", &ctx->in_fence_fd, fence->submit_fence.fence_fd)) { /* error */ } } @@ -220,18 +209,20 @@ fd_fence_server_signal(struct pipe_context *pctx, int fd_fence_get_fd(struct pipe_screen *pscreen, struct pipe_fence_handle *fence) { + assert(fence->submit_fence.use_fence_fd); + /* NOTE: in the deferred fence case, the pctx we want is the threaded-ctx * but if TC is not used, this will be null. Which is fine, we won't call * threaded_context_flush() in that case */ fence_flush(&fence->ctx->tc->base, fence, PIPE_TIMEOUT_INFINITE); - return os_dupfd_cloexec(fence->fence_fd); + return os_dupfd_cloexec(fence->submit_fence.fence_fd); } bool fd_fence_is_fd(struct pipe_fence_handle *fence) { - return fence->fence_fd != -1; + return fence->submit_fence.use_fence_fd; } struct pipe_fence_handle * @@ -243,8 +234,20 @@ fd_fence_create(struct fd_batch *batch) void fd_fence_set_batch(struct pipe_fence_handle *fence, struct fd_batch *batch) { - assert(!fence->batch); - fence->batch = batch; + if (batch) { + assert(!fence->batch); + fence->batch = batch; + } else { + fence->batch = NULL; + + /* When the batch is dis-associated with the fence, we can signal TC + * that the fence is flushed + */ + if (fence->needs_signal) { + util_queue_fence_signal(&fence->ready); + fence->needs_signal = false; + } + } } struct pipe_fence_handle * diff --git a/src/gallium/drivers/freedreno/freedreno_fence.h b/src/gallium/drivers/freedreno/freedreno_fence.h index 5c307228946..947ac97b122 100644 --- a/src/gallium/drivers/freedreno/freedreno_fence.h +++ b/src/gallium/drivers/freedreno/freedreno_fence.h @@ -30,12 +30,15 @@ #include "pipe/p_context.h" #include "util/u_queue.h" +#include "drm/freedreno_ringbuffer.h" + struct pipe_fence_handle { struct pipe_reference reference; - /* fence holds a weak reference to the batch until the batch is flushed, - * at which point fd_fence_populate() is called and timestamp and possibly - * fence_fd become valid and the week reference is dropped. + /* fence holds a weak reference to the batch until the batch is flushed, to + * accommodate PIPE_FLUSH_DEFERRED. When the batch is actually flushed, it + * is cleared (before the batch reference is dropped). If we need to wait + * on a fence, and the batch is not NULL, we need to flush it. * * Note that with u_threaded_context async flushes, if a fence is requested * by the frontend, the fence is initially created without a weak reference @@ -48,8 +51,8 @@ struct pipe_fence_handle { struct tc_unflushed_batch_token *tc_token; bool needs_signal; - /* For threaded_context async flushes, we must wait on the fence, signalled - * in fd_fence_populate(), to know that the rendering has been actually + /* For threaded_context async flushes, we must wait on the fence, signaled + * when fence->batch is cleared, to know that the rendering has been actually * flushed from the driver thread. * * The ready fence is created signaled for non-async-flush fences, and only @@ -64,13 +67,10 @@ struct pipe_fence_handle { struct fd_context *ctx; struct fd_pipe *pipe; struct fd_screen *screen; - int fence_fd; - uint32_t timestamp; + struct fd_submit_fence submit_fence; uint32_t syncobj; }; -void fd_fence_populate(struct pipe_fence_handle *fence, uint32_t timestamp, - int fence_fd); void fd_fence_repopulate(struct pipe_fence_handle *fence, struct pipe_fence_handle *last_fence); void fd_fence_ref(struct pipe_fence_handle **ptr, diff --git a/src/gallium/drivers/freedreno/freedreno_gmem.c b/src/gallium/drivers/freedreno/freedreno_gmem.c index 7df72ab41b0..f6380797288 100644 --- a/src/gallium/drivers/freedreno/freedreno_gmem.c +++ b/src/gallium/drivers/freedreno/freedreno_gmem.c @@ -654,18 +654,14 @@ render_sysmem(struct fd_batch *batch) assert_dt static void flush_ring(struct fd_batch *batch) { - uint32_t timestamp = 0; - int out_fence_fd = -1; - if (FD_DBG(NOHW)) return; fd_submit_flush(batch->submit, batch->in_fence_fd, - batch->needs_out_fence_fd ? &out_fence_fd : NULL, - batch->fence ? ×tamp : NULL); + batch->fence ? &batch->fence->submit_fence : NULL); if (batch->fence) - fd_fence_populate(batch->fence, timestamp, out_fence_fd); + fd_fence_set_batch(batch->fence, NULL); } void