[AMD Official Use Only - General] Ping! Hi Felix, what do you think? Thanks! Regards, Lang >-----Original Message----- >From: Koenig, Christian <Christian.Koenig@xxxxxxx> >Sent: Tuesday, June 14, 2022 5:08 PM >To: Yu, Lang <Lang.Yu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Deucher, Alexander ><Alexander.Deucher@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx> >Subject: Re: [PATCH v3] drm/amdkfd: simplify vm_validate_pt_pd_bos > >Am 14.06.22 um 11:03 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. >> >> The special case is when turning a GFX VM to a compute VM, if >> vm_update_mode changed, we should make sure root PD gets mapped. So >> just map root PD after updating vm->update_funcs in >> amdgpu_vm_make_compute whether the vm_update_mode changed or >not. >> >> v3: >> - Add some comments suggested by Christian. >> >> v2: >> - Don't rename vm_validate_pt_pd_bos and make it public. >> >> Signed-off-by: Lang Yu <Lang.Yu@xxxxxxx> > >I can't judge the kfd part, but the VM stuff looks good to me now. > >Acked-by: Christian König <christian.koenig@xxxxxxx> > >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 14 ------------- >- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++++ >> 2 files changed, 8 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 50bb590c3306..c9ef242177e2 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >> @@ -439,22 +439,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..3a6b827e540c 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -2225,6 +2225,14 @@ int amdgpu_vm_make_compute(struct >amdgpu_device *adev, struct amdgpu_vm *vm) >> } else { >> vm->update_funcs = &amdgpu_vm_sdma_funcs; >> } >> + /* >> + * Make sure root PD gets mapped. As vm_update_mode could be >changed >> + * when turning a GFX VM into a compute VM. >> + */ >> + 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;