* [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops [not found] <20250722133526.3550547-1-himal.prasad.ghimiray@intel.com> @ 2025-07-22 13:35 ` Himal Prasad Ghimiray 2025-07-22 13:38 ` Danilo Krummrich 2025-07-27 21:18 ` Matthew Brost 0 siblings, 2 replies; 9+ messages in thread From: Himal Prasad Ghimiray @ 2025-07-22 13:35 UTC (permalink / raw) To: intel-xe Cc: Matthew Brost, Thomas Hellström, Himal Prasad Ghimiray, Danilo Krummrich, Boris Brezillon, dri-devel - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input range. - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the user-provided range and split the existing non-GEM object VMA if the start or end of the input range lies within it. The operations can create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be used by the Xe driver to assign attributes to GPUVMA's within the user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, the operation with this flag will never have UNMAPs and merges, and can be without any final operations. v2 - use drm_gpuvm_sm_map_ops_create with flags instead of defining new ops_create (Danilo) - Add doc (Danilo) v3 - Fix doc - Fix unmapping check v4 - Fix mapping for non madvise ops Cc: Danilo Krummrich <dakr@redhat.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: Boris Brezillon <bbrezillon@kernel.org> Cc: <dri-devel@lists.freedesktop.org> Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> --- drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + drivers/gpu/drm/xe/xe_vm.c | 1 + include/drm/drm_gpuvm.h | 25 ++++++- 4 files changed, 98 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c index e89b932e987c..c7779588ea38 100644 --- a/drivers/gpu/drm/drm_gpuvm.c +++ b/drivers/gpu/drm/drm_gpuvm.c @@ -2103,10 +2103,13 @@ static int __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, const struct drm_gpuvm_ops *ops, void *priv, u64 req_addr, u64 req_range, + enum drm_gpuvm_sm_map_ops_flags flags, struct drm_gem_object *req_obj, u64 req_offset) { struct drm_gpuva *va, *next; u64 req_end = req_addr + req_range; + bool is_madvise_ops = (flags == DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE); + bool needs_map = !is_madvise_ops; int ret; if (unlikely(!drm_gpuvm_range_valid(gpuvm, req_addr, req_range))) @@ -2119,26 +2122,35 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, u64 range = va->va.range; u64 end = addr + range; bool merge = !!va->gem.obj; + bool skip_madvise_ops = is_madvise_ops && merge; + needs_map = !is_madvise_ops; if (addr == req_addr) { merge &= obj == req_obj && offset == req_offset; if (end == req_end) { - ret = op_unmap_cb(ops, priv, va, merge); - if (ret) - return ret; + if (!is_madvise_ops) { + ret = op_unmap_cb(ops, priv, va, merge); + if (ret) + return ret; + } break; } if (end < req_end) { - ret = op_unmap_cb(ops, priv, va, merge); - if (ret) - return ret; + if (!is_madvise_ops) { + ret = op_unmap_cb(ops, priv, va, merge); + if (ret) + return ret; + } continue; } if (end > req_end) { + if (skip_madvise_ops) + break; + struct drm_gpuva_op_map n = { .va.addr = req_end, .va.range = range - req_range, @@ -2153,6 +2165,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, ret = op_remap_cb(ops, priv, NULL, &n, &u); if (ret) return ret; + + if (is_madvise_ops) + needs_map = true; break; } } else if (addr < req_addr) { @@ -2170,20 +2185,42 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, u.keep = merge; if (end == req_end) { + if (skip_madvise_ops) + break; + ret = op_remap_cb(ops, priv, &p, NULL, &u); if (ret) return ret; + + if (is_madvise_ops) + needs_map = true; + break; } if (end < req_end) { + if (skip_madvise_ops) + continue; + ret = op_remap_cb(ops, priv, &p, NULL, &u); if (ret) return ret; + + if (is_madvise_ops) { + ret = op_map_cb(ops, priv, req_addr, + min(end - req_addr, req_end - end), + NULL, req_offset); + if (ret) + return ret; + } + continue; } if (end > req_end) { + if (skip_madvise_ops) + break; + struct drm_gpuva_op_map n = { .va.addr = req_end, .va.range = end - req_end, @@ -2195,6 +2232,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, ret = op_remap_cb(ops, priv, &p, &n, &u); if (ret) return ret; + + if (is_madvise_ops) + needs_map = true; break; } } else if (addr > req_addr) { @@ -2203,20 +2243,29 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, (addr - req_addr); if (end == req_end) { - ret = op_unmap_cb(ops, priv, va, merge); - if (ret) - return ret; + if (!is_madvise_ops) { + ret = op_unmap_cb(ops, priv, va, merge); + if (ret) + return ret; + } + break; } if (end < req_end) { - ret = op_unmap_cb(ops, priv, va, merge); - if (ret) - return ret; + if (!is_madvise_ops) { + ret = op_unmap_cb(ops, priv, va, merge); + if (ret) + return ret; + } + continue; } if (end > req_end) { + if (skip_madvise_ops) + break; + struct drm_gpuva_op_map n = { .va.addr = req_end, .va.range = end - req_end, @@ -2231,14 +2280,16 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, ret = op_remap_cb(ops, priv, NULL, &n, &u); if (ret) return ret; + + if (is_madvise_ops) + return op_map_cb(ops, priv, addr, + (req_end - addr), NULL, req_offset); break; } } } - - return op_map_cb(ops, priv, - req_addr, req_range, - req_obj, req_offset); + return needs_map ? op_map_cb(ops, priv, req_addr, + req_range, req_obj, req_offset) : 0; } static int @@ -2337,15 +2388,15 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv, struct drm_gem_object *req_obj, u64 req_offset) { const struct drm_gpuvm_ops *ops = gpuvm->ops; + enum drm_gpuvm_sm_map_ops_flags flags = DRM_GPUVM_SM_MAP_NOT_MADVISE; if (unlikely(!(ops && ops->sm_step_map && ops->sm_step_remap && ops->sm_step_unmap))) return -EINVAL; - return __drm_gpuvm_sm_map(gpuvm, ops, priv, - req_addr, req_range, - req_obj, req_offset); + return __drm_gpuvm_sm_map(gpuvm, ops, priv, req_addr, req_range, + flags, req_obj, req_offset); } EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map); @@ -2487,6 +2538,7 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = { * @gpuvm: the &drm_gpuvm representing the GPU VA space * @req_addr: the start address of the new mapping * @req_range: the range of the new mapping + * @drm_gpuvm_sm_map_ops_flag: ops flag determining madvise or not * @req_obj: the &drm_gem_object to map * @req_offset: the offset within the &drm_gem_object * @@ -2517,6 +2569,7 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = { struct drm_gpuva_ops * drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, u64 req_addr, u64 req_range, + enum drm_gpuvm_sm_map_ops_flags flags, struct drm_gem_object *req_obj, u64 req_offset) { struct drm_gpuva_ops *ops; @@ -2536,7 +2589,7 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, args.ops = ops; ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args, - req_addr, req_range, + req_addr, req_range, flags, req_obj, req_offset); if (ret) goto err_free_ops; diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c index 48f105239f42..26e13fcdbdb8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c @@ -1303,6 +1303,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job, op->ops = drm_gpuvm_sm_map_ops_create(&uvmm->base, op->va.addr, op->va.range, + DRM_GPUVM_SM_MAP_NOT_MADVISE, op->gem.obj, op->gem.offset); if (IS_ERR(op->ops)) { diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c index 2035604121e6..b2ed99551b6e 100644 --- a/drivers/gpu/drm/xe/xe_vm.c +++ b/drivers/gpu/drm/xe/xe_vm.c @@ -2318,6 +2318,7 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops, case DRM_XE_VM_BIND_OP_MAP: case DRM_XE_VM_BIND_OP_MAP_USERPTR: ops = drm_gpuvm_sm_map_ops_create(&vm->gpuvm, addr, range, + DRM_GPUVM_SM_MAP_NOT_MADVISE, obj, bo_offset_or_userptr); break; case DRM_XE_VM_BIND_OP_UNMAP: diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index 2a9629377633..c589b886a4fd 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -211,6 +211,27 @@ enum drm_gpuvm_flags { DRM_GPUVM_USERBITS = BIT(1), }; +/** + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops + */ +enum drm_gpuvm_sm_map_ops_flags { + /** + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops + */ + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, + + /** + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the + * user-provided range and split the existing non-GEM object VMA if the + * start or end of the input range lies within it. The operations can + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags + * in default mode, the operation with this flag will never have UNMAPs and + * merges, and can be without any final operations. + */ + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), +}; + /** * struct drm_gpuvm - DRM GPU VA Manager * @@ -1059,8 +1080,8 @@ struct drm_gpuva_ops { #define drm_gpuva_next_op(op) list_next_entry(op, entry) struct drm_gpuva_ops * -drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, - u64 addr, u64 range, +drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, u64 addr, u64 range, + enum drm_gpuvm_sm_map_ops_flags flags, struct drm_gem_object *obj, u64 offset); struct drm_gpuva_ops * drm_gpuvm_sm_unmap_ops_create(struct drm_gpuvm *gpuvm, -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-22 13:35 ` [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops Himal Prasad Ghimiray @ 2025-07-22 13:38 ` Danilo Krummrich 2025-07-24 0:43 ` Matthew Brost 2025-07-24 10:02 ` Ghimiray, Himal Prasad 2025-07-27 21:18 ` Matthew Brost 1 sibling, 2 replies; 9+ messages in thread From: Danilo Krummrich @ 2025-07-22 13:38 UTC (permalink / raw) To: Himal Prasad Ghimiray Cc: intel-xe, Matthew Brost, Thomas Hellström, Danilo Krummrich, Boris Brezillon, Caterina Shablia, dri-devel (Cc: Caterina) On Tue Jul 22, 2025 at 3:35 PM CEST, Himal Prasad Ghimiray wrote: > - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input > range. > > - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > user-provided range and split the existing non-GEM object VMA if the > start or end of the input range lies within it. The operations can > create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be > used by the Xe driver to assign attributes to GPUVMA's within the > user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, > the operation with this flag will never have UNMAPs and > merges, and can be without any final operations. > > v2 > - use drm_gpuvm_sm_map_ops_create with flags instead of defining new > ops_create (Danilo) > - Add doc (Danilo) > > v3 > - Fix doc > - Fix unmapping check > > v4 > - Fix mapping for non madvise ops > > Cc: Danilo Krummrich <dakr@redhat.com> > Cc: Matthew Brost <matthew.brost@intel.com> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: <dri-devel@lists.freedesktop.org> > Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> > --- > drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + > drivers/gpu/drm/xe/xe_vm.c | 1 + What about the other drivers using GPUVM, aren't they affected by the changes? > include/drm/drm_gpuvm.h | 25 ++++++- > 4 files changed, 98 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > index e89b932e987c..c7779588ea38 100644 > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -2103,10 +2103,13 @@ static int > __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > const struct drm_gpuvm_ops *ops, void *priv, > u64 req_addr, u64 req_range, > + enum drm_gpuvm_sm_map_ops_flags flags, Please coordinate with Boris and Caterina here. They're adding a new request structure, struct drm_gpuvm_map_req. I think we can define it as struct drm_gpuvm_map_req { struct drm_gpuva_op_map map; struct drm_gpuvm_sm_map_ops_flags flags; } eventually. Please also coordinate on the changes in __drm_gpuvm_sm_map() below regarding Caterina's series [1], it looks like they're conflicting. [1] https://lore.kernel.org/all/20250707170442.1437009-1-caterina.shablia@collabora.com/ > +/** > + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops > + */ > +enum drm_gpuvm_sm_map_ops_flags { > + /** > + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops > + */ > + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, Why would we name this "NOT_MADVISE"? What if we add more flags for other purposes? > + /** > + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > + * user-provided range and split the existing non-GEM object VMA if the > + * start or end of the input range lies within it. The operations can > + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags > + * in default mode, the operation with this flag will never have UNMAPs and > + * merges, and can be without any final operations. > + */ > + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), > +}; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-22 13:38 ` Danilo Krummrich @ 2025-07-24 0:43 ` Matthew Brost 2025-07-24 10:05 ` Ghimiray, Himal Prasad 2025-07-24 10:32 ` Caterina Shablia 2025-07-24 10:02 ` Ghimiray, Himal Prasad 1 sibling, 2 replies; 9+ messages in thread From: Matthew Brost @ 2025-07-24 0:43 UTC (permalink / raw) To: Danilo Krummrich Cc: Himal Prasad Ghimiray, intel-xe, Thomas Hellström, Danilo Krummrich, Boris Brezillon, Caterina Shablia, dri-devel On Tue, Jul 22, 2025 at 03:38:14PM +0200, Danilo Krummrich wrote: > (Cc: Caterina) > > On Tue Jul 22, 2025 at 3:35 PM CEST, Himal Prasad Ghimiray wrote: > > - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input > > range. > > > > - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > > drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > > user-provided range and split the existing non-GEM object VMA if the > > start or end of the input range lies within it. The operations can > > create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be > > used by the Xe driver to assign attributes to GPUVMA's within the > > user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, > > the operation with this flag will never have UNMAPs and > > merges, and can be without any final operations. > > > > v2 > > - use drm_gpuvm_sm_map_ops_create with flags instead of defining new > > ops_create (Danilo) > > - Add doc (Danilo) > > > > v3 > > - Fix doc > > - Fix unmapping check > > > > v4 > > - Fix mapping for non madvise ops > > > > Cc: Danilo Krummrich <dakr@redhat.com> > > Cc: Matthew Brost <matthew.brost@intel.com> > > Cc: Boris Brezillon <bbrezillon@kernel.org> > > Cc: <dri-devel@lists.freedesktop.org> > > Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> > > --- > > drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ > > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + > > drivers/gpu/drm/xe/xe_vm.c | 1 + > > What about the other drivers using GPUVM, aren't they affected by the changes? > Yes, this seemly would break the build or other users. If the baseline includes the patch below that I suggest to pull in this is a moot point though. > > include/drm/drm_gpuvm.h | 25 ++++++- > > 4 files changed, 98 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > > index e89b932e987c..c7779588ea38 100644 > > --- a/drivers/gpu/drm/drm_gpuvm.c > > +++ b/drivers/gpu/drm/drm_gpuvm.c > > @@ -2103,10 +2103,13 @@ static int > > __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > > const struct drm_gpuvm_ops *ops, void *priv, > > u64 req_addr, u64 req_range, > > + enum drm_gpuvm_sm_map_ops_flags flags, > > Please coordinate with Boris and Caterina here. They're adding a new request > structure, struct drm_gpuvm_map_req. > > I think we can define it as > > struct drm_gpuvm_map_req { > struct drm_gpuva_op_map map; > struct drm_gpuvm_sm_map_ops_flags flags; > } +1, I see the patch [2] and the suggested change to drm_gpuva_op_map [3]. Both patch and your suggestion look good to me. Perhaps we try to accelerate [2] landing ahead of either series as overall just looks like a good cleanup which can be merged asap. Himal - I'd rebase on top [2], with Danilo suggestion in [3] if this hasn't landed by your next rev. [2] https://lore.kernel.org/all/20250707170442.1437009-4-caterina.shablia@collabora.com/ [3] https://lore.kernel.org/all/DB61N61AKIJ3.FG7GUJBG386P@kernel.org/ > > eventually. > > Please also coordinate on the changes in __drm_gpuvm_sm_map() below regarding > Caterina's series [1], it looks like they're conflicting. > It looks pretty minor actually. I'm sure if really matter who this is race but yes, always good to coordinate. > [1] https://lore.kernel.org/all/20250707170442.1437009-1-caterina.shablia@collabora.com/ > > > +/** > > + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops > > + */ > > +enum drm_gpuvm_sm_map_ops_flags { > > + /** > > + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops > > + */ > > + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, > > Why would we name this "NOT_MADVISE"? What if we add more flags for other > purposes? > How about... s/DRM_GPUVM_SM_MAP_NOT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_NONE/ > > + /** > > + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > > + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > > + * user-provided range and split the existing non-GEM object VMA if the > > + * start or end of the input range lies within it. The operations can > > + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags > > + * in default mode, the operation with this flag will never have UNMAPs and > > + * merges, and can be without any final operations. > > + */ > > + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), Then normalize this one... s/DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE/ Matt > > +}; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-24 0:43 ` Matthew Brost @ 2025-07-24 10:05 ` Ghimiray, Himal Prasad 2025-07-24 10:32 ` Caterina Shablia 1 sibling, 0 replies; 9+ messages in thread From: Ghimiray, Himal Prasad @ 2025-07-24 10:05 UTC (permalink / raw) To: Matthew Brost, Danilo Krummrich Cc: intel-xe, Thomas Hellström, Danilo Krummrich, Boris Brezillon, Caterina Shablia, dri-devel On 24-07-2025 06:13, Matthew Brost wrote: > On Tue, Jul 22, 2025 at 03:38:14PM +0200, Danilo Krummrich wrote: >> (Cc: Caterina) >> >> On Tue Jul 22, 2025 at 3:35 PM CEST, Himal Prasad Ghimiray wrote: >>> - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input >>> range. >>> >>> - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >>> drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >>> user-provided range and split the existing non-GEM object VMA if the >>> start or end of the input range lies within it. The operations can >>> create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be >>> used by the Xe driver to assign attributes to GPUVMA's within the >>> user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, >>> the operation with this flag will never have UNMAPs and >>> merges, and can be without any final operations. >>> >>> v2 >>> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new >>> ops_create (Danilo) >>> - Add doc (Danilo) >>> >>> v3 >>> - Fix doc >>> - Fix unmapping check >>> >>> v4 >>> - Fix mapping for non madvise ops >>> >>> Cc: Danilo Krummrich <dakr@redhat.com> >>> Cc: Matthew Brost <matthew.brost@intel.com> >>> Cc: Boris Brezillon <bbrezillon@kernel.org> >>> Cc: <dri-devel@lists.freedesktop.org> >>> Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> >>> --- >>> drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ >>> drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + >>> drivers/gpu/drm/xe/xe_vm.c | 1 + >> >> What about the other drivers using GPUVM, aren't they affected by the changes? >> > > Yes, this seemly would break the build or other users. If the baseline > includes the patch below that I suggest to pull in this is a moot point > though. > >>> include/drm/drm_gpuvm.h | 25 ++++++- >>> 4 files changed, 98 insertions(+), 22 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c >>> index e89b932e987c..c7779588ea38 100644 >>> --- a/drivers/gpu/drm/drm_gpuvm.c >>> +++ b/drivers/gpu/drm/drm_gpuvm.c >>> @@ -2103,10 +2103,13 @@ static int >>> __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >>> const struct drm_gpuvm_ops *ops, void *priv, >>> u64 req_addr, u64 req_range, >>> + enum drm_gpuvm_sm_map_ops_flags flags, >> >> Please coordinate with Boris and Caterina here. They're adding a new request >> structure, struct drm_gpuvm_map_req. >> >> I think we can define it as >> >> struct drm_gpuvm_map_req { >> struct drm_gpuva_op_map map; >> struct drm_gpuvm_sm_map_ops_flags flags; >> } > > +1, I see the patch [2] and the suggested change to drm_gpuva_op_map > [3]. Both patch and your suggestion look good to me. > > Perhaps we try to accelerate [2] landing ahead of either series as > overall just looks like a good cleanup which can be merged asap. > > Himal - I'd rebase on top [2], with Danilo suggestion in [3] if this > hasn't landed by your next rev. > > [2] https://lore.kernel.org/all/20250707170442.1437009-4-caterina.shablia@collabora.com/ > [3] https://lore.kernel.org/all/DB61N61AKIJ3.FG7GUJBG386P@kernel.org/ > Sure will take care of this. >> >> eventually. >> >> Please also coordinate on the changes in __drm_gpuvm_sm_map() below regarding >> Caterina's series [1], it looks like they're conflicting. >> > > It looks pretty minor actually. I'm sure if really matter who this is > race but yes, always good to coordinate. > >> [1] https://lore.kernel.org/all/20250707170442.1437009-1-caterina.shablia@collabora.com/ >> >>> +/** >>> + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops >>> + */ >>> +enum drm_gpuvm_sm_map_ops_flags { >>> + /** >>> + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops >>> + */ >>> + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, >> >> Why would we name this "NOT_MADVISE"? What if we add more flags for other >> purposes? >> > > How about... > > s/DRM_GPUVM_SM_MAP_NOT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_NONE/ I was thinking DRM_GPUVM_SM_MAP_DEFAULT, but DRM_GPUVM_SM_MAP_OPS_FLAG_NONE looks better. will update it in next rev. > >>> + /** >>> + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >>> + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >>> + * user-provided range and split the existing non-GEM object VMA if the >>> + * start or end of the input range lies within it. The operations can >>> + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags >>> + * in default mode, the operation with this flag will never have UNMAPs and >>> + * merges, and can be without any final operations. >>> + */ >>> + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), > > Then normalize this one... > > s/DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE/ Sure > > Matt > >>> +}; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-24 0:43 ` Matthew Brost 2025-07-24 10:05 ` Ghimiray, Himal Prasad @ 2025-07-24 10:32 ` Caterina Shablia 2025-07-28 10:20 ` Ghimiray, Himal Prasad 1 sibling, 1 reply; 9+ messages in thread From: Caterina Shablia @ 2025-07-24 10:32 UTC (permalink / raw) To: Danilo Krummrich, Matthew Brost Cc: Himal Prasad Ghimiray, intel-xe, Thomas Hellström, Danilo Krummrich, Boris Brezillon, dri-devel El jueves, 24 de julio de 2025 2:43:56 (hora de verano de Europa central), Matthew Brost escribió: > On Tue, Jul 22, 2025 at 03:38:14PM +0200, Danilo Krummrich wrote: > > (Cc: Caterina) > > > > On Tue Jul 22, 2025 at 3:35 PM CEST, Himal Prasad Ghimiray wrote: > > > - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input > > > > > > range. > > > > > > - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > > > > > > drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > > > > > > user-provided range and split the existing non-GEM object VMA if the > > > start or end of the input range lies within it. The operations can > > > create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be > > > used by the Xe driver to assign attributes to GPUVMA's within the > > > user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, > > > the operation with this flag will never have UNMAPs and > > > merges, and can be without any final operations. > > > > > > v2 > > > - use drm_gpuvm_sm_map_ops_create with flags instead of defining new > > > > > > ops_create (Danilo) > > > > > > - Add doc (Danilo) > > > > > > v3 > > > - Fix doc > > > - Fix unmapping check > > > > > > v4 > > > - Fix mapping for non madvise ops > > > > > > Cc: Danilo Krummrich <dakr@redhat.com> > > > Cc: Matthew Brost <matthew.brost@intel.com> > > > Cc: Boris Brezillon <bbrezillon@kernel.org> > > > Cc: <dri-devel@lists.freedesktop.org> > > > Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> > > > --- > > > > > > drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ > > > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + > > > drivers/gpu/drm/xe/xe_vm.c | 1 + > > > > What about the other drivers using GPUVM, aren't they affected by the > > changes? > Yes, this seemly would break the build or other users. If the baseline > includes the patch below that I suggest to pull in this is a moot point > though. > > > > include/drm/drm_gpuvm.h | 25 ++++++- > > > 4 files changed, 98 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > > > index e89b932e987c..c7779588ea38 100644 > > > --- a/drivers/gpu/drm/drm_gpuvm.c > > > +++ b/drivers/gpu/drm/drm_gpuvm.c > > > @@ -2103,10 +2103,13 @@ static int > > > > > > __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > > > > > > const struct drm_gpuvm_ops *ops, void *priv, > > > u64 req_addr, u64 req_range, > > > > > > + enum drm_gpuvm_sm_map_ops_flags flags, > > > > Please coordinate with Boris and Caterina here. They're adding a new > > request structure, struct drm_gpuvm_map_req. > > > > I think we can define it as > > > > struct drm_gpuvm_map_req { > > > > struct drm_gpuva_op_map map; > > struct drm_gpuvm_sm_map_ops_flags flags; > > > > } > > +1, I see the patch [2] and the suggested change to drm_gpuva_op_map > [3]. Both patch and your suggestion look good to me. > > Perhaps we try to accelerate [2] landing ahead of either series as > overall just looks like a good cleanup which can be merged asap. I'm not sure my patchset would be in a mergeable state any time soon -- I've discovered some issues with split/merge of repeated mappings while writing the doc, so it will be a while before I'll be submitting that again. [2] itself is in a good shape, absolutely feel free to submit that as part of your series. > > Himal - I'd rebase on top [2], with Danilo suggestion in [3] if this > hasn't landed by your next rev. > > [2] > https://lore.kernel.org/all/20250707170442.1437009-4-caterina.shablia@colla > bora.com/ [3] > https://lore.kernel.org/all/DB61N61AKIJ3.FG7GUJBG386P@kernel.org/ > > eventually. > > > > Please also coordinate on the changes in __drm_gpuvm_sm_map() below > > regarding Caterina's series [1], it looks like they're conflicting. > > It looks pretty minor actually. I'm sure if really matter who this is > race but yes, always good to coordinate. > > > [1] > > https://lore.kernel.org/all/20250707170442.1437009-1-caterina.shablia@col > > labora.com/> > > > +/** > > > + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge > > > ops > > > + */ > > > +enum drm_gpuvm_sm_map_ops_flags { > > > + /** > > > + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops > > > + */ > > > + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, > > > > Why would we name this "NOT_MADVISE"? What if we add more flags for other > > purposes? > > How about... > > s/DRM_GPUVM_SM_MAP_NOT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_NONE/ > > > > + /** > > > + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > > > + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > > > + * user-provided range and split the existing non-GEM object VMA if > > > the > > > + * start or end of the input range lies within it. The operations can > > > + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags > > > + * in default mode, the operation with this flag will never have > > > UNMAPs and + * merges, and can be without any final operations. > > > + */ > > > + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), > > Then normalize this one... > > s/DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MA > DVISE/ > > Matt > > > > +}; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-24 10:32 ` Caterina Shablia @ 2025-07-28 10:20 ` Ghimiray, Himal Prasad 0 siblings, 0 replies; 9+ messages in thread From: Ghimiray, Himal Prasad @ 2025-07-28 10:20 UTC (permalink / raw) To: Caterina Shablia, Danilo Krummrich, Matthew Brost Cc: intel-xe, Thomas Hellström, Danilo Krummrich, Boris Brezillon, dri-devel On 24-07-2025 16:02, Caterina Shablia wrote: > El jueves, 24 de julio de 2025 2:43:56 (hora de verano de Europa central), > Matthew Brost escribió: >> On Tue, Jul 22, 2025 at 03:38:14PM +0200, Danilo Krummrich wrote: >>> (Cc: Caterina) >>> >>> On Tue Jul 22, 2025 at 3:35 PM CEST, Himal Prasad Ghimiray wrote: >>>> - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input >>>> >>>> range. >>>> >>>> - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >>>> >>>> drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >>>> >>>> user-provided range and split the existing non-GEM object VMA if the >>>> start or end of the input range lies within it. The operations can >>>> create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be >>>> used by the Xe driver to assign attributes to GPUVMA's within the >>>> user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, >>>> the operation with this flag will never have UNMAPs and >>>> merges, and can be without any final operations. >>>> >>>> v2 >>>> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new >>>> >>>> ops_create (Danilo) >>>> >>>> - Add doc (Danilo) >>>> >>>> v3 >>>> - Fix doc >>>> - Fix unmapping check >>>> >>>> v4 >>>> - Fix mapping for non madvise ops >>>> >>>> Cc: Danilo Krummrich <dakr@redhat.com> >>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>> Cc: Boris Brezillon <bbrezillon@kernel.org> >>>> Cc: <dri-devel@lists.freedesktop.org> >>>> Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> >>>> --- >>>> >>>> drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ >>>> drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + >>>> drivers/gpu/drm/xe/xe_vm.c | 1 + >>> >>> What about the other drivers using GPUVM, aren't they affected by the >>> changes? >> Yes, this seemly would break the build or other users. If the baseline >> includes the patch below that I suggest to pull in this is a moot point >> though. >> >>>> include/drm/drm_gpuvm.h | 25 ++++++- >>>> 4 files changed, 98 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c >>>> index e89b932e987c..c7779588ea38 100644 >>>> --- a/drivers/gpu/drm/drm_gpuvm.c >>>> +++ b/drivers/gpu/drm/drm_gpuvm.c >>>> @@ -2103,10 +2103,13 @@ static int >>>> >>>> __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >>>> >>>> const struct drm_gpuvm_ops *ops, void *priv, >>>> u64 req_addr, u64 req_range, >>>> >>>> + enum drm_gpuvm_sm_map_ops_flags flags, >>> >>> Please coordinate with Boris and Caterina here. They're adding a new >>> request structure, struct drm_gpuvm_map_req. >>> >>> I think we can define it as >>> >>> struct drm_gpuvm_map_req { >>> >>> struct drm_gpuva_op_map map; >>> struct drm_gpuvm_sm_map_ops_flags flags; >>> >>> } >> >> +1, I see the patch [2] and the suggested change to drm_gpuva_op_map >> [3]. Both patch and your suggestion look good to me. >> >> Perhaps we try to accelerate [2] landing ahead of either series as >> overall just looks like a good cleanup which can be merged asap. > I'm not sure my patchset would be in a mergeable state any time soon -- I've > discovered some issues with split/merge of repeated mappings while writing the > doc, so it will be a while before I'll be submitting that again. [2] itself is > in a good shape, absolutely feel free to submit that as part of your series. Thanks for the confirmation. Will update next rev accordingly. >> >> Himal - I'd rebase on top [2], with Danilo suggestion in [3] if this >> hasn't landed by your next rev. >> >> [2] >> https://lore.kernel.org/all/20250707170442.1437009-4-caterina.shablia@colla >> bora.com/ [3] >> https://lore.kernel.org/all/DB61N61AKIJ3.FG7GUJBG386P@kernel.org/ >>> eventually. >>> >>> Please also coordinate on the changes in __drm_gpuvm_sm_map() below >>> regarding Caterina's series [1], it looks like they're conflicting. >> >> It looks pretty minor actually. I'm sure if really matter who this is >> race but yes, always good to coordinate. >> >>> [1] >>> https://lore.kernel.org/all/20250707170442.1437009-1-caterina.shablia@col >>> labora.com/> >>>> +/** >>>> + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge >>>> ops >>>> + */ >>>> +enum drm_gpuvm_sm_map_ops_flags { >>>> + /** >>>> + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops >>>> + */ >>>> + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, >>> >>> Why would we name this "NOT_MADVISE"? What if we add more flags for other >>> purposes? >> >> How about... >> >> s/DRM_GPUVM_SM_MAP_NOT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_NONE/ >> >>>> + /** >>>> + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >>>> + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >>>> + * user-provided range and split the existing non-GEM object VMA > if >>>> the >>>> + * start or end of the input range lies within it. The operations > can >>>> + * create up to 2 REMAPS and 2 MAPs. Unlike > drm_gpuvm_sm_map_ops_flags >>>> + * in default mode, the operation with this flag will never have >>>> UNMAPs and + * merges, and can be without any final operations. >>>> + */ >>>> + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), >> >> Then normalize this one... >> >> s/DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE/DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MA >> DVISE/ >> >> Matt >> >>>> +}; > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-22 13:38 ` Danilo Krummrich 2025-07-24 0:43 ` Matthew Brost @ 2025-07-24 10:02 ` Ghimiray, Himal Prasad 1 sibling, 0 replies; 9+ messages in thread From: Ghimiray, Himal Prasad @ 2025-07-24 10:02 UTC (permalink / raw) To: Danilo Krummrich Cc: intel-xe, Matthew Brost, Thomas Hellström, Danilo Krummrich, Boris Brezillon, Caterina Shablia, dri-devel On 22-07-2025 19:08, Danilo Krummrich wrote: > (Cc: Caterina) > > On Tue Jul 22, 2025 at 3:35 PM CEST, Himal Prasad Ghimiray wrote: >> - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input >> range. >> >> - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >> drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >> user-provided range and split the existing non-GEM object VMA if the >> start or end of the input range lies within it. The operations can >> create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be >> used by the Xe driver to assign attributes to GPUVMA's within the >> user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, >> the operation with this flag will never have UNMAPs and >> merges, and can be without any final operations. >> >> v2 >> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new >> ops_create (Danilo) >> - Add doc (Danilo) >> >> v3 >> - Fix doc >> - Fix unmapping check >> >> v4 >> - Fix mapping for non madvise ops >> >> Cc: Danilo Krummrich <dakr@redhat.com> >> Cc: Matthew Brost <matthew.brost@intel.com> >> Cc: Boris Brezillon <bbrezillon@kernel.org> >> Cc: <dri-devel@lists.freedesktop.org> >> Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> >> --- >> drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ >> drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + >> drivers/gpu/drm/xe/xe_vm.c | 1 + > > What about the other drivers using GPUVM, aren't they affected by the changes? Apart from xe, nouveau_uvmm.c is the only user of drm_gpuvm_sm_map_ops_create api and patch takes care for nouveau_uvmm.c > >> include/drm/drm_gpuvm.h | 25 ++++++- >> 4 files changed, 98 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c >> index e89b932e987c..c7779588ea38 100644 >> --- a/drivers/gpu/drm/drm_gpuvm.c >> +++ b/drivers/gpu/drm/drm_gpuvm.c >> @@ -2103,10 +2103,13 @@ static int >> __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> const struct drm_gpuvm_ops *ops, void *priv, >> u64 req_addr, u64 req_range, >> + enum drm_gpuvm_sm_map_ops_flags flags, > > Please coordinate with Boris and Caterina here. They're adding a new request > structure, struct drm_gpuvm_map_req. > > I think we can define it as > > struct drm_gpuvm_map_req { > struct drm_gpuva_op_map map; > struct drm_gpuvm_sm_map_ops_flags flags; > } > > eventually. Sure will check this. > > Please also coordinate on the changes in __drm_gpuvm_sm_map() below regarding > Caterina's series [1], it looks like they're conflicting. Will give it a look > > [1] https://lore.kernel.org/all/20250707170442.1437009-1-caterina.shablia@collabora.com/ > >> +/** >> + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops >> + */ >> +enum drm_gpuvm_sm_map_ops_flags { >> + /** >> + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops >> + */ >> + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, > > Why would we name this "NOT_MADVISE"? What if we add more flags for other > purposes? How about something like DRM_GPUVM_SM_MAP_DEFAULT ? > >> + /** >> + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >> + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >> + * user-provided range and split the existing non-GEM object VMA if the >> + * start or end of the input range lies within it. The operations can >> + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags >> + * in default mode, the operation with this flag will never have UNMAPs and >> + * merges, and can be without any final operations. >> + */ >> + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), >> +}; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-22 13:35 ` [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops Himal Prasad Ghimiray 2025-07-22 13:38 ` Danilo Krummrich @ 2025-07-27 21:18 ` Matthew Brost 2025-07-28 6:16 ` Ghimiray, Himal Prasad 1 sibling, 1 reply; 9+ messages in thread From: Matthew Brost @ 2025-07-27 21:18 UTC (permalink / raw) To: Himal Prasad Ghimiray Cc: intel-xe, Thomas Hellström, Danilo Krummrich, Boris Brezillon, dri-devel On Tue, Jul 22, 2025 at 07:05:04PM +0530, Himal Prasad Ghimiray wrote: > - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input > range. > > - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > user-provided range and split the existing non-GEM object VMA if the > start or end of the input range lies within it. The operations can > create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be > used by the Xe driver to assign attributes to GPUVMA's within the > user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, > the operation with this flag will never have UNMAPs and > merges, and can be without any final operations. > > v2 > - use drm_gpuvm_sm_map_ops_create with flags instead of defining new > ops_create (Danilo) > - Add doc (Danilo) > > v3 > - Fix doc > - Fix unmapping check > > v4 > - Fix mapping for non madvise ops > > Cc: Danilo Krummrich <dakr@redhat.com> > Cc: Matthew Brost <matthew.brost@intel.com> > Cc: Boris Brezillon <bbrezillon@kernel.org> > Cc: <dri-devel@lists.freedesktop.org> > Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> > --- > drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + > drivers/gpu/drm/xe/xe_vm.c | 1 + > include/drm/drm_gpuvm.h | 25 ++++++- > 4 files changed, 98 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > index e89b932e987c..c7779588ea38 100644 > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -2103,10 +2103,13 @@ static int > __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > const struct drm_gpuvm_ops *ops, void *priv, > u64 req_addr, u64 req_range, > + enum drm_gpuvm_sm_map_ops_flags flags, > struct drm_gem_object *req_obj, u64 req_offset) > { > struct drm_gpuva *va, *next; > u64 req_end = req_addr + req_range; > + bool is_madvise_ops = (flags == DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE); > + bool needs_map = !is_madvise_ops; > int ret; > > if (unlikely(!drm_gpuvm_range_valid(gpuvm, req_addr, req_range))) > @@ -2119,26 +2122,35 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > u64 range = va->va.range; > u64 end = addr + range; > bool merge = !!va->gem.obj; > + bool skip_madvise_ops = is_madvise_ops && merge; > > + needs_map = !is_madvise_ops; > if (addr == req_addr) { > merge &= obj == req_obj && > offset == req_offset; > > if (end == req_end) { > - ret = op_unmap_cb(ops, priv, va, merge); > - if (ret) > - return ret; > + if (!is_madvise_ops) { > + ret = op_unmap_cb(ops, priv, va, merge); > + if (ret) > + return ret; > + } > break; > } > > if (end < req_end) { > - ret = op_unmap_cb(ops, priv, va, merge); > - if (ret) > - return ret; > + if (!is_madvise_ops) { > + ret = op_unmap_cb(ops, priv, va, merge); > + if (ret) > + return ret; > + } > continue; > } > > if (end > req_end) { > + if (skip_madvise_ops) > + break; > + > struct drm_gpuva_op_map n = { > .va.addr = req_end, > .va.range = range - req_range, > @@ -2153,6 +2165,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > ret = op_remap_cb(ops, priv, NULL, &n, &u); > if (ret) > return ret; > + > + if (is_madvise_ops) > + needs_map = true; > break; > } > } else if (addr < req_addr) { > @@ -2170,20 +2185,42 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > u.keep = merge; > > if (end == req_end) { > + if (skip_madvise_ops) > + break; > + > ret = op_remap_cb(ops, priv, &p, NULL, &u); > if (ret) > return ret; > + > + if (is_madvise_ops) > + needs_map = true; > + > break; > } > > if (end < req_end) { > + if (skip_madvise_ops) > + continue; > + > ret = op_remap_cb(ops, priv, &p, NULL, &u); > if (ret) > return ret; > + > + if (is_madvise_ops) { > + ret = op_map_cb(ops, priv, req_addr, > + min(end - req_addr, req_end - end), This doesn't look right. This creating a new MAP operation to replace what the REMAP operation unmapped but didn't remap. In Xe debug speak, this is where we are: REMAP:UNMAP REMAP:PREV MAP <-- This is the calculation we are doing. We want to 'MAP' to size here to be: 'REMAP:UNMAP.end - REMAP:PREV.end' Which is 'end - req_addr'. So delete the min statement here and replace with 'end - req_addr'. Matt > + NULL, req_offset); > + if (ret) > + return ret; > + } > + > continue; > } > > if (end > req_end) { > + if (skip_madvise_ops) > + break; > + > struct drm_gpuva_op_map n = { > .va.addr = req_end, > .va.range = end - req_end, > @@ -2195,6 +2232,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > ret = op_remap_cb(ops, priv, &p, &n, &u); > if (ret) > return ret; > + > + if (is_madvise_ops) > + needs_map = true; > break; > } > } else if (addr > req_addr) { > @@ -2203,20 +2243,29 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > (addr - req_addr); > > if (end == req_end) { > - ret = op_unmap_cb(ops, priv, va, merge); > - if (ret) > - return ret; > + if (!is_madvise_ops) { > + ret = op_unmap_cb(ops, priv, va, merge); > + if (ret) > + return ret; > + } > + > break; > } > > if (end < req_end) { > - ret = op_unmap_cb(ops, priv, va, merge); > - if (ret) > - return ret; > + if (!is_madvise_ops) { > + ret = op_unmap_cb(ops, priv, va, merge); > + if (ret) > + return ret; > + } > + > continue; > } > > if (end > req_end) { > + if (skip_madvise_ops) > + break; > + > struct drm_gpuva_op_map n = { > .va.addr = req_end, > .va.range = end - req_end, > @@ -2231,14 +2280,16 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, > ret = op_remap_cb(ops, priv, NULL, &n, &u); > if (ret) > return ret; > + > + if (is_madvise_ops) > + return op_map_cb(ops, priv, addr, > + (req_end - addr), NULL, req_offset); > break; > } > } > } > - > - return op_map_cb(ops, priv, > - req_addr, req_range, > - req_obj, req_offset); > + return needs_map ? op_map_cb(ops, priv, req_addr, > + req_range, req_obj, req_offset) : 0; > } > > static int > @@ -2337,15 +2388,15 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv, > struct drm_gem_object *req_obj, u64 req_offset) > { > const struct drm_gpuvm_ops *ops = gpuvm->ops; > + enum drm_gpuvm_sm_map_ops_flags flags = DRM_GPUVM_SM_MAP_NOT_MADVISE; > > if (unlikely(!(ops && ops->sm_step_map && > ops->sm_step_remap && > ops->sm_step_unmap))) > return -EINVAL; > > - return __drm_gpuvm_sm_map(gpuvm, ops, priv, > - req_addr, req_range, > - req_obj, req_offset); > + return __drm_gpuvm_sm_map(gpuvm, ops, priv, req_addr, req_range, > + flags, req_obj, req_offset); > } > EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map); > > @@ -2487,6 +2538,7 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = { > * @gpuvm: the &drm_gpuvm representing the GPU VA space > * @req_addr: the start address of the new mapping > * @req_range: the range of the new mapping > + * @drm_gpuvm_sm_map_ops_flag: ops flag determining madvise or not > * @req_obj: the &drm_gem_object to map > * @req_offset: the offset within the &drm_gem_object > * > @@ -2517,6 +2569,7 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = { > struct drm_gpuva_ops * > drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, > u64 req_addr, u64 req_range, > + enum drm_gpuvm_sm_map_ops_flags flags, > struct drm_gem_object *req_obj, u64 req_offset) > { > struct drm_gpuva_ops *ops; > @@ -2536,7 +2589,7 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, > args.ops = ops; > > ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args, > - req_addr, req_range, > + req_addr, req_range, flags, > req_obj, req_offset); > if (ret) > goto err_free_ops; > diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c > index 48f105239f42..26e13fcdbdb8 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c > @@ -1303,6 +1303,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job, > op->ops = drm_gpuvm_sm_map_ops_create(&uvmm->base, > op->va.addr, > op->va.range, > + DRM_GPUVM_SM_MAP_NOT_MADVISE, > op->gem.obj, > op->gem.offset); > if (IS_ERR(op->ops)) { > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c > index 2035604121e6..b2ed99551b6e 100644 > --- a/drivers/gpu/drm/xe/xe_vm.c > +++ b/drivers/gpu/drm/xe/xe_vm.c > @@ -2318,6 +2318,7 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops, > case DRM_XE_VM_BIND_OP_MAP: > case DRM_XE_VM_BIND_OP_MAP_USERPTR: > ops = drm_gpuvm_sm_map_ops_create(&vm->gpuvm, addr, range, > + DRM_GPUVM_SM_MAP_NOT_MADVISE, > obj, bo_offset_or_userptr); > break; > case DRM_XE_VM_BIND_OP_UNMAP: > diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h > index 2a9629377633..c589b886a4fd 100644 > --- a/include/drm/drm_gpuvm.h > +++ b/include/drm/drm_gpuvm.h > @@ -211,6 +211,27 @@ enum drm_gpuvm_flags { > DRM_GPUVM_USERBITS = BIT(1), > }; > > +/** > + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops > + */ > +enum drm_gpuvm_sm_map_ops_flags { > + /** > + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops > + */ > + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, > + > + /** > + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by > + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the > + * user-provided range and split the existing non-GEM object VMA if the > + * start or end of the input range lies within it. The operations can > + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags > + * in default mode, the operation with this flag will never have UNMAPs and > + * merges, and can be without any final operations. > + */ > + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), > +}; > + > /** > * struct drm_gpuvm - DRM GPU VA Manager > * > @@ -1059,8 +1080,8 @@ struct drm_gpuva_ops { > #define drm_gpuva_next_op(op) list_next_entry(op, entry) > > struct drm_gpuva_ops * > -drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, > - u64 addr, u64 range, > +drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, u64 addr, u64 range, > + enum drm_gpuvm_sm_map_ops_flags flags, > struct drm_gem_object *obj, u64 offset); > struct drm_gpuva_ops * > drm_gpuvm_sm_unmap_ops_create(struct drm_gpuvm *gpuvm, > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops 2025-07-27 21:18 ` Matthew Brost @ 2025-07-28 6:16 ` Ghimiray, Himal Prasad 0 siblings, 0 replies; 9+ messages in thread From: Ghimiray, Himal Prasad @ 2025-07-28 6:16 UTC (permalink / raw) To: Matthew Brost Cc: intel-xe, Thomas Hellström, Danilo Krummrich, Boris Brezillon, dri-devel On 28-07-2025 02:48, Matthew Brost wrote: > On Tue, Jul 22, 2025 at 07:05:04PM +0530, Himal Prasad Ghimiray wrote: >> - DRM_GPUVM_SM_MAP_NOT_MADVISE: Default sm_map operations for the input >> range. >> >> - DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >> drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >> user-provided range and split the existing non-GEM object VMA if the >> start or end of the input range lies within it. The operations can >> create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be >> used by the Xe driver to assign attributes to GPUVMA's within the >> user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode, >> the operation with this flag will never have UNMAPs and >> merges, and can be without any final operations. >> >> v2 >> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new >> ops_create (Danilo) >> - Add doc (Danilo) >> >> v3 >> - Fix doc >> - Fix unmapping check >> >> v4 >> - Fix mapping for non madvise ops >> >> Cc: Danilo Krummrich <dakr@redhat.com> >> Cc: Matthew Brost <matthew.brost@intel.com> >> Cc: Boris Brezillon <bbrezillon@kernel.org> >> Cc: <dri-devel@lists.freedesktop.org> >> Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com> >> --- >> drivers/gpu/drm/drm_gpuvm.c | 93 ++++++++++++++++++++------ >> drivers/gpu/drm/nouveau/nouveau_uvmm.c | 1 + >> drivers/gpu/drm/xe/xe_vm.c | 1 + >> include/drm/drm_gpuvm.h | 25 ++++++- >> 4 files changed, 98 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c >> index e89b932e987c..c7779588ea38 100644 >> --- a/drivers/gpu/drm/drm_gpuvm.c >> +++ b/drivers/gpu/drm/drm_gpuvm.c >> @@ -2103,10 +2103,13 @@ static int >> __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> const struct drm_gpuvm_ops *ops, void *priv, >> u64 req_addr, u64 req_range, >> + enum drm_gpuvm_sm_map_ops_flags flags, >> struct drm_gem_object *req_obj, u64 req_offset) >> { >> struct drm_gpuva *va, *next; >> u64 req_end = req_addr + req_range; >> + bool is_madvise_ops = (flags == DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE); >> + bool needs_map = !is_madvise_ops; >> int ret; >> >> if (unlikely(!drm_gpuvm_range_valid(gpuvm, req_addr, req_range))) >> @@ -2119,26 +2122,35 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> u64 range = va->va.range; >> u64 end = addr + range; >> bool merge = !!va->gem.obj; >> + bool skip_madvise_ops = is_madvise_ops && merge; >> >> + needs_map = !is_madvise_ops; >> if (addr == req_addr) { >> merge &= obj == req_obj && >> offset == req_offset; >> >> if (end == req_end) { >> - ret = op_unmap_cb(ops, priv, va, merge); >> - if (ret) >> - return ret; >> + if (!is_madvise_ops) { >> + ret = op_unmap_cb(ops, priv, va, merge); >> + if (ret) >> + return ret; >> + } >> break; >> } >> >> if (end < req_end) { >> - ret = op_unmap_cb(ops, priv, va, merge); >> - if (ret) >> - return ret; >> + if (!is_madvise_ops) { >> + ret = op_unmap_cb(ops, priv, va, merge); >> + if (ret) >> + return ret; >> + } >> continue; >> } >> >> if (end > req_end) { >> + if (skip_madvise_ops) >> + break; >> + >> struct drm_gpuva_op_map n = { >> .va.addr = req_end, >> .va.range = range - req_range, >> @@ -2153,6 +2165,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> ret = op_remap_cb(ops, priv, NULL, &n, &u); >> if (ret) >> return ret; >> + >> + if (is_madvise_ops) >> + needs_map = true; >> break; >> } >> } else if (addr < req_addr) { >> @@ -2170,20 +2185,42 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> u.keep = merge; >> >> if (end == req_end) { >> + if (skip_madvise_ops) >> + break; >> + >> ret = op_remap_cb(ops, priv, &p, NULL, &u); >> if (ret) >> return ret; >> + >> + if (is_madvise_ops) >> + needs_map = true; >> + >> break; >> } >> >> if (end < req_end) { >> + if (skip_madvise_ops) >> + continue; >> + >> ret = op_remap_cb(ops, priv, &p, NULL, &u); >> if (ret) >> return ret; >> + >> + if (is_madvise_ops) { >> + ret = op_map_cb(ops, priv, req_addr, >> + min(end - req_addr, req_end - end), > > This doesn't look right. > > This creating a new MAP operation to replace what the REMAP operation > unmapped but didn't remap. In Xe debug speak, this is where we are: > > REMAP:UNMAP > REMAP:PREV > MAP <-- This is the calculation we are doing. > > We want to 'MAP' to size here to be: > > 'REMAP:UNMAP.end - REMAP:PREV.end' > > Which is 'end - req_addr'. So delete the min statement here and replace > with 'end - req_addr'. > > Matt True, will fix this. > >> + NULL, req_offset); >> + if (ret) >> + return ret; >> + } >> + >> continue; >> } >> >> if (end > req_end) { >> + if (skip_madvise_ops) >> + break; >> + >> struct drm_gpuva_op_map n = { >> .va.addr = req_end, >> .va.range = end - req_end, >> @@ -2195,6 +2232,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> ret = op_remap_cb(ops, priv, &p, &n, &u); >> if (ret) >> return ret; >> + >> + if (is_madvise_ops) >> + needs_map = true; >> break; >> } >> } else if (addr > req_addr) { >> @@ -2203,20 +2243,29 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> (addr - req_addr); >> >> if (end == req_end) { >> - ret = op_unmap_cb(ops, priv, va, merge); >> - if (ret) >> - return ret; >> + if (!is_madvise_ops) { >> + ret = op_unmap_cb(ops, priv, va, merge); >> + if (ret) >> + return ret; >> + } >> + >> break; >> } >> >> if (end < req_end) { >> - ret = op_unmap_cb(ops, priv, va, merge); >> - if (ret) >> - return ret; >> + if (!is_madvise_ops) { >> + ret = op_unmap_cb(ops, priv, va, merge); >> + if (ret) >> + return ret; >> + } >> + >> continue; >> } >> >> if (end > req_end) { >> + if (skip_madvise_ops) >> + break; >> + >> struct drm_gpuva_op_map n = { >> .va.addr = req_end, >> .va.range = end - req_end, >> @@ -2231,14 +2280,16 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, >> ret = op_remap_cb(ops, priv, NULL, &n, &u); >> if (ret) >> return ret; >> + >> + if (is_madvise_ops) >> + return op_map_cb(ops, priv, addr, >> + (req_end - addr), NULL, req_offset); >> break; >> } >> } >> } >> - >> - return op_map_cb(ops, priv, >> - req_addr, req_range, >> - req_obj, req_offset); >> + return needs_map ? op_map_cb(ops, priv, req_addr, >> + req_range, req_obj, req_offset) : 0; >> } >> >> static int >> @@ -2337,15 +2388,15 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv, >> struct drm_gem_object *req_obj, u64 req_offset) >> { >> const struct drm_gpuvm_ops *ops = gpuvm->ops; >> + enum drm_gpuvm_sm_map_ops_flags flags = DRM_GPUVM_SM_MAP_NOT_MADVISE; >> >> if (unlikely(!(ops && ops->sm_step_map && >> ops->sm_step_remap && >> ops->sm_step_unmap))) >> return -EINVAL; >> >> - return __drm_gpuvm_sm_map(gpuvm, ops, priv, >> - req_addr, req_range, >> - req_obj, req_offset); >> + return __drm_gpuvm_sm_map(gpuvm, ops, priv, req_addr, req_range, >> + flags, req_obj, req_offset); >> } >> EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map); >> >> @@ -2487,6 +2538,7 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = { >> * @gpuvm: the &drm_gpuvm representing the GPU VA space >> * @req_addr: the start address of the new mapping >> * @req_range: the range of the new mapping >> + * @drm_gpuvm_sm_map_ops_flag: ops flag determining madvise or not >> * @req_obj: the &drm_gem_object to map >> * @req_offset: the offset within the &drm_gem_object >> * >> @@ -2517,6 +2569,7 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = { >> struct drm_gpuva_ops * >> drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, >> u64 req_addr, u64 req_range, >> + enum drm_gpuvm_sm_map_ops_flags flags, >> struct drm_gem_object *req_obj, u64 req_offset) >> { >> struct drm_gpuva_ops *ops; >> @@ -2536,7 +2589,7 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, >> args.ops = ops; >> >> ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args, >> - req_addr, req_range, >> + req_addr, req_range, flags, >> req_obj, req_offset); >> if (ret) >> goto err_free_ops; >> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c >> index 48f105239f42..26e13fcdbdb8 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c >> @@ -1303,6 +1303,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job, >> op->ops = drm_gpuvm_sm_map_ops_create(&uvmm->base, >> op->va.addr, >> op->va.range, >> + DRM_GPUVM_SM_MAP_NOT_MADVISE, >> op->gem.obj, >> op->gem.offset); >> if (IS_ERR(op->ops)) { >> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c >> index 2035604121e6..b2ed99551b6e 100644 >> --- a/drivers/gpu/drm/xe/xe_vm.c >> +++ b/drivers/gpu/drm/xe/xe_vm.c >> @@ -2318,6 +2318,7 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops, >> case DRM_XE_VM_BIND_OP_MAP: >> case DRM_XE_VM_BIND_OP_MAP_USERPTR: >> ops = drm_gpuvm_sm_map_ops_create(&vm->gpuvm, addr, range, >> + DRM_GPUVM_SM_MAP_NOT_MADVISE, >> obj, bo_offset_or_userptr); >> break; >> case DRM_XE_VM_BIND_OP_UNMAP: >> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h >> index 2a9629377633..c589b886a4fd 100644 >> --- a/include/drm/drm_gpuvm.h >> +++ b/include/drm/drm_gpuvm.h >> @@ -211,6 +211,27 @@ enum drm_gpuvm_flags { >> DRM_GPUVM_USERBITS = BIT(1), >> }; >> >> +/** >> + * enum drm_gpuvm_sm_map_ops_flags - flags for drm_gpuvm split/merge ops >> + */ >> +enum drm_gpuvm_sm_map_ops_flags { >> + /** >> + * @DRM_GPUVM_SM_MAP_NOT_MADVISE: DEFAULT sm_map ops >> + */ >> + DRM_GPUVM_SM_MAP_NOT_MADVISE = 0, >> + >> + /** >> + * @DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE: This flag is used by >> + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the >> + * user-provided range and split the existing non-GEM object VMA if the >> + * start or end of the input range lies within it. The operations can >> + * create up to 2 REMAPS and 2 MAPs. Unlike drm_gpuvm_sm_map_ops_flags >> + * in default mode, the operation with this flag will never have UNMAPs and >> + * merges, and can be without any final operations. >> + */ >> + DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE = BIT(0), >> +}; >> + >> /** >> * struct drm_gpuvm - DRM GPU VA Manager >> * >> @@ -1059,8 +1080,8 @@ struct drm_gpuva_ops { >> #define drm_gpuva_next_op(op) list_next_entry(op, entry) >> >> struct drm_gpuva_ops * >> -drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, >> - u64 addr, u64 range, >> +drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm, u64 addr, u64 range, >> + enum drm_gpuvm_sm_map_ops_flags flags, >> struct drm_gem_object *obj, u64 offset); >> struct drm_gpuva_ops * >> drm_gpuvm_sm_unmap_ops_create(struct drm_gpuvm *gpuvm, >> -- >> 2.34.1 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-28 10:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20250722133526.3550547-1-himal.prasad.ghimiray@intel.com> 2025-07-22 13:35 ` [PATCH v5 01/23] Introduce drm_gpuvm_sm_map_ops_flags enums for sm_map_ops Himal Prasad Ghimiray 2025-07-22 13:38 ` Danilo Krummrich 2025-07-24 0:43 ` Matthew Brost 2025-07-24 10:05 ` Ghimiray, Himal Prasad 2025-07-24 10:32 ` Caterina Shablia 2025-07-28 10:20 ` Ghimiray, Himal Prasad 2025-07-24 10:02 ` Ghimiray, Himal Prasad 2025-07-27 21:18 ` Matthew Brost 2025-07-28 6:16 ` Ghimiray, Himal Prasad
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).