Am 13.06.22 um 10:41 schrieb Lang Yu:
On 06/13/ , Christian König wrote:
Am 13.06.22 um 10:26 schrieb Lang Yu:
On 06/13/ , Christian König wrote:
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.
From my observations, if root PD is evicted. amdgpu_vm_validate_pt_bos
will validate and map it.
And amdgpu_cs_list_validate always validates root PD after
amdgpu_vm_validate_pt_bos has done that, it is actually unnecessary.
Do you think it's worth skiping root PD validation in
amdgpu_cs_list_validate? Thanks!
No, it's just your change is completely irrelevant and just complicates
things.
Validating a BO twice should have basically no overhead at all.
And especially adding the manual call to map_table() in
amdgpu_vm_make_compute() is a no-go. We don't want such specific handling
for compute contexts.
That's because when turning a GFX VM to a compute VM, if vm_update_mode changed,
we need to map the root PD again.
Ah, ok that sounds valid. But please add a comment explaining this.
If we always validate and map it in vm_validate_pt_pd_bos, that's so confused.
It's indeed not really clean, we shouldn't leak requirements like this
outside of the VM code.
Regards,
Christian.
Regards,
Lang
Regards,
Christian.
Regards,
Lang
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;
