Re: [PATCH 1/2] drm/ttm: further cleanup ttm_mem_reg handling

Am 07.07.20 um 21:16 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 1/2] drm/ttm: further cleanup ttm_mem_reg handling

Stop touching the backend private pointer alltogether and make sure we never put the same mem twice by.

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

drivers/gpu/drm/ttm/ttm_bo.c | 46 +++++++++++++++++++-------------- include/drm/ttm/ttm_bo_driver.h | 2 -- 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 0c13fe96c7e3..7be36b9996ed 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -312,7 +312,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (bdev->driver->move_notify) bdev->driver->move_notify(bo, evict, mem); bo->mem = *mem;

• 	mem->mm_node = NULL;
  goto moved;
  

  } }

@@ -616,7 +615,6 @@ static void ttm_bo_release(struct kref *kref) ttm_bo_cleanup_memtype_use(bo); dma_resv_unlock(bo->base.resv);

• BUG_ON(bo->mem.mm_node != NULL); atomic_dec(&ttm_bo_glob.bo_count); dma_fence_put(bo->moving); if (!ttm_bo_uses_embedded_gem_object(bo))

@@ -843,12 +841,29 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, return ret; }

+static int ttm_bo_mem_get(struct ttm_buffer_object *bo,

• 	  const struct ttm_place *place,
  

• 	  struct ttm_mem_reg *mem)
  

+{

• struct ttm_mem_type_manager *man = &bo->bdev->man[mem->mem_type];
• mem->mm_node = NULL;
• if (!man->func || !man->func->get_node)

• return 0;
  

• return man->func->get_node(man, bo, place, mem); }

Should not we export this as a symbol, so that it can be used similar to ttm_bo_mem_put in drm drivers??

No, it's a well known design problem of TTM that the exported counter part of ttm_bo_mem_put() is ttm_bo_mem_space().

I have it on my TODO list to fix this, but that needs a bigger change.

Regards, Christian.

Regards, Madhav

void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem) { struct ttm_mem_type_manager *man = &bo->bdev->man[mem->mem_type];

• if (mem->mm_node)

• (*man->func->put_node)(man, mem);
  

• if (!man->func || !man->func->put_node)

• return;
  

• man->func->put_node(man, mem);
• mem->mm_node = NULL;
• mem->mem_type = TTM_PL_SYSTEM; } EXPORT_SYMBOL(ttm_bo_mem_put);

@@ -902,7 +917,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,

ticket = dma_resv_locking_ctx(bo->base.resv); do {

• ret = (*man->func->get_node)(man, bo, place, mem);
  

• ret = ttm_bo_mem_get(bo, place, mem);
  

  if (likely(!ret)) break; if (unlikely(ret != -ENOSPC))

@@ -1032,7 +1047,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, if (unlikely(ret)) return ret;

• mem->mm_node = NULL; for (i = 0; i < placement->num_placement; ++i) { const struct ttm_place *place = &placement->placement[i]; struct ttm_mem_type_manager *man;

@@ -1044,20 +1058,16 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, goto error;

type_found = true;

• mem->mm_node = NULL;
  

• if (mem->mem_type == TTM_PL_SYSTEM)
  

• 	return 0;
  

• man = &bdev->man[mem->mem_type];
  

• ret = (*man->func->get_node)(man, bo, place, mem);
  

• ret = ttm_bo_mem_get(bo, place, mem);
  

  if (ret == -ENOSPC) continue; if (unlikely(ret)) goto error;

• man = &bdev->man[mem->mem_type];
  

  ret = ttm_bo_add_move_fence(bo, man, mem, ctx->no_wait_gpu); if (unlikely(ret)) {

• 	(*man->func->put_node)(man, mem);
  

• 	ttm_bo_mem_put(bo, mem);
  if (ret == -EBUSY)
  	continue;
  
  

@@ -1076,12 +1086,8 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, goto error;

type_found = true;

• mem->mm_node = NULL;
  

• if (mem->mem_type == TTM_PL_SYSTEM)
  

• 	return 0;
  

• ret = ttm_bo_mem_force_space(bo, place, mem, ctx);

• if (ret == 0 && mem->mm_node)
  

• if (likely(!ret))
  return 0;
  
  

  if (ret && ret != -EBUSY)

@@ -1129,7 +1135,7 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo, goto out_unlock; ret = ttm_bo_handle_move_mem(bo, &mem, false, ctx); out_unlock:

• if (ret && mem.mm_node)

• if (ret) ttm_bo_mem_put(bo, &mem); return ret; }

@@ -1144,7 +1150,7 @@ static bool ttm_bo_places_compat(const struct ttm_place *places, for (i = 0; i < num_placement; i++) { const struct ttm_place *heap = &places[i];

• if (mem->mm_node && (mem->start < heap->fpfn ||
  

• if ((mem->start < heap->fpfn ||
  (heap->lpfn != 0 && (mem->start + mem->num_pages) > heap->lpfn)))
  continue;
  
  

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index aa1f398c2ea7..732167cad130 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -564,8 +564,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx);

void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem); -void ttm_bo_mem_put_locked(struct ttm_buffer_object *bo,

• 	   struct ttm_mem_reg *mem);
  
  

  int ttm_bo_device_release(struct ttm_bo_device *bdev);

-- 2.17.1

---

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