[Mesa-dev] [PATCH 09/14] gallium/radeon: enable suballocations for VRAM with no CPU access

[Nicolai Hähnle]

On 29.06.2017 21:47, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> ---
>   src/gallium/drivers/radeon/radeon_winsys.h    | 15 ++++++++++++---
>   src/gallium/winsys/amdgpu/drm/amdgpu_bo.c     |  3 +++
>   src/gallium/winsys/radeon/drm/radeon_drm_bo.c |  3 +++
>   3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h
> index 95543bb..4ecd73f 100644
> --- a/src/gallium/drivers/radeon/radeon_winsys.h
> +++ b/src/gallium/drivers/radeon/radeon_winsys.h
> @@ -652,70 +652,79 @@ static inline void radeon_emit(struct radeon_winsys_cs *cs, uint32_t 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_NO_CPU_ACCESS,
>       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_NO_CPU_ACCESS:
>       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_NO_CPU_ACCESS:
> +        return RADEON_FLAG_GTT_WC | RADEON_FLAG_NO_CPU_ACCESS;
>       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);
> +    /* NO_CPU_ACCESS implies VRAM only. */
> +    assert(!(flags & RADEON_FLAG_NO_CPU_ACCESS) || domain == RADEON_DOMAIN_VRAM);
>   
> -    /* Unsupported flags: NO_CPU_ACCESS, NO_SUBALLOC, SPARSE. */
> -    if (flags & ~RADEON_FLAG_GTT_WC)
> +    /* Unsupported flags: NO_SUBALLOC, SPARSE. */
> +    if (flags & ~(RADEON_FLAG_GTT_WC | RADEON_FLAG_NO_CPU_ACCESS))
>           return -1;
>   
>       switch (domain) {
>       case RADEON_DOMAIN_VRAM:
> -        return RADEON_HEAP_VRAM;
> +        if (flags & RADEON_FLAG_NO_CPU_ACCESS)
> +            return RADEON_HEAP_VRAM_NO_CPU_ACCESS;
> +        else
> +            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;
>   }
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> index 5ebe59f..38aaa89 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> @@ -1131,20 +1131,23 @@ amdgpu_bo_create(struct radeon_winsys *rws,
>                    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;
> +   /* NO_CPU_ACCESS is valid with VRAM only. */
> +   if (domain != RADEON_DOMAIN_VRAM)
> +      flags &= ~RADEON_FLAG_NO_CPU_ACCESS;

I'd also prefer this as an assert. With that changed:

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;
>         int heap = radeon_get_heap_index(domain, flags);
>   
>         if (heap < 0 || heap >= RADEON_MAX_SLAB_HEAPS)
>            goto no_slab;
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index a9421d6..cef88a6 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> @@ -918,20 +918,23 @@ radeon_winsys_bo_create(struct radeon_winsys *rws,
>   
>       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;
> +    /* NO_CPU_ACCESS is valid with VRAM only. */
> +    if (domain != RADEON_DOMAIN_VRAM)
> +        flags &= ~RADEON_FLAG_NO_CPU_ACCESS;
>   
>       /* 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;
>           int heap = radeon_get_heap_index(domain, flags);
>   
>           if (heap < 0 || heap >= RADEON_MAX_SLAB_HEAPS)
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.