From 13d509c7e66439e3e85d24f3326c037a47d0ffc5 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Tue, 3 Nov 2020 10:18:33 -0800 Subject: [PATCH] freedreno/drm: Rework APPEND() macro In particular I wanted the nr_foo increment to be after assignment.. mostly just to track down a potential race. (This wasn't it, but I like this color for the bikeshed better.) Signed-off-by: Rob Clark Part-of: --- src/freedreno/drm/msm_priv.h | 12 +++---- src/freedreno/drm/msm_ringbuffer.c | 60 ++++++++++++++--------------------- src/freedreno/drm/msm_ringbuffer_sp.c | 22 +++++-------- 3 files changed, 37 insertions(+), 57 deletions(-) diff --git a/src/freedreno/drm/msm_priv.h b/src/freedreno/drm/msm_priv.h index 37adb13..c578078 100644 --- a/src/freedreno/drm/msm_priv.h +++ b/src/freedreno/drm/msm_priv.h @@ -117,25 +117,25 @@ static inline void get_abs_timeout(struct drm_msm_timespec *tv, uint64_t ns) * Stupid/simple growable array implementation: */ -static inline void * -grow(void *ptr, uint16_t nr, uint16_t *max, uint16_t sz) +static inline void +grow(void **ptr, uint16_t nr, uint16_t *max, uint16_t sz) { if ((nr + 1) > *max) { if ((*max * 2) < (nr + 1)) *max = nr + 5; else *max = *max * 2; - ptr = realloc(ptr, *max * sz); + *ptr = realloc(*ptr, *max * sz); } - return ptr; } #define DECLARE_ARRAY(type, name) \ unsigned short nr_ ## name, max_ ## name; \ type * name; -#define APPEND(x, name) ({ \ - (x)->name = grow((x)->name, (x)->nr_ ## name, &(x)->max_ ## name, sizeof((x)->name[0])); \ +#define APPEND(x, name, ...) ({ \ + grow((void **)&(x)->name, (x)->nr_ ## name, &(x)->max_ ## name, sizeof((x)->name[0])); \ + (x)->name[(x)->nr_ ## name] = __VA_ARGS__; \ (x)->nr_ ## name ++; \ }) diff --git a/src/freedreno/drm/msm_ringbuffer.c b/src/freedreno/drm/msm_ringbuffer.c index 0538859..240e2db 100644 --- a/src/freedreno/drm/msm_ringbuffer.c +++ b/src/freedreno/drm/msm_ringbuffer.c @@ -150,15 +150,12 @@ append_bo(struct msm_submit *submit, struct fd_bo *bo) /* found */ idx = (uint32_t)(uintptr_t)entry->data; } else { - idx = APPEND(submit, submit_bos); - idx = APPEND(submit, bos); - - submit->submit_bos[idx].flags = bo->flags & - (MSM_SUBMIT_BO_READ | MSM_SUBMIT_BO_WRITE); - submit->submit_bos[idx].handle = bo->handle; - submit->submit_bos[idx].presumed = 0; - - submit->bos[idx] = fd_bo_ref(bo); + idx = APPEND(submit, submit_bos, (struct drm_msm_gem_submit_bo){ + .flags = bo->flags & (MSM_SUBMIT_BO_READ | MSM_SUBMIT_BO_WRITE), + .handle = bo->handle, + .presumed = 0, + }); + APPEND(submit, bos, fd_bo_ref(bo)); _mesa_hash_table_insert_pre_hashed(submit->bo_table, hash, bo, (void *)(uintptr_t)idx); @@ -464,12 +461,9 @@ finalize_current_cmd(struct fd_ringbuffer *ring) debug_assert(msm_ring->cmd->ring_bo == msm_ring->ring_bo); - unsigned idx = APPEND(&msm_ring->u, cmds); - - msm_ring->u.cmds[idx] = msm_ring->cmd; + msm_ring->cmd->size = offset_bytes(ring->cur, ring->start); + APPEND(&msm_ring->u, cmds, msm_ring->cmd); msm_ring->cmd = NULL; - - msm_ring->u.cmds[idx]->size = offset_bytes(ring->cur, ring->start); } static void @@ -501,9 +495,7 @@ msm_ringbuffer_emit_reloc(struct fd_ringbuffer *ring, unsigned reloc_idx; if (ring->flags & _FD_RINGBUFFER_OBJECT) { - unsigned idx = APPEND(&msm_ring->u, reloc_bos); - - msm_ring->u.reloc_bos[idx] = fd_bo_ref(reloc->bo); + unsigned idx = APPEND(&msm_ring->u, reloc_bos, fd_bo_ref(reloc->bo)); /* this gets fixed up at submit->flush() time, since this state- * object rb can be used with many different submits @@ -520,30 +512,24 @@ msm_ringbuffer_emit_reloc(struct fd_ringbuffer *ring, pipe = msm_ring->u.submit->pipe; } - struct drm_msm_gem_submit_reloc *r; - unsigned idx = APPEND(msm_ring->cmd, relocs); - - r = &msm_ring->cmd->relocs[idx]; - - r->reloc_idx = reloc_idx; - r->reloc_offset = reloc->offset; - r->or = reloc->or; - r->shift = reloc->shift; - r->submit_offset = offset_bytes(ring->cur, ring->start) + - msm_ring->offset; + APPEND(msm_ring->cmd, relocs, (struct drm_msm_gem_submit_reloc){ + .reloc_idx = reloc_idx, + .reloc_offset = reloc->offset, + .or = reloc->or, + .shift = reloc->shift, + .submit_offset = offset_bytes(ring->cur, ring->start) + msm_ring->offset, + }); ring->cur++; if (pipe->gpu_id >= 500) { - idx = APPEND(msm_ring->cmd, relocs); - r = &msm_ring->cmd->relocs[idx]; - - r->reloc_idx = reloc_idx; - r->reloc_offset = reloc->offset; - r->or = reloc->orhi; - r->shift = reloc->shift - 32; - r->submit_offset = offset_bytes(ring->cur, ring->start) + - msm_ring->offset; + APPEND(msm_ring->cmd, relocs, (struct drm_msm_gem_submit_reloc){ + .reloc_idx = reloc_idx, + .reloc_offset = reloc->offset, + .or = reloc->orhi, + .shift = reloc->shift - 32, + .submit_offset = offset_bytes(ring->cur, ring->start) + msm_ring->offset, + }); ring->cur++; } diff --git a/src/freedreno/drm/msm_ringbuffer_sp.c b/src/freedreno/drm/msm_ringbuffer_sp.c index 62a937d..4b7ca30 100644 --- a/src/freedreno/drm/msm_ringbuffer_sp.c +++ b/src/freedreno/drm/msm_ringbuffer_sp.c @@ -126,9 +126,7 @@ msm_submit_append_bo(struct msm_submit_sp *submit, struct fd_bo *bo) /* found */ idx = (uint32_t)(uintptr_t)entry->data; } else { - idx = APPEND(submit, bos); - - submit->bos[idx] = fd_bo_ref(bo); + idx = APPEND(submit, bos, fd_bo_ref(bo)); _mesa_hash_table_insert_pre_hashed(submit->bo_table, hash, bo, (void *)(uintptr_t)idx); @@ -272,7 +270,7 @@ msm_submit_sp_flush(struct fd_submit *submit, int in_fence_fd, submit_bos[i].handle = msm_submit->bos[i]->handle; submit_bos[i].presumed = 0; } - req.bos = VOID2U64(&submit_bos), + req.bos = VOID2U64(submit_bos), req.nr_bos = msm_submit->nr_bos; req.cmds = VOID2U64(cmds), req.nr_cmds = primary->u.nr_cmds; @@ -366,10 +364,10 @@ finalize_current_cmd(struct fd_ringbuffer *ring) debug_assert(!(ring->flags & _FD_RINGBUFFER_OBJECT)); struct msm_ringbuffer_sp *msm_ring = to_msm_ringbuffer_sp(ring); - unsigned idx = APPEND(&msm_ring->u, cmds); - - msm_ring->u.cmds[idx].ring_bo = fd_bo_ref(msm_ring->ring_bo); - msm_ring->u.cmds[idx].size = offset_bytes(ring->cur, ring->start); + APPEND(&msm_ring->u, cmds, (struct msm_cmd_sp){ + .ring_bo = fd_bo_ref(msm_ring->ring_bo), + .size = offset_bytes(ring->cur, ring->start), + }); } static void @@ -413,8 +411,7 @@ msm_ringbuffer_sp_emit_reloc(struct fd_ringbuffer *ring, } } if (!found) { - unsigned idx = APPEND(&msm_ring->u, reloc_bos); - msm_ring->u.reloc_bos[idx] = fd_bo_ref(reloc->bo); + APPEND(&msm_ring->u, reloc_bos, fd_bo_ref(reloc->bo)); } pipe = msm_ring->u.pipe; @@ -474,10 +471,7 @@ msm_ringbuffer_sp_emit_reloc_ring(struct fd_ringbuffer *ring, if (ring->flags & _FD_RINGBUFFER_OBJECT) { for (unsigned i = 0; i < msm_target->u.nr_reloc_bos; i++) { - unsigned idx = APPEND(&msm_ring->u, reloc_bos); - - msm_ring->u.reloc_bos[idx] = - fd_bo_ref(msm_target->u.reloc_bos[i]); + APPEND(&msm_ring->u, reloc_bos, fd_bo_ref(msm_target->u.reloc_bos[i])); } } else { // TODO it would be nice to know whether we have already -- 2.7.4