[Mesa-dev] [PATCH 10/14] gallium/radeon: clean up pb_cache bucket/usage determination

Nicolai Hähnle nhaehnle at gmail.com
Mon Jul 3 19:51:06 UTC 2017


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    | 20 ++++++++++++++++++++
>   src/gallium/winsys/amdgpu/drm/amdgpu_bo.c     | 21 +++++----------------
>   src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 21 +++++----------------
>   3 files changed, 30 insertions(+), 32 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h
> index 4ecd73f..239b6ab 100644
> --- a/src/gallium/drivers/radeon/radeon_winsys.h
> +++ b/src/gallium/drivers/radeon/radeon_winsys.h
> @@ -658,20 +658,21 @@ static inline void radeon_emit_array(struct radeon_winsys_cs *cs,
>       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,
> +    RADEON_MAX_CACHED_HEAPS = 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;
> @@ -692,20 +693,39 @@ static inline unsigned radeon_flags_from_heap(enum radeon_heap 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;
>       }
>   }
>   
> +/* The pb cache bucket is chosen to minimize pb_cache misses.
> + * It must be between 0 and 3 inclusive.
> + */
> +static inline unsigned radeon_get_pb_cache_bucket_index(enum radeon_heap heap)
> +{
> +    switch (heap) {
> +    case RADEON_HEAP_VRAM_NO_CPU_ACCESS:
> +        return 0;
> +    case RADEON_HEAP_VRAM:
> +    case RADEON_HEAP_VRAM_GTT:
> +        return 1;

Why are VRAM and VRAM+GTT in the same heap?

We don't even use VRAM+GTT on amdgpu anymore, but maybe this affects 
radeon in an adverse way...

Cheers,
Nicolai


> +    case RADEON_HEAP_GTT_WC:
> +        return 2;
> +    case RADEON_HEAP_GTT:
> +    default:
> +        return 3;
> +    }
> +}
> +
>   /* 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_SUBALLOC, SPARSE. */
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> index 38aaa89..d512f7b 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> @@ -1182,36 +1182,25 @@ no_slab:
>      /* This flag is irrelevant for the cache. */
>      flags &= ~RADEON_FLAG_NO_SUBALLOC;
>   
>      /* Align size to page size. This is the minimum alignment for normal
>       * BOs. Aligning this here helps the cached bufmgr. Especially small BOs,
>       * like constant/uniform buffers, can benefit from better and more reuse.
>       */
>      size = align64(size, ws->info.gart_page_size);
>      alignment = align(alignment, ws->info.gart_page_size);
>   
> -   /* Only set one usage bit each for domains and flags, or the cache manager
> -    * might consider different sets of domains / flags compatible
> -    */
> -   if (domain == RADEON_DOMAIN_VRAM_GTT)
> -      usage = 1 << 2;
> -   else
> -      usage = domain >> 1;
> -   assert(flags < sizeof(usage) * 8 - 3);
> -   usage |= 1 << (flags + 3);
> -
> -   /* Determine the pb_cache bucket for minimizing pb_cache misses. */
> -   pb_cache_bucket = 0;
> -   if (domain & RADEON_DOMAIN_VRAM) /* VRAM or VRAM+GTT */
> -      pb_cache_bucket += 1;
> -   if (flags == RADEON_FLAG_GTT_WC) /* WC */
> -      pb_cache_bucket += 2;
> +   int heap = radeon_get_heap_index(domain, flags);
> +   assert(heap >= 0 && heap < RADEON_MAX_CACHED_HEAPS);
> +   usage = 1 << heap; /* Only set one usage bit for each heap. */
> +
> +   pb_cache_bucket = radeon_get_pb_cache_bucket_index(heap);
>      assert(pb_cache_bucket < ARRAY_SIZE(ws->bo_cache.buckets));
>   
>      /* Get a buffer from the cache. */
>      bo = (struct amdgpu_winsys_bo*)
>           pb_cache_reclaim_buffer(&ws->bo_cache, size, alignment, usage,
>                                   pb_cache_bucket);
>      if (bo)
>         return &bo->base;
>   
>      /* Create a new one. */
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index cef88a6..bbdb60d 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> @@ -962,36 +962,25 @@ no_slab:
>       /* This flag is irrelevant for the cache. */
>       flags &= ~RADEON_FLAG_NO_SUBALLOC;
>   
>       /* Align size to page size. This is the minimum alignment for normal
>        * BOs. Aligning this here helps the cached bufmgr. Especially small BOs,
>        * like constant/uniform buffers, can benefit from better and more reuse.
>        */
>       size = align(size, ws->info.gart_page_size);
>       alignment = align(alignment, ws->info.gart_page_size);
>   
> -    /* Only set one usage bit each for domains and flags, or the cache manager
> -     * might consider different sets of domains / flags compatible
> -     */
> -    if (domain == RADEON_DOMAIN_VRAM_GTT)
> -        usage = 1 << 2;
> -    else
> -        usage = (unsigned)domain >> 1;
> -    assert(flags < sizeof(usage) * 8 - 3);
> -    usage |= 1 << (flags + 3);
> -
> -    /* Determine the pb_cache bucket for minimizing pb_cache misses. */
> -    pb_cache_bucket = 0;
> -    if (domain & RADEON_DOMAIN_VRAM) /* VRAM or VRAM+GTT */
> -       pb_cache_bucket += 1;
> -    if (flags == RADEON_FLAG_GTT_WC) /* WC */
> -       pb_cache_bucket += 2;
> +    int heap = radeon_get_heap_index(domain, flags);
> +    assert(heap >= 0 && heap < RADEON_MAX_CACHED_HEAPS);
> +    usage = 1 << heap; /* Only set one usage bit for each heap. */
> +
> +    pb_cache_bucket = radeon_get_pb_cache_bucket_index(heap);
>       assert(pb_cache_bucket < ARRAY_SIZE(ws->bo_cache.buckets));
>   
>       bo = radeon_bo(pb_cache_reclaim_buffer(&ws->bo_cache, size, alignment,
>                                              usage, pb_cache_bucket));
>       if (bo)
>           return &bo->base;
>   
>       bo = radeon_create_bo(ws, size, alignment, usage, domain, flags,
>                             pb_cache_bucket);
>       if (!bo) {
> 


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


More information about the mesa-dev mailing list