[Mesa-dev] [PATCH 08/14] gallium/radeon: clean up (domain, flags) <-> (slab heap) translations
Nicolai Hähnle
nhaehnle at gmail.com
Mon Jul 3 19:47:48 UTC 2017
On 29.06.2017 21:47, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at 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.haehnle at 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.
More information about the mesa-dev
mailing list