winsys/amdgpu: don't set GTT with GDS & OA placements on APUs

Submitted by Marek Olšák on April 15, 2019, 5:21 p.m.

Details

Message ID 20190415172127.25209-1-maraeo@gmail.com
State New
Headers show
Series "winsys/amdgpu: don't set GTT with GDS & OA placements on APUs" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Marek Olšák April 15, 2019, 5:21 p.m.
From: Marek Olšák <marek.olsak@amd.com>

---
 src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

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;

Comments

On 2019-04-15 7:21 p.m., Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@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@amd.com>