Re: [PATCH 5/9] drm/amdgpu: remove GTT accounting

[Felix Kuehling <felix.kuehling@xxxxxxx>]

On 2022-02-09 03:40, Christian König 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.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> Tested-by: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
Should amdgpu_preempt_mgr also be updated?

Regards,
  Felix


>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c     |  8 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  2 -
>   3 files changed, 15 insertions(+), 44 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);
> -		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;
> +	}
>   
>   	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_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);