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 <[email protected]>
Cc: Matthew Brost <[email protected]>
Cc: Boris Brezillon <[email protected]>
Cc: <[email protected]>
Signed-off-by: Himal Prasad Ghimiray<[email protected]>
---
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/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
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/[email protected]/
+/**
+ * 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
+};
