Am 08.07.20 um 09:35 schrieb Chauhan, Madhav:
[AMD Public Use]
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Christian König
Sent: Monday, July 6, 2020 11:18 PM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
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@xxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmadhav.chauhan%40amd.com%7C9c9a919e6d10450ac9df08d821d4c5d1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637296545052666024&sdata=DSjWjB76AeiABFB06VesHBPUp6KhaDvaQOyR%2FgZSr98%3D&reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel