On 29.06.2017 21:47, Marek Olšák wrote:
From: Marek Olšák <marek.ol...@amd.com>

This is cleaner, and we are down to 4 slabs.
---
  src/gallium/drivers/radeon/radeon_winsys.h        | 62 +++++++++++++++++++++++
  src/gallium/winsys/amdgpu/drm/amdgpu_bo.c         | 44 +++-------------
  src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c     |  2 +-
  src/gallium/winsys/radeon/drm/radeon_drm_bo.c     | 44 +++-------------
  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c |  2 +-
  5 files changed, 80 insertions(+), 74 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_winsys.h 
b/src/gallium/drivers/radeon/radeon_winsys.h
index 1be94f7..95543bb 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -651,11 +651,73 @@ static inline void radeon_emit(struct radeon_winsys_cs 
*cs, uint32_t value)
      cs->current.buf[cs->current.cdw++] = value;
  }
static inline void radeon_emit_array(struct radeon_winsys_cs *cs,
                                     const uint32_t *values, unsigned count)
  {
      memcpy(cs->current.buf + cs->current.cdw, values, count * 4);
      cs->current.cdw += count;
  }
+enum radeon_heap {
+    RADEON_HEAP_VRAM,
+    RADEON_HEAP_VRAM_GTT, /* combined heaps */
+    RADEON_HEAP_GTT_WC,
+    RADEON_HEAP_GTT,
+    RADEON_MAX_SLAB_HEAPS,
+};
+
+static inline enum radeon_bo_domain radeon_domain_from_heap(enum radeon_heap 
heap)
+{
+    switch (heap) {
+    case RADEON_HEAP_VRAM:
+        return RADEON_DOMAIN_VRAM;
+    case RADEON_HEAP_VRAM_GTT:
+        return RADEON_DOMAIN_VRAM_GTT;
+    case RADEON_HEAP_GTT_WC:
+    case RADEON_HEAP_GTT:
+        return RADEON_DOMAIN_GTT;
+    default:
+        assert(0);
+        return 0;
+    }
+}
+
+static inline unsigned radeon_flags_from_heap(enum radeon_heap heap)
+{
+    switch (heap) {
+    case RADEON_HEAP_VRAM:
+    case RADEON_HEAP_VRAM_GTT:
+    case RADEON_HEAP_GTT_WC:
+        return RADEON_FLAG_GTT_WC;
+    case RADEON_HEAP_GTT:
+    default:
+        return 0;
+    }
+}
+
+/* Return the heap index for winsys allocators, or -1 on failure. */
+static inline int radeon_get_heap_index(enum radeon_bo_domain domain,
+                                        enum radeon_bo_flag flags)
+{
+    /* VRAM implies WC (write combining) */
+    assert(!(domain & RADEON_DOMAIN_VRAM) || flags & RADEON_FLAG_GTT_WC);
+
+    /* Unsupported flags: NO_CPU_ACCESS, NO_SUBALLOC, SPARSE. */
+    if (flags & ~RADEON_FLAG_GTT_WC)
+        return -1;
+
+    switch (domain) {
+    case RADEON_DOMAIN_VRAM:
+        return RADEON_HEAP_VRAM;
+    case RADEON_DOMAIN_VRAM_GTT:
+        return RADEON_HEAP_VRAM_GTT;
+    case RADEON_DOMAIN_GTT:
+        if (flags & RADEON_FLAG_GTT_WC)
+            return RADEON_HEAP_GTT_WC;
+        else
+            return RADEON_HEAP_GTT;
+    }
+    return -1;
+}
+
  #endif
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c 
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index 9736f44a..5ebe59f 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -488,43 +488,27 @@ static const struct pb_vtbl amdgpu_winsys_bo_slab_vtbl = {
     amdgpu_bo_slab_destroy
     /* other functions are never called */
  };
struct pb_slab *amdgpu_bo_slab_alloc(void *priv, unsigned heap,
                                       unsigned entry_size,
                                       unsigned group_index)
  {
     struct amdgpu_winsys *ws = priv;
     struct amdgpu_slab *slab = CALLOC_STRUCT(amdgpu_slab);
-   enum radeon_bo_domain domains;
-   enum radeon_bo_flag flags = 0;
+   enum radeon_bo_domain domains = radeon_domain_from_heap(heap);
+   enum radeon_bo_flag flags = radeon_flags_from_heap(heap);
     uint32_t base_id;
if (!slab)
        return NULL;
- if (heap & 1)
-      flags |= RADEON_FLAG_GTT_WC;
-
-   switch (heap >> 2) {
-   case 0:
-      domains = RADEON_DOMAIN_VRAM;
-      break;
-   default:
-   case 1:
-      domains = RADEON_DOMAIN_VRAM_GTT;
-      break;
-   case 2:
-      domains = RADEON_DOMAIN_GTT;
-      break;
-   }
-
     slab->buffer = amdgpu_winsys_bo(amdgpu_bo_create(&ws->base,
                                                      64 * 1024, 64 * 1024,
                                                      domains, flags));
     if (!slab->buffer)
        goto fail;
assert(slab->buffer->bo); slab->base.num_entries = slab->buffer->base.size / entry_size;
     slab->base.num_free = slab->base.num_entries;
@@ -1144,45 +1128,33 @@ static struct pb_buffer *
  amdgpu_bo_create(struct radeon_winsys *rws,
                   uint64_t size,
                   unsigned alignment,
                   enum radeon_bo_domain domain,
                   enum radeon_bo_flag flags)
  {
     struct amdgpu_winsys *ws = amdgpu_winsys(rws);
     struct amdgpu_winsys_bo *bo;
     unsigned usage = 0, pb_cache_bucket;
+ /* VRAM implies WC. This is not optional. */
+   if (domain & RADEON_DOMAIN_VRAM)
+      flags |= RADEON_FLAG_GTT_WC;

I'd prefer this as an assert. Otherwise callers might be confused about being able to leave the flag clear.

With that changed, patches 1-8 are

Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>


+
     /* Sub-allocate small buffers from slabs. */
     if (!(flags & (RADEON_FLAG_NO_SUBALLOC | RADEON_FLAG_SPARSE)) &&
         size <= (1 << AMDGPU_SLAB_MAX_SIZE_LOG2) &&
         alignment <= MAX2(1 << AMDGPU_SLAB_MIN_SIZE_LOG2, 
util_next_power_of_two(size))) {
        struct pb_slab_entry *entry;
-      unsigned heap = 0;
-
-      if (flags & RADEON_FLAG_GTT_WC)
-         heap |= 1;
-      if (flags & ~RADEON_FLAG_GTT_WC)
-         goto no_slab;
+      int heap = radeon_get_heap_index(domain, flags);
- switch (domain) {
-      case RADEON_DOMAIN_VRAM:
-         heap |= 0 * 4;
-         break;
-      case RADEON_DOMAIN_VRAM_GTT:
-         heap |= 1 * 4;
-         break;
-      case RADEON_DOMAIN_GTT:
-         heap |= 2 * 4;
-         break;
-      default:
+      if (heap < 0 || heap >= RADEON_MAX_SLAB_HEAPS)
           goto no_slab;
-      }
entry = pb_slab_alloc(&ws->bo_slabs, size, heap);
        if (!entry) {
           /* Clear the cache and try again. */
           pb_cache_release_all_buffers(&ws->bo_cache);
entry = pb_slab_alloc(&ws->bo_slabs, size, heap);
        }
        if (!entry)
           return NULL;
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c 
b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 2148c49..ef7b04a 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -280,21 +280,21 @@ amdgpu_winsys_create(int fd, unsigned flags,
     if (!do_winsys_init(ws, fd))
        goto fail_alloc;
/* Create managers. */
     pb_cache_init(&ws->bo_cache, 500000, ws->check_vm ? 1.0f : 2.0f, 0,
                   (ws->info.vram_size + ws->info.gart_size) / 8,
                   amdgpu_bo_destroy, amdgpu_bo_can_reclaim);
if (!pb_slabs_init(&ws->bo_slabs,
                        AMDGPU_SLAB_MIN_SIZE_LOG2, AMDGPU_SLAB_MAX_SIZE_LOG2,
-                      12, /* number of heaps (domain/flags combinations) */
+                      RADEON_MAX_SLAB_HEAPS,
                        ws,
                        amdgpu_bo_can_reclaim_slab,
                        amdgpu_bo_slab_alloc,
                        amdgpu_bo_slab_free))
        goto fail_cache;
ws->info.min_alloc_size = 1 << AMDGPU_SLAB_MIN_SIZE_LOG2; /* init reference */
     pipe_reference_init(&ws->reference, 1);
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index 81a59e5..a9421d6 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -722,43 +722,27 @@ static const struct pb_vtbl radeon_winsys_bo_slab_vtbl = {
      radeon_bo_slab_destroy
      /* other functions are never called */
  };
struct pb_slab *radeon_bo_slab_alloc(void *priv, unsigned heap,
                                       unsigned entry_size,
                                       unsigned group_index)
  {
      struct radeon_drm_winsys *ws = priv;
      struct radeon_slab *slab = CALLOC_STRUCT(radeon_slab);
-    enum radeon_bo_domain domains;
-    enum radeon_bo_flag flags = 0;
+    enum radeon_bo_domain domains = radeon_domain_from_heap(heap);
+    enum radeon_bo_flag flags = radeon_flags_from_heap(heap);
      unsigned base_hash;
if (!slab)
          return NULL;
- if (heap & 1)
-        flags |= RADEON_FLAG_GTT_WC;
-
-    switch (heap >> 2) {
-    case 0:
-        domains = RADEON_DOMAIN_VRAM;
-        break;
-    default:
-    case 1:
-        domains = RADEON_DOMAIN_VRAM_GTT;
-        break;
-    case 2:
-        domains = RADEON_DOMAIN_GTT;
-        break;
-    }
-
      slab->buffer = radeon_bo(radeon_winsys_bo_create(&ws->base,
                                                       64 * 1024, 64 * 1024,
                                                       domains, flags));
      if (!slab->buffer)
          goto fail;
assert(slab->buffer->handle); slab->base.num_entries = slab->buffer->base.size / entry_size;
      slab->base.num_free = slab->base.num_entries;
@@ -931,47 +915,35 @@ radeon_winsys_bo_create(struct radeon_winsys *rws,
      struct radeon_drm_winsys *ws = radeon_drm_winsys(rws);
      struct radeon_bo *bo;
      unsigned usage = 0, pb_cache_bucket;
assert(!(flags & RADEON_FLAG_SPARSE)); /* not supported */ /* Only 32-bit sizes are supported. */
      if (size > UINT_MAX)
          return NULL;
+ /* VRAM implies WC. This is not optional. */
+    if (domain & RADEON_DOMAIN_VRAM)
+        flags |= RADEON_FLAG_GTT_WC;
+
      /* Sub-allocate small buffers from slabs. */
      if (!(flags & RADEON_FLAG_NO_SUBALLOC) &&
          size <= (1 << RADEON_SLAB_MAX_SIZE_LOG2) &&
          ws->info.has_virtual_memory &&
          alignment <= MAX2(1 << RADEON_SLAB_MIN_SIZE_LOG2, 
util_next_power_of_two(size))) {
          struct pb_slab_entry *entry;
-        unsigned heap = 0;
+        int heap = radeon_get_heap_index(domain, flags);
- if (flags & RADEON_FLAG_GTT_WC)
-            heap |= 1;
-        if (flags & ~RADEON_FLAG_GTT_WC)
+        if (heap < 0 || heap >= RADEON_MAX_SLAB_HEAPS)
              goto no_slab;
- switch (domain) {
-        case RADEON_DOMAIN_VRAM:
-            heap |= 0 * 4;
-            break;
-        case RADEON_DOMAIN_VRAM_GTT:
-            heap |= 1 * 4;
-            break;
-        case RADEON_DOMAIN_GTT:
-            heap |= 2 * 4;
-            break;
-        default:
-            goto no_slab;
-        }
-
          entry = pb_slab_alloc(&ws->bo_slabs, size, heap);
          if (!entry) {
              /* Clear the cache and try again. */
              pb_cache_release_all_buffers(&ws->bo_cache);
entry = pb_slab_alloc(&ws->bo_slabs, size, heap);
          }
          if (!entry)
              return NULL;
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 8e43b68..ad1db3c 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -769,21 +769,21 @@ radeon_drm_winsys_create(int fd, unsigned flags,
                    radeon_bo_destroy,
                    radeon_bo_can_reclaim);
if (ws->info.has_virtual_memory) {
          /* There is no fundamental obstacle to using slab buffer allocation
           * without GPUVM, but enabling it requires making sure that the 
drivers
           * honor the address offset.
           */
          if (!pb_slabs_init(&ws->bo_slabs,
                             RADEON_SLAB_MIN_SIZE_LOG2, 
RADEON_SLAB_MAX_SIZE_LOG2,
-                           12,
+                           RADEON_MAX_SLAB_HEAPS,
                             ws,
                             radeon_bo_can_reclaim_slab,
                             radeon_bo_slab_alloc,
                             radeon_bo_slab_free))
              goto fail_cache;
ws->info.min_alloc_size = 1 << RADEON_SLAB_MIN_SIZE_LOG2;
      } else {
          ws->info.min_alloc_size = ws->info.gart_page_size;
      }



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to