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 <robdclark@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7342>

diff --git a/src/freedreno/drm/msm_priv.h b/src/freedreno/drm/msm_priv.h
index 37adb13..c578078 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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