Re: [PATCH v2] drm/amdkfd: simplify vm_validate_pt_pd_bos

[Christian König <christian.koenig@xxxxxxx>]

Am 13.06.22 um 09:59 schrieb Lang Yu:
> We don't need to validate and map root PD specially here,
> it would be validated and mapped by amdgpu_vm_validate_pt_bos
> if it is evicted.

I'm not sure if that's correct. Traditionally we have handled the root 
PD differently to the rest in the VM.

It doesn't make much sense any more today, but I need to double check if 
that isn't still the case.

Christian.

> The special case is when turning a GFX VM to a compute VM,
> if vm_update_mode changed, we need to map the root PD again.
> So just move root PD mapping to amdgpu_vm_make_compute.
>
> v2:
>   - Don't rename vm_validate_pt_pd_bos and make it public.
>
> Signed-off-by: Lang Yu <Lang.Yu@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 14 --------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c           |  5 +++++
>   2 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 6a3bd8b9a08f..3805eef9ab69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -425,22 +425,8 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
>   		return ret;
>   	}
>   
> -	ret = amdgpu_amdkfd_validate_vm_bo(NULL, pd);
> -	if (ret) {
> -		pr_err("failed to validate PD\n");
> -		return ret;
> -	}
> -
>   	vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.bo);
>   
> -	if (vm->use_cpu_for_update) {
> -		ret = amdgpu_bo_kmap(pd, NULL);
> -		if (ret) {
> -			pr_err("failed to kmap PD, ret=%d\n", ret);
> -			return ret;
> -		}
> -	}
> -
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 703552f9a6d7..08fda57f5aa2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2225,6 +2225,11 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	} else {
>   		vm->update_funcs = &amdgpu_vm_sdma_funcs;
>   	}
> +
> +	r = vm->update_funcs->map_table(to_amdgpu_bo_vm(vm->root.bo));
> +	if (r)
> +		goto unreserve_bo;
> +
>   	dma_fence_put(vm->last_update);
>   	vm->last_update = NULL;
>   	vm->is_compute_context = true;