Re: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2

Am 14.02.22 um 12:10 schrieb Matthew Auld:

On Mon, 14 Feb 2022 at 09:34, Christian König ckoenig.leichtzumerken@gmail.com wrote:

...

This is provided by TTM now.

Also switch man->size to bytes instead of pages and fix the double printing of size and usage in debugfs.

v2: fix size checking as well

Signed-off-by: Christian König christian.koenig@amd.com Tested-by: Bas Nieuwenhuizen bas@basnieuwenhuizen.nl

---

drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++---------------- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - 4 files changed, 16 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index e0c7fbe01d93..3bcd27ae379d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev, struct ttm_resource_manager *man;

     man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);

•   return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
  

•   return sysfs_emit(buf, "%llu\n", man->size);
  

  }

  /**

@@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev, { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = drm_to_adev(ddev);

•   struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
  
  

•   return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
  

•   return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
  

  }

  static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,

@@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, struct amdgpu_gtt_node *node; int r;

•   if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
  

•       atomic64_add_return(num_pages, &mgr->used) >  man->size) {
  

•           atomic64_sub(num_pages, &mgr->used);
  

I guess this behaviour is now slightly different, since TEMPORARY will now get accounted like everything else. Hopefully that is not a concern.

Yeah, that's correct but also unproblematic. See a few lines below.

Otherwise seems mechanical, Reviewed-by: Matthew Auld matthew.auld@intel.com

...

•           return -ENOSPC;
  

•   }
  

•    node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
  

•   if (!node) {
  

•           r = -ENOMEM;
  

•           goto err_out;
  

•   }
  

•   if (!node)
  

•           return -ENOMEM;
  
     node->tbo = tbo;
     ttm_resource_init(tbo, place, &node->base.base);
  

•   if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
  

•       ttm_resource_manager_usage(man) > man->size) {
  

•           r = -ENOSPC;
  

•           goto err_free;
  

•   }
  

This here now takes care of TTM_PL_FLAG_TEMPORARY.

Regards, Christian.

...

     if (place->lpfn) {
             spin_lock(&mgr->lock);

@@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, err_free: ttm_resource_fini(man, &node->base.base); kfree(node);

-err_out:

•   if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
  

•           atomic64_sub(num_pages, &mgr->used);
  

•    return r;
  

  }

@@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, drm_mm_remove_node(&node->base.mm_nodes[0]); spin_unlock(&mgr->lock);

•   if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
  

•           atomic64_sub(res->num_pages, &mgr->used);
  

•    ttm_resource_fini(man, res);
     kfree(node);
  

  }

-/**

• • amdgpu_gtt_mgr_usage - return usage of GTT domain
• • @mgr: amdgpu_gtt_mgr pointer
• • Return how many bytes are used in the GTT domain
• */

-uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr) -{

•   return atomic64_read(&mgr->used) * PAGE_SIZE;
  

-}

• /**
  • amdgpu_gtt_mgr_recover - re-init gart

@@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man, spin_lock(&mgr->lock); drm_mm_print(&mgr->mm, printer); spin_unlock(&mgr->lock);

•   drm_printf(printer, "man size:%llu pages,  gtt used:%llu pages\n",
  

•              man->size, atomic64_read(&mgr->used));
  

  }

  static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {

@@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) man->use_tt = true; man->func = &amdgpu_gtt_mgr_func;

•   ttm_resource_manager_init(man, &adev->mman.bdev,
  

•                             gtt_size >> PAGE_SHIFT);
  

•   ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
  
     start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
     size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
     drm_mm_init(&mgr->mm, start, size);
     spin_lock_init(&mgr->lock);
  

•   atomic64_set(&mgr->used, 0);
  
     ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
     ttm_resource_manager_set_used(man, true);
  

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 1ebb91db2274..9ff4aced5da7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr); return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0; case AMDGPU_INFO_GTT_USAGE:

•           ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
  

•           ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
             return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
     case AMDGPU_INFO_GDS_CONFIG: {
             struct drm_amdgpu_info_gds gds_info;
  

@@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) case AMDGPU_INFO_MEMORY: { struct drm_amdgpu_memory_info mem; struct ttm_resource_manager *gtt_man =

•                   ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
  

•                   &adev->mman.gtt_mgr.manager;
  

•            memset(&mem, 0, sizeof(mem));
             mem.vram.total_heap_size = adev->gmc.real_vram_size;
             mem.vram.usable_heap_size = adev->gmc.real_vram_size -
  

@@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) mem.gtt.total_heap_size *= PAGE_SIZE; mem.gtt.usable_heap_size = mem.gtt.total_heap_size - atomic64_read(&adev->gart_pin_size);

•           mem.gtt.heap_usage =
  

•                   amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
  

•           mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
             mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
  
             return copy_to_user(out, &mem,
  

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 5661b82d84d4..514754142f69 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -451,7 +451,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev, if (domain & AMDGPU_GEM_DOMAIN_GTT) { man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);

•           if (size < (man->size << PAGE_SHIFT))
  

•           if (size < man->size)
                     return true;
             else
                     goto fail;
  

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index f8f48be16d80..120b69ec9885 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr { struct ttm_resource_manager manager; struct drm_mm mm; spinlock_t lock;

•   atomic64_t used;
  

  };

  struct amdgpu_preempt_mgr {

@@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev); void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);

bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem); -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr); int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);

uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);

2.25.1