Am 04.03.2018 um 03:34 schrieb Felix Kuehling:
> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 82 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
> 2 files changed, 83 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5afbc5e..58153ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2350,6 +2350,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> INIT_KFIFO(vm->faults);
> vm->fault_credit = 16;
>
> + vm->vm_context = vm_context;
I think we should drop the vm_context parameter and all the related code
in amdgpu_vm_init(). But that can be part of a later cleanup patch as well.
> +
> return 0;
>
> error_free_root:
> @@ -2364,6 +2366,86 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> }
>
> /**
> + * amdgpu_vm_make_compute - Turn a GFX VM into a compute VM
> + *
> + * This only works on GFX VMs that don't have any BOs added and no
> + * page tables allocated yet.
> + *
> + * Changes the following VM parameters:
> + * - vm_context
> + * - use_cpu_for_update
> + * - pte_supports_ats
> + * - pasid (old PASID is released, because compute manages its own PASIDs)
> + *
> + * Reinitializes the page directory to reflect the changed ATS
> + * setting. May leave behind an unused shadow BO for the page
> + * directory when switching from SDMA updates to CPU updates.
> + *
> + * Returns 0 for success, -errno for errors.
> + */
> +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> +{
> + bool pte_support_ats = (adev->asic_type == CHIP_RAVEN);
> + int r;
> +
> + r = amdgpu_bo_reserve(vm->root.base.bo, true);
> + if (r)
> + return r;
> +
> + /* Sanity checks */
> + if (vm->vm_context == AMDGPU_VM_CONTEXT_COMPUTE) {
> + /* Can happen if ioctl is interrupted by a signal after
> + * this function already completed. Just return success.
> + */
> + r = 0;
> + goto error;
> + }
Ok, that is actually a show stopper. An interrupted IOCTL should never
have a visible effect.
Is that just a theoretical issue or did you run into this in practice?
Regards,
Christian.
> + if (!RB_EMPTY_ROOT(&vm->va.rb_root) || vm->root.entries) {
> + r = -EINVAL;
> + goto error;
> + }
> +
> + /* Check if PD needs to be reinitialized and do it before
> + * changing any other state, in case it fails.
> + */
> + if (pte_support_ats != vm->pte_support_ats) {
> + /* TODO: This depends on a patch series by Christian.
> + * It's only needed for GFX9 GPUs, which aren't
> + * supported by upstream KFD yet.
> + r = amdgpu_vm_clear_bo(adev, vm, vm->root.base.bo,
> + adev->vm_manager.root_level,
> + pte_support_ats);
> + if (r)
> + goto error;
> + */
> + }
> +
> + /* Update VM state */
> + vm->vm_context = AMDGPU_VM_CONTEXT_COMPUTE;
> + vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
> + AMDGPU_VM_USE_CPU_FOR_COMPUTE);
> + vm->pte_support_ats = pte_support_ats;
> + DRM_DEBUG_DRIVER("VM update mode is %s\n",
> + vm->use_cpu_for_update ? "CPU" : "SDMA");
> + WARN_ONCE((vm->use_cpu_for_update & !amdgpu_vm_is_large_bar(adev)),
> + "CPU update of VM recommended only for large BAR system\n");
> +
> + if (vm->pasid) {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
> + idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
> + spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
> +
> + vm->pasid = 0;
> + }
> +
> +error:
> + amdgpu_bo_unreserve(vm->root.base.bo);
> + return r;
> +}
> +
> +/**
> * amdgpu_vm_free_levels - free PD/PT levels
> *
> * @adev: amdgpu device structure
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 40b4e09..7f50a38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -263,6 +263,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev);
> void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
> int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> int vm_context, unsigned int pasid);
> +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev,
> unsigned int pasid);
