drm: Branch 'master' - 10 commits
[email protected] (GitLab Mirror) Sat, 13 Oct 2018 21:29:21 +0000 (UTC)
| Newsgroups | gmane.comp.video.dri.patches |
|---|---|
| Message-ID | <[email protected]> |
freedreno/freedreno-symbol-check | 7 - freedreno/freedreno_priv.h | 12 - freedreno/freedreno_ringbuffer.c | 94 +++----------- freedreno/freedreno_ringbuffer.h | 53 ++++++-- freedreno/kgsl/kgsl_ringbuffer.c | 9 - freedreno/msm/msm_pipe.c | 6 freedreno/msm/msm_priv.h | 11 + freedreno/msm/msm_ringbuffer.c | 247 ++++++++++++++++++++++++--------------- 8 files changed, 248 insertions(+), 191 deletions(-) New commits: commit 032258b7f77974e14fd319bc3a61d91cc403a21a Author: Rob Clark <[email protected]> Date: Thu Oct 11 09:55:03 2018 -0400 freedreno/msm: handle ring-reloc to other stateobjs Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index 44a329a7..594c2ba9 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -340,6 +340,16 @@ static void flush_reset(struct fd_ringbuffer *ring) fd_bo_del(&msm_bo->base); } + for (i = 0; i < msm_ring->nr_cmds; i++) { + struct msm_cmd *msm_cmd = msm_ring->cmds[i]; + + if (msm_cmd->ring == ring) + continue; + + if (msm_cmd->ring->flags & FD_RINGBUFFER_OBJECT) + fd_ringbuffer_del(msm_cmd->ring); + } + msm_ring->submit.nr_cmds = 0; msm_ring->submit.nr_bos = 0; msm_ring->nr_cmds = 0; @@ -426,6 +436,24 @@ handle_stateobj_relocs(struct fd_ringbuffer *parent, struct fd_ringbuffer *state relocs[i].reloc_idx = bo2idx(parent, bo, flags); } + /* stateobj rb's could have reloc's to other stateobj rb's which didn't + * get propagated to the parent rb at _emit_reloc_ring() time (because + * the parent wasn't known then), so fix that up now: + */ + for (i = 0; i < msm_ring->nr_cmds; i++) { + struct msm_cmd *msm_cmd = msm_ring->cmds[i]; + struct drm_msm_gem_submit_cmd *cmd = &msm_ring->submit.cmds[i]; + + if (msm_ring->cmds[i]->ring == stateobj) + continue; + + assert(msm_cmd->ring->flags & FD_RINGBUFFER_OBJECT); + + if (get_cmd(parent, msm_cmd, cmd->submit_offset, cmd->size, cmd->type)) { + fd_ringbuffer_ref(msm_cmd->ring); + } + } + return relocs; } @@ -456,7 +484,6 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start /* for each of the cmd's fix up their reloc's: */ for (i = 0; i < msm_ring->submit.nr_cmds; i++) { - struct drm_msm_gem_submit_cmd *cmd = &msm_ring->submit.cmds[i]; struct msm_cmd *msm_cmd = msm_ring->cmds[i]; struct drm_msm_gem_submit_reloc *relocs = msm_cmd->relocs; unsigned nr_relocs = msm_cmd->nr_relocs; @@ -472,6 +499,7 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start relocs, nr_relocs); } + struct drm_msm_gem_submit_cmd *cmd = &msm_ring->submit.cmds[i]; cmd->relocs = VOID2U64(relocs); cmd->nr_relocs = nr_relocs; } @@ -506,9 +534,6 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start struct drm_msm_gem_submit_cmd *cmd = &msm_ring->submit.cmds[i]; struct msm_cmd *msm_cmd = msm_ring->cmds[i]; if (msm_cmd->ring->flags & FD_RINGBUFFER_OBJECT) { - /* we could have dropped last reference: */ - msm_ring->cmds[i] = NULL; - fd_ringbuffer_del(msm_cmd->ring); free(U642VOID(cmd->relocs)); } } commit 413a49a0688441e36beff361b7ad100a7f80e051 Author: Rob Clark <[email protected]> Date: Thu Oct 11 09:43:58 2018 -0400 freedreno/msm: simplify msm_ringbuffer_flush() Now that it doesn't have to deal with ringmarkers we can simplify the reloc handling. Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index 5cd97709..44a329a7 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -318,21 +318,6 @@ static void * msm_ringbuffer_hostptr(struct fd_ringbuffer *ring) return base + to_msm_ringbuffer(ring)->offset; } -static uint32_t find_next_reloc_idx(struct msm_cmd *msm_cmd, - uint32_t start, uint32_t offset) -{ - uint32_t i; - - /* a binary search would be more clever.. */ - for (i = start; i < msm_cmd->nr_relocs; i++) { - struct drm_msm_gem_submit_reloc *reloc = &msm_cmd->relocs[i]; - if (reloc->submit_offset >= offset) - return i; - } - - return i; -} - static void delete_cmds(struct msm_ringbuffer *msm_ring) { struct msm_cmd *cmd, *tmp; @@ -473,10 +458,8 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start for (i = 0; i < msm_ring->submit.nr_cmds; i++) { struct drm_msm_gem_submit_cmd *cmd = &msm_ring->submit.cmds[i]; struct msm_cmd *msm_cmd = msm_ring->cmds[i]; - uint32_t a = find_next_reloc_idx(msm_cmd, 0, cmd->submit_offset); - uint32_t b = find_next_reloc_idx(msm_cmd, a, cmd->submit_offset + cmd->size); - struct drm_msm_gem_submit_reloc *relocs = &msm_cmd->relocs[a]; - unsigned nr_relocs = (b > a) ? b - a : 0; + struct drm_msm_gem_submit_reloc *relocs = msm_cmd->relocs; + unsigned nr_relocs = msm_cmd->nr_relocs; /* for reusable stateobjs, the reloc table has reloc_idx that * points into it's own private bos table, rather than the global commit 880871b3787d2664daefe228bc99ead298d5a76e Author: Rob Clark <[email protected]> Date: Wed Oct 10 12:51:33 2018 -0400 freedreno/msm: use hashtable to track submit.cmds table With streaming stateobjs to upload uniforms, the submit.cmds table gets much larger, and iterating over it for each ring to ring reloc starts getting expensive. TODO if we have flag to pass when constructing parent rb, we could avoid dynamically allocating this and bo_table in get_cmd() or bo2idx Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index 24aa9fd4..5cd97709 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -46,6 +46,9 @@ struct msm_cmd { DECLARE_ARRAY(struct drm_msm_gem_submit_reloc, relocs); uint32_t size; + + /* has cmd already been added to parent rb's submit.cmds table? */ + int is_appended_to_submit; }; struct msm_ringbuffer { @@ -91,6 +94,18 @@ struct msm_ringbuffer { /* maps fd_bo to idx: */ void *bo_table; + + /* maps msm_cmd to drm_msm_gem_submit_cmd in parent rb. Each rb has a + * list of msm_cmd's which correspond to each chunk of cmdstream in + * a 'growable' rb. For each of those we need to create one + * drm_msm_gem_submit_cmd in the parent rb which collects the state + * for the submit ioctl. Because we can have multiple IB's to the same + * target rb (for example, or same stateobj emit multiple times), and + * because in theory we can have multiple different rb's that have a + * reference to a given target, we need a hashtable to track this per + * rb. + */ + void *cmd_table; }; static inline struct msm_ringbuffer * to_msm_ringbuffer(struct fd_ringbuffer *x) @@ -234,13 +249,6 @@ static uint32_t bo2idx(struct fd_ringbuffer *ring, struct fd_bo *bo, uint32_t fl return idx; } -static int check_cmd_bo(struct fd_ringbuffer *ring, - struct drm_msm_gem_submit_cmd *cmd, struct fd_bo *bo) -{ - struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); - return msm_ring->submit.bos[cmd->submit_idx].handle == bo->handle; -} - /* Ensure that submit has corresponding entry in cmds table for the * target cmdstream buffer: * @@ -253,15 +261,31 @@ static int get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd, struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); struct drm_msm_gem_submit_cmd *cmd; uint32_t i; + void *val; - /* figure out if we already have a cmd buf: */ - for (i = 0; i < msm_ring->submit.nr_cmds; i++) { + if (!msm_ring->cmd_table) + msm_ring->cmd_table = drmHashCreate(); + + /* figure out if we already have a cmd buf.. short-circuit hash + * lookup if: + * - target cmd has never been added to submit.cmds + * - target cmd is not a streaming stateobj (which unlike longer + * lived CSO stateobj, is not expected to be reused with multiple + * submits) + */ + if (target_cmd->is_appended_to_submit && + !(target_cmd->ring->flags & FD_RINGBUFFER_STREAMING) && + !drmHashLookup(msm_ring->cmd_table, (unsigned long)target_cmd, &val)) { + i = VOID2U64(val); cmd = &msm_ring->submit.cmds[i]; - if ((cmd->submit_offset == submit_offset) && - (cmd->size == size) && - (cmd->type == type) && - check_cmd_bo(ring, cmd, target_cmd->ring_bo)) - return FALSE; + + assert(cmd->submit_offset == submit_offset); + assert(cmd->size == size); + assert(cmd->type == type); + assert(msm_ring->submit.bos[cmd->submit_idx].handle == + target_cmd->ring_bo->handle); + + return FALSE; } /* create cmd buf if not: */ @@ -275,6 +299,13 @@ static int get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd, cmd->size = size; cmd->pad = 0; + target_cmd->is_appended_to_submit = TRUE; + + if (!(target_cmd->ring->flags & FD_RINGBUFFER_STREAMING)) { + drmHashInsert(msm_ring->cmd_table, (unsigned long)target_cmd, + U642VOID(i)); + } + target_cmd->size = size; return TRUE; @@ -334,6 +365,11 @@ static void flush_reset(struct fd_ringbuffer *ring) msm_ring->bo_table = NULL; } + if (msm_ring->cmd_table) { + drmHashDestroy(msm_ring->cmd_table); + msm_ring->cmd_table = NULL; + } + if (msm_ring->is_growable) { delete_cmds(msm_ring); } else { commit 5c4722e9078a845dce097168731f1be88cf61e75 Author: Rob Clark <[email protected]> Date: Wed Oct 10 11:10:39 2018 -0400 freedreno/msm: simplify emit_reloc_ring() vfunc Now that it doesn't have to deal with the ringmarker case, we can make some simplifications. Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/freedreno_priv.h b/freedreno/freedreno_priv.h index 48cd6380..b8eac4b2 100644 --- a/freedreno/freedreno_priv.h +++ b/freedreno/freedreno_priv.h @@ -140,8 +140,7 @@ struct fd_ringbuffer_funcs { void (*emit_reloc)(struct fd_ringbuffer *ring, const struct fd_reloc *reloc); uint32_t (*emit_reloc_ring)(struct fd_ringbuffer *ring, - struct fd_ringbuffer *target, uint32_t cmd_idx, - uint32_t submit_offset, uint32_t size); + struct fd_ringbuffer *target, uint32_t cmd_idx); uint32_t (*cmd_count)(struct fd_ringbuffer *ring); void (*destroy)(struct fd_ringbuffer *ring); }; diff --git a/freedreno/freedreno_ringbuffer.c b/freedreno/freedreno_ringbuffer.c index ddaa4632..8f0093ae 100644 --- a/freedreno/freedreno_ringbuffer.c +++ b/freedreno/freedreno_ringbuffer.c @@ -166,8 +166,7 @@ drm_public uint32_t fd_ringbuffer_emit_reloc_ring_full(struct fd_ringbuffer *ring, struct fd_ringbuffer *target, uint32_t cmd_idx) { - uint32_t size = offset_bytes(target->cur, target->start); - return ring->funcs->emit_reloc_ring(ring, target, cmd_idx, 0, size); + return ring->funcs->emit_reloc_ring(ring, target, cmd_idx); } drm_public uint32_t diff --git a/freedreno/kgsl/kgsl_ringbuffer.c b/freedreno/kgsl/kgsl_ringbuffer.c index 7361f7d5..42a0e0aa 100644 --- a/freedreno/kgsl/kgsl_ringbuffer.c +++ b/freedreno/kgsl/kgsl_ringbuffer.c @@ -174,13 +174,12 @@ static void kgsl_ringbuffer_emit_reloc(struct fd_ringbuffer *ring, } static uint32_t kgsl_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, - struct fd_ringbuffer *target, uint32_t cmd_idx, - uint32_t submit_offset, uint32_t size) + struct fd_ringbuffer *target, uint32_t cmd_idx) { struct kgsl_ringbuffer *target_ring = to_kgsl_ringbuffer(target); assert(cmd_idx == 0); - (*ring->cur++) = target_ring->bo->gpuaddr + submit_offset; - return size; + (*ring->cur++) = target_ring->bo->gpuaddr; + return offset_bytes(target->cur, target->start); } static void kgsl_ringbuffer_destroy(struct fd_ringbuffer *ring) diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index 304be119..24aa9fd4 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -566,15 +566,14 @@ static void msm_ringbuffer_emit_reloc(struct fd_ringbuffer *ring, } static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, - struct fd_ringbuffer *target, uint32_t cmd_idx, - uint32_t submit_offset, uint32_t size) + struct fd_ringbuffer *target, uint32_t cmd_idx) { struct msm_cmd *cmd = NULL; struct msm_ringbuffer *msm_target = to_msm_ringbuffer(target); uint32_t idx = 0; int added_cmd = FALSE; - - submit_offset += msm_target->offset; + uint32_t size; + uint32_t submit_offset = msm_target->offset; LIST_FOR_EACH_ENTRY(cmd, &msm_target->cmd_list, list) { if (idx == cmd_idx) @@ -593,6 +592,7 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, size = cmd->size; } else { struct fd_ringbuffer *parent = ring->parent ? ring->parent : ring; + size = offset_bytes(target->cur, target->start); added_cmd = get_cmd(parent, cmd, submit_offset, size, MSM_SUBMIT_CMD_IB_TARGET_BUF); } commit 96b625240cb621531b49b8cb05249dafba4e0817 Author: Rob Clark <[email protected]> Date: Thu Oct 11 13:51:14 2018 -0400 freedreno/msm: remove reset of linked rings The msm_cmd isn't refcount'd, so with stateobj rb's that have independent lifecycle, this is no longer a safe thing to do. Really, now that there is a bo-cache for rb's, fd_ringbuffer_reset() should be deprecated because it adds a bunch of pointless complexity. Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/freedreno_ringbuffer.h b/freedreno/freedreno_ringbuffer.h index 3e951dca..bc41a31c 100644 --- a/freedreno/freedreno_ringbuffer.h +++ b/freedreno/freedreno_ringbuffer.h @@ -102,6 +102,7 @@ struct fd_ringbuffer *fd_ringbuffer_ref(struct fd_ringbuffer *ring); void fd_ringbuffer_del(struct fd_ringbuffer *ring); void fd_ringbuffer_set_parent(struct fd_ringbuffer *ring, struct fd_ringbuffer *parent); +will_be_deprecated void fd_ringbuffer_reset(struct fd_ringbuffer *ring); int fd_ringbuffer_flush(struct fd_ringbuffer *ring); /* in_fence_fd: -1 for no in-fence, else fence fd diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index a9190886..304be119 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -324,14 +324,6 @@ static void flush_reset(struct fd_ringbuffer *ring) fd_bo_del(&msm_bo->base); } - /* for each of the cmd buffers, clear their reloc's: */ - for (i = 0; i < msm_ring->submit.nr_cmds; i++) { - struct msm_cmd *target_cmd = msm_ring->cmds[i]; - if (!target_cmd) - continue; - target_cmd->nr_relocs = 0; - } - msm_ring->submit.nr_cmds = 0; msm_ring->submit.nr_bos = 0; msm_ring->nr_cmds = 0; commit 09cbccff558988c57e52685ffb359441db8c2bd7 Author: Rob Clark <[email protected]> Date: Wed Oct 10 10:47:17 2018 -0400 freedreno: remove deprecated ringmarker API It's usage in mesa was removed more than two years ago. And it stands in the way of some optimizations needed to reduce the overhead of hw stateobjs (ie. CP_SET_DRAW_STATE, where the # of cmds in the submit ioctl goes up significantly). Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/freedreno-symbol-check b/freedreno/freedreno-symbol-check index 3216d487..978026c0 100755 --- a/freedreno/freedreno-symbol-check +++ b/freedreno/freedreno-symbol-check @@ -57,12 +57,7 @@ fd_ringbuffer_reset fd_ringbuffer_set_parent fd_ringbuffer_size fd_ringbuffer_timestamp -fd_ringmarker_del -fd_ringmarker_dwords -fd_ringmarker_flush fd_ringbuffer_flush2 -fd_ringmarker_mark -fd_ringmarker_new EOF done) diff --git a/freedreno/freedreno_priv.h b/freedreno/freedreno_priv.h index e997ef07..48cd6380 100644 --- a/freedreno/freedreno_priv.h +++ b/freedreno/freedreno_priv.h @@ -131,11 +131,6 @@ struct fd_pipe { const struct fd_pipe_funcs *funcs; }; -struct fd_ringmarker { - struct fd_ringbuffer *ring; - uint32_t *cur; -}; - struct fd_ringbuffer_funcs { void * (*hostptr)(struct fd_ringbuffer *ring); int (*flush)(struct fd_ringbuffer *ring, uint32_t *last_start, diff --git a/freedreno/freedreno_ringbuffer.c b/freedreno/freedreno_ringbuffer.c index 28fcc937..ddaa4632 100644 --- a/freedreno/freedreno_ringbuffer.c +++ b/freedreno/freedreno_ringbuffer.c @@ -155,21 +155,6 @@ drm_public void fd_ringbuffer_reloc2(struct fd_ringbuffer *ring, ring->funcs->emit_reloc(ring, reloc); } -drm_public void fd_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, - struct fd_ringmarker *target, struct fd_ringmarker *end) -{ - uint32_t submit_offset, size; - - /* This function is deprecated and not supported on 64b devices: */ - assert(ring->pipe->gpu_id < 500); - assert(target->ring == end->ring); - - submit_offset = offset_bytes(target->cur, target->ring->start); - size = offset_bytes(end->cur, target->cur); - - ring->funcs->emit_reloc_ring(ring, target->ring, 0, submit_offset, size); -} - drm_public uint32_t fd_ringbuffer_cmd_count(struct fd_ringbuffer *ring) { if (!ring->funcs->cmd_count) @@ -196,45 +181,3 @@ fd_ringbuffer_size(struct fd_ringbuffer *ring) return offset_bytes(ring->cur, ring->start); } -/* - * Deprecated ringmarker API: - */ - -drm_public struct fd_ringmarker * fd_ringmarker_new(struct fd_ringbuffer *ring) -{ - struct fd_ringmarker *marker = NULL; - - marker = calloc(1, sizeof(*marker)); - if (!marker) { - ERROR_MSG("allocation failed"); - return NULL; - } - - marker->ring = ring; - - marker->cur = marker->ring->cur; - - return marker; -} - -drm_public void fd_ringmarker_del(struct fd_ringmarker *marker) -{ - free(marker); -} - -drm_public void fd_ringmarker_mark(struct fd_ringmarker *marker) -{ - marker->cur = marker->ring->cur; -} - -drm_public uint32_t fd_ringmarker_dwords(struct fd_ringmarker *start, - struct fd_ringmarker *end) -{ - return end->cur - start->cur; -} - -drm_public int fd_ringmarker_flush(struct fd_ringmarker *marker) -{ - struct fd_ringbuffer *ring = marker->ring; - return ring->funcs->flush(ring, marker->cur, -1, NULL); -} diff --git a/freedreno/freedreno_ringbuffer.h b/freedreno/freedreno_ringbuffer.h index ad615e8f..3e951dca 100644 --- a/freedreno/freedreno_ringbuffer.h +++ b/freedreno/freedreno_ringbuffer.h @@ -37,7 +37,6 @@ */ struct fd_ringbuffer_funcs; -struct fd_ringmarker; enum fd_ringbuffer_flags { @@ -134,18 +133,9 @@ struct fd_reloc { void fd_ringbuffer_reloc2(struct fd_ringbuffer *ring, const struct fd_reloc *reloc); will_be_deprecated void fd_ringbuffer_reloc(struct fd_ringbuffer *ring, const struct fd_reloc *reloc); -will_be_deprecated void fd_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, - struct fd_ringmarker *target, struct fd_ringmarker *end); uint32_t fd_ringbuffer_cmd_count(struct fd_ringbuffer *ring); uint32_t fd_ringbuffer_emit_reloc_ring_full(struct fd_ringbuffer *ring, struct fd_ringbuffer *target, uint32_t cmd_idx); uint32_t fd_ringbuffer_size(struct fd_ringbuffer *ring); -will_be_deprecated struct fd_ringmarker * fd_ringmarker_new(struct fd_ringbuffer *ring); -will_be_deprecated void fd_ringmarker_del(struct fd_ringmarker *marker); -will_be_deprecated void fd_ringmarker_mark(struct fd_ringmarker *marker); -will_be_deprecated uint32_t fd_ringmarker_dwords(struct fd_ringmarker *start, - struct fd_ringmarker *end); -will_be_deprecated int fd_ringmarker_flush(struct fd_ringmarker *marker); - #endif /* FREEDRENO_RINGBUFFER_H_ */ commit 33faf339c3de8e70dcef5240f9e88e1e2bde3713 Author: Rob Clark <[email protected]> Date: Mon Oct 8 15:33:02 2018 -0400 freedreno/msm: support suballocation for stateobj rb's Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/freedreno_ringbuffer.h b/freedreno/freedreno_ringbuffer.h index c5aebaf2..ad615e8f 100644 --- a/freedreno/freedreno_ringbuffer.h +++ b/freedreno/freedreno_ringbuffer.h @@ -46,6 +46,20 @@ enum fd_ringbuffer_flags { * to a parent ringbuffer. */ FD_RINGBUFFER_OBJECT = 0x1, + + /* Hint that the stateobj will be used for streaming state + * that is used once or a few times and then discarded. + * + * For sub-allocation, non streaming stateobj's should be + * sub-allocated from a page size buffer, so one long lived + * state obj doesn't prevent other pages from being freed. + * (Ie. it would be no worse than allocating a page sized + * bo for each small non-streaming stateobj). + * + * But streaming stateobj's could be sub-allocated from a + * larger buffer to reduce the alloc/del overhead. + */ + FD_RINGBUFFER_STREAMING = 0x2, }; struct fd_ringbuffer { diff --git a/freedreno/msm/msm_pipe.c b/freedreno/msm/msm_pipe.c index f28778ef..e070b317 100644 --- a/freedreno/msm/msm_pipe.c +++ b/freedreno/msm/msm_pipe.c @@ -138,6 +138,12 @@ static void msm_pipe_destroy(struct fd_pipe *pipe) { struct msm_pipe *msm_pipe = to_msm_pipe(pipe); close_submitqueue(pipe, msm_pipe->queue_id); + + if (msm_pipe->suballoc_ring) { + fd_ringbuffer_del(msm_pipe->suballoc_ring); + msm_pipe->suballoc_ring = NULL; + } + free(msm_pipe); } diff --git a/freedreno/msm/msm_priv.h b/freedreno/msm/msm_priv.h index ee0eecb8..cc951fba 100644 --- a/freedreno/msm/msm_priv.h +++ b/freedreno/msm/msm_priv.h @@ -57,6 +57,17 @@ struct msm_pipe { uint32_t gmem; uint32_t chip_id; uint32_t queue_id; + + /* Allow for sub-allocation of stateobj ring buffers (ie. sharing + * the same underlying bo).. + * + * This takes advantage of each context having it's own fd_pipe, + * so we don't have to worry about access from multiple threads. + * + * We also rely on previous stateobj having been fully constructed + * so we can reclaim extra space at it's end. + */ + struct fd_ringbuffer *suballoc_ring; }; static inline struct msm_pipe * to_msm_pipe(struct fd_pipe *x) diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index a102ca35..a9190886 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -85,6 +85,8 @@ struct msm_ringbuffer { int is_growable; unsigned cmd_count; + unsigned offset; /* for sub-allocated stateobj rb's */ + unsigned seqno; /* maps fd_bo to idx: */ @@ -100,6 +102,13 @@ static inline struct msm_ringbuffer * to_msm_ringbuffer(struct fd_ringbuffer *x) static pthread_mutex_t idx_lock = PTHREAD_MUTEX_INITIALIZER; +static struct msm_cmd *current_cmd(struct fd_ringbuffer *ring) +{ + struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); + assert(!LIST_IS_EMPTY(&msm_ring->cmd_list)); + return LIST_LAST_ENTRY(&msm_ring->cmd_list, struct msm_cmd, list); +} + static void ring_cmd_del(struct msm_cmd *cmd) { fd_bo_del(cmd->ring_bo); @@ -109,7 +118,8 @@ static void ring_cmd_del(struct msm_cmd *cmd) free(cmd); } -static struct msm_cmd * ring_cmd_new(struct fd_ringbuffer *ring, uint32_t size) +static struct msm_cmd * ring_cmd_new(struct fd_ringbuffer *ring, uint32_t size, + enum fd_ringbuffer_flags flags) { struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); struct msm_cmd *cmd = calloc(1, sizeof(*cmd)); @@ -118,7 +128,48 @@ static struct msm_cmd * ring_cmd_new(struct fd_ringbuffer *ring, uint32_t size) return NULL; cmd->ring = ring; - cmd->ring_bo = fd_bo_new_ring(ring->pipe->dev, size, 0); + + /* TODO separate suballoc buffer for small non-streaming state, using + * smaller page-sized backing bo's. + */ + if (flags & FD_RINGBUFFER_STREAMING) { + struct msm_pipe *msm_pipe = to_msm_pipe(ring->pipe); + unsigned suballoc_offset = 0; + struct fd_bo *suballoc_bo = NULL; + + if (msm_pipe->suballoc_ring) { + struct msm_ringbuffer *suballoc_ring = to_msm_ringbuffer(msm_pipe->suballoc_ring); + + assert(msm_pipe->suballoc_ring->flags & FD_RINGBUFFER_OBJECT); + assert(suballoc_ring->cmd_count == 1); + + suballoc_bo = current_cmd(msm_pipe->suballoc_ring)->ring_bo; + + suballoc_offset = fd_ringbuffer_size(msm_pipe->suballoc_ring) + + suballoc_ring->offset; + + suballoc_offset = ALIGN(suballoc_offset, 0x10); + + if ((size + suballoc_offset) > suballoc_bo->size) { + suballoc_bo = NULL; + } + } + + if (!suballoc_bo) { + cmd->ring_bo = fd_bo_new_ring(ring->pipe->dev, 0x8000, 0); + msm_ring->offset = 0; + } else { + cmd->ring_bo = fd_bo_ref(suballoc_bo); + msm_ring->offset = suballoc_offset; + } + + if (msm_pipe->suballoc_ring) + fd_ringbuffer_del(msm_pipe->suballoc_ring); + + msm_pipe->suballoc_ring = fd_ringbuffer_ref(ring); + } else { + cmd->ring_bo = fd_bo_new_ring(ring->pipe->dev, size, 0); + } if (!cmd->ring_bo) goto fail; @@ -132,13 +183,6 @@ fail: return NULL; } -static struct msm_cmd *current_cmd(struct fd_ringbuffer *ring) -{ - struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); - assert(!LIST_IS_EMPTY(&msm_ring->cmd_list)); - return LIST_LAST_ENTRY(&msm_ring->cmd_list, struct msm_cmd, list); -} - static uint32_t append_bo(struct fd_ringbuffer *ring, struct fd_bo *bo) { struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); @@ -238,7 +282,9 @@ static int get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd, static void * msm_ringbuffer_hostptr(struct fd_ringbuffer *ring) { - return fd_bo_map(current_cmd(ring)->ring_bo); + struct msm_cmd *cmd = current_cmd(ring); + uint8_t *base = fd_bo_map(cmd->ring_bo); + return base + to_msm_ringbuffer(ring)->offset; } static uint32_t find_next_reloc_idx(struct msm_cmd *msm_cmd, @@ -374,9 +420,10 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start int in_fence_fd, int *out_fence_fd) { struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); + struct msm_pipe *msm_pipe = to_msm_pipe(ring->pipe); struct drm_msm_gem_submit req = { - .flags = to_msm_pipe(ring->pipe)->pipe, - .queueid = to_msm_pipe(ring->pipe)->queue_id, + .flags = msm_pipe->pipe, + .queueid = msm_pipe->queue_id, }; uint32_t i; int ret; @@ -464,7 +511,7 @@ static void msm_ringbuffer_grow(struct fd_ringbuffer *ring, uint32_t size) { assert(to_msm_ringbuffer(ring)->is_growable); finalize_current_cmd(ring, ring->last_start); - ring_cmd_new(ring, size); + ring_cmd_new(ring, size, 0); } static void msm_ringbuffer_reset(struct fd_ringbuffer *ring) @@ -488,7 +535,8 @@ static void msm_ringbuffer_emit_reloc(struct fd_ringbuffer *ring, reloc->reloc_offset = r->offset; reloc->or = r->or; reloc->shift = r->shift; - reloc->submit_offset = offset_bytes(ring->cur, ring->start); + reloc->submit_offset = offset_bytes(ring->cur, ring->start) + + to_msm_ringbuffer(ring)->offset; addr = msm_bo->presumed; if (reloc->shift < 0) @@ -513,7 +561,8 @@ static void msm_ringbuffer_emit_reloc(struct fd_ringbuffer *ring, reloc_hi->reloc_offset = r->offset; reloc_hi->or = r->orhi; reloc_hi->shift = r->shift - 32; - reloc_hi->submit_offset = offset_bytes(ring->cur, ring->start); + reloc_hi->submit_offset = offset_bytes(ring->cur, ring->start) + + to_msm_ringbuffer(ring)->offset; addr = msm_bo->presumed >> 32; if (reloc_hi->shift < 0) @@ -529,10 +578,13 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, uint32_t submit_offset, uint32_t size) { struct msm_cmd *cmd = NULL; + struct msm_ringbuffer *msm_target = to_msm_ringbuffer(target); uint32_t idx = 0; int added_cmd = FALSE; - LIST_FOR_EACH_ENTRY(cmd, &to_msm_ringbuffer(target)->cmd_list, list) { + submit_offset += msm_target->offset; + + LIST_FOR_EACH_ENTRY(cmd, &msm_target->cmd_list, list) { if (idx == cmd_idx) break; idx++; @@ -540,7 +592,7 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, assert(cmd && (idx == cmd_idx)); - if (idx < (to_msm_ringbuffer(target)->cmd_count - 1)) { + if (idx < (msm_target->cmd_count - 1)) { /* All but the last cmd buffer is fully "baked" (ie. already has * done get_cmd() to add it to the cmds table). But in this case, * the size we get is invalid (since it is calculated from the @@ -628,7 +680,7 @@ drm_private struct fd_ringbuffer * msm_ringbuffer_new(struct fd_pipe *pipe, ring->size = size; ring->pipe = pipe; /* needed in ring_cmd_new() */ - ring_cmd_new(ring, size); + ring_cmd_new(ring, size, flags); return ring; } commit bf001648a92c8f5e10d95322f486a346febd2c09 Author: Rob Clark <[email protected]> Date: Wed Oct 10 08:37:23 2018 -0400 freedreno: add flags param for rb creation For now, we want a way for gallium to be able to provide hints for the upcoming rb suballocation. But could be useful for other things down the road. Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/freedreno-symbol-check b/freedreno/freedreno-symbol-check index 002a3989..3216d487 100755 --- a/freedreno/freedreno-symbol-check +++ b/freedreno/freedreno-symbol-check @@ -48,6 +48,7 @@ fd_ringbuffer_emit_reloc_ring_full fd_ringbuffer_flush fd_ringbuffer_grow fd_ringbuffer_new +fd_ringbuffer_new_flags fd_ringbuffer_new_object fd_ringbuffer_ref fd_ringbuffer_reloc diff --git a/freedreno/freedreno_priv.h b/freedreno/freedreno_priv.h index 84dbc5c6..e997ef07 100644 --- a/freedreno/freedreno_priv.h +++ b/freedreno/freedreno_priv.h @@ -115,10 +115,6 @@ drm_private int fd_bo_cache_free(struct fd_bo_cache *cache, struct fd_bo *bo); /* for where @table_lock is already held: */ drm_private void fd_device_del_locked(struct fd_device *dev); -enum fd_ringbuffer_flags { - FD_RINGBUFFER_OBJECT = 0x1, -}; - struct fd_pipe_funcs { struct fd_ringbuffer * (*ringbuffer_new)(struct fd_pipe *pipe, uint32_t size, enum fd_ringbuffer_flags flags); diff --git a/freedreno/freedreno_ringbuffer.c b/freedreno/freedreno_ringbuffer.c index 80af736f..28fcc937 100644 --- a/freedreno/freedreno_ringbuffer.c +++ b/freedreno/freedreno_ringbuffer.c @@ -32,12 +32,19 @@ #include "freedreno_priv.h" #include "freedreno_ringbuffer.h" -static struct fd_ringbuffer * -ringbuffer_new(struct fd_pipe *pipe, uint32_t size, +drm_public struct fd_ringbuffer * +fd_ringbuffer_new_flags(struct fd_pipe *pipe, uint32_t size, enum fd_ringbuffer_flags flags) { struct fd_ringbuffer *ring; + /* we can't really support "growable" rb's in general for + * stateobj's since we need a single gpu addr (ie. can't + * do the trick of a chain of IB packets): + */ + if (flags & FD_RINGBUFFER_OBJECT) + assert(size); + ring = pipe->funcs->ringbuffer_new(pipe, size, flags); if (!ring) return NULL; @@ -55,18 +62,13 @@ ringbuffer_new(struct fd_pipe *pipe, uint32_t size, drm_public struct fd_ringbuffer * fd_ringbuffer_new(struct fd_pipe *pipe, uint32_t size) { - return ringbuffer_new(pipe, size, 0); + return fd_ringbuffer_new_flags(pipe, size, 0); } drm_public struct fd_ringbuffer * fd_ringbuffer_new_object(struct fd_pipe *pipe, uint32_t size) { - /* we can't really support "growable" rb's in general for - * stateobj's since we need a single gpu addr (ie. can't - * do the trick of a chain of IB packets): - */ - assert(size); - return ringbuffer_new(pipe, size, FD_RINGBUFFER_OBJECT); + return fd_ringbuffer_new_flags(pipe, size, FD_RINGBUFFER_OBJECT); } drm_public void fd_ringbuffer_del(struct fd_ringbuffer *ring) diff --git a/freedreno/freedreno_ringbuffer.h b/freedreno/freedreno_ringbuffer.h index b2e8024d..c5aebaf2 100644 --- a/freedreno/freedreno_ringbuffer.h +++ b/freedreno/freedreno_ringbuffer.h @@ -39,6 +39,15 @@ struct fd_ringbuffer_funcs; struct fd_ringmarker; +enum fd_ringbuffer_flags { + + /* Ringbuffer is a "state object", which is potentially reused + * many times, rather than being used in one-shot mode linked + * to a parent ringbuffer. + */ + FD_RINGBUFFER_OBJECT = 0x1, +}; + struct fd_ringbuffer { int size; uint32_t *cur, *end, *start, *last_start; @@ -52,7 +61,7 @@ struct fd_ringbuffer { */ void *user; - uint32_t flags; + enum fd_ringbuffer_flags flags; /* This is a bit gross, but we can't use atomic_t in exported * headers. OTOH, we don't need the refcnt to be publicly @@ -70,8 +79,11 @@ struct fd_ringbuffer { struct fd_ringbuffer * fd_ringbuffer_new(struct fd_pipe *pipe, uint32_t size); +will_be_deprecated struct fd_ringbuffer * fd_ringbuffer_new_object(struct fd_pipe *pipe, uint32_t size); +struct fd_ringbuffer * fd_ringbuffer_new_flags(struct fd_pipe *pipe, + uint32_t size, enum fd_ringbuffer_flags flags); struct fd_ringbuffer *fd_ringbuffer_ref(struct fd_ringbuffer *ring); void fd_ringbuffer_del(struct fd_ringbuffer *ring); commit a8a006192634d1390acf3a09d4edec27084c8507 Author: Rob Clark <[email protected]> Date: Tue Oct 9 15:33:39 2018 -0400 freedreno: expose refcnt'ing on ringbuffers Move this out of msm_ringbuffer backend so that the gallium driver can refcnt rb's Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/freedreno-symbol-check b/freedreno/freedreno-symbol-check index 6da9d667..002a3989 100755 --- a/freedreno/freedreno-symbol-check +++ b/freedreno/freedreno-symbol-check @@ -49,6 +49,7 @@ fd_ringbuffer_flush fd_ringbuffer_grow fd_ringbuffer_new fd_ringbuffer_new_object +fd_ringbuffer_ref fd_ringbuffer_reloc fd_ringbuffer_reloc2 fd_ringbuffer_reset diff --git a/freedreno/freedreno_ringbuffer.c b/freedreno/freedreno_ringbuffer.c index 1fa33b7a..80af736f 100644 --- a/freedreno/freedreno_ringbuffer.c +++ b/freedreno/freedreno_ringbuffer.c @@ -71,11 +71,21 @@ fd_ringbuffer_new_object(struct fd_pipe *pipe, uint32_t size) drm_public void fd_ringbuffer_del(struct fd_ringbuffer *ring) { - if (!(ring->flags & FD_RINGBUFFER_OBJECT)) - fd_ringbuffer_reset(ring); + if (!atomic_dec_and_test(&ring->refcnt)) + return; + + fd_ringbuffer_reset(ring); ring->funcs->destroy(ring); } +drm_public struct fd_ringbuffer * +fd_ringbuffer_ref(struct fd_ringbuffer *ring) +{ + STATIC_ASSERT(sizeof(ring->refcnt) <= sizeof(ring->__pad)); + atomic_inc(&ring->refcnt); + return ring; +} + /* ringbuffers which are IB targets should set the toplevel rb (ie. * the IB source) as it's parent before emitting reloc's, to ensure * the bookkeeping works out properly. diff --git a/freedreno/freedreno_ringbuffer.h b/freedreno/freedreno_ringbuffer.h index 69e7ed99..b2e8024d 100644 --- a/freedreno/freedreno_ringbuffer.h +++ b/freedreno/freedreno_ringbuffer.h @@ -53,6 +53,19 @@ struct fd_ringbuffer { void *user; uint32_t flags; + + /* This is a bit gross, but we can't use atomic_t in exported + * headers. OTOH, we don't need the refcnt to be publicly + * visible. The only reason that this struct is exported is + * because fd_ringbuffer_emit needs to be something that can + * be inlined for performance reasons. + */ + union { +#ifdef HAS_ATOMIC_OPS + atomic_t refcnt; +#endif + uint64_t __pad; + }; }; struct fd_ringbuffer * fd_ringbuffer_new(struct fd_pipe *pipe, @@ -60,6 +73,7 @@ struct fd_ringbuffer * fd_ringbuffer_new(struct fd_pipe *pipe, struct fd_ringbuffer * fd_ringbuffer_new_object(struct fd_pipe *pipe, uint32_t size); +struct fd_ringbuffer *fd_ringbuffer_ref(struct fd_ringbuffer *ring); void fd_ringbuffer_del(struct fd_ringbuffer *ring); void fd_ringbuffer_set_parent(struct fd_ringbuffer *ring, struct fd_ringbuffer *parent); diff --git a/freedreno/kgsl/kgsl_ringbuffer.c b/freedreno/kgsl/kgsl_ringbuffer.c index e4fdf342..7361f7d5 100644 --- a/freedreno/kgsl/kgsl_ringbuffer.c +++ b/freedreno/kgsl/kgsl_ringbuffer.c @@ -216,6 +216,8 @@ drm_private struct fd_ringbuffer * kgsl_ringbuffer_new(struct fd_pipe *pipe, } ring = &kgsl_ring->base; + atomic_set(&ring->refcnt, 1); + ring->funcs = &funcs; ring->size = size; diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index 5a088818..a102ca35 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -29,6 +29,7 @@ #include <assert.h> #include <inttypes.h> +#include "xf86atomic.h" #include "freedreno_ringbuffer.h" #include "msm_priv.h" @@ -50,8 +51,6 @@ struct msm_cmd { struct msm_ringbuffer { struct fd_ringbuffer base; - atomic_t refcnt; - /* submit ioctl related tables: * Note that bos and cmds are tracked by the parent ringbuffer, since * that is global to the submit ioctl call. The reloc's table is tracked @@ -97,9 +96,6 @@ static inline struct msm_ringbuffer * to_msm_ringbuffer(struct fd_ringbuffer *x) return (struct msm_ringbuffer *)x; } -static void msm_ringbuffer_unref(struct fd_ringbuffer *ring); -static void msm_ringbuffer_ref(struct fd_ringbuffer *ring); - #define INIT_SIZE 0x1000 static pthread_mutex_t idx_lock = PTHREAD_MUTEX_INITIALIZER; @@ -454,7 +450,7 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start if (msm_cmd->ring->flags & FD_RINGBUFFER_OBJECT) { /* we could have dropped last reference: */ msm_ring->cmds[i] = NULL; - msm_ringbuffer_unref(msm_cmd->ring); + fd_ringbuffer_del(msm_cmd->ring); free(U642VOID(cmd->relocs)); } } @@ -568,7 +564,7 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring, * destroyed after emitted but before flush, so we must hold a ref: */ if (added_cmd && (target->flags & FD_RINGBUFFER_OBJECT)) { - msm_ringbuffer_ref(target); + fd_ringbuffer_ref(target); } return size; @@ -579,13 +575,10 @@ static uint32_t msm_ringbuffer_cmd_count(struct fd_ringbuffer *ring) return to_msm_ringbuffer(ring)->cmd_count; } -static void msm_ringbuffer_unref(struct fd_ringbuffer *ring) +static void msm_ringbuffer_destroy(struct fd_ringbuffer *ring) { struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); - if (!atomic_dec_and_test(&msm_ring->refcnt)) - return; - flush_reset(ring); delete_cmds(msm_ring); @@ -596,12 +589,6 @@ static void msm_ringbuffer_unref(struct fd_ringbuffer *ring) free(msm_ring); } -static void msm_ringbuffer_ref(struct fd_ringbuffer *ring) -{ - struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring); - atomic_inc(&msm_ring->refcnt); -} - static const struct fd_ringbuffer_funcs funcs = { .hostptr = msm_ringbuffer_hostptr, .flush = msm_ringbuffer_flush, @@ -610,7 +597,7 @@ static const struct fd_ringbuffer_funcs funcs = { .emit_reloc = msm_ringbuffer_emit_reloc, .emit_reloc_ring = msm_ringbuffer_emit_reloc_ring, .cmd_count = msm_ringbuffer_cmd_count, - .destroy = msm_ringbuffer_unref, + .destroy = msm_ringbuffer_destroy, }; drm_private struct fd_ringbuffer * msm_ringbuffer_new(struct fd_pipe *pipe, @@ -633,9 +620,10 @@ drm_private struct fd_ringbuffer * msm_ringbuffer_new(struct fd_pipe *pipe, list_inithead(&msm_ring->cmd_list); msm_ring->seqno = ++to_msm_device(pipe->dev)->ring_cnt; - atomic_set(&msm_ring->refcnt, 1); ring = &msm_ring->base; + atomic_set(&ring->refcnt, 1); + ring->funcs = &funcs; ring->size = size; ring->pipe = pipe; /* needed in ring_cmd_new() */ commit 566b1d9f93e4922edf5b13b80cb46179e9d85802 Author: Rob Clark <[email protected]> Date: Tue Oct 9 08:24:06 2018 -0400 freedreno/msm: get rid of ring_bo unref hack Since 28328298 'freedreno: move ring_cache behind fd_bo_del()' this hack is no longer necessary. Signed-off-by: Rob Clark <[email protected]> diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c index ac6221c2..5a088818 100644 --- a/freedreno/msm/msm_ringbuffer.c +++ b/freedreno/msm/msm_ringbuffer.c @@ -106,8 +106,7 @@ static pthread_mutex_t idx_lock = PTHREAD_MUTEX_INITIALIZER; static void ring_cmd_del(struct msm_cmd *cmd) { - if (cmd->ring_bo) - fd_bo_del(cmd->ring_bo); + fd_bo_del(cmd->ring_bo); list_del(&cmd->list); to_msm_ringbuffer(cmd->ring)->cmd_count--; free(cmd->relocs); @@ -455,14 +454,6 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start if (msm_cmd->ring->flags & FD_RINGBUFFER_OBJECT) { /* we could have dropped last reference: */ msm_ring->cmds[i] = NULL; - - /* need to drop ring_bo ref prior to unref'ing the ring, - * because ring_bo_del assumes it is dropping the *last* - * reference: - */ - fd_bo_del(msm_ring->bos[cmd->submit_idx]); - msm_ring->bos[cmd->submit_idx] = NULL; - msm_ringbuffer_unref(msm_cmd->ring); free(U642VOID(cmd->relocs)); } --