[PATCH] drm/amdgpu: fix amdgpu_vm_handle_moved as well v2

[zhoucm1]

Reviewed-by: Chunming Zhou <david1.zhou at amd.com>, but one commit inline...


On 2017年09月11日 22:55, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> There is no guarantee that the last BO_VA actually needed an update.
>
> Additional to that all command submissions must wait for moved BOs to
> be cleared, not just the first one.
>
> v2: Don't overwrite any newer fence.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 24 ++++++++++--------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  3 +--
>   3 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 4681dcc..b59749d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -805,7 +805,7 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>   
>   	}
>   
> -	r = amdgpu_vm_handle_moved(adev, vm, &p->job->sync);
> +	r = amdgpu_vm_handle_moved(adev, vm);
>   	if (r)
>   		return r;
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5042f09..6b95ba6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1742,7 +1742,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   	dma_addr_t *pages_addr = NULL;
>   	struct ttm_mem_reg *mem;
>   	struct drm_mm_node *nodes;
> -	struct dma_fence *exclusive;
> +	struct dma_fence *exclusive, **last_update;
>   	uint64_t flags;
>   	int r;
>   
> @@ -1768,6 +1768,11 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   	else
>   		flags = 0x0;
>   
> +	if (clear || (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv))
> +		last_update = &vm->last_update;
> +	else
> +		last_update = &bo_va->last_pt_update;

I still prefer to move this out of bo_update and place to where calling 
amdgpu_vm_bo_update, this way, we will know we're syncing which fence of 
which path explicitly.

> +
>   	if (!clear && bo_va->base.moved) {
>   		bo_va->base.moved = false;
>   		list_splice_init(&bo_va->valids, &bo_va->invalids);
> @@ -1779,7 +1784,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   	list_for_each_entry(mapping, &bo_va->invalids, list) {
>   		r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, vm,
>   					       mapping, flags, nodes,
> -					       &bo_va->last_pt_update);
> +					       last_update);
>   		if (r)
>   			return r;
>   	}
> @@ -1802,12 +1807,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			trace_amdgpu_vm_bo_mapping(mapping);
>   	}
>   
> -	if (bo_va->base.bo &&
> -	    bo_va->base.bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> -		dma_fence_put(vm->last_update);
> -		vm->last_update = dma_fence_get(bo_va->last_pt_update);
> -	}
> -
>   	return 0;
>   }
>   
> @@ -2005,15 +2004,15 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>    * PTs have to be reserved!
>    */
>   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> -			   struct amdgpu_vm *vm,
> -			   struct amdgpu_sync *sync)
> +			   struct amdgpu_vm *vm)
>   {
> -	struct amdgpu_bo_va *bo_va = NULL;
>   	bool clear;
>   	int r = 0;
>   
>   	spin_lock(&vm->status_lock);
>   	while (!list_empty(&vm->moved)) {
> +		struct amdgpu_bo_va *bo_va;
> +
>   		bo_va = list_first_entry(&vm->moved,
>   			struct amdgpu_bo_va, base.vm_status);
>   		spin_unlock(&vm->status_lock);
> @@ -2029,9 +2028,6 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>   	}
>   	spin_unlock(&vm->status_lock);
>   
> -	if (bo_va)
> -		r = amdgpu_sync_fence(adev, sync, bo_va->last_pt_update);
> -
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index cb6a622..48c58ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -250,8 +250,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>   			  struct amdgpu_vm *vm,
>   			  struct dma_fence **fence);
>   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> -			   struct amdgpu_vm *vm,
> -			   struct amdgpu_sync *sync);
> +			   struct amdgpu_vm *vm);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
>   			bool clear);