[PATCH] drm/amdgpu: use a 64bit interval tree for VM management

Deucher, Alexander Alexander.Deucher at amd.com
Thu Mar 30 13:49:46 UTC 2017


> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Thursday, March 30, 2017 8:24 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: use a 64bit interval tree for VM management
> 
> From: Christian König <christian.koenig at amd.com>
> 
> Starting with Vega10 we have a 48bit VA, so the pfn won't
> fit into 32bits any more.
> 
> Signed-off-by: Christian König <christian.koenig at amd.com>

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  7 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c    | 12 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c    |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 12 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   |  4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c   |  4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    | 93 ++++++++++++++++---
> ------------
>  7 files changed, 71 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3115cdc..86fba1a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -32,7 +32,7 @@
>  #include <linux/wait.h>
>  #include <linux/list.h>
>  #include <linux/kref.h>
> -#include <linux/interval_tree.h>
> +#include <linux/rbtree.h>
>  #include <linux/hashtable.h>
>  #include <linux/fence.h>
> 
> @@ -381,7 +381,10 @@ struct amdgpu_bo_list_entry {
> 
>  struct amdgpu_bo_va_mapping {
>  	struct list_head		list;
> -	struct interval_tree_node	it;
> +	struct rb_node			rb;
> +	uint64_t			start;
> +	uint64_t			last;
> +	uint64_t			__subtree_last;
>  	uint64_t			offset;
>  	uint64_t			flags;
>  };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index dde1ecf..3389f1b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -949,7 +949,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
> *adev,
>  			}
> 
>  			if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
> -			    (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
> +			    (m->last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>  				DRM_ERROR("IB va_start+ib_bytes is
> invalid\n");
>  				return -EINVAL;
>  			}
> @@ -960,7 +960,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
> *adev,
>  				return r;
>  			}
> 
> -			offset = ((uint64_t)m->it.start) *
> AMDGPU_GPU_PAGE_SIZE;
> +			offset = m->start * AMDGPU_GPU_PAGE_SIZE;
>  			kptr += chunk_ib->va_start - offset;
> 
>  			r =  amdgpu_ib_get(adev, vm, chunk_ib->ib_bytes,
> ib);
> @@ -1388,8 +1388,8 @@ amdgpu_cs_find_mapping(struct
> amdgpu_cs_parser *parser,
>  			continue;
> 
>  		list_for_each_entry(mapping, &lobj->bo_va->valids, list) {
> -			if (mapping->it.start > addr ||
> -			    addr > mapping->it.last)
> +			if (mapping->start > addr ||
> +			    addr > mapping->last)
>  				continue;
> 
>  			*bo = lobj->bo_va->bo;
> @@ -1397,8 +1397,8 @@ amdgpu_cs_find_mapping(struct
> amdgpu_cs_parser *parser,
>  		}
> 
>  		list_for_each_entry(mapping, &lobj->bo_va->invalids, list) {
> -			if (mapping->it.start > addr ||
> -			    addr > mapping->it.last)
> +			if (mapping->start > addr ||
> +			    addr > mapping->last)
>  				continue;
> 
>  			*bo = lobj->bo_va->bo;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> index 7ea3cac..38f739f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> @@ -31,6 +31,7 @@
>  #include <linux/firmware.h>
>  #include <linux/module.h>
>  #include <linux/mmu_notifier.h>
> +#include <linux/interval_tree.h>
>  #include <drm/drmP.h>
>  #include <drm/drm.h>
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> index f38e5e2..381b770 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> @@ -226,8 +226,8 @@ TRACE_EVENT(amdgpu_vm_bo_map,
> 
>  	    TP_fast_assign(
>  			   __entry->bo = bo_va ? bo_va->bo : NULL;
> -			   __entry->start = mapping->it.start;
> -			   __entry->last = mapping->it.last;
> +			   __entry->start = mapping->start;
> +			   __entry->last = mapping->last;
>  			   __entry->offset = mapping->offset;
>  			   __entry->flags = mapping->flags;
>  			   ),
> @@ -250,8 +250,8 @@ TRACE_EVENT(amdgpu_vm_bo_unmap,
> 
>  	    TP_fast_assign(
>  			   __entry->bo = bo_va->bo;
> -			   __entry->start = mapping->it.start;
> -			   __entry->last = mapping->it.last;
> +			   __entry->start = mapping->start;
> +			   __entry->last = mapping->last;
>  			   __entry->offset = mapping->offset;
>  			   __entry->flags = mapping->flags;
>  			   ),
> @@ -270,8 +270,8 @@ DECLARE_EVENT_CLASS(amdgpu_vm_mapping,
>  			     ),
> 
>  	    TP_fast_assign(
> -			   __entry->soffset = mapping->it.start;
> -			   __entry->eoffset = mapping->it.last + 1;
> +			   __entry->soffset = mapping->start;
> +			   __entry->eoffset = mapping->last + 1;
>  			   __entry->flags = mapping->flags;
>  			   ),
>  	    TP_printk("soffs=%010llx, eoffs=%010llx, flags=%08x",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index e1a838e..ff8ae50 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -741,10 +741,10 @@ static int amdgpu_uvd_cs_pass2(struct
> amdgpu_uvd_cs_ctx *ctx)
> 
>  	start = amdgpu_bo_gpu_offset(bo);
> 
> -	end = (mapping->it.last + 1 - mapping->it.start);
> +	end = (mapping->last + 1 - mapping->start);
>  	end = end * AMDGPU_GPU_PAGE_SIZE + start;
> 
> -	addr -= ((uint64_t)mapping->it.start) * AMDGPU_GPU_PAGE_SIZE;
> +	addr -= mapping->start * AMDGPU_GPU_PAGE_SIZE;
>  	start += addr;
> 
>  	amdgpu_set_ib_value(ctx->parser, ctx->ib_idx, ctx->data0,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index eccd70a..e43c83f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -595,13 +595,13 @@ static int amdgpu_vce_cs_reloc(struct
> amdgpu_cs_parser *p, uint32_t ib_idx,
>  	}
> 
>  	if ((addr + (uint64_t)size) >
> -	    ((uint64_t)mapping->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
> +	    (mapping->last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>  		DRM_ERROR("BO to small for addr 0x%010Lx %d %d\n",
>  			  addr, lo, hi);
>  		return -EINVAL;
>  	}
> 
> -	addr -= ((uint64_t)mapping->it.start) * AMDGPU_GPU_PAGE_SIZE;
> +	addr -= mapping->start * AMDGPU_GPU_PAGE_SIZE;
>  	addr += amdgpu_bo_gpu_offset(bo);
>  	addr -= ((uint64_t)size) * ((uint64_t)index);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index c11d15e..43adc4b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -26,6 +26,7 @@
>   *          Jerome Glisse
>   */
>  #include <linux/fence-array.h>
> +#include <linux/interval_tree_generic.h>
>  #include <drm/drmP.h>
>  #include <drm/amdgpu_drm.h>
>  #include "amdgpu.h"
> @@ -51,6 +52,15 @@
>   * SI supports 16.
>   */
> 
> +#define START(node) ((node)->start)
> +#define LAST(node) ((node)->last)
> +
> +INTERVAL_TREE_DEFINE(struct amdgpu_bo_va_mapping, rb, uint64_t,
> __subtree_last,
> +		     START, LAST, static, amdgpu_vm_it)
> +
> +#undef START
> +#undef LAST
> +
>  /* Local structure. Encapsulate some VM table update parameters to reduce
>   * the number of function parameters
>   */
> @@ -1301,7 +1311,7 @@ static int amdgpu_vm_bo_split_mapping(struct
> amdgpu_device *adev,
>  				      struct drm_mm_node *nodes,
>  				      struct fence **fence)
>  {
> -	uint64_t pfn, src = 0, start = mapping->it.start;
> +	uint64_t pfn, src = 0, start = mapping->start;
>  	int r;
> 
>  	/* normally,bo_va->flags only contians READABLE and WIRTEABLE bit
> go here
> @@ -1353,7 +1363,7 @@ static int amdgpu_vm_bo_split_mapping(struct
> amdgpu_device *adev,
>  		}
>  		addr += pfn << PAGE_SHIFT;
> 
> -		last = min((uint64_t)mapping->it.last, start + max_entries -
> 1);
> +		last = min((uint64_t)mapping->last, start + max_entries - 1);
>  		r = amdgpu_vm_bo_update_mapping(adev, exclusive,
>  						src, pages_addr, vm,
>  						start, last, flags, addr,
> @@ -1368,7 +1378,7 @@ static int amdgpu_vm_bo_split_mapping(struct
> amdgpu_device *adev,
>  		}
>  		start = last + 1;
> 
> -	} while (unlikely(start != mapping->it.last + 1));
> +	} while (unlikely(start != mapping->last + 1));
> 
>  	return 0;
>  }
> @@ -1724,9 +1734,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device
> *adev,
>  		     uint64_t saddr, uint64_t offset,
>  		     uint64_t size, uint64_t flags)
>  {
> -	struct amdgpu_bo_va_mapping *mapping;
> +	struct amdgpu_bo_va_mapping *mapping, *tmp;
>  	struct amdgpu_vm *vm = bo_va->vm;
> -	struct interval_tree_node *it;
>  	uint64_t eaddr;
> 
>  	/* validate the parameters */
> @@ -1743,14 +1752,12 @@ int amdgpu_vm_bo_map(struct amdgpu_device
> *adev,
>  	saddr /= AMDGPU_GPU_PAGE_SIZE;
>  	eaddr /= AMDGPU_GPU_PAGE_SIZE;
> 
> -	it = interval_tree_iter_first(&vm->va, saddr, eaddr);
> -	if (it) {
> -		struct amdgpu_bo_va_mapping *tmp;
> -		tmp = container_of(it, struct amdgpu_bo_va_mapping, it);
> +	tmp = amdgpu_vm_it_iter_first(&vm->va, saddr, eaddr);
> +	if (tmp) {
>  		/* bo and tmp overlap, invalid addr */
>  		dev_err(adev->dev, "bo %p va 0x%010Lx-0x%010Lx conflict
> with "
> -			"0x%010lx-0x%010lx\n", bo_va->bo, saddr, eaddr,
> -			tmp->it.start, tmp->it.last + 1);
> +			"0x%010Lx-0x%010Lx\n", bo_va->bo, saddr, eaddr,
> +			tmp->start, tmp->last + 1);
>  		return -EINVAL;
>  	}
> 
> @@ -1759,13 +1766,13 @@ int amdgpu_vm_bo_map(struct amdgpu_device
> *adev,
>  		return -ENOMEM;
> 
>  	INIT_LIST_HEAD(&mapping->list);
> -	mapping->it.start = saddr;
> -	mapping->it.last = eaddr;
> +	mapping->start = saddr;
> +	mapping->last = eaddr;
>  	mapping->offset = offset;
>  	mapping->flags = flags;
> 
>  	list_add(&mapping->list, &bo_va->invalids);
> -	interval_tree_insert(&mapping->it, &vm->va);
> +	amdgpu_vm_it_insert(mapping, &vm->va);
> 
>  	if (flags & AMDGPU_PTE_PRT)
>  		amdgpu_vm_prt_get(adev);
> @@ -1823,13 +1830,13 @@ int amdgpu_vm_bo_replace_map(struct
> amdgpu_device *adev,
>  	saddr /= AMDGPU_GPU_PAGE_SIZE;
>  	eaddr /= AMDGPU_GPU_PAGE_SIZE;
> 
> -	mapping->it.start = saddr;
> -	mapping->it.last = eaddr;
> +	mapping->start = saddr;
> +	mapping->last = eaddr;
>  	mapping->offset = offset;
>  	mapping->flags = flags;
> 
>  	list_add(&mapping->list, &bo_va->invalids);
> -	interval_tree_insert(&mapping->it, &vm->va);
> +	amdgpu_vm_it_insert(mapping, &vm->va);
> 
>  	if (flags & AMDGPU_PTE_PRT)
>  		amdgpu_vm_prt_get(adev);
> @@ -1860,7 +1867,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device
> *adev,
>  	saddr /= AMDGPU_GPU_PAGE_SIZE;
> 
>  	list_for_each_entry(mapping, &bo_va->valids, list) {
> -		if (mapping->it.start == saddr)
> +		if (mapping->start == saddr)
>  			break;
>  	}
> 
> @@ -1868,7 +1875,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device
> *adev,
>  		valid = false;
> 
>  		list_for_each_entry(mapping, &bo_va->invalids, list) {
> -			if (mapping->it.start == saddr)
> +			if (mapping->start == saddr)
>  				break;
>  		}
> 
> @@ -1877,7 +1884,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device
> *adev,
>  	}
> 
>  	list_del(&mapping->list);
> -	interval_tree_remove(&mapping->it, &vm->va);
> +	amdgpu_vm_it_remove(mapping, &vm->va);
>  	trace_amdgpu_vm_bo_unmap(bo_va, mapping);
> 
>  	if (valid)
> @@ -1905,7 +1912,6 @@ int amdgpu_vm_bo_clear_mappings(struct
> amdgpu_device *adev,
>  				uint64_t saddr, uint64_t size)
>  {
>  	struct amdgpu_bo_va_mapping *before, *after, *tmp, *next;
> -	struct interval_tree_node *it;
>  	LIST_HEAD(removed);
>  	uint64_t eaddr;
> 
> @@ -1927,43 +1933,42 @@ int amdgpu_vm_bo_clear_mappings(struct
> amdgpu_device *adev,
>  	INIT_LIST_HEAD(&after->list);
> 
>  	/* Now gather all removed mappings */
> -	it = interval_tree_iter_first(&vm->va, saddr, eaddr);
> -	while (it) {
> -		tmp = container_of(it, struct amdgpu_bo_va_mapping, it);
> -		it = interval_tree_iter_next(it, saddr, eaddr);
> -
> +	tmp = amdgpu_vm_it_iter_first(&vm->va, saddr, eaddr);
> +	while (tmp) {
>  		/* Remember mapping split at the start */
> -		if (tmp->it.start < saddr) {
> -			before->it.start = tmp->it.start;
> -			before->it.last = saddr - 1;
> +		if (tmp->start < saddr) {
> +			before->start = tmp->start;
> +			before->last = saddr - 1;
>  			before->offset = tmp->offset;
>  			before->flags = tmp->flags;
>  			list_add(&before->list, &tmp->list);
>  		}
> 
>  		/* Remember mapping split at the end */
> -		if (tmp->it.last > eaddr) {
> -			after->it.start = eaddr + 1;
> -			after->it.last = tmp->it.last;
> +		if (tmp->last > eaddr) {
> +			after->start = eaddr + 1;
> +			after->last = tmp->last;
>  			after->offset = tmp->offset;
> -			after->offset += after->it.start - tmp->it.start;
> +			after->offset += after->start - tmp->start;
>  			after->flags = tmp->flags;
>  			list_add(&after->list, &tmp->list);
>  		}
> 
>  		list_del(&tmp->list);
>  		list_add(&tmp->list, &removed);
> +
> +		tmp = amdgpu_vm_it_iter_next(tmp, saddr, eaddr);
>  	}
> 
>  	/* And free them up */
>  	list_for_each_entry_safe(tmp, next, &removed, list) {
> -		interval_tree_remove(&tmp->it, &vm->va);
> +		amdgpu_vm_it_remove(tmp, &vm->va);
>  		list_del(&tmp->list);
> 
> -		if (tmp->it.start < saddr)
> -		    tmp->it.start = saddr;
> -		if (tmp->it.last > eaddr)
> -		    tmp->it.last = eaddr;
> +		if (tmp->start < saddr)
> +		    tmp->start = saddr;
> +		if (tmp->last > eaddr)
> +		    tmp->last = eaddr;
> 
>  		list_add(&tmp->list, &vm->freed);
>  		trace_amdgpu_vm_bo_unmap(NULL, tmp);
> @@ -1971,7 +1976,7 @@ int amdgpu_vm_bo_clear_mappings(struct
> amdgpu_device *adev,
> 
>  	/* Insert partial mapping before the range */
>  	if (!list_empty(&before->list)) {
> -		interval_tree_insert(&before->it, &vm->va);
> +		amdgpu_vm_it_insert(before, &vm->va);
>  		if (before->flags & AMDGPU_PTE_PRT)
>  			amdgpu_vm_prt_get(adev);
>  	} else {
> @@ -1980,7 +1985,7 @@ int amdgpu_vm_bo_clear_mappings(struct
> amdgpu_device *adev,
> 
>  	/* Insert partial mapping after the range */
>  	if (!list_empty(&after->list)) {
> -		interval_tree_insert(&after->it, &vm->va);
> +		amdgpu_vm_it_insert(after, &vm->va);
>  		if (after->flags & AMDGPU_PTE_PRT)
>  			amdgpu_vm_prt_get(adev);
>  	} else {
> @@ -2014,13 +2019,13 @@ void amdgpu_vm_bo_rmv(struct
> amdgpu_device *adev,
> 
>  	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
>  		list_del(&mapping->list);
> -		interval_tree_remove(&mapping->it, &vm->va);
> +		amdgpu_vm_it_remove(mapping, &vm->va);
>  		trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>  		list_add(&mapping->list, &vm->freed);
>  	}
>  	list_for_each_entry_safe(mapping, next, &bo_va->invalids, list) {
>  		list_del(&mapping->list);
> -		interval_tree_remove(&mapping->it, &vm->va);
> +		amdgpu_vm_it_remove(mapping, &vm->va);
>  		amdgpu_vm_free_mapping(adev, vm, mapping,
>  				       bo_va->last_pt_update);
>  	}
> @@ -2162,9 +2167,9 @@ void amdgpu_vm_fini(struct amdgpu_device
> *adev, struct amdgpu_vm *vm)
>  	if (!RB_EMPTY_ROOT(&vm->va)) {
>  		dev_err(adev->dev, "still active bo inside vm\n");
>  	}
> -	rbtree_postorder_for_each_entry_safe(mapping, tmp, &vm->va,
> it.rb) {
> +	rbtree_postorder_for_each_entry_safe(mapping, tmp, &vm->va,
> rb) {
>  		list_del(&mapping->list);
> -		interval_tree_remove(&mapping->it, &vm->va);
> +		amdgpu_vm_it_remove(mapping, &vm->va);
>  		kfree(mapping);
>  	}
>  	list_for_each_entry_safe(mapping, tmp, &vm->freed, list) {
> --
> 2.5.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list