[PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed

[Christian König]

Am 23.03.2017 um 20:27 schrieb Nicolai Hähnle:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> We will add the fence to freed buffer objects in a later commit, to ensure
> that the underlying memory can only be re-used after all references in
> page tables have been cleared.
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Reviewed-by: Christian König <christian.koenig at amd.com> for both.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
>   4 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 55d553a..85e6070 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>   	int i, r;
>   
>   	r = amdgpu_vm_update_page_directory(adev, vm);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_sync_fence(adev, &p->job->sync, vm->page_directory_fence);
>   	if (r)
>   		return r;
>   
> -	r = amdgpu_vm_clear_freed(adev, vm);
> +	r = amdgpu_vm_clear_freed(adev, vm, NULL);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
>   	if (r)
>   		return r;
>   
>   	r = amdgpu_sync_fence(adev, &p->job->sync,
>   			      fpriv->prt_va->last_pt_update);
>   	if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index be9fb2c..4a53c43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
>   
>   	r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,
>   				      NULL);
>   	if (r)
>   		goto error;
>   
>   	r = amdgpu_vm_update_page_directory(adev, vm);
>   	if (r)
>   		goto error;
>   
> -	r = amdgpu_vm_clear_freed(adev, vm);
> +	r = amdgpu_vm_clear_freed(adev, vm, NULL);
>   	if (r)
>   		goto error;
>   
>   	if (operation == AMDGPU_VA_OP_MAP ||
>   	    operation == AMDGPU_VA_OP_REPLACE)
>   		r = amdgpu_vm_bo_update(adev, bo_va, false);
>   
>   error:
>   	if (r && r != -ERESTARTSYS)
>   		DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index dd7df45..2c95a75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	}
>   
>   	kfree(shared);
>   }
>   
>   /**
>    * amdgpu_vm_clear_freed - clear freed BOs in the PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
> + * @fence: optional resulting fence (unchanged if no work needed to be done
> + * or if an error occurred)
>    *
>    * Make sure all freed BOs are cleared in the PT.
>    * Returns 0 for success.
>    *
>    * PTs have to be reserved and mutex must be locked!
>    */
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm)
> +			  struct amdgpu_vm *vm,
> +			  struct fence **fence)
>   {
>   	struct amdgpu_bo_va_mapping *mapping;
> -	struct fence *fence = NULL;
> +	struct fence *f = NULL;
>   	int r;
>   
>   	while (!list_empty(&vm->freed)) {
>   		mapping = list_first_entry(&vm->freed,
>   			struct amdgpu_bo_va_mapping, list);
>   		list_del(&mapping->list);
>   
>   		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
> -					       0, 0, &fence);
> -		amdgpu_vm_free_mapping(adev, vm, mapping, fence);
> +					       0, 0, &f);
> +		amdgpu_vm_free_mapping(adev, vm, mapping, f);
>   		if (r) {
> -			fence_put(fence);
> +			fence_put(f);
>   			return r;
>   		}
> +	}
>   
> +	if (fence && f) {
> +		fence_put(*fence);
> +		*fence = f;
> +	} else {
> +		fence_put(f);
>   	}
> -	fence_put(fence);
> +
>   	return 0;
>   
>   }
>   
>   /**
>    * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requested vm
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index ff10fa5..9d5a572 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			struct amdgpu_vm *vm,
>   			uint64_t saddr, uint64_t size);
>   int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
>   		      struct amdgpu_sync *sync, struct fence *fence,
>   		      struct amdgpu_job *job);
>   int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
>   void amdgpu_vm_reset_id(struct amdgpu_device *adev, unsigned vm_id);
>   int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
>   				    struct amdgpu_vm *vm);
>   int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm);
> +			  struct amdgpu_vm *vm,
> +			  struct fence **fence);
>   int amdgpu_vm_clear_invalids(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			     struct amdgpu_sync *sync);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
>   			bool clear);
>   void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   			     struct amdgpu_bo *bo);
>   struct amdgpu_bo_va *amdgpu_vm_bo_find(struct amdgpu_vm *vm,
>   				       struct amdgpu_bo *bo);
>   struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,