All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-xe] [PATCH v2 0/6] More VM cleanups
@ 2023-07-20  4:10 Matthew Brost
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds Matthew Brost
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe

Continuation of [1]. These are the last changes in [1] that will not
break the uAPI.

Matt

[1] https://patchwork.freedesktop.org/series/117156/

Matthew Brost (6):
  drm/xe: Avoid doing rebinds
  drm/xe: Reduce the number list links in xe_vma
  drm/xe: Replace list_del_init with list_del for
    userptr.invalidate_link cleanup
  drm/xe: Change tile masks from u64 to u8
  drm/xe: Combine destroy_cb and destroy_work in xe_vma into union
  drm/xe: Only alloc userptr part of xe_vma for userptrs

 drivers/gpu/drm/xe/xe_bo.c       |   6 +-
 drivers/gpu/drm/xe/xe_exec.c     |   2 +-
 drivers/gpu/drm/xe/xe_pt.c       |   3 +-
 drivers/gpu/drm/xe/xe_vm.c       | 135 ++++++++++++++++++++++---------
 drivers/gpu/drm/xe/xe_vm_types.h | 131 +++++++++++++++++-------------
 5 files changed, 178 insertions(+), 99 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
@ 2023-07-20  4:10 ` Matthew Brost
  2023-07-20 15:56   ` Rodrigo Vivi
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 2/6] drm/xe: Reduce the number list links in xe_vma Matthew Brost
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe

If we dont change page sizes we can avoid doing rebinds rather just do a
partial unbind. The algorithm to determine its page size is greedy as we
assume all pages in the removed VMA are the largest page used in the
VMA.

v2: Don't exceed 100 lines
v3: struct xe_vma_op_unmap remove in different patch, remove XXX comment

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_pt.c       |  1 +
 drivers/gpu/drm/xe/xe_vm.c       | 72 +++++++++++++++++++++++++++-----
 drivers/gpu/drm/xe/xe_vm_types.h |  7 ++++
 3 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index 851ea7c01b91..bbf37e5c8abb 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -668,6 +668,7 @@ xe_pt_stage_bind_entry(struct xe_ptw *parent, pgoff_t offset,
 		if (!is_null)
 			xe_res_next(curs, next - addr);
 		xe_walk->va_curs_start = next;
+		xe_walk->vma->gpuva.flags |= (XE_VMA_PTE_4K << level);
 		*action = ACTION_CONTINUE;
 
 		return ret;
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 87907a97a8c3..19ce6b0c6564 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -2384,6 +2384,16 @@ static struct xe_vma *new_vma(struct xe_vm *vm, struct drm_gpuva_op_map *op,
 	return vma;
 }
 
+static u64 xe_vma_max_pte_size(struct xe_vma *vma)
+{
+	if (vma->gpuva.flags & XE_VMA_PTE_1G)
+		return SZ_1G;
+	else if (vma->gpuva.flags & XE_VMA_PTE_2M)
+		return SZ_2M;
+
+	return SZ_4K;
+}
+
 /*
  * Parse operations list and create any resources needed for the operations
  * prior to fully committing to the operations. This setup can fail.
@@ -2460,6 +2470,13 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
 				break;
 			}
 			case DRM_GPUVA_OP_REMAP:
+			{
+				struct xe_vma *old =
+					gpuva_to_vma(op->base.remap.unmap->va);
+
+				op->remap.start = xe_vma_start(old);
+				op->remap.range = xe_vma_size(old);
+
 				if (op->base.remap.prev) {
 					struct xe_vma *vma;
 					bool read_only =
@@ -2478,6 +2495,20 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
 					}
 
 					op->remap.prev = vma;
+
+					/*
+					 * Userptr creates a new SG mapping so
+					 * we must also rebind.
+					 */
+					op->remap.skip_prev = !xe_vma_is_userptr(old) &&
+						IS_ALIGNED(xe_vma_end(vma),
+							   xe_vma_max_pte_size(old));
+					if (op->remap.skip_prev) {
+						op->remap.range -=
+							xe_vma_end(vma) -
+							xe_vma_start(old);
+						op->remap.start = xe_vma_end(vma);
+					}
 				}
 
 				if (op->base.remap.next) {
@@ -2499,14 +2530,21 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
 					}
 
 					op->remap.next = vma;
-				}
 
-				/* XXX: Support no doing remaps */
-				op->remap.start =
-					xe_vma_start(gpuva_to_vma(op->base.remap.unmap->va));
-				op->remap.range =
-					xe_vma_size(gpuva_to_vma(op->base.remap.unmap->va));
+					/*
+					 * Userptr creates a new SG mapping so
+					 * we must also rebind.
+					 */
+					op->remap.skip_next = !xe_vma_is_userptr(old) &&
+						IS_ALIGNED(xe_vma_start(vma),
+							   xe_vma_max_pte_size(old));
+					if (op->remap.skip_next)
+						op->remap.range -=
+							xe_vma_end(old) -
+							xe_vma_start(vma);
+				}
 				break;
+			}
 			case DRM_GPUVA_OP_UNMAP:
 			case DRM_GPUVA_OP_PREFETCH:
 				/* Nothing to do */
@@ -2549,10 +2587,23 @@ static int xe_vma_op_commit(struct xe_vm *vm, struct xe_vma_op *op)
 	case DRM_GPUVA_OP_REMAP:
 		prep_vma_destroy(vm, gpuva_to_vma(op->base.remap.unmap->va),
 				 true);
-		if (op->remap.prev)
+
+		if (op->remap.prev) {
 			err |= xe_vm_insert_vma(vm, op->remap.prev);
-		if (op->remap.next)
+			if (!err && op->remap.skip_prev)
+				op->remap.prev = NULL;
+		}
+		if (op->remap.next) {
 			err |= xe_vm_insert_vma(vm, op->remap.next);
+			if (!err && op->remap.skip_next)
+				op->remap.next = NULL;
+		}
+
+		/* Adjust for partial unbind after removin VMA from VM */
+		if (!err) {
+			op->base.remap.unmap->va->va.addr = op->remap.start;
+			op->base.remap.unmap->va->va.range = op->remap.range;
+		}
 		break;
 	case DRM_GPUVA_OP_UNMAP:
 		prep_vma_destroy(vm, gpuva_to_vma(op->base.unmap.va), true);
@@ -2622,9 +2673,10 @@ static int __xe_vma_op_execute(struct xe_vm *vm, struct xe_vma *vma,
 		bool next = !!op->remap.next;
 
 		if (!op->remap.unmap_done) {
-			vm->async_ops.munmap_rebind_inflight = true;
-			if (prev || next)
+			if (prev || next) {
+				vm->async_ops.munmap_rebind_inflight = true;
 				vma->gpuva.flags |= XE_VMA_FIRST_REBIND;
+			}
 			err = xe_vm_unbind(vm, vma, op->engine, op->syncs,
 					   op->num_syncs,
 					   !prev && !next ? op->fence : NULL,
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index dbe6aed6d2cf..29ec04fa1e26 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -29,6 +29,9 @@ struct xe_vm;
 #define XE_VMA_ATOMIC_PTE_BIT	(DRM_GPUVA_USERBITS << 2)
 #define XE_VMA_FIRST_REBIND	(DRM_GPUVA_USERBITS << 3)
 #define XE_VMA_LAST_REBIND	(DRM_GPUVA_USERBITS << 4)
+#define XE_VMA_PTE_4K		(DRM_GPUVA_USERBITS << 5)
+#define XE_VMA_PTE_2M		(DRM_GPUVA_USERBITS << 6)
+#define XE_VMA_PTE_1G		(DRM_GPUVA_USERBITS << 7)
 
 struct xe_vma {
 	/** @gpuva: Base GPUVA object */
@@ -336,6 +339,10 @@ struct xe_vma_op_remap {
 	u64 start;
 	/** @range: range of the VMA unmap */
 	u64 range;
+	/** @skip_prev: skip prev rebind */
+	bool skip_prev;
+	/** @skip_next: skip next rebind */
+	bool skip_next;
 	/** @unmap_done: unmap operation in done */
 	bool unmap_done;
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-xe] [PATCH v2 2/6] drm/xe: Reduce the number list links in xe_vma
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds Matthew Brost
@ 2023-07-20  4:10 ` Matthew Brost
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 3/6] drm/xe: Replace list_del_init with list_del for userptr.invalidate_link cleanup Matthew Brost
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe; +Cc: Rodrigo Vivi

Combine the userptr, rebind, and destroy links into a union as
the lists these links belong to are mutually exclusive.

v2: Adjust which lists are combined (Thomas H)
v3: Add kernel doc why this is safe (Thomas H), remove related change
of list_del_init -> list_del (Rodrigo)

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_bo.c       |  6 +++--
 drivers/gpu/drm/xe/xe_exec.c     |  2 +-
 drivers/gpu/drm/xe/xe_pt.c       |  2 +-
 drivers/gpu/drm/xe/xe_vm.c       | 43 ++++++++++++++++----------------
 drivers/gpu/drm/xe/xe_vm_types.h | 37 ++++++++++++++++-----------
 5 files changed, 49 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 828a9fea420b..538501c46b8b 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -474,8 +474,10 @@ static int xe_bo_trigger_rebind(struct xe_device *xe, struct xe_bo *bo,
 			}
 
 			xe_vm_assert_held(vm);
-			if (list_empty(&vma->rebind_link) && vma->tile_present)
-				list_add_tail(&vma->rebind_link, &vm->rebind_list);
+			if (list_empty(&vma->combined_links.rebind) &&
+			    vma->tile_present)
+				list_add_tail(&vma->combined_links.rebind,
+					      &vm->rebind_list);
 
 			if (vm_resv_locked)
 				dma_resv_unlock(&vm->resv);
diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c
index 1f64d64a94fa..fff4a9d9d12a 100644
--- a/drivers/gpu/drm/xe/xe_exec.c
+++ b/drivers/gpu/drm/xe/xe_exec.c
@@ -120,7 +120,7 @@ static int xe_exec_begin(struct xe_engine *e, struct ww_acquire_ctx *ww,
 	 * BOs have valid placements possibly moving an evicted BO back
 	 * to a location where the GPU can access it).
 	 */
-	list_for_each_entry(vma, &vm->rebind_list, rebind_link) {
+	list_for_each_entry(vma, &vm->rebind_list, combined_links.rebind) {
 		XE_WARN_ON(xe_vma_is_null(vma));
 
 		if (xe_vma_is_userptr(vma))
diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index bbf37e5c8abb..303313adf3ef 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -1690,7 +1690,7 @@ __xe_pt_unbind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_engine *e
 	}
 
 	if (!vma->tile_present)
-		list_del_init(&vma->rebind_link);
+		list_del_init(&vma->combined_links.rebind);
 
 	if (unbind_pt_update.locked) {
 		XE_WARN_ON(!xe_vma_is_userptr(vma));
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 19ce6b0c6564..87faf4b723b3 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -467,7 +467,8 @@ int xe_vm_lock_dma_resv(struct xe_vm *vm, struct ww_acquire_ctx *ww,
 
 		list_del_init(&vma->notifier.rebind_link);
 		if (vma->tile_present && !(vma->gpuva.flags & XE_VMA_DESTROYED))
-			list_move_tail(&vma->rebind_link, &vm->rebind_list);
+			list_move_tail(&vma->combined_links.rebind,
+				       &vm->rebind_list);
 	}
 	spin_unlock(&vm->notifier.list_lock);
 
@@ -608,7 +609,7 @@ static void preempt_rebind_work_func(struct work_struct *w)
 	if (err)
 		goto out_unlock;
 
-	list_for_each_entry(vma, &vm->rebind_list, rebind_link) {
+	list_for_each_entry(vma, &vm->rebind_list, combined_links.rebind) {
 		if (xe_vma_has_no_bo(vma) ||
 		    vma->gpuva.flags & XE_VMA_DESTROYED)
 			continue;
@@ -780,17 +781,20 @@ int xe_vm_userptr_pin(struct xe_vm *vm)
 	list_for_each_entry_safe(vma, next, &vm->userptr.invalidated,
 				 userptr.invalidate_link) {
 		list_del_init(&vma->userptr.invalidate_link);
-		list_move_tail(&vma->userptr_link, &vm->userptr.repin_list);
+		if (list_empty(&vma->combined_links.userptr))
+			list_move_tail(&vma->combined_links.userptr,
+				       &vm->userptr.repin_list);
 	}
 	spin_unlock(&vm->userptr.invalidated_lock);
 
 	/* Pin and move to temporary list */
-	list_for_each_entry_safe(vma, next, &vm->userptr.repin_list, userptr_link) {
+	list_for_each_entry_safe(vma, next, &vm->userptr.repin_list,
+				 combined_links.userptr) {
 		err = xe_vma_userptr_pin_pages(vma);
 		if (err < 0)
 			goto out_err;
 
-		list_move_tail(&vma->userptr_link, &tmp_evict);
+		list_move_tail(&vma->combined_links.userptr, &tmp_evict);
 	}
 
 	/* Take lock and move to rebind_list for rebinding. */
@@ -798,10 +802,8 @@ int xe_vm_userptr_pin(struct xe_vm *vm)
 	if (err)
 		goto out_err;
 
-	list_for_each_entry_safe(vma, next, &tmp_evict, userptr_link) {
-		list_del_init(&vma->userptr_link);
-		list_move_tail(&vma->rebind_link, &vm->rebind_list);
-	}
+	list_for_each_entry_safe(vma, next, &tmp_evict, combined_links.userptr)
+		list_move_tail(&vma->combined_links.rebind, &vm->rebind_list);
 
 	dma_resv_unlock(&vm->resv);
 
@@ -845,10 +847,11 @@ struct dma_fence *xe_vm_rebind(struct xe_vm *vm, bool rebind_worker)
 		return NULL;
 
 	xe_vm_assert_held(vm);
-	list_for_each_entry_safe(vma, next, &vm->rebind_list, rebind_link) {
+	list_for_each_entry_safe(vma, next, &vm->rebind_list,
+				 combined_links.rebind) {
 		XE_WARN_ON(!vma->tile_present);
 
-		list_del_init(&vma->rebind_link);
+		list_del_init(&vma->combined_links.rebind);
 		dma_fence_put(fence);
 		if (rebind_worker)
 			trace_xe_vma_rebind_worker(vma);
@@ -883,9 +886,7 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
 		return vma;
 	}
 
-	INIT_LIST_HEAD(&vma->rebind_link);
-	INIT_LIST_HEAD(&vma->unbind_link);
-	INIT_LIST_HEAD(&vma->userptr_link);
+	INIT_LIST_HEAD(&vma->combined_links.rebind);
 	INIT_LIST_HEAD(&vma->userptr.invalidate_link);
 	INIT_LIST_HEAD(&vma->notifier.rebind_link);
 	INIT_LIST_HEAD(&vma->extobj.link);
@@ -1058,7 +1059,7 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
 	struct xe_vm *vm = xe_vma_vm(vma);
 
 	lockdep_assert_held_write(&vm->lock);
-	XE_BUG_ON(!list_empty(&vma->unbind_link));
+	XE_BUG_ON(!list_empty(&vma->combined_links.destroy));
 
 	if (xe_vma_is_userptr(vma)) {
 		XE_WARN_ON(!(vma->gpuva.flags & XE_VMA_DESTROYED));
@@ -1066,7 +1067,6 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
 		spin_lock(&vm->userptr.invalidated_lock);
 		list_del_init(&vma->userptr.invalidate_link);
 		spin_unlock(&vm->userptr.invalidated_lock);
-		list_del(&vma->userptr_link);
 	} else if (!xe_vma_is_null(vma)) {
 		xe_bo_assert_held(xe_vma_bo(vma));
 
@@ -1087,9 +1087,6 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
 	}
 
 	xe_vm_assert_held(vm);
-	if (!list_empty(&vma->rebind_link))
-		list_del(&vma->rebind_link);
-
 	if (fence) {
 		int ret = dma_fence_add_callback(fence, &vma->destroy_cb,
 						 vma_destroy_cb);
@@ -1438,11 +1435,12 @@ void xe_vm_close_and_put(struct xe_vm *vm)
 
 		/* easy case, remove from VMA? */
 		if (xe_vma_has_no_bo(vma) || xe_vma_bo(vma)->vm) {
+			list_del_init(&vma->combined_links.rebind);
 			xe_vma_destroy(vma, NULL);
 			continue;
 		}
 
-		list_add_tail(&vma->unbind_link, &contested);
+		list_move_tail(&vma->combined_links.destroy, &contested);
 	}
 
 	/*
@@ -1470,8 +1468,9 @@ void xe_vm_close_and_put(struct xe_vm *vm)
 	 * Since we hold a refcount to the bo, we can remove and free
 	 * the members safely without locking.
 	 */
-	list_for_each_entry_safe(vma, next_vma, &contested, unbind_link) {
-		list_del_init(&vma->unbind_link);
+	list_for_each_entry_safe(vma, next_vma, &contested,
+				 combined_links.destroy) {
+		list_del_init(&vma->combined_links.destroy);
 		xe_vma_destroy_unlocked(vma);
 	}
 
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index 29ec04fa1e26..f1f3b619d996 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -49,21 +49,28 @@ struct xe_vma {
 	 */
 	u64 tile_present;
 
-	/** @userptr_link: link into VM repin list if userptr */
-	struct list_head userptr_link;
-
-	/**
-	 * @rebind_link: link into VM if this VMA needs rebinding, and
-	 * if it's a bo (not userptr) needs validation after a possible
-	 * eviction. Protected by the vm's resv lock.
-	 */
-	struct list_head rebind_link;
-
-	/**
-	 * @unbind_link: link or list head if an unbind of multiple VMAs, in
-	 * single unbind op, is being done.
-	 */
-	struct list_head unbind_link;
+	/** @combined_links: links into lists which are mutually exclusive */
+	union {
+		/**
+		 * @userptr: link into VM repin list if userptr. Protected by
+		 * vm->lock in write mode.
+		 */
+		struct list_head userptr;
+		/**
+		 * @rebind: link into VM if this VMA needs rebinding, and
+		 * if it's a bo (not userptr) needs validation after a possible
+		 * eviction. Protected by the vm's resv lock and typically
+		 * vm->lock is also held in write mode. The only place where
+		 * vm->lock isn't held is the BO eviction path which has
+		 * mutually exclusive execution with userptr.
+		 */
+		struct list_head rebind;
+		/**
+		 * @destroy: link to contested list when VM is being closed.
+		 * Protected by vm->lock in write mode and vm's resv lock.
+		 */
+		struct list_head destroy;
+	} combined_links;
 
 	/** @destroy_cb: callback to destroy VMA when unbind job is done */
 	struct dma_fence_cb destroy_cb;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-xe] [PATCH v2 3/6] drm/xe: Replace list_del_init with list_del for userptr.invalidate_link cleanup
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds Matthew Brost
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 2/6] drm/xe: Reduce the number list links in xe_vma Matthew Brost
@ 2023-07-20  4:10 ` Matthew Brost
  2023-07-20 15:53   ` Rodrigo Vivi
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 4/6] drm/xe: Change tile masks from u64 to u8 Matthew Brost
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe

This list isn't used again, list_del is the proper call.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_vm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 87faf4b723b3..1ed3bc2541f2 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1065,7 +1065,7 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
 		XE_WARN_ON(!(vma->gpuva.flags & XE_VMA_DESTROYED));
 
 		spin_lock(&vm->userptr.invalidated_lock);
-		list_del_init(&vma->userptr.invalidate_link);
+		list_del(&vma->userptr.invalidate_link);
 		spin_unlock(&vm->userptr.invalidated_lock);
 	} else if (!xe_vma_is_null(vma)) {
 		xe_bo_assert_held(xe_vma_bo(vma));
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-xe] [PATCH v2 4/6] drm/xe: Change tile masks from u64 to u8
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
                   ` (2 preceding siblings ...)
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 3/6] drm/xe: Replace list_del_init with list_del for userptr.invalidate_link cleanup Matthew Brost
@ 2023-07-20  4:10 ` Matthew Brost
  2023-07-20 15:48   ` Rodrigo Vivi
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 5/6] drm/xe: Combine destroy_cb and destroy_work in xe_vma into union Matthew Brost
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe

This will save us a few bytes in the xe_vma structure.

v2: Use hweight8 rather than hweight_long (Rodrigo)

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_vm.c       | 12 ++++++------
 drivers/gpu/drm/xe/xe_vm_types.h | 28 ++++++++++++++--------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 1ed3bc2541f2..92d97d347a44 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -871,7 +871,7 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
 				    u64 start, u64 end,
 				    bool read_only,
 				    bool is_null,
-				    u64 tile_mask)
+				    u8 tile_mask)
 {
 	struct xe_vma *vma;
 	struct xe_tile *tile;
@@ -1574,7 +1574,7 @@ xe_vm_unbind_vma(struct xe_vma *vma, struct xe_engine *e,
 	struct dma_fence_array *cf = NULL;
 	struct xe_vm *vm = xe_vma_vm(vma);
 	int cur_fence = 0, i;
-	int number_tiles = hweight_long(vma->tile_present);
+	int number_tiles = hweight8(vma->tile_present);
 	int err;
 	u8 id;
 
@@ -1649,7 +1649,7 @@ xe_vm_bind_vma(struct xe_vma *vma, struct xe_engine *e,
 	struct dma_fence_array *cf = NULL;
 	struct xe_vm *vm = xe_vma_vm(vma);
 	int cur_fence = 0, i;
-	int number_tiles = hweight_long(vma->tile_mask);
+	int number_tiles = hweight8(vma->tile_mask);
 	int err;
 	u8 id;
 
@@ -2245,7 +2245,7 @@ static void print_op(struct xe_device *xe, struct drm_gpuva_op *op)
 static struct drm_gpuva_ops *
 vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
 			 u64 bo_offset_or_userptr, u64 addr, u64 range,
-			 u32 operation, u64 tile_mask, u32 region)
+			 u32 operation, u8 tile_mask, u32 region)
 {
 	struct drm_gem_object *obj = bo ? &bo->ttm.base : NULL;
 	struct ww_acquire_ctx ww;
@@ -2342,7 +2342,7 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
 }
 
 static struct xe_vma *new_vma(struct xe_vm *vm, struct drm_gpuva_op_map *op,
-			      u64 tile_mask, bool read_only, bool is_null)
+			      u8 tile_mask, bool read_only, bool is_null)
 {
 	struct xe_bo *bo = op->gem.obj ? gem_to_xe_bo(op->gem.obj) : NULL;
 	struct xe_vma *vma;
@@ -3327,7 +3327,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		u64 addr = bind_ops[i].addr;
 		u32 op = bind_ops[i].op;
 		u64 obj_offset = bind_ops[i].obj_offset;
-		u64 tile_mask = bind_ops[i].tile_mask;
+		u8 tile_mask = bind_ops[i].tile_mask;
 		u32 region = bind_ops[i].region;
 
 		ops[i] = vm_bind_ioctl_ops_create(vm, bos[i], obj_offset,
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index f1f3b619d996..268d2475f2ae 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -37,18 +37,6 @@ struct xe_vma {
 	/** @gpuva: Base GPUVA object */
 	struct drm_gpuva gpuva;
 
-	/** @tile_mask: Tile mask of where to create binding for this VMA */
-	u64 tile_mask;
-
-	/**
-	 * @tile_present: GT mask of binding are present for this VMA.
-	 * protected by vm->lock, vm->resv and for userptrs,
-	 * vm->userptr.notifier_lock for writing. Needs either for reading,
-	 * but if reading is done under the vm->lock only, it needs to be held
-	 * in write mode.
-	 */
-	u64 tile_present;
-
 	/** @combined_links: links into lists which are mutually exclusive */
 	union {
 		/**
@@ -106,9 +94,21 @@ struct xe_vma {
 	/** @usm: unified shared memory state */
 	struct {
 		/** @tile_invalidated: VMA has been invalidated */
-		u64 tile_invalidated;
+		u8 tile_invalidated;
 	} usm;
 
+	/** @tile_mask: Tile mask of where to create binding for this VMA */
+	u8 tile_mask;
+
+	/**
+	 * @tile_present: GT mask of binding are present for this VMA.
+	 * protected by vm->lock, vm->resv and for userptrs,
+	 * vm->userptr.notifier_lock for writing. Needs either for reading,
+	 * but if reading is done under the vm->lock only, it needs to be held
+	 * in write mode.
+	 */
+	u8 tile_present;
+
 	struct {
 		struct list_head rebind_link;
 	} notifier;
@@ -395,7 +395,7 @@ struct xe_vma_op {
 	 */
 	struct async_op_fence *fence;
 	/** @tile_mask: gt mask for this operation */
-	u64 tile_mask;
+	u8 tile_mask;
 	/** @flags: operation flags */
 	enum xe_vma_op_flags flags;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-xe] [PATCH v2 5/6] drm/xe: Combine destroy_cb and destroy_work in xe_vma into union
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
                   ` (3 preceding siblings ...)
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 4/6] drm/xe: Change tile masks from u64 to u8 Matthew Brost
@ 2023-07-20  4:10 ` Matthew Brost
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 6/6] drm/xe: Only alloc userptr part of xe_vma for userptrs Matthew Brost
  2023-07-20  4:13 ` [Intel-xe] ✗ CI.Patch_applied: failure for More VM cleanups (rev2) Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe; +Cc: Rodrigo Vivi

The callback kicks the worker thus mutually exclusive execution,
combining saves a bit of space in xe_vma.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_vm_types.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index 268d2475f2ae..809cfe734d43 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -60,11 +60,12 @@ struct xe_vma {
 		struct list_head destroy;
 	} combined_links;
 
-	/** @destroy_cb: callback to destroy VMA when unbind job is done */
-	struct dma_fence_cb destroy_cb;
-
-	/** @destroy_work: worker to destroy this BO */
-	struct work_struct destroy_work;
+	union {
+		/** @destroy_cb: callback to destroy VMA when unbind job is done */
+		struct dma_fence_cb destroy_cb;
+		/** @destroy_work: worker to destroy this BO */
+		struct work_struct destroy_work;
+	};
 
 	/** @userptr: user pointer state */
 	struct {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-xe] [PATCH v2 6/6] drm/xe: Only alloc userptr part of xe_vma for userptrs
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
                   ` (4 preceding siblings ...)
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 5/6] drm/xe: Combine destroy_cb and destroy_work in xe_vma into union Matthew Brost
@ 2023-07-20  4:10 ` Matthew Brost
  2023-07-20  4:13 ` [Intel-xe] ✗ CI.Patch_applied: failure for More VM cleanups (rev2) Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Matthew Brost @ 2023-07-20  4:10 UTC (permalink / raw)
  To: intel-xe; +Cc: Rodrigo Vivi

Only alloc userptr part of xe_vma for userptrs, this will save on space
in the common BO case.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_vm.c       |  6 +++-
 drivers/gpu/drm/xe/xe_vm_types.h | 56 ++++++++++++++++++--------------
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 92d97d347a44..766830d21e01 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -880,7 +880,11 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
 	XE_BUG_ON(start >= end);
 	XE_BUG_ON(end >= vm->size);
 
-	vma = kzalloc(sizeof(*vma), GFP_KERNEL);
+	if (!bo && !is_null)	/* userptr */
+		vma = kzalloc(sizeof(*vma), GFP_KERNEL);
+	else
+		vma = kzalloc(sizeof(*vma) - sizeof(struct xe_userptr),
+			      GFP_KERNEL);
 	if (!vma) {
 		vma = ERR_PTR(-ENOMEM);
 		return vma;
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index 809cfe734d43..621cf13d0a7d 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -33,6 +33,31 @@ struct xe_vm;
 #define XE_VMA_PTE_2M		(DRM_GPUVA_USERBITS << 6)
 #define XE_VMA_PTE_1G		(DRM_GPUVA_USERBITS << 7)
 
+/** struct xe_userptr - User pointer */
+struct xe_userptr {
+	/** @invalidate_link: Link for the vm::userptr.invalidated list */
+	struct list_head invalidate_link;
+	/**
+	 * @notifier: MMU notifier for user pointer (invalidation call back)
+	 */
+	struct mmu_interval_notifier notifier;
+	/** @sgt: storage for a scatter gather table */
+	struct sg_table sgt;
+	/** @sg: allocated scatter gather table */
+	struct sg_table *sg;
+	/** @notifier_seq: notifier sequence number */
+	unsigned long notifier_seq;
+	/**
+	 * @initial_bind: user pointer has been bound at least once.
+	 * write: vm->userptr.notifier_lock in read mode and vm->resv held.
+	 * read: vm->userptr.notifier_lock in write mode or vm->resv held.
+	 */
+	bool initial_bind;
+#if IS_ENABLED(CONFIG_DRM_XE_USERPTR_INVAL_INJECT)
+	u32 divisor;
+#endif
+};
+
 struct xe_vma {
 	/** @gpuva: Base GPUVA object */
 	struct drm_gpuva gpuva;
@@ -67,31 +92,6 @@ struct xe_vma {
 		struct work_struct destroy_work;
 	};
 
-	/** @userptr: user pointer state */
-	struct {
-		/** @invalidate_link: Link for the vm::userptr.invalidated list */
-		struct list_head invalidate_link;
-		/**
-		 * @notifier: MMU notifier for user pointer (invalidation call back)
-		 */
-		struct mmu_interval_notifier notifier;
-		/** @sgt: storage for a scatter gather table */
-		struct sg_table sgt;
-		/** @sg: allocated scatter gather table */
-		struct sg_table *sg;
-		/** @notifier_seq: notifier sequence number */
-		unsigned long notifier_seq;
-		/**
-		 * @initial_bind: user pointer has been bound at least once.
-		 * write: vm->userptr.notifier_lock in read mode and vm->resv held.
-		 * read: vm->userptr.notifier_lock in write mode or vm->resv held.
-		 */
-		bool initial_bind;
-#if IS_ENABLED(CONFIG_DRM_XE_USERPTR_INVAL_INJECT)
-		u32 divisor;
-#endif
-	} userptr;
-
 	/** @usm: unified shared memory state */
 	struct {
 		/** @tile_invalidated: VMA has been invalidated */
@@ -121,6 +121,12 @@ struct xe_vma {
 		 */
 		struct list_head link;
 	} extobj;
+
+	/**
+	 * @userptr: user pointer state, only allocated for VMAs that are
+	 * user pointers
+	 */
+	struct xe_userptr userptr;
 };
 
 struct xe_device;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-xe] ✗ CI.Patch_applied: failure for More VM cleanups (rev2)
  2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
                   ` (5 preceding siblings ...)
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 6/6] drm/xe: Only alloc userptr part of xe_vma for userptrs Matthew Brost
@ 2023-07-20  4:13 ` Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2023-07-20  4:13 UTC (permalink / raw)
  To: Matthew Brost; +Cc: intel-xe

== Series Details ==

Series: More VM cleanups (rev2)
URL   : https://patchwork.freedesktop.org/series/120574/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-xe-next' with base: ===
Base commit: f5459dca4 drm/xe: Cleanup style warnings and errors
=== git am output follows ===
error: patch failed: drivers/gpu/drm/xe/xe_vm.c:2499
error: drivers/gpu/drm/xe/xe_vm.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch
Applying: drm/xe: Avoid doing rebinds
Patch failed at 0001 drm/xe: Avoid doing rebinds
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-xe] [PATCH v2 4/6] drm/xe: Change tile masks from u64 to u8
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 4/6] drm/xe: Change tile masks from u64 to u8 Matthew Brost
@ 2023-07-20 15:48   ` Rodrigo Vivi
  0 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2023-07-20 15:48 UTC (permalink / raw)
  To: Matthew Brost; +Cc: intel-xe

On Wed, Jul 19, 2023 at 09:10:55PM -0700, Matthew Brost wrote:
> This will save us a few bytes in the xe_vma structure.
> 
> v2: Use hweight8 rather than hweight_long (Rodrigo)
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_vm.c       | 12 ++++++------
>  drivers/gpu/drm/xe/xe_vm_types.h | 28 ++++++++++++++--------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 1ed3bc2541f2..92d97d347a44 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -871,7 +871,7 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
>  				    u64 start, u64 end,
>  				    bool read_only,
>  				    bool is_null,
> -				    u64 tile_mask)
> +				    u8 tile_mask)
>  {
>  	struct xe_vma *vma;
>  	struct xe_tile *tile;
> @@ -1574,7 +1574,7 @@ xe_vm_unbind_vma(struct xe_vma *vma, struct xe_engine *e,
>  	struct dma_fence_array *cf = NULL;
>  	struct xe_vm *vm = xe_vma_vm(vma);
>  	int cur_fence = 0, i;
> -	int number_tiles = hweight_long(vma->tile_present);
> +	int number_tiles = hweight8(vma->tile_present);
>  	int err;
>  	u8 id;
>  
> @@ -1649,7 +1649,7 @@ xe_vm_bind_vma(struct xe_vma *vma, struct xe_engine *e,
>  	struct dma_fence_array *cf = NULL;
>  	struct xe_vm *vm = xe_vma_vm(vma);
>  	int cur_fence = 0, i;
> -	int number_tiles = hweight_long(vma->tile_mask);
> +	int number_tiles = hweight8(vma->tile_mask);
>  	int err;
>  	u8 id;
>  
> @@ -2245,7 +2245,7 @@ static void print_op(struct xe_device *xe, struct drm_gpuva_op *op)
>  static struct drm_gpuva_ops *
>  vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
>  			 u64 bo_offset_or_userptr, u64 addr, u64 range,
> -			 u32 operation, u64 tile_mask, u32 region)
> +			 u32 operation, u8 tile_mask, u32 region)
>  {
>  	struct drm_gem_object *obj = bo ? &bo->ttm.base : NULL;
>  	struct ww_acquire_ctx ww;
> @@ -2342,7 +2342,7 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
>  }
>  
>  static struct xe_vma *new_vma(struct xe_vm *vm, struct drm_gpuva_op_map *op,
> -			      u64 tile_mask, bool read_only, bool is_null)
> +			      u8 tile_mask, bool read_only, bool is_null)
>  {
>  	struct xe_bo *bo = op->gem.obj ? gem_to_xe_bo(op->gem.obj) : NULL;
>  	struct xe_vma *vma;
> @@ -3327,7 +3327,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  		u64 addr = bind_ops[i].addr;
>  		u32 op = bind_ops[i].op;
>  		u64 obj_offset = bind_ops[i].obj_offset;
> -		u64 tile_mask = bind_ops[i].tile_mask;
> +		u8 tile_mask = bind_ops[i].tile_mask;
>  		u32 region = bind_ops[i].region;
>  
>  		ops[i] = vm_bind_ioctl_ops_create(vm, bos[i], obj_offset,
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index f1f3b619d996..268d2475f2ae 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -37,18 +37,6 @@ struct xe_vma {
>  	/** @gpuva: Base GPUVA object */
>  	struct drm_gpuva gpuva;
>  
> -	/** @tile_mask: Tile mask of where to create binding for this VMA */
> -	u64 tile_mask;
> -
> -	/**
> -	 * @tile_present: GT mask of binding are present for this VMA.
> -	 * protected by vm->lock, vm->resv and for userptrs,
> -	 * vm->userptr.notifier_lock for writing. Needs either for reading,
> -	 * but if reading is done under the vm->lock only, it needs to be held
> -	 * in write mode.
> -	 */
> -	u64 tile_present;
> -
>  	/** @combined_links: links into lists which are mutually exclusive */
>  	union {
>  		/**
> @@ -106,9 +94,21 @@ struct xe_vma {
>  	/** @usm: unified shared memory state */
>  	struct {
>  		/** @tile_invalidated: VMA has been invalidated */
> -		u64 tile_invalidated;
> +		u8 tile_invalidated;
>  	} usm;
>  
> +	/** @tile_mask: Tile mask of where to create binding for this VMA */
> +	u8 tile_mask;
> +
> +	/**
> +	 * @tile_present: GT mask of binding are present for this VMA.
> +	 * protected by vm->lock, vm->resv and for userptrs,
> +	 * vm->userptr.notifier_lock for writing. Needs either for reading,
> +	 * but if reading is done under the vm->lock only, it needs to be held
> +	 * in write mode.
> +	 */
> +	u8 tile_present;
> +
>  	struct {
>  		struct list_head rebind_link;
>  	} notifier;
> @@ -395,7 +395,7 @@ struct xe_vma_op {
>  	 */
>  	struct async_op_fence *fence;
>  	/** @tile_mask: gt mask for this operation */
> -	u64 tile_mask;
> +	u8 tile_mask;
>  	/** @flags: operation flags */
>  	enum xe_vma_op_flags flags;
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-xe] [PATCH v2 3/6] drm/xe: Replace list_del_init with list_del for userptr.invalidate_link cleanup
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 3/6] drm/xe: Replace list_del_init with list_del for userptr.invalidate_link cleanup Matthew Brost
@ 2023-07-20 15:53   ` Rodrigo Vivi
  0 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2023-07-20 15:53 UTC (permalink / raw)
  To: Matthew Brost; +Cc: intel-xe

On Wed, Jul 19, 2023 at 09:10:54PM -0700, Matthew Brost wrote:
> This list isn't used again, list_del is the proper call.
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_vm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 87faf4b723b3..1ed3bc2541f2 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1065,7 +1065,7 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
>  		XE_WARN_ON(!(vma->gpuva.flags & XE_VMA_DESTROYED));
>  
>  		spin_lock(&vm->userptr.invalidated_lock);
> -		list_del_init(&vma->userptr.invalidate_link);
> +		list_del(&vma->userptr.invalidate_link);
>  		spin_unlock(&vm->userptr.invalidated_lock);
>  	} else if (!xe_vma_is_null(vma)) {
>  		xe_bo_assert_held(xe_vma_bo(vma));
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds
  2023-07-20  4:10 ` [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds Matthew Brost
@ 2023-07-20 15:56   ` Rodrigo Vivi
  0 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2023-07-20 15:56 UTC (permalink / raw)
  To: Matthew Brost; +Cc: intel-xe

On Wed, Jul 19, 2023 at 09:10:52PM -0700, Matthew Brost wrote:
> If we dont change page sizes we can avoid doing rebinds rather just do a
> partial unbind. The algorithm to determine its page size is greedy as we
> assume all pages in the removed VMA are the largest page used in the
> VMA.
> 
> v2: Don't exceed 100 lines
> v3: struct xe_vma_op_unmap remove in different patch, remove XXX comment

thanks

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pt.c       |  1 +
>  drivers/gpu/drm/xe/xe_vm.c       | 72 +++++++++++++++++++++++++++-----
>  drivers/gpu/drm/xe/xe_vm_types.h |  7 ++++
>  3 files changed, 70 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 851ea7c01b91..bbf37e5c8abb 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -668,6 +668,7 @@ xe_pt_stage_bind_entry(struct xe_ptw *parent, pgoff_t offset,
>  		if (!is_null)
>  			xe_res_next(curs, next - addr);
>  		xe_walk->va_curs_start = next;
> +		xe_walk->vma->gpuva.flags |= (XE_VMA_PTE_4K << level);
>  		*action = ACTION_CONTINUE;
>  
>  		return ret;
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 87907a97a8c3..19ce6b0c6564 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2384,6 +2384,16 @@ static struct xe_vma *new_vma(struct xe_vm *vm, struct drm_gpuva_op_map *op,
>  	return vma;
>  }
>  
> +static u64 xe_vma_max_pte_size(struct xe_vma *vma)
> +{
> +	if (vma->gpuva.flags & XE_VMA_PTE_1G)
> +		return SZ_1G;
> +	else if (vma->gpuva.flags & XE_VMA_PTE_2M)
> +		return SZ_2M;
> +
> +	return SZ_4K;
> +}
> +
>  /*
>   * Parse operations list and create any resources needed for the operations
>   * prior to fully committing to the operations. This setup can fail.
> @@ -2460,6 +2470,13 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
>  				break;
>  			}
>  			case DRM_GPUVA_OP_REMAP:
> +			{
> +				struct xe_vma *old =
> +					gpuva_to_vma(op->base.remap.unmap->va);
> +
> +				op->remap.start = xe_vma_start(old);
> +				op->remap.range = xe_vma_size(old);
> +
>  				if (op->base.remap.prev) {
>  					struct xe_vma *vma;
>  					bool read_only =
> @@ -2478,6 +2495,20 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
>  					}
>  
>  					op->remap.prev = vma;
> +
> +					/*
> +					 * Userptr creates a new SG mapping so
> +					 * we must also rebind.
> +					 */
> +					op->remap.skip_prev = !xe_vma_is_userptr(old) &&
> +						IS_ALIGNED(xe_vma_end(vma),
> +							   xe_vma_max_pte_size(old));
> +					if (op->remap.skip_prev) {
> +						op->remap.range -=
> +							xe_vma_end(vma) -
> +							xe_vma_start(old);
> +						op->remap.start = xe_vma_end(vma);
> +					}
>  				}
>  
>  				if (op->base.remap.next) {
> @@ -2499,14 +2530,21 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
>  					}
>  
>  					op->remap.next = vma;
> -				}
>  
> -				/* XXX: Support no doing remaps */
> -				op->remap.start =
> -					xe_vma_start(gpuva_to_vma(op->base.remap.unmap->va));
> -				op->remap.range =
> -					xe_vma_size(gpuva_to_vma(op->base.remap.unmap->va));
> +					/*
> +					 * Userptr creates a new SG mapping so
> +					 * we must also rebind.
> +					 */
> +					op->remap.skip_next = !xe_vma_is_userptr(old) &&
> +						IS_ALIGNED(xe_vma_start(vma),
> +							   xe_vma_max_pte_size(old));
> +					if (op->remap.skip_next)
> +						op->remap.range -=
> +							xe_vma_end(old) -
> +							xe_vma_start(vma);
> +				}
>  				break;
> +			}
>  			case DRM_GPUVA_OP_UNMAP:
>  			case DRM_GPUVA_OP_PREFETCH:
>  				/* Nothing to do */
> @@ -2549,10 +2587,23 @@ static int xe_vma_op_commit(struct xe_vm *vm, struct xe_vma_op *op)
>  	case DRM_GPUVA_OP_REMAP:
>  		prep_vma_destroy(vm, gpuva_to_vma(op->base.remap.unmap->va),
>  				 true);
> -		if (op->remap.prev)
> +
> +		if (op->remap.prev) {
>  			err |= xe_vm_insert_vma(vm, op->remap.prev);
> -		if (op->remap.next)
> +			if (!err && op->remap.skip_prev)
> +				op->remap.prev = NULL;
> +		}
> +		if (op->remap.next) {
>  			err |= xe_vm_insert_vma(vm, op->remap.next);
> +			if (!err && op->remap.skip_next)
> +				op->remap.next = NULL;
> +		}
> +
> +		/* Adjust for partial unbind after removin VMA from VM */
> +		if (!err) {
> +			op->base.remap.unmap->va->va.addr = op->remap.start;
> +			op->base.remap.unmap->va->va.range = op->remap.range;
> +		}
>  		break;
>  	case DRM_GPUVA_OP_UNMAP:
>  		prep_vma_destroy(vm, gpuva_to_vma(op->base.unmap.va), true);
> @@ -2622,9 +2673,10 @@ static int __xe_vma_op_execute(struct xe_vm *vm, struct xe_vma *vma,
>  		bool next = !!op->remap.next;
>  
>  		if (!op->remap.unmap_done) {
> -			vm->async_ops.munmap_rebind_inflight = true;
> -			if (prev || next)
> +			if (prev || next) {
> +				vm->async_ops.munmap_rebind_inflight = true;
>  				vma->gpuva.flags |= XE_VMA_FIRST_REBIND;
> +			}
>  			err = xe_vm_unbind(vm, vma, op->engine, op->syncs,
>  					   op->num_syncs,
>  					   !prev && !next ? op->fence : NULL,
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index dbe6aed6d2cf..29ec04fa1e26 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -29,6 +29,9 @@ struct xe_vm;
>  #define XE_VMA_ATOMIC_PTE_BIT	(DRM_GPUVA_USERBITS << 2)
>  #define XE_VMA_FIRST_REBIND	(DRM_GPUVA_USERBITS << 3)
>  #define XE_VMA_LAST_REBIND	(DRM_GPUVA_USERBITS << 4)
> +#define XE_VMA_PTE_4K		(DRM_GPUVA_USERBITS << 5)
> +#define XE_VMA_PTE_2M		(DRM_GPUVA_USERBITS << 6)
> +#define XE_VMA_PTE_1G		(DRM_GPUVA_USERBITS << 7)
>  
>  struct xe_vma {
>  	/** @gpuva: Base GPUVA object */
> @@ -336,6 +339,10 @@ struct xe_vma_op_remap {
>  	u64 start;
>  	/** @range: range of the VMA unmap */
>  	u64 range;
> +	/** @skip_prev: skip prev rebind */
> +	bool skip_prev;
> +	/** @skip_next: skip next rebind */
> +	bool skip_next;
>  	/** @unmap_done: unmap operation in done */
>  	bool unmap_done;
>  };
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-07-20 15:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-20  4:10 [Intel-xe] [PATCH v2 0/6] More VM cleanups Matthew Brost
2023-07-20  4:10 ` [Intel-xe] [PATCH v2 1/6] drm/xe: Avoid doing rebinds Matthew Brost
2023-07-20 15:56   ` Rodrigo Vivi
2023-07-20  4:10 ` [Intel-xe] [PATCH v2 2/6] drm/xe: Reduce the number list links in xe_vma Matthew Brost
2023-07-20  4:10 ` [Intel-xe] [PATCH v2 3/6] drm/xe: Replace list_del_init with list_del for userptr.invalidate_link cleanup Matthew Brost
2023-07-20 15:53   ` Rodrigo Vivi
2023-07-20  4:10 ` [Intel-xe] [PATCH v2 4/6] drm/xe: Change tile masks from u64 to u8 Matthew Brost
2023-07-20 15:48   ` Rodrigo Vivi
2023-07-20  4:10 ` [Intel-xe] [PATCH v2 5/6] drm/xe: Combine destroy_cb and destroy_work in xe_vma into union Matthew Brost
2023-07-20  4:10 ` [Intel-xe] [PATCH v2 6/6] drm/xe: Only alloc userptr part of xe_vma for userptrs Matthew Brost
2023-07-20  4:13 ` [Intel-xe] ✗ CI.Patch_applied: failure for More VM cleanups (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.