[PATCH 10/12] drm/amdgpu: immedially invalidate PTEs

[Felix Kuehling]

On 2020-01-06 10:03 a.m., Christian König wrote:
> When a BO is evicted immedially invalidate the mapped PTEs.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a03cfbe670c4..6844ba7467a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2569,6 +2569,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   			     struct amdgpu_bo *bo, bool evicted)
>   {
>   	struct amdgpu_vm_bo_base *bo_base;
> +	int r;
>   
>   	/* shadow bo doesn't have bo base, its validation needs its parent */
>   	if (bo->parent && bo->parent->shadow == bo)
> @@ -2576,8 +2577,22 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   
>   	for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
>   		struct amdgpu_vm *vm = bo_base->vm;
> +		struct dma_resv *resv = vm->root.base.bo->tbo.base.resv;
> +
> +		if (bo->tbo.type != ttm_bo_type_kernel) {
> +			struct amdgpu_bo_va *bo_va;
> +
> +			bo_va = container_of(bo_base, struct amdgpu_bo_va,
> +					     base);
> +			r = amdgpu_vm_bo_update(adev, bo_va,
> +						bo->tbo.base.resv != resv);

Will this update PTEs for per-VM BOs without validating them first?


> +			if (!r) {
> +				amdgpu_vm_bo_idle(bo_base);

Is this the right state? The description of amdgpu_vm_bo_idle says that 
this is for "PDs/PTs and per VM BOs". For regular BOs, I think this 
should call amdgpu_vm_bo_done.


> +				continue;

This skips a bunch of state machine logic below. Maybe some of that 
could be cleaned up.


> +			}
> +		}
>   
> -		if (evicted && bo->tbo.base.resv == vm->root.base.bo->tbo.base.resv) {
> +		if (evicted && bo->tbo.base.resv == resv) {
>   			amdgpu_vm_bo_evicted(bo_base);

It will never get here for per-VM BOs now (except if bo_update failed). 
Not sure if that's a problem.


>   			continue;
>   		}
>
>                  if (bo_base->moved)
>                          continue;
>                  bo_base->moved = true;

Maybe the whole PT invalidation should be after this to avoid multiple 
invalidations off the same BO?


>
>                  if (bo->tbo.type == ttm_bo_type_kernel)
>                          amdgpu_vm_bo_relocated(bo_base);
>                  else if (bo->tbo.resv == vm->root.base.bo->tbo.resv)
>                          amdgpu_vm_bo_moved(bo_base);
>                  else
>                          amdgpu_vm_bo_invalidated(bo_base);

I believe the last two cases are unreachable now (except if bo_update 
failed).

Regards,
   Felix