[Mesa-dev] [PATCH 06/12] winsys/amdgpu: extract adding a new buffer list entry into its own function

[Nicolai Hähnle]

From: Nicolai Hähnle <nicolai.haehnle at amd.com>

While at it, try to be a little more robust in the face of memory allocation
failure.
---
 src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 113 ++++++++++++++++++------------
 1 file changed, 70 insertions(+), 43 deletions(-)

diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
index 7cbf19b..638c2d5 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
@@ -284,77 +284,104 @@ int amdgpu_lookup_buffer(struct amdgpu_cs_context *cs, struct amdgpu_winsys_bo *
           *         AAAAAAAAAAABBBBBBBBBBBBBBCCCCCCCC
           * will collide here: ^ and here:   ^,
           * meaning that we should get very few collisions in the end. */
          cs->buffer_indices_hashlist[hash] = i;
          return i;
       }
    }
    return -1;
 }
 
-static unsigned amdgpu_add_buffer(struct amdgpu_cs *acs,
-                                 struct amdgpu_winsys_bo *bo,
-                                 enum radeon_bo_usage usage,
-                                 enum radeon_bo_domain domains,
-                                 unsigned priority,
-                                 enum radeon_bo_domain *added_domains)
+static int
+amdgpu_lookup_or_add_buffer(struct amdgpu_cs_context *cs, struct amdgpu_winsys_bo *bo)
 {
-   struct amdgpu_cs_context *cs = acs->csc;
    struct amdgpu_cs_buffer *buffer;
-   unsigned hash = bo->unique_id & (ARRAY_SIZE(cs->buffer_indices_hashlist)-1);
-   int i = -1;
+   unsigned hash;
+   int idx = amdgpu_lookup_buffer(cs, bo);
 
-   assert(priority < 64);
-   *added_domains = 0;
-
-   i = amdgpu_lookup_buffer(cs, bo);
-
-   if (i >= 0) {
-      buffer = &cs->buffers[i];
-      buffer->priority_usage |= 1llu << priority;
-      buffer->usage |= usage;
-      *added_domains = domains & ~buffer->domains;
-      buffer->domains |= domains;
-      cs->flags[i] = MAX2(cs->flags[i], priority / 4);
-      return i;
-   }
+   if (idx >= 0)
+      return idx;
 
    /* New buffer, check if the backing array is large enough. */
    if (cs->num_buffers >= cs->max_num_buffers) {
-      uint32_t size;
-      cs->max_num_buffers += 10;
+      unsigned new_max =
+         MAX2(cs->max_num_buffers + 16, (unsigned)(cs->max_num_buffers * 1.3));
+      struct amdgpu_cs_buffer *new_buffers;
+      amdgpu_bo_handle *new_handles;
+      uint8_t *new_flags;
+
+      new_buffers = MALLOC(new_max * sizeof(*new_buffers));
+      new_handles = MALLOC(new_max * sizeof(*new_handles));
+      new_flags = MALLOC(new_max * sizeof(*new_flags));
+
+      if (!new_buffers || !new_handles || !new_flags) {
+         fprintf(stderr, "amdgpu_lookup_or_add_buffer: allocation failed\n");
+         FREE(new_buffers);
+         FREE(new_handles);
+         FREE(new_flags);
+         return -1;
+      }
 
-      size = cs->max_num_buffers * sizeof(struct amdgpu_cs_buffer);
-      cs->buffers = realloc(cs->buffers, size);
+      memcpy(new_buffers, cs->buffers, cs->num_buffers * sizeof(*new_buffers));
+      memcpy(new_handles, cs->handles, cs->num_buffers * sizeof(*new_handles));
+      memcpy(new_flags, cs->flags, cs->num_buffers * sizeof(*new_flags));
 
-      size = cs->max_num_buffers * sizeof(amdgpu_bo_handle);
-      cs->handles = realloc(cs->handles, size);
+      FREE(cs->buffers);
+      FREE(cs->handles);
+      FREE(cs->flags);
 
-      cs->flags = realloc(cs->flags, cs->max_num_buffers);
+      cs->max_num_buffers = new_max;
+      cs->buffers = new_buffers;
+      cs->handles = new_handles;
+      cs->flags = new_flags;
    }
 
-   /* Initialize the new buffer. */
-   cs->buffers[cs->num_buffers].bo = NULL;
-   amdgpu_winsys_bo_reference(&cs->buffers[cs->num_buffers].bo, bo);
-   cs->handles[cs->num_buffers] = bo->bo;
-   cs->flags[cs->num_buffers] = priority / 4;
+   idx = cs->num_buffers;
+   buffer = &cs->buffers[idx];
+   memset(buffer, 0, sizeof(*buffer));
+   amdgpu_winsys_bo_reference(&buffer->bo, bo);
+   cs->handles[idx] = bo->bo;
+   cs->flags[idx] = 0;
    p_atomic_inc(&bo->num_cs_references);
-   buffer = &cs->buffers[cs->num_buffers];
-   buffer->bo = bo;
-   buffer->priority_usage = 1llu << priority;
-   buffer->usage = usage;
-   buffer->domains = domains;
+   cs->num_buffers++;
 
-   cs->buffer_indices_hashlist[hash] = cs->num_buffers;
+   hash = bo->unique_id & (ARRAY_SIZE(cs->buffer_indices_hashlist)-1);
+   cs->buffer_indices_hashlist[hash] = idx;
+
+   return idx;
+}
+
+static unsigned amdgpu_add_buffer(struct amdgpu_cs *acs,
+                                 struct amdgpu_winsys_bo *bo,
+                                 enum radeon_bo_usage usage,
+                                 enum radeon_bo_domain domains,
+                                 unsigned priority,
+                                 enum radeon_bo_domain *added_domains)
+{
+   struct amdgpu_cs_context *cs = acs->csc;
+   struct amdgpu_cs_buffer *buffer;
+   int i = amdgpu_lookup_or_add_buffer(cs, bo);
+
+   assert(priority < 64);
+
+   if (i < 0) {
+      *added_domains = 0;
+      return ~0;
+   }
 
-   *added_domains = domains;
-   return cs->num_buffers++;
+   buffer = &cs->buffers[i];
+   buffer->priority_usage |= 1llu << priority;
+   buffer->usage |= usage;
+   *added_domains = domains & ~buffer->domains;
+   buffer->domains |= domains;
+   cs->flags[i] = MAX2(cs->flags[i], priority / 4);
+   return i;
 }
 
 static unsigned amdgpu_cs_add_buffer(struct radeon_winsys_cs *rcs,
                                     struct pb_buffer *buf,
                                     enum radeon_bo_usage usage,
                                     enum radeon_bo_domain domains,
                                     enum radeon_bo_priority priority)
 {
    /* Don't use the "domains" parameter. Amdgpu doesn't support changing
     * the buffer placement during command submission.
-- 
2.7.4