On 06/07/ , Felix Kuehling wrote:
> Am 2022-06-07 um 05:59 schrieb Lang Yu:
> > This will remove some redundant codes.
> >
> > Signed-off-by: Lang Yu <Lang.Yu@xxxxxxx>
>
> The redundancy is quite small, and amdgpu_amdkfd_gpuvm_validate_pt_pd_bos
> and amdgpu_amdkfd_bo_validate are quite a bit more complex and handle more
> different cases. Someone changing those functions in the future may not
> realize the effect that may have on the SVM code.
>
> I'd prefer to keep the svm_range_bo_validate function in kfd_svm.c to make
> the code easier to understand and maintain. If anything, I'd move it closer
> to where its used, because it's only used in one place.
Thanks for your comments. I got it. By the way,
is it necessary to update vm->pd_phys_addr here?
I noticed that vm->pd_phys_addr is updated in
vm_validate_pt_pd_bos()? Thanks!
Regards,
Lang
> Regards,
> Felix
>
>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 13 +------------
> > 1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> > index d6fc00d51c8c..03e07d1d1d1a 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> > @@ -625,15 +625,6 @@ svm_range_get_pdd_by_adev(struct svm_range *prange, struct amdgpu_device *adev)
> > return kfd_process_device_from_gpuidx(p, gpu_idx);
> > }
> > -static int svm_range_bo_validate(void *param, struct amdgpu_bo *bo)
> > -{
> > - struct ttm_operation_ctx ctx = { false, false };
> > -
> > - amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_VRAM);
> > -
> > - return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> > -}
> > -
> > static int
> > svm_range_check_attr(struct kfd_process *p,
> > uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs)
> > @@ -1428,9 +1419,7 @@ static int svm_range_reserve_bos(struct svm_validate_context *ctx)
> > goto unreserve_out;
> > }
> > - r = amdgpu_vm_validate_pt_bos(pdd->dev->adev,
> > - drm_priv_to_vm(pdd->drm_priv),
> > - svm_range_bo_validate, NULL);
> > + r = amdgpu_amdkfd_gpuvm_validate_pt_pd_bos(drm_priv_to_vm(pdd->drm_priv));
> > if (r) {
> > pr_debug("failed %d validate pt bos\n", r);
> > goto unreserve_out;
