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