Am 12.05.2017 um 04:39 schrieb Harish Kasiviswanathan:
> Add VM context module param (amdgpu.vm_update_context) that can used to
> control how the VM pde/pte are updated for Graphics and Compute.
>
> BIT0 controls Graphics and BIT1 Compute.
> BIT0 [= 0] Graphics updated by SDMA [= 1] by CPU
> BIT1 [= 0] Compute updated by SDMA [= 1] by CPU
>
> By default, only for large BAR system vm_update_context = 2, indicating
> that Graphics VMs will be updated via SDMA and Compute VMs will be
> updated via CPU. And for all all other systems (by default)
> vm_update_context = 0
>
> Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 29 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 16 +++++++++++++++-
> 5 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index fadeb55..d927153 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -94,6 +94,7 @@
> extern int amdgpu_vm_block_size;
> extern int amdgpu_vm_fault_stop;
> extern int amdgpu_vm_debug;
> +extern int amdgpu_vm_update_context;
> extern int amdgpu_dc;
> extern int amdgpu_sched_jobs;
> extern int amdgpu_sched_hw_submission;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 26fce4d..f07bcae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -93,6 +93,7 @@
> int amdgpu_vm_block_size = -1;
> int amdgpu_vm_fault_stop = 0;
> int amdgpu_vm_debug = 0;
> +int amdgpu_vm_update_context = -1;
> int amdgpu_vram_page_split = 1024;
> int amdgpu_exp_hw_support = 0;
> int amdgpu_dc = -1;
> @@ -180,6 +181,9 @@
> MODULE_PARM_DESC(vm_debug, "Debug VM handling (0 = disabled (default), 1 = enabled)");
> module_param_named(vm_debug, amdgpu_vm_debug, int, 0644);
>
> +MODULE_PARM_DESC(vm_update_context, "VM update using CPU (0 = never (default except for large BAR(LB)), 1 = Graphics only, 2 = Compute only (default for LB), 3 = Both");
> +module_param_named(vm_update_context, amdgpu_vm_update_context, int, 0444);
> +
> MODULE_PARM_DESC(vram_page_split, "Number of pages after we split VRAM allocations (default 1024, -1 = disable)");
> module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index d167949..c524053 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -774,7 +774,9 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
> goto out_suspend;
> }
>
> - r = amdgpu_vm_init(adev, &fpriv->vm);
> + r = amdgpu_vm_init(adev, &fpriv->vm,
> + !!(amdgpu_vm_update_context &
> + AMDGPU_VM_USE_CPU_FOR_GFX));
> if (r) {
> kfree(fpriv);
> goto out_suspend;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5d92e73..ff6cf33 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -721,6 +721,11 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
> return true;
> }
>
> +static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
> +{
> + return (adev->mc.real_vram_size == adev->mc.visible_vram_size);
> +}
> +
> /**
> * amdgpu_vm_flush - hardware flush the vm
> *
> @@ -2292,10 +2297,12 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size)
> *
> * @adev: amdgpu_device pointer
> * @vm: requested vm
> + * @vm_update_mode: FALSE use SDMA, TRUE use CPU to update page tables
> *
> * Init @vm fields.
> */
> -int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> +int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> + bool vm_update_mode)
> {
> const unsigned align = min(AMDGPU_VM_PTB_ALIGN_SIZE,
> AMDGPU_VM_PTE_COUNT(adev) * 8);
> @@ -2324,6 +2331,10 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> if (r)
> return r;
>
> + vm->is_vm_update_mode_cpu = vm_update_mode;
> + DRM_DEBUG_DRIVER("VM update mode is %s\n",
> + vm->is_vm_update_mode_cpu ? "CPU" : "SDMA");
> +
> vm->last_dir_update = NULL;
>
> r = amdgpu_bo_create(adev, amdgpu_vm_bo_size(adev, 0), align, true,
> @@ -2455,6 +2466,22 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
> atomic64_set(&adev->vm_manager.client_counter, 0);
> spin_lock_init(&adev->vm_manager.prt_lock);
> atomic_set(&adev->vm_manager.num_prt_users, 0);
> +
> + /* If not overridden by the user, by default, only in large BAR systems
> + * Compute VM tables will be updated by CPU
> + */
> +#ifdef CONFIG_X86_64
> + if (amdgpu_vm_update_context == -1) {
> + if (amdgpu_vm_is_large_bar(adev))
> + amdgpu_vm_update_context =
> + AMDGPU_VM_USE_CPU_FOR_COMPUTE;
> + else
> + amdgpu_vm_update_context = 0;
> + }
> +#else
> + amdgpu_vm_update_context = 0;
> +#endif
> +
amdgpu_vm_update_context is a module parameter, updating it here
depending on device specific infos is not allowed.
Please add a field to adev->vm_manager and initialize it here depending
on the module parameter.
Additionally to that I would call this vm_update_mode and not
vm_update_context.
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 27546df..564f7a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -96,6 +96,16 @@ struct amdgpu_vm_pt {
> unsigned last_entry_used;
> };
>
> +/* amdgpu_vm_update_context - User parameter that controls how VM pde/pte are
> + * updated for Graphics and Compute. BIT0 controls Graphics and BIT1 controls
> + * Compute.
> + * BIT0 [= 0] Graphics updated by SDMA [= 1] by CPU
> + * BIT1 [= 0] Compute updated by SDMA [= 1] by CPU
> + * NOTE: CPU update is recommneded only for large BAR systems
> + */
> +#define AMDGPU_VM_USE_CPU_FOR_GFX (1 << 0)
> +#define AMDGPU_VM_USE_CPU_FOR_COMPUTE (1 << 1)
> +
> struct amdgpu_vm {
> /* tree of virtual addresses mapped */
> struct rb_root va;
> @@ -129,6 +139,9 @@ struct amdgpu_vm {
> struct amdgpu_vm_id *reserved_vmid[AMDGPU_MAX_VMHUBS];
> /* each VM will map on CSA */
> struct amdgpu_bo_va *csa_bo_va;
> +
> + /* Flag to indicate if VM tables are updated by CPU or GPU (SDMA) */
> + bool is_vm_update_mode_cpu;
Better call this "use_cpu_updates" or something like this.
Same for the function parameter.
Regards,
Christian.
> };
>
> struct amdgpu_vm_id {
> @@ -190,7 +203,8 @@ struct amdgpu_vm_manager {
>
> 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 amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> + bool vm_update_mode);
> void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
> struct list_head *validated,
