[PATCH 2/2] drm/amdgpu: stop allocating dummy GTT nodes

Christian König ckoenig.leichtzumerken at gmail.com
Wed Jul 8 09:47:29 UTC 2020


Am 08.07.20 um 09:35 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Christian König
> Sent: Monday, July 6, 2020 11:18 PM
> To: amd-gfx at lists.freedesktop.org; dri-devel at 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 at 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 at 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



More information about the amd-gfx mailing list