Re: [PATCH 3/3] drm/amdgpu: rework amdgpu_cs_find_mapping

[Leo Liu <leo.liu@xxxxxxx>]

On 09/06/2017 11:51 AM, Christian König wrote:
> From: Christian König <christian.koenig@xxxxxxx>
>
> Use the VM instead of the BO list to find the BO for a virtual address.
>
> This fixes UVD/VCE in physical mode with VM local BOs.

The series is
Tested-by: Leo Liu <leo.liu@xxxxxxx>

> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 42 +++++++-----------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 17 ++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     |  2 ++
>   4 files changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index bcb3548..132e17a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1374,46 +1374,24 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser,
>   			   uint64_t addr, struct amdgpu_bo **bo,
>   			   struct amdgpu_bo_va_mapping **map)
>   {
> +	struct amdgpu_fpriv *fpriv = parser->filp->driver_priv;
> +	struct amdgpu_vm *vm = &fpriv->vm;
>   	struct amdgpu_bo_va_mapping *mapping;
> -	unsigned i;
>   	int r;
>   
> -	if (!parser->bo_list)
> -		return 0;
> -
>   	addr /= AMDGPU_GPU_PAGE_SIZE;
>   
> -	for (i = 0; i < parser->bo_list->num_entries; i++) {
> -		struct amdgpu_bo_list_entry *lobj;
> -
> -		lobj = &parser->bo_list->array[i];
> -		if (!lobj->bo_va)
> -			continue;
> -
> -		list_for_each_entry(mapping, &lobj->bo_va->valids, list) {
> -			if (mapping->start > addr ||
> -			    addr > mapping->last)
> -				continue;
> -
> -			*bo = lobj->bo_va->base.bo;
> -			*map = mapping;
> -			goto found;
> -		}
> -
> -		list_for_each_entry(mapping, &lobj->bo_va->invalids, list) {
> -			if (mapping->start > addr ||
> -			    addr > mapping->last)
> -				continue;
> +	mapping = amdgpu_vm_bo_lookup_mapping(vm, addr);
> +	if (!mapping || !mapping->bo_va || !mapping->bo_va->base.bo)
> +		return -EINVAL;
>   
> -			*bo = lobj->bo_va->base.bo;
> -			*map = mapping;
> -			goto found;
> -		}
> -	}
> +	*bo = mapping->bo_va->base.bo;
> +	*map = mapping;
>   
> -	return -EINVAL;
> +	/* Double check that the BO is reserved by this CS */
> +	if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket)
> +		return -EINVAL;
>   
> -found:
>   	r = amdgpu_ttm_bind(&(*bo)->tbo, &(*bo)->tbo.mem);
>   	if (unlikely(r))
>   		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 42492e6..a4891be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -35,6 +35,7 @@
>   
>   /* bo virtual addresses in a vm */
>   struct amdgpu_bo_va_mapping {
> +	struct amdgpu_bo_va		*bo_va;
>   	struct list_head		list;
>   	struct rb_node			rb;
>   	uint64_t			start;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index cfc8de8..ab9dfd7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2083,6 +2083,7 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device *adev,
>   	struct amdgpu_vm *vm = bo_va->base.vm;
>   	struct amdgpu_bo *bo = bo_va->base.bo;
>   
> +	mapping->bo_va = bo_va;
>   	list_add(&mapping->list, &bo_va->invalids);
>   	amdgpu_vm_it_insert(mapping, &vm->va);
>   
> @@ -2260,6 +2261,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>   
>   	list_del(&mapping->list);
>   	amdgpu_vm_it_remove(mapping, &vm->va);
> +	mapping->bo_va = NULL;
>   	trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>   
>   	if (valid)
> @@ -2345,6 +2347,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
>   		if (tmp->last > eaddr)
>   		    tmp->last = eaddr;
>   
> +		tmp->bo_va = NULL;
>   		list_add(&tmp->list, &vm->freed);
>   		trace_amdgpu_vm_bo_unmap(NULL, tmp);
>   	}
> @@ -2371,6 +2374,19 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
>   }
>   
>   /**
> + * amdgpu_vm_bo_lookup_mapping - find mapping by address
> + *
> + * @vm: the requested VM
> + *
> + * Find a mapping by it's address.
> + */
> +struct amdgpu_bo_va_mapping *amdgpu_vm_bo_lookup_mapping(struct amdgpu_vm *vm,
> +							 uint64_t addr)
> +{
> +	return amdgpu_vm_it_iter_first(&vm->va, addr, addr);
> +}
> +
> +/**
>    * amdgpu_vm_bo_rmv - remove a bo to a specific vm
>    *
>    * @adev: amdgpu_device pointer
> @@ -2395,6 +2411,7 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>   	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
>   		list_del(&mapping->list);
>   		amdgpu_vm_it_remove(mapping, &vm->va);
> +		mapping->bo_va = NULL;
>   		trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>   		list_add(&mapping->list, &vm->freed);
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 90b7741..c1accd1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -276,6 +276,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>   int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
>   				struct amdgpu_vm *vm,
>   				uint64_t saddr, uint64_t size);
> +struct amdgpu_bo_va_mapping *amdgpu_vm_bo_lookup_mapping(struct amdgpu_vm *vm,
> +							 uint64_t addr);
>   void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>   		      struct amdgpu_bo_va *bo_va);
>   void amdgpu_vm_set_fragment_size(struct amdgpu_device *adev,

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel