On Fri, Jun 13, 2025 at 4:57 PM Rob Clark <[email protected]> wrote: > > For UNMAP/REMAP steps we could be needing to lock objects that are not > explicitly listed in the VM_BIND ioctl in order to tear-down unmapped > VAs. These helpers handle locking/preparing the needed objects. > > Note that these functions do not strictly require the VM changes to be > applied before the next drm_gpuvm_sm_map_lock()/_unmap_lock() call. In > the case that VM changes from an earlier drm_gpuvm_sm_map()/_unmap() > call result in a differing sequence of steps when the VM changes are > actually applied, it will be the same set of GEM objects involved, so > the locking is still correct. > > Signed-off-by: Rob Clark <[email protected]>
And if it wasn't clear/obvious, the expected usage is something that looks like: https://gitlab.freedesktop.org/robclark/msm/-/blob/sparse-newer/drivers/gpu/drm/msm/msm_gem_vma.c?ref_type=heads#L1150 ie. the caller handles the drm_exec_until_all_locked bit BR, -R > --- > drivers/gpu/drm/drm_gpuvm.c | 81 +++++++++++++++++++++++++++++++++++++ > include/drm/drm_gpuvm.h | 8 ++++ > 2 files changed, 89 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > index 0ca717130541..197066dd390b 100644 > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -2390,6 +2390,87 @@ drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm, void *priv, > } > EXPORT_SYMBOL_GPL(drm_gpuvm_sm_unmap); > > +static int > +drm_gpuva_sm_step_lock(struct drm_gpuva_op *op, void *priv) > +{ > + struct drm_exec *exec = priv; > + > + switch (op->op) { > + case DRM_GPUVA_OP_REMAP: > + if (op->remap.unmap->va->gem.obj) > + return drm_exec_lock_obj(exec, > op->remap.unmap->va->gem.obj); > + return 0; > + case DRM_GPUVA_OP_UNMAP: > + if (op->unmap.va->gem.obj) > + return drm_exec_lock_obj(exec, op->unmap.va->gem.obj); > + return 0; > + default: > + return 0; > + } > +} > + > +static const struct drm_gpuvm_ops lock_ops = { > + .sm_step_map = drm_gpuva_sm_step_lock, > + .sm_step_remap = drm_gpuva_sm_step_lock, > + .sm_step_unmap = drm_gpuva_sm_step_lock, > +}; > + > +/** > + * drm_gpuvm_sm_map_lock() - locks the objects touched by a > drm_gpuvm_sm_map() > + * @gpuvm: the &drm_gpuvm representing the GPU VA space > + * @exec: the &drm_exec locking context > + * @num_fences: for newly mapped objects, the # of fences to reserve > + * @req_addr: the start address of the range to unmap > + * @req_range: the range of the mappings to unmap > + * @req_obj: the &drm_gem_object to map > + * @req_offset: the offset within the &drm_gem_object > + * > + * This function locks (drm_exec_lock_obj()) objects that will be unmapped/ > + * remapped, and locks+prepares (drm_exec_prepare_object()) objects that > + * will be newly mapped. > + * > + * Returns: 0 on success or a negative error code > + */ > +int > +drm_gpuvm_sm_map_lock(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, unsigned int num_fences, > + u64 req_addr, u64 req_range, > + struct drm_gem_object *req_obj, u64 req_offset) > +{ > + if (req_obj) { > + int ret = drm_exec_prepare_obj(exec, req_obj, num_fences); > + if (ret) > + return ret; > + } > + > + return __drm_gpuvm_sm_map(gpuvm, &lock_ops, exec, > + req_addr, req_range, > + req_obj, req_offset); > + > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map_lock); > + > +/** > + * drm_gpuvm_sm_unmap_lock() - locks the objects touched by > drm_gpuvm_sm_unmap() > + * @gpuvm: the &drm_gpuvm representing the GPU VA space > + * @exec: the &drm_exec locking context > + * @req_addr: the start address of the range to unmap > + * @req_range: the range of the mappings to unmap > + * > + * This function locks (drm_exec_lock_obj()) objects that will be unmapped/ > + * remapped by drm_gpuvm_sm_unmap(). > + * > + * Returns: 0 on success or a negative error code > + */ > +int > +drm_gpuvm_sm_unmap_lock(struct drm_gpuvm *gpuvm, struct drm_exec *exec, > + u64 req_addr, u64 req_range) > +{ > + return __drm_gpuvm_sm_unmap(gpuvm, &lock_ops, exec, > + req_addr, req_range); > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_sm_unmap_lock); > + > static struct drm_gpuva_op * > gpuva_op_alloc(struct drm_gpuvm *gpuvm) > { > diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h > index 0ef837a331d6..a769dccfb3ae 100644 > --- a/include/drm/drm_gpuvm.h > +++ b/include/drm/drm_gpuvm.h > @@ -1211,6 +1211,14 @@ int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void > *priv, > int drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm, void *priv, > u64 req_addr, u64 req_range); > > +int drm_gpuvm_sm_map_lock(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, unsigned int num_fences, > + u64 req_addr, u64 req_range, > + struct drm_gem_object *obj, u64 offset); > + > +int drm_gpuvm_sm_unmap_lock(struct drm_gpuvm *gpuvm, struct drm_exec *exec, > + u64 req_addr, u64 req_range); > + > void drm_gpuva_map(struct drm_gpuvm *gpuvm, > struct drm_gpuva *va, > struct drm_gpuva_op_map *op); > -- > 2.49.0 >
