All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chauhan, Madhav" <Madhav.Chauhan@amd.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: RE: [PATCH 1/2] drm/ttm: further cleanup ttm_mem_reg handling
Date: Tue, 7 Jul 2020 19:16:25 +0000	[thread overview]
Message-ID: <BL0PR12MB2433182BDAF36FEC1A0551AF9C660@BL0PR12MB2433.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20200706174811.14755-1-christian.koenig@amd.com>

[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??

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.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cmadhav.chauhan%40amd.com%7C886113da714f433267d308d821d4c4fe%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637296545014835325&amp;sdata=bemDP4gp7%2BcWeCqxy99r71eUndax2ruO7FeCv30g2PA%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Chauhan, Madhav" <Madhav.Chauhan@amd.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: RE: [PATCH 1/2] drm/ttm: further cleanup ttm_mem_reg handling
Date: Tue, 7 Jul 2020 19:16:25 +0000	[thread overview]
Message-ID: <BL0PR12MB2433182BDAF36FEC1A0551AF9C660@BL0PR12MB2433.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20200706174811.14755-1-christian.koenig@amd.com>

[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??

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.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cmadhav.chauhan%40amd.com%7C886113da714f433267d308d821d4c4fe%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637296545014835325&amp;sdata=bemDP4gp7%2BcWeCqxy99r71eUndax2ruO7FeCv30g2PA%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  parent reply	other threads:[~2020-07-07 19:16 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 17:48 [PATCH 1/2] drm/ttm: further cleanup ttm_mem_reg handling Christian König
2020-07-06 17:48 ` Christian König
2020-07-06 17:48 ` [PATCH 2/2] drm/amdgpu: stop allocating dummy GTT nodes Christian König
2020-07-06 17:48   ` Christian König
2020-07-08  7:35   ` Chauhan, Madhav
2020-07-08  7:35     ` Chauhan, Madhav
2020-07-08  9:47     ` Christian König
2020-07-08  9:47       ` Christian König
2020-07-07 19:16 ` Chauhan, Madhav [this message]
2020-07-07 19:16   ` [PATCH 1/2] drm/ttm: further cleanup ttm_mem_reg handling Chauhan, Madhav
2020-07-08  9:43   ` Christian König
2020-07-08  9:43     ` Christian König
2020-07-09 15:09 Christian König
2020-07-09 15:09 ` Christian König
2020-07-09 15:29 ` Chauhan, Madhav
2020-07-09 15:29   ` Chauhan, Madhav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL0PR12MB2433182BDAF36FEC1A0551AF9C660@BL0PR12MB2433.namprd12.prod.outlook.com \
    --to=madhav.chauhan@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.