[PATCH 4/7] drm/xe: Relax runtime pm protection around VM

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Sent: Saturday, May 4, 2024 12:43 AM
> To: intel-xe at lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi at intel.com>; Brost, Matthew
> <matthew.brost at intel.com>; Dugast, Francois <francois.dugast at intel.com>;
> thomas.hellstrom at linux.intel.com; Auld, Matthew
> <matthew.auld at intel.com>; Gupta, Anshuman
> <anshuman.gupta at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Subject: [PATCH 4/7] drm/xe: Relax runtime pm protection around VM
> 
> In the regular use case scenario, user space will create a VM, and keep it alive
> for the entire duration of its workload.
> 
> For the regular desktop cases, it means that the VM is alive even on idle
> scenarios where display goes off. This is unacceptable since this would
> entirely block runtime PM indefinitely, blocking deeper Package-C state. This
> would be a waste drainage of power.
> 
> So, let's limit the protection only for the long running workloads, which
> memory might be mapped and accessed during this entire workload.
> 
> This indeed opens up a risk of use case without display, and without long-
> running workload, where memory might be mapped and accessed with direct
> read and write operations without any gpu execution involved. Because of
> this, we are also adding here, the extra protection for the special vm_op
> access callback.
AFAIU for this particular case we are already having protection, as we are invalidating the pte upon entry to runtime suspend.
So a pagefault should wake the device. Therefore we don't need this extra precaution.
Thanks,
Anshuman
> 
> In the ideal case of the mmapped scenario of vm_ops, we would also get
> references in the 'open' and 'mmap' callbacks, and put it back on the 'close'
> callback, for a balanced case.
> However, this would also block the regular desktop case, so we are not doing
> this.
> 
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Francois Dugast <francois.dugast at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 17 ++++++++++++++++-
> drivers/gpu/drm/xe/xe_vm.c |  6 +++---
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c index
> 52a16cb4e736..48eca9f2651a 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1157,11 +1157,26 @@ static vm_fault_t xe_gem_fault(struct vm_fault
> *vmf)
>  	return ret;
>  }
> 
> +static int xe_vm_access(struct vm_area_struct *vma, unsigned long addr,
> +			void *buf, int len, int write)
> +{
> +	struct ttm_buffer_object *tbo = vma->vm_private_data;
> +	struct drm_device *ddev = tbo->base.dev;
> +	struct xe_device *xe = to_xe_device(ddev);
> +	int ret;
> +
> +	xe_pm_runtime_get(xe);
> +	ret = ttm_bo_vm_access(vma, addr, buf, len, write);
> +	xe_pm_runtime_put(xe);
> +
> +	return ret;
> +}
> +
>  static const struct vm_operations_struct xe_gem_vm_ops = {
>  	.fault = xe_gem_fault,
>  	.open = ttm_bo_vm_open,
>  	.close = ttm_bo_vm_close,
> -	.access = ttm_bo_vm_access
> +	.access = xe_vm_access
>  };
> 
>  static const struct drm_gem_object_funcs xe_gem_object_funcs = { diff --git
> a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c index
> dfd31b346021..aa298b768620 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1347,7 +1347,7 @@ struct xe_vm *xe_vm_create(struct xe_device *xe,
> u32 flags)
> 
>  	vm->pt_ops = &xelp_pt_ops;
> 
> -	if (!(flags & XE_VM_FLAG_MIGRATION))
> +	if (flags & XE_VM_FLAG_LR_MODE)
>  		xe_pm_runtime_get_noresume(xe);
> 
>  	vm_resv_obj = drm_gpuvm_resv_object_alloc(&xe->drm);
> @@ -1457,7 +1457,7 @@ struct xe_vm *xe_vm_create(struct xe_device *xe,
> u32 flags)
>  	for_each_tile(tile, xe, id)
>  		xe_range_fence_tree_fini(&vm->rftree[id]);
>  	kfree(vm);
> -	if (!(flags & XE_VM_FLAG_MIGRATION))
> +	if (flags & XE_VM_FLAG_LR_MODE)
>  		xe_pm_runtime_put(xe);
>  	return ERR_PTR(err);
>  }
> @@ -1592,7 +1592,7 @@ static void vm_destroy_work_func(struct
> work_struct *w)
> 
>  	mutex_destroy(&vm->snap_mutex);
> 
> -	if (!(vm->flags & XE_VM_FLAG_MIGRATION))
> +	if (vm->flags & XE_VM_FLAG_LR_MODE)
>  		xe_pm_runtime_put(xe);
> 
>  	for_each_tile(tile, xe, id)
> --
> 2.44.0