[Mesa-dev] [PATCH] winsys/amdgpu: don't set GTT with GDS & OA placements on APUs

[Michel Dänzer]

On 2019-04-15 7:21 p.m., Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> ---
>  src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> index c1863057370..09cf9247755 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> @@ -471,37 +471,39 @@ static struct amdgpu_winsys_bo *amdgpu_create_bo(struct amdgpu_winsys *ws,
>        return NULL;
>     }
>  
>     if (heap >= 0) {
>        pb_cache_init_entry(&ws->bo_cache, &bo->u.real.cache_entry, &bo->base,
>                            heap);
>     }
>     request.alloc_size = size;
>     request.phys_alignment = alignment;
>  
> -   if (initial_domain & RADEON_DOMAIN_VRAM)
> +   if (initial_domain & RADEON_DOMAIN_VRAM) {
>        request.preferred_heap |= AMDGPU_GEM_DOMAIN_VRAM;
> +
> +      /* Since VRAM and GTT have almost the same performance on APUs, we could
> +       * just set GTT. However, in order to decrease GTT(RAM) usage, which is
> +       * shared with the OS, allow VRAM placements too. The idea is not to use
> +       * VRAM usefully, but to use it so that it's not unused and wasted.
> +       */
> +      if (!ws->info.has_dedicated_vram)
> +         request.preferred_heap |= AMDGPU_GEM_DOMAIN_GTT;
> +   }
> +
>     if (initial_domain & RADEON_DOMAIN_GTT)
>        request.preferred_heap |= AMDGPU_GEM_DOMAIN_GTT;
>     if (initial_domain & RADEON_DOMAIN_GDS)
>        request.preferred_heap |= AMDGPU_GEM_DOMAIN_GDS;
>     if (initial_domain & RADEON_DOMAIN_OA)
>        request.preferred_heap |= AMDGPU_GEM_DOMAIN_OA;
>  
> -   /* Since VRAM and GTT have almost the same performance on APUs, we could
> -    * just set GTT. However, in order to decrease GTT(RAM) usage, which is
> -    * shared with the OS, allow VRAM placements too. The idea is not to use
> -    * VRAM usefully, but to use it so that it's not unused and wasted.
> -    */
> -   if (!ws->info.has_dedicated_vram)
> -      request.preferred_heap |= AMDGPU_GEM_DOMAIN_GTT;
> -
>     if (flags & RADEON_FLAG_NO_CPU_ACCESS)
>        request.flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
>     if (flags & RADEON_FLAG_GTT_WC)
>        request.flags |= AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>     if (flags & RADEON_FLAG_NO_INTERPROCESS_SHARING &&
>         ws->info.has_local_buffers)
>        request.flags |= AMDGPU_GEM_CREATE_VM_ALWAYS_VALID;
>     if (ws->zero_all_vram_allocs &&
>         (request.preferred_heap & AMDGPU_GEM_DOMAIN_VRAM))
>        request.flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
> 

Does this fix a problem which could be mentioned in the commit log?
Either way,

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer