Re: [PATCH 2/2] drm/amdgpu: stop allocating dummy GTT nodes

Am 08.07.20 um 09:35 schrieb Chauhan, Madhav:

[AMD Public Use]

-----Original Message----- From: amd-gfx amd-gfx-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Monday, July 6, 2020 11:18 PM To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: [PATCH 2/2] drm/amdgpu: stop allocating dummy GTT nodes

Now that TTM is fixed up we can finally stop that nonsense.

Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 104 ++++++-------------- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 18 +++- 2 files changed, 42 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 2c20d23d62d1..62cf4fbd803a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -150,60 +150,7 @@ static int amdgpu_gtt_mgr_fini(struct ttm_mem_type_manager *man) */ bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_mem_reg *mem) {

• struct amdgpu_gtt_node *node = mem->mm_node;
• return (node->node.start != AMDGPU_BO_INVALID_OFFSET);

-}

-/**

• • amdgpu_gtt_mgr_alloc - allocate new ranges
• • @man: TTM memory type manager
• • @tbo: TTM BO we need this range for
• • @place: placement flags and restrictions
• • @mem: the resulting mem object
• • Allocate the address space for a node.
• */

-static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,

• 		struct ttm_buffer_object *tbo,
  

• 		const struct ttm_place *place,
  

• 		struct ttm_mem_reg *mem)
  

-{

• struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
• struct amdgpu_gtt_mgr *mgr = man->priv;
• struct amdgpu_gtt_node *node = mem->mm_node;
• enum drm_mm_insert_mode mode;
• unsigned long fpfn, lpfn;
• int r;
• if (amdgpu_gtt_mgr_has_gart_addr(mem))

• return 0;
  

• if (place)

• fpfn = place->fpfn;
  

• else

• fpfn = 0;
  

• if (place && place->lpfn)

• lpfn = place->lpfn;
  

• else

• lpfn = adev->gart.num_cpu_pages;
  

• mode = DRM_MM_INSERT_BEST;
• if (place && place->flags & TTM_PL_FLAG_TOPDOWN)

• mode = DRM_MM_INSERT_HIGH;
  

• spin_lock(&mgr->lock);
• r = drm_mm_insert_node_in_range(&mgr->mm, &node->node, mem->num_pages,

• 			mem->page_alignment, 0, fpfn, lpfn,
  

• 			mode);
  

• spin_unlock(&mgr->lock);
• if (!r)

• mem->start = node->node.start;
  

• return r;

• return mem->mm_node != NULL; }

  /**

@@ -234,29 +181,37 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man, atomic64_sub(mem->num_pages, &mgr->available); spin_unlock(&mgr->lock);

• if (!place->lpfn) {

• mem->mm_node = NULL;
  

• mem->start = AMDGPU_BO_INVALID_OFFSET;
  

• return 0;
  

• }
• node = kzalloc(sizeof(*node), GFP_KERNEL); if (!node) { r = -ENOMEM; goto err_out; }

• node->node.start = AMDGPU_BO_INVALID_OFFSET;

• node->node.size = mem->num_pages; node->tbo = tbo;

• mem->mm_node = node;

• if (place->fpfn || place->lpfn || place->flags & TTM_PL_FLAG_TOPDOWN) {

• r = amdgpu_gtt_mgr_alloc(man, tbo, place, mem);
  

• if (unlikely(r)) {
  

• 	kfree(node);
  

• 	mem->mm_node = NULL;
  

• 	goto err_out;
  

• }
  

• } else {

• mem->start = node->node.start;
  

• }

• spin_lock(&mgr->lock);

• r = drm_mm_insert_node_in_range(&mgr->mm, &node->node, mem->num_pages,

• 			mem->page_alignment, 0, place->fpfn,
  

• 			place->lpfn, DRM_MM_INSERT_BEST);
  

• spin_unlock(&mgr->lock);

• if (unlikely(r))

• goto err_free;
  

• mem->mm_node = node;

• mem->start = node->node.start;

  return 0;

+err_free:

• kfree(node);
• err_out: atomic64_add(mem->num_pages, &mgr->available);

@@ -279,17 +234,14 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man, struct amdgpu_gtt_mgr *mgr = man->priv; struct amdgpu_gtt_node *node = mem->mm_node;

• if (!node)

• return;
  

• spin_lock(&mgr->lock);
• if (node->node.start != AMDGPU_BO_INVALID_OFFSET)

• if (node) {

• spin_lock(&mgr->lock);
  

  drm_mm_remove_node(&node->node);

• spin_unlock(&mgr->lock);
• atomic64_add(mem->num_pages, &mgr->available);

• spin_unlock(&mgr->lock);
  

• kfree(node);
  

• }

• kfree(node);
• mem->mm_node = NULL;

• atomic64_add(mem->num_pages, &mgr->available); }

Looks fine to me, nitpick: Should we update the documentation of amdgpu_gtt_mgr_new() which still says "Dummy"??

Yes, that's probably a good point.

Thanks, Christian.

Regards, Madhav

/** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 38d2a7fb5698..50503f860fce 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -429,12 +429,22 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev, }

src_offset = src->offset;

• src_mm = amdgpu_find_mm_node(src->mem, &src_offset);
• src_node_size = (src_mm->size << PAGE_SHIFT) - src_offset;

• if (src->mem->mm_node) {

• src_mm = amdgpu_find_mm_node(src->mem, &src_offset);
  

• src_node_size = (src_mm->size << PAGE_SHIFT) - src_offset;
  

• } else {

• src_mm = NULL;
  

• src_node_size = ULLONG_MAX;
  

• }

  dst_offset = dst->offset;

• dst_mm = amdgpu_find_mm_node(dst->mem, &dst_offset);
• dst_node_size = (dst_mm->size << PAGE_SHIFT) - dst_offset;

• if (dst->mem->mm_node) {

• dst_mm = amdgpu_find_mm_node(dst->mem, &dst_offset);
  

• dst_node_size = (dst_mm->size << PAGE_SHIFT) - dst_offset;
  

• } else {

• dst_mm = NULL;
  

• dst_node_size = ULLONG_MAX;
  

• }

  mutex_lock(&adev->mman.gtt_window_lock);

-- 2.17.1

---

amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.free...