| Message ID | 20200706174811.14755-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/ttm: further cleanup ttm_mem_reg handling | expand |
[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"?? 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
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.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cmadhav.chauhan%40amd.com%7C9c9a919e6d10450ac9df08d821d4c5d1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637296545052666024&sdata=DSjWjB76AeiABFB06VesHBPUp6KhaDvaQOyR%2FgZSr98%3D&reserved=0
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); } /** 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);
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(-)