[PATCH 1/6] drm/amdgpu: add amdgpu_bo_vm bo type

[PATCH 1/6] drm/amdgpu: add amdgpu_bo_vm bo type

[Nirmoy Das]

Add new BO subclass that will be used by amdgpu vm code.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 32 ++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 +++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 3f85ba8222ef..6870cc297ae6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -694,6 +694,38 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 	*ubo_ptr = to_amdgpu_bo_user(bo_ptr);
 	return r;
 }
+
+/**
+ * amdgpu_bo_create_vm - create an &amdgpu_bo_vm buffer object
+ * @adev: amdgpu device object
+ * @bp: parameters to be used for the buffer object
+ * @vmbo_ptr: pointer to the buffer object pointer
+ *
+ * Create a BO to be for GPUVM.
+ *
+ * Returns:
+ * 0 for success or a negative error code on failure.
+ */
+
+int amdgpu_bo_create_vm(struct amdgpu_device *adev,
+			struct amdgpu_bo_param *bp,
+			struct amdgpu_bo_vm **vmbo_ptr)
+{
+	struct amdgpu_bo *bo_ptr;
+	int r;
+
+	/* bo_ptr_size will be determined by the caller and it depends on
+	 * num of amdgpu_vm_pt entries.
+	 */
+	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+	r = amdgpu_bo_create(adev, bp, &bo_ptr);
+	if (r)
+		return r;
+
+	*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);
+	return r;
+}
+
 /**
  * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
  * @bo: pointer to the buffer object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 11480c5a2716..a7fbf5f7051e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -44,6 +44,7 @@
 #define AMDGPU_AMDKFD_CREATE_SVM_BO	(1ULL << 62)
 
 #define to_amdgpu_bo_user(abo) container_of((abo), struct amdgpu_bo_user, bo)
+#define to_amdgpu_bo_vm(abo) container_of((abo), struct amdgpu_bo_vm, bo)
 
 struct amdgpu_bo_param {
 	unsigned long			size;
@@ -125,6 +126,12 @@ struct amdgpu_bo_user {
 
 };
 
+struct amdgpu_bo_vm {
+	struct amdgpu_bo		bo;
+	struct amdgpu_bo		*shadow;
+	struct amdgpu_vm_pt             entries[];
+};
+
 static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)
 {
 	return container_of(tbo, struct amdgpu_bo, tbo);
@@ -272,6 +279,9 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
 int amdgpu_bo_create_user(struct amdgpu_device *adev,
 			  struct amdgpu_bo_param *bp,
 			  struct amdgpu_bo_user **ubo_ptr);
+int amdgpu_bo_create_vm(struct amdgpu_device *adev,
+			struct amdgpu_bo_param *bp,
+			struct amdgpu_bo_vm **ubo_ptr);
 void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
 			   void **cpu_addr);
 int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 2/6] drm/amdgpu: move shadow bo validation to VM code

[Nirmoy Das]

Do the shadow bo validation in the VM code as
VM code knows/owns shadow BOs.

v2: Fix a typo.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 23 ++++-------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  5 +++++
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 90136f9dedd6..f6a8f0c5a52f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -396,10 +396,10 @@ void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
 	spin_unlock(&adev->mm_stats.lock);
 }

-static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
-				 struct amdgpu_bo *bo)
+static int amdgpu_cs_bo_validate(void *param, struct amdgpu_bo *bo)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	struct amdgpu_cs_parser *p = param;
 	struct ttm_operation_ctx ctx = {
 		.interruptible = true,
 		.no_wait_gpu = false,
@@ -451,21 +451,6 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
 	return r;
 }

-static int amdgpu_cs_validate(void *param, struct amdgpu_bo *bo)
-{
-	struct amdgpu_cs_parser *p = param;
-	int r;
-
-	r = amdgpu_cs_bo_validate(p, bo);
-	if (r)
-		return r;
-
-	if (bo->shadow)
-		r = amdgpu_cs_bo_validate(p, bo->shadow);
-
-	return r;
-}
-
 static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
 			    struct list_head *validated)
 {
@@ -493,7 +478,7 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
 						     lobj->user_pages);
 		}

-		r = amdgpu_cs_validate(p, bo);
+		r = amdgpu_cs_bo_validate(p, bo);
 		if (r)
 			return r;

@@ -593,7 +578,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	p->bytes_moved_vis = 0;

 	r = amdgpu_vm_validate_pt_bos(p->adev, &fpriv->vm,
-				      amdgpu_cs_validate, p);
+				      amdgpu_cs_bo_validate, p);
 	if (r) {
 		DRM_ERROR("amdgpu_vm_validate_pt_bos() failed.\n");
 		goto error_validate;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index da155c276c51..6bc7566cc193 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -696,6 +696,11 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		r = validate(param, bo);
 		if (r)
 			return r;
+		if (bo->shadow) {
+			r = validate(param, bo->shadow);
+			if (r)
+				return r;
+		}

 		if (bo->tbo.type != ttm_bo_type_kernel) {
 			amdgpu_vm_bo_moved(bo_base);
--
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/6] drm/admgpu: add two shadow BO helper functions

[Nirmoy Das]

Add amdgpu_bo_add_to_shadow_list() to handle shadow list
additions and amdgpu_bo_shadowed() to check if a BO is shadowed.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 16 ++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 17 +++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6870cc297ae6..a63b450cd603 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -760,6 +760,22 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
 	return r;
 }
 
+/**
+ * amdgpu_bo_add_to_shadow_list - add a BO to the shadow list
+ *
+ * @bo: BO that will be inserted into the shadow list
+ *
+ * Insert a BO to the shadow list.
+ */
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+	mutex_lock(&adev->shadow_list_lock);
+	list_add_tail(&bo->shadow_list, &adev->shadow_list);
+	mutex_unlock(&adev->shadow_list_lock);
+}
+
 /**
  * amdgpu_bo_restore_shadow - restore an &amdgpu_bo shadow
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index a7fbf5f7051e..9afccf6c66f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -259,6 +259,22 @@ static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
 	return bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED;
 }
 
+/**
+ * amdgpu_bo_shadowed - check if the BO is shadowed
+ *
+ * @bo: BO to be tested.
+ *
+ * Returns:
+ * NULL if not shadowed or else return a BO pointer.
+ */
+static inline struct amdgpu_bo *amdgpu_bo_shadowed(struct amdgpu_bo *bo)
+{
+	if (bo->tbo.type == ttm_bo_type_kernel)
+		return to_amdgpu_bo_vm(bo)->shadow;
+
+	return NULL;
+}
+
 bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
 void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
 
@@ -322,6 +338,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
 int amdgpu_bo_validate(struct amdgpu_bo *bo);
 void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
 				uint64_t *gtt_mem, uint64_t *cpu_mem);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
 int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 			     struct dma_fence **fence);
 uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v3 4/6] drm/amdgpu: switch to amdgpu_bo_vm for vm code

[Nirmoy Das]

The subclass, amdgpu_bo_vm is intended for PT/PD BOs which are also
shadowed, so switch to amdgpu_bo_vm BO for PT/PD BOs.

v3: simplify code.
    check also if shadow bo exist, instead of checking only bo's type.
v2: squash three related patches.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c      | 93 +++++++++++++--------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 18 ++--
 2 files changed, 68 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6bc7566cc193..d723873df765 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -652,15 +652,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
 	spin_lock(&adev->mman.bdev.lru_lock);
 	list_for_each_entry(bo_base, &vm->idle, vm_status) {
 		struct amdgpu_bo *bo = bo_base->bo;
+		struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);

 		if (!bo->parent)
 			continue;

 		ttm_bo_move_to_lru_tail(&bo->tbo, &bo->tbo.mem,
 					&vm->lru_bulk_move);
-		if (bo->shadow)
-			ttm_bo_move_to_lru_tail(&bo->shadow->tbo,
-						&bo->shadow->tbo.mem,
+		if (shadow)
+			ttm_bo_move_to_lru_tail(&shadow->tbo, &shadow->tbo.mem,
 						&vm->lru_bulk_move);
 	}
 	spin_unlock(&adev->mman.bdev.lru_lock);
@@ -692,12 +692,13 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,

 	list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
 		struct amdgpu_bo *bo = bo_base->bo;
+		struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);

 		r = validate(param, bo);
 		if (r)
 			return r;
-		if (bo->shadow) {
-			r = validate(param, bo->shadow);
+		if (shadow) {
+			r = validate(param, shadow);
 			if (r)
 				return r;
 		}
@@ -754,6 +755,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
 	unsigned level = adev->vm_manager.root_level;
 	struct amdgpu_vm_update_params params;
 	struct amdgpu_bo *ancestor = bo;
+	struct amdgpu_bo *shadow;
 	unsigned entries, ats_entries;
 	uint64_t addr;
 	int r;
@@ -793,9 +795,9 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
 	if (r)
 		return r;

-	if (bo->shadow) {
-		r = ttm_bo_validate(&bo->shadow->tbo, &bo->shadow->placement,
-				    &ctx);
+	shadow = amdgpu_bo_shadowed(bo);
+	if (shadow) {
+		r = ttm_bo_validate(&shadow->tbo, &shadow->placement, &ctx);
 		if (r)
 			return r;
 	}
@@ -863,14 +865,16 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
  * @vm: requesting vm
  * @level: the page table level
  * @immediate: use a immediate update
- * @bo: pointer to the buffer object pointer
+ * @vmbo: pointer to the buffer object pointer
  */
 static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
 			       struct amdgpu_vm *vm,
 			       int level, bool immediate,
-			       struct amdgpu_bo **bo)
+			       struct amdgpu_bo_vm **vmbo)
 {
 	struct amdgpu_bo_param bp;
+	struct amdgpu_bo *bo;
+	struct dma_resv *resv;
 	int r;

 	memset(&bp, 0, sizeof(bp));
@@ -881,7 +885,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
 	bp.domain = amdgpu_bo_get_preferred_pin_domain(adev, bp.domain);
 	bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
 		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
+	bp.bo_ptr_size = sizeof(struct amdgpu_bo_vm);
 	if (vm->use_cpu_for_update)
 		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;

@@ -890,26 +894,41 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
 	if (vm->root.base.bo)
 		bp.resv = vm->root.base.bo->tbo.base.resv;

-	r = amdgpu_bo_create(adev, &bp, bo);
+	r = amdgpu_bo_create_vm(adev, &bp, vmbo);
 	if (r)
 		return r;

-	if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
+	bo = &(*vmbo)->bo;
+	if (vm->is_compute_context && (adev->flags & AMD_IS_APU)) {
+		(*vmbo)->shadow = NULL;
 		return 0;
+	}

 	if (!bp.resv)
-		WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
+		WARN_ON(dma_resv_lock(bo->tbo.base.resv,
 				      NULL));
-	r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
+	resv = bp.resv;
+	memset(&bp, 0, sizeof(bp));
+	bp.size = amdgpu_vm_bo_size(adev, level);
+	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
+	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+	bp.type = ttm_bo_type_kernel;
+	bp.resv = bo->tbo.base.resv;
+	bp.bo_ptr_size = sizeof(struct amdgpu_bo);

-	if (!bp.resv)
-		dma_resv_unlock((*bo)->tbo.base.resv);
+	r = amdgpu_bo_create(adev, &bp, &(*vmbo)->shadow);
+
+	if (!resv)
+		dma_resv_unlock(bo->tbo.base.resv);

 	if (r) {
-		amdgpu_bo_unref(bo);
+		amdgpu_bo_unref(&bo);
 		return r;
 	}

+	(*vmbo)->shadow->parent = amdgpu_bo_ref(bo);
+	amdgpu_bo_add_to_shadow_list((*vmbo)->shadow);
+
 	return 0;
 }

@@ -933,7 +952,8 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 			       bool immediate)
 {
 	struct amdgpu_vm_pt *entry = cursor->entry;
-	struct amdgpu_bo *pt;
+	struct amdgpu_bo *pt_bo;
+	struct amdgpu_bo_vm *pt;
 	int r;

 	if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
@@ -957,10 +977,11 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 	/* Keep a reference to the root directory to avoid
 	 * freeing them up in the wrong order.
 	 */
-	pt->parent = amdgpu_bo_ref(cursor->parent->base.bo);
-	amdgpu_vm_bo_base_init(&entry->base, vm, pt);
+	pt_bo = &pt->bo;
+	pt_bo->parent = amdgpu_bo_ref(cursor->parent->base.bo);
+	amdgpu_vm_bo_base_init(&entry->base, vm, pt_bo);

-	r = amdgpu_vm_clear_bo(adev, vm, pt, immediate);
+	r = amdgpu_vm_clear_bo(adev, vm, pt_bo, immediate);
 	if (r)
 		goto error_free_pt;

@@ -968,7 +989,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,

 error_free_pt:
 	amdgpu_bo_unref(&pt->shadow);
-	amdgpu_bo_unref(&pt);
+	amdgpu_bo_unref(&pt_bo);
 	return r;
 }

@@ -979,10 +1000,13 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
  */
 static void amdgpu_vm_free_table(struct amdgpu_vm_pt *entry)
 {
+	struct amdgpu_bo *shadow;
+
 	if (entry->base.bo) {
+		shadow = amdgpu_bo_shadowed(entry->base.bo);
 		entry->base.bo->vm_bo = NULL;
 		list_del(&entry->base.vm_status);
-		amdgpu_bo_unref(&entry->base.bo->shadow);
+		amdgpu_bo_unref(&shadow);
 		amdgpu_bo_unref(&entry->base.bo);
 	}
 	kvfree(entry->entries);
@@ -2674,7 +2698,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
 	struct amdgpu_vm_bo_base *bo_base;

 	/* shadow bo doesn't have bo base, its validation needs its parent */
-	if (bo->parent && bo->parent->shadow == bo)
+	if (bo->parent && (amdgpu_bo_shadowed(bo->parent) == bo))
 		bo = bo->parent;

 	for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
@@ -2843,7 +2867,8 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
  */
 int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
 {
-	struct amdgpu_bo *root;
+	struct amdgpu_bo *root_bo;
+	struct amdgpu_bo_vm *root;
 	int r, i;

 	vm->va = RB_ROOT_CACHED;
@@ -2897,18 +2922,18 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
 				false, &root);
 	if (r)
 		goto error_free_delayed;
-
-	r = amdgpu_bo_reserve(root, true);
+	root_bo = &root->bo;
+	r = amdgpu_bo_reserve(root_bo, true);
 	if (r)
 		goto error_free_root;

-	r = dma_resv_reserve_shared(root->tbo.base.resv, 1);
+	r = dma_resv_reserve_shared(root_bo->tbo.base.resv, 1);
 	if (r)
 		goto error_unreserve;

-	amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
+	amdgpu_vm_bo_base_init(&vm->root.base, vm, root_bo);

-	r = amdgpu_vm_clear_bo(adev, vm, root, false);
+	r = amdgpu_vm_clear_bo(adev, vm, root_bo, false);
 	if (r)
 		goto error_unreserve;

@@ -2935,8 +2960,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
 	amdgpu_bo_unreserve(vm->root.base.bo);

 error_free_root:
-	amdgpu_bo_unref(&vm->root.base.bo->shadow);
-	amdgpu_bo_unref(&vm->root.base.bo);
+	amdgpu_bo_unref(&root->shadow);
+	amdgpu_bo_unref(&root_bo);
 	vm->root.base.bo = NULL;

 error_free_delayed:
@@ -3078,7 +3103,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	}

 	/* Free the shadow bo for compute VM */
-	amdgpu_bo_unref(&vm->root.base.bo->shadow);
+	amdgpu_bo_unref(&to_amdgpu_bo_vm(vm->root.base.bo)->shadow);

 	if (pasid)
 		vm->pasid = pasid;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index a83a646759c5..c9ef025c43f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -41,10 +41,7 @@ static int amdgpu_vm_sdma_map_table(struct amdgpu_bo *table)
 	if (r)
 		return r;

-	if (table->shadow)
-		r = amdgpu_ttm_alloc_gart(&table->shadow->tbo);
-
-	return r;
+	return amdgpu_ttm_alloc_gart(&to_amdgpu_bo_vm(table)->shadow->tbo);
 }

 /**
@@ -201,17 +198,20 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
 				 uint64_t addr, unsigned count, uint32_t incr,
 				 uint64_t flags)
 {
+	struct amdgpu_bo *shadow;
 	enum amdgpu_ib_pool_type pool = p->immediate ? AMDGPU_IB_POOL_IMMEDIATE
 		: AMDGPU_IB_POOL_DELAYED;
 	unsigned int i, ndw, nptes;
 	uint64_t *pte;
 	int r;

+	shadow = amdgpu_bo_shadowed(bo);
 	/* Wait for PD/PT moves to be completed */
 	r = amdgpu_sync_fence(&p->job->sync, bo->tbo.moving);
 	if (r)
 		return r;

+
 	do {
 		ndw = p->num_dw_left;
 		ndw -= p->job->ibs->length_dw;
@@ -238,8 +238,8 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,

 		if (!p->pages_addr) {
 			/* set page commands needed */
-			if (bo->shadow)
-				amdgpu_vm_sdma_set_ptes(p, bo->shadow, pe, addr,
+			if (shadow)
+				amdgpu_vm_sdma_set_ptes(p, shadow, pe, addr,
 							count, incr, flags);
 			amdgpu_vm_sdma_set_ptes(p, bo, pe, addr, count,
 						incr, flags);
@@ -248,7 +248,7 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,

 		/* copy commands needed */
 		ndw -= p->adev->vm_manager.vm_pte_funcs->copy_pte_num_dw *
-			(bo->shadow ? 2 : 1);
+			(shadow ? 2 : 1);

 		/* for padding */
 		ndw -= 7;
@@ -263,8 +263,8 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
 			pte[i] |= flags;
 		}

-		if (bo->shadow)
-			amdgpu_vm_sdma_copy_ptes(p, bo->shadow, pe, nptes);
+		if (shadow)
+			amdgpu_vm_sdma_copy_ptes(p, shadow, pe, nptes);
 		amdgpu_vm_sdma_copy_ptes(p, bo, pe, nptes);

 		pe += nptes * 8;
--
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 5/6] drm/amdgpu: remove unused code

[Nirmoy Das]

Remove unused code related to shadow BO.

v2: removing shadow bo ptr from base class.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 29 ----------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 -----
 2 files changed, 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index a63b450cd603..db9c64836556 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -638,35 +638,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	return r;
 }

-int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
-			    unsigned long size,
-			    struct amdgpu_bo *bo)
-{
-	struct amdgpu_bo_param bp;
-	int r;
-
-	if (bo->shadow)
-		return 0;
-
-	memset(&bp, 0, sizeof(bp));
-	bp.size = size;
-	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
-	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-	bp.type = ttm_bo_type_kernel;
-	bp.resv = bo->tbo.base.resv;
-	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
-
-	r = amdgpu_bo_create(adev, &bp, &bo->shadow);
-	if (!r) {
-		bo->shadow->parent = amdgpu_bo_ref(bo);
-		mutex_lock(&adev->shadow_list_lock);
-		list_add_tail(&bo->shadow->shadow_list, &adev->shadow_list);
-		mutex_unlock(&adev->shadow_list_lock);
-	}
-
-	return r;
-}
-
 /**
  * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
  * @adev: amdgpu device object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 9afccf6c66f2..fa75251148be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -104,9 +104,6 @@ struct amdgpu_bo {
 	struct amdgpu_vm_bo_base	*vm_bo;
 	/* Constant after initialization */
 	struct amdgpu_bo		*parent;
-	struct amdgpu_bo		*shadow;
-
-

 #ifdef CONFIG_MMU_NOTIFIER
 	struct mmu_interval_notifier	notifier;
@@ -300,9 +297,6 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 			struct amdgpu_bo_vm **ubo_ptr);
 void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
 			   void **cpu_addr);
-int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
-			    unsigned long size,
-			    struct amdgpu_bo *bo);
 int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
 void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
 void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
--
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 6/6] drm/amdgpu: do not allocate entries separately

[Nirmoy Das]

Allocate PD/PT entries while allocating VM BOs and use that
instead of allocating those entries separately.

v2: create a new var for num entries.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 +++++++++++++++-----------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d723873df765..70df453863cb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -876,6 +876,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
 	struct amdgpu_bo *bo;
 	struct dma_resv *resv;
 	int r;
+	unsigned int num_entries;

 	memset(&bp, 0, sizeof(bp));

@@ -885,7 +886,14 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
 	bp.domain = amdgpu_bo_get_preferred_pin_domain(adev, bp.domain);
 	bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
 		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-	bp.bo_ptr_size = sizeof(struct amdgpu_bo_vm);
+
+	if (level < AMDGPU_VM_PTB)
+		num_entries = amdgpu_vm_num_entries(adev, level);
+	else
+		num_entries = 0;
+
+	bp.bo_ptr_size = struct_size((*vmbo), entries, num_entries);
+
 	if (vm->use_cpu_for_update)
 		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;

@@ -956,19 +964,14 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 	struct amdgpu_bo_vm *pt;
 	int r;

-	if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
-		unsigned num_entries;
-
-		num_entries = amdgpu_vm_num_entries(adev, cursor->level);
-		entry->entries = kvmalloc_array(num_entries,
-						sizeof(*entry->entries),
-						GFP_KERNEL | __GFP_ZERO);
-		if (!entry->entries)
-			return -ENOMEM;
-	}
-
-	if (entry->base.bo)
+	if (entry->base.bo) {
+		if (cursor->level < AMDGPU_VM_PTB)
+			entry->entries =
+				to_amdgpu_bo_vm(entry->base.bo)->entries;
+		else
+			entry->entries = NULL;
 		return 0;
+	}

 	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt);
 	if (r)
@@ -980,6 +983,10 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 	pt_bo = &pt->bo;
 	pt_bo->parent = amdgpu_bo_ref(cursor->parent->base.bo);
 	amdgpu_vm_bo_base_init(&entry->base, vm, pt_bo);
+	if (cursor->level < AMDGPU_VM_PTB)
+		entry->entries = pt->entries;
+	else
+		entry->entries = NULL;

 	r = amdgpu_vm_clear_bo(adev, vm, pt_bo, immediate);
 	if (r)
@@ -1009,7 +1016,6 @@ static void amdgpu_vm_free_table(struct amdgpu_vm_pt *entry)
 		amdgpu_bo_unref(&shadow);
 		amdgpu_bo_unref(&entry->base.bo);
 	}
-	kvfree(entry->entries);
 	entry->entries = NULL;
 }

--
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/admgpu: add two shadow BO helper functions

[Christian König]

Am 27.05.21 um 13:53 schrieb Nirmoy Das:
> Add amdgpu_bo_add_to_shadow_list() to handle shadow list
> additions and amdgpu_bo_shadowed() to check if a BO is shadowed.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 16 ++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 17 +++++++++++++++++
>   2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6870cc297ae6..a63b450cd603 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -760,6 +760,22 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
>   	return r;
>   }
>   
> +/**
> + * amdgpu_bo_add_to_shadow_list - add a BO to the shadow list
> + *
> + * @bo: BO that will be inserted into the shadow list
> + *
> + * Insert a BO to the shadow list.
> + */
> +void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +
> +	mutex_lock(&adev->shadow_list_lock);
> +	list_add_tail(&bo->shadow_list, &adev->shadow_list);
> +	mutex_unlock(&adev->shadow_list_lock);
> +}
> +
>   /**
>    * amdgpu_bo_restore_shadow - restore an &amdgpu_bo shadow
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index a7fbf5f7051e..9afccf6c66f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -259,6 +259,22 @@ static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
>   	return bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED;
>   }
>   
> +/**
> + * amdgpu_bo_shadowed - check if the BO is shadowed
> + *
> + * @bo: BO to be tested.
> + *
> + * Returns:
> + * NULL if not shadowed or else return a BO pointer.
> + */
> +static inline struct amdgpu_bo *amdgpu_bo_shadowed(struct amdgpu_bo *bo)
> +{
> +	if (bo->tbo.type == ttm_bo_type_kernel)
> +		return to_amdgpu_bo_vm(bo)->shadow;
> +
> +	return NULL;
> +}
> +
>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
>   void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
>   
> @@ -322,6 +338,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
>   int amdgpu_bo_validate(struct amdgpu_bo *bo);
>   void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
>   				uint64_t *gtt_mem, uint64_t *cpu_mem);
> +void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
>   int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
>   			     struct dma_fence **fence);
>   uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 4/6] drm/amdgpu: switch to amdgpu_bo_vm for vm code

[Christian König]

Am 27.05.21 um 13:53 schrieb Nirmoy Das:
> The subclass, amdgpu_bo_vm is intended for PT/PD BOs which are also
> shadowed, so switch to amdgpu_bo_vm BO for PT/PD BOs.
>
> v3: simplify code.
>      check also if shadow bo exist, instead of checking only bo's type.
> v2: squash three related patches.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c      | 93 +++++++++++++--------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 18 ++--
>   2 files changed, 68 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 6bc7566cc193..d723873df765 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -652,15 +652,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
>   	spin_lock(&adev->mman.bdev.lru_lock);
>   	list_for_each_entry(bo_base, &vm->idle, vm_status) {
>   		struct amdgpu_bo *bo = bo_base->bo;
> +		struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
>
>   		if (!bo->parent)
>   			continue;
>
>   		ttm_bo_move_to_lru_tail(&bo->tbo, &bo->tbo.mem,
>   					&vm->lru_bulk_move);
> -		if (bo->shadow)
> -			ttm_bo_move_to_lru_tail(&bo->shadow->tbo,
> -						&bo->shadow->tbo.mem,
> +		if (shadow)
> +			ttm_bo_move_to_lru_tail(&shadow->tbo, &shadow->tbo.mem,
>   						&vm->lru_bulk_move);
>   	}
>   	spin_unlock(&adev->mman.bdev.lru_lock);
> @@ -692,12 +692,13 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>
>   	list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
>   		struct amdgpu_bo *bo = bo_base->bo;
> +		struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
>
>   		r = validate(param, bo);
>   		if (r)
>   			return r;
> -		if (bo->shadow) {
> -			r = validate(param, bo->shadow);
> +		if (shadow) {
> +			r = validate(param, shadow);
>   			if (r)
>   				return r;
>   		}
> @@ -754,6 +755,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   	unsigned level = adev->vm_manager.root_level;
>   	struct amdgpu_vm_update_params params;
>   	struct amdgpu_bo *ancestor = bo;
> +	struct amdgpu_bo *shadow;
>   	unsigned entries, ats_entries;
>   	uint64_t addr;
>   	int r;
> @@ -793,9 +795,9 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   	if (r)
>   		return r;
>
> -	if (bo->shadow) {
> -		r = ttm_bo_validate(&bo->shadow->tbo, &bo->shadow->placement,
> -				    &ctx);
> +	shadow = amdgpu_bo_shadowed(bo);
> +	if (shadow) {
> +		r = ttm_bo_validate(&shadow->tbo, &shadow->placement, &ctx);
>   		if (r)
>   			return r;
>   	}
> @@ -863,14 +865,16 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>    * @vm: requesting vm
>    * @level: the page table level
>    * @immediate: use a immediate update
> - * @bo: pointer to the buffer object pointer
> + * @vmbo: pointer to the buffer object pointer
>    */
>   static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   			       struct amdgpu_vm *vm,
>   			       int level, bool immediate,
> -			       struct amdgpu_bo **bo)
> +			       struct amdgpu_bo_vm **vmbo)
>   {
>   	struct amdgpu_bo_param bp;
> +	struct amdgpu_bo *bo;
> +	struct dma_resv *resv;
>   	int r;
>
>   	memset(&bp, 0, sizeof(bp));
> @@ -881,7 +885,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   	bp.domain = amdgpu_bo_get_preferred_pin_domain(adev, bp.domain);
>   	bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>   		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo_vm);
>   	if (vm->use_cpu_for_update)
>   		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>
> @@ -890,26 +894,41 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   	if (vm->root.base.bo)
>   		bp.resv = vm->root.base.bo->tbo.base.resv;
>
> -	r = amdgpu_bo_create(adev, &bp, bo);
> +	r = amdgpu_bo_create_vm(adev, &bp, vmbo);
>   	if (r)
>   		return r;
>
> -	if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
> +	bo = &(*vmbo)->bo;
> +	if (vm->is_compute_context && (adev->flags & AMD_IS_APU)) {
> +		(*vmbo)->shadow = NULL;
>   		return 0;
> +	}
>
>   	if (!bp.resv)
> -		WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
> +		WARN_ON(dma_resv_lock(bo->tbo.base.resv,
>   				      NULL));
> -	r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
> +	resv = bp.resv;
> +	memset(&bp, 0, sizeof(bp));
> +	bp.size = amdgpu_vm_bo_size(adev, level);
> +	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
> +	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> +	bp.type = ttm_bo_type_kernel;
> +	bp.resv = bo->tbo.base.resv;
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>
> -	if (!bp.resv)
> -		dma_resv_unlock((*bo)->tbo.base.resv);
> +	r = amdgpu_bo_create(adev, &bp, &(*vmbo)->shadow);
> +
> +	if (!resv)
> +		dma_resv_unlock(bo->tbo.base.resv);
>
>   	if (r) {
> -		amdgpu_bo_unref(bo);
> +		amdgpu_bo_unref(&bo);
>   		return r;
>   	}
>
> +	(*vmbo)->shadow->parent = amdgpu_bo_ref(bo);
> +	amdgpu_bo_add_to_shadow_list((*vmbo)->shadow);
> +
>   	return 0;
>   }
>
> @@ -933,7 +952,8 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			       bool immediate)
>   {
>   	struct amdgpu_vm_pt *entry = cursor->entry;
> -	struct amdgpu_bo *pt;
> +	struct amdgpu_bo *pt_bo;
> +	struct amdgpu_bo_vm *pt;
>   	int r;
>
>   	if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
> @@ -957,10 +977,11 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   	/* Keep a reference to the root directory to avoid
>   	 * freeing them up in the wrong order.
>   	 */
> -	pt->parent = amdgpu_bo_ref(cursor->parent->base.bo);
> -	amdgpu_vm_bo_base_init(&entry->base, vm, pt);
> +	pt_bo = &pt->bo;
> +	pt_bo->parent = amdgpu_bo_ref(cursor->parent->base.bo);
> +	amdgpu_vm_bo_base_init(&entry->base, vm, pt_bo);
>
> -	r = amdgpu_vm_clear_bo(adev, vm, pt, immediate);
> +	r = amdgpu_vm_clear_bo(adev, vm, pt_bo, immediate);
>   	if (r)
>   		goto error_free_pt;
>
> @@ -968,7 +989,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>
>   error_free_pt:
>   	amdgpu_bo_unref(&pt->shadow);
> -	amdgpu_bo_unref(&pt);
> +	amdgpu_bo_unref(&pt_bo);
>   	return r;
>   }
>
> @@ -979,10 +1000,13 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>    */
>   static void amdgpu_vm_free_table(struct amdgpu_vm_pt *entry)
>   {
> +	struct amdgpu_bo *shadow;
> +
>   	if (entry->base.bo) {
> +		shadow = amdgpu_bo_shadowed(entry->base.bo);
>   		entry->base.bo->vm_bo = NULL;
>   		list_del(&entry->base.vm_status);
> -		amdgpu_bo_unref(&entry->base.bo->shadow);
> +		amdgpu_bo_unref(&shadow);
>   		amdgpu_bo_unref(&entry->base.bo);
>   	}
>   	kvfree(entry->entries);
> @@ -2674,7 +2698,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   	struct amdgpu_vm_bo_base *bo_base;
>
>   	/* shadow bo doesn't have bo base, its validation needs its parent */
> -	if (bo->parent && bo->parent->shadow == bo)
> +	if (bo->parent && (amdgpu_bo_shadowed(bo->parent) == bo))
>   		bo = bo->parent;
>
>   	for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
> @@ -2843,7 +2867,8 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
>    */
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   {
> -	struct amdgpu_bo *root;
> +	struct amdgpu_bo *root_bo;
> +	struct amdgpu_bo_vm *root;
>   	int r, i;
>
>   	vm->va = RB_ROOT_CACHED;
> @@ -2897,18 +2922,18 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   				false, &root);
>   	if (r)
>   		goto error_free_delayed;
> -
> -	r = amdgpu_bo_reserve(root, true);
> +	root_bo = &root->bo;
> +	r = amdgpu_bo_reserve(root_bo, true);
>   	if (r)
>   		goto error_free_root;
>
> -	r = dma_resv_reserve_shared(root->tbo.base.resv, 1);
> +	r = dma_resv_reserve_shared(root_bo->tbo.base.resv, 1);
>   	if (r)
>   		goto error_unreserve;
>
> -	amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
> +	amdgpu_vm_bo_base_init(&vm->root.base, vm, root_bo);
>
> -	r = amdgpu_vm_clear_bo(adev, vm, root, false);
> +	r = amdgpu_vm_clear_bo(adev, vm, root_bo, false);
>   	if (r)
>   		goto error_unreserve;
>
> @@ -2935,8 +2960,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   	amdgpu_bo_unreserve(vm->root.base.bo);
>
>   error_free_root:
> -	amdgpu_bo_unref(&vm->root.base.bo->shadow);
> -	amdgpu_bo_unref(&vm->root.base.bo);
> +	amdgpu_bo_unref(&root->shadow);
> +	amdgpu_bo_unref(&root_bo);
>   	vm->root.base.bo = NULL;
>
>   error_free_delayed:
> @@ -3078,7 +3103,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	}
>
>   	/* Free the shadow bo for compute VM */
> -	amdgpu_bo_unref(&vm->root.base.bo->shadow);
> +	amdgpu_bo_unref(&to_amdgpu_bo_vm(vm->root.base.bo)->shadow);
>
>   	if (pasid)
>   		vm->pasid = pasid;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index a83a646759c5..c9ef025c43f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -41,10 +41,7 @@ static int amdgpu_vm_sdma_map_table(struct amdgpu_bo *table)
>   	if (r)
>   		return r;
>
> -	if (table->shadow)
> -		r = amdgpu_ttm_alloc_gart(&table->shadow->tbo);
> -
> -	return r;
> +	return amdgpu_ttm_alloc_gart(&to_amdgpu_bo_vm(table)->shadow->tbo);

Here you also need to check if shadow isn't NULL.

I think it would be simpler if you make the parameter to 
amdgpu_vm_sdma_map_table a vmbo in the first place.

>   }
>
>   /**
> @@ -201,17 +198,20 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>   				 uint64_t addr, unsigned count, uint32_t incr,
>   				 uint64_t flags)
>   {
> +	struct amdgpu_bo *shadow;
>   	enum amdgpu_ib_pool_type pool = p->immediate ? AMDGPU_IB_POOL_IMMEDIATE
>   		: AMDGPU_IB_POOL_DELAYED;
>   	unsigned int i, ndw, nptes;
>   	uint64_t *pte;
>   	int r;
>
> +	shadow = amdgpu_bo_shadowed(bo);

Same for amdgpu_vm_sdma_map_table(), just make the parameter a vmbo in 
the first place.

Apart from that patch looks good to me.

Christian.

>   	/* Wait for PD/PT moves to be completed */
>   	r = amdgpu_sync_fence(&p->job->sync, bo->tbo.moving);
>   	if (r)
>   		return r;
>
> +
>   	do {
>   		ndw = p->num_dw_left;
>   		ndw -= p->job->ibs->length_dw;
> @@ -238,8 +238,8 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>
>   		if (!p->pages_addr) {
>   			/* set page commands needed */
> -			if (bo->shadow)
> -				amdgpu_vm_sdma_set_ptes(p, bo->shadow, pe, addr,
> +			if (shadow)
> +				amdgpu_vm_sdma_set_ptes(p, shadow, pe, addr,
>   							count, incr, flags);
>   			amdgpu_vm_sdma_set_ptes(p, bo, pe, addr, count,
>   						incr, flags);
> @@ -248,7 +248,7 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>
>   		/* copy commands needed */
>   		ndw -= p->adev->vm_manager.vm_pte_funcs->copy_pte_num_dw *
> -			(bo->shadow ? 2 : 1);
> +			(shadow ? 2 : 1);
>
>   		/* for padding */
>   		ndw -= 7;
> @@ -263,8 +263,8 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>   			pte[i] |= flags;
>   		}
>
> -		if (bo->shadow)
> -			amdgpu_vm_sdma_copy_ptes(p, bo->shadow, pe, nptes);
> +		if (shadow)
> +			amdgpu_vm_sdma_copy_ptes(p, shadow, pe, nptes);
>   		amdgpu_vm_sdma_copy_ptes(p, bo, pe, nptes);
>
>   		pe += nptes * 8;
> --
> 2.31.1
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 5/6] drm/amdgpu: remove unused code

[Christian König]

Am 27.05.21 um 13:53 schrieb Nirmoy Das:
> Remove unused code related to shadow BO.
>
> v2: removing shadow bo ptr from base class.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 29 ----------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 -----
>   2 files changed, 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index a63b450cd603..db9c64836556 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -638,35 +638,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	return r;
>   }
>
> -int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
> -			    unsigned long size,
> -			    struct amdgpu_bo *bo)
> -{
> -	struct amdgpu_bo_param bp;
> -	int r;
> -
> -	if (bo->shadow)
> -		return 0;
> -
> -	memset(&bp, 0, sizeof(bp));
> -	bp.size = size;
> -	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
> -	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -	bp.type = ttm_bo_type_kernel;
> -	bp.resv = bo->tbo.base.resv;
> -	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
> -
> -	r = amdgpu_bo_create(adev, &bp, &bo->shadow);
> -	if (!r) {
> -		bo->shadow->parent = amdgpu_bo_ref(bo);
> -		mutex_lock(&adev->shadow_list_lock);
> -		list_add_tail(&bo->shadow->shadow_list, &adev->shadow_list);
> -		mutex_unlock(&adev->shadow_list_lock);
> -	}
> -
> -	return r;
> -}
> -
>   /**
>    * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
>    * @adev: amdgpu device object
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 9afccf6c66f2..fa75251148be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -104,9 +104,6 @@ struct amdgpu_bo {
>   	struct amdgpu_vm_bo_base	*vm_bo;
>   	/* Constant after initialization */
>   	struct amdgpu_bo		*parent;
> -	struct amdgpu_bo		*shadow;
> -
> -
>
>   #ifdef CONFIG_MMU_NOTIFIER
>   	struct mmu_interval_notifier	notifier;
> @@ -300,9 +297,6 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
>   			struct amdgpu_bo_vm **ubo_ptr);
>   void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
>   			   void **cpu_addr);
> -int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
> -			    unsigned long size,
> -			    struct amdgpu_bo *bo);
>   int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
>   void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
>   void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
> --
> 2.31.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH v3 4/6] drm/amdgpu: switch to amdgpu_bo_vm for vm code

[Das, Nirmoy]

[AMD Official Use Only]

Thanks Christian, I will resend with your suggestions included.

Nirmoy

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com> 
Sent: Friday, May 28, 2021 10:01 AM
To: Das, Nirmoy <Nirmoy.Das@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH v3 4/6] drm/amdgpu: switch to amdgpu_bo_vm for vm code



Am 27.05.21 um 13:53 schrieb Nirmoy Das:
> The subclass, amdgpu_bo_vm is intended for PT/PD BOs which are also 
> shadowed, so switch to amdgpu_bo_vm BO for PT/PD BOs.
>
> v3: simplify code.
>      check also if shadow bo exist, instead of checking only bo's type.
> v2: squash three related patches.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c      | 93 +++++++++++++--------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 18 ++--
>   2 files changed, 68 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 6bc7566cc193..d723873df765 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -652,15 +652,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
>   	spin_lock(&adev->mman.bdev.lru_lock);
>   	list_for_each_entry(bo_base, &vm->idle, vm_status) {
>   		struct amdgpu_bo *bo = bo_base->bo;
> +		struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
>
>   		if (!bo->parent)
>   			continue;
>
>   		ttm_bo_move_to_lru_tail(&bo->tbo, &bo->tbo.mem,
>   					&vm->lru_bulk_move);
> -		if (bo->shadow)
> -			ttm_bo_move_to_lru_tail(&bo->shadow->tbo,
> -						&bo->shadow->tbo.mem,
> +		if (shadow)
> +			ttm_bo_move_to_lru_tail(&shadow->tbo, &shadow->tbo.mem,
>   						&vm->lru_bulk_move);
>   	}
>   	spin_unlock(&adev->mman.bdev.lru_lock);
> @@ -692,12 +692,13 @@ int amdgpu_vm_validate_pt_bos(struct 
> amdgpu_device *adev, struct amdgpu_vm *vm,
>
>   	list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
>   		struct amdgpu_bo *bo = bo_base->bo;
> +		struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
>
>   		r = validate(param, bo);
>   		if (r)
>   			return r;
> -		if (bo->shadow) {
> -			r = validate(param, bo->shadow);
> +		if (shadow) {
> +			r = validate(param, shadow);
>   			if (r)
>   				return r;
>   		}
> @@ -754,6 +755,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   	unsigned level = adev->vm_manager.root_level;
>   	struct amdgpu_vm_update_params params;
>   	struct amdgpu_bo *ancestor = bo;
> +	struct amdgpu_bo *shadow;
>   	unsigned entries, ats_entries;
>   	uint64_t addr;
>   	int r;
> @@ -793,9 +795,9 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   	if (r)
>   		return r;
>
> -	if (bo->shadow) {
> -		r = ttm_bo_validate(&bo->shadow->tbo, &bo->shadow->placement,
> -				    &ctx);
> +	shadow = amdgpu_bo_shadowed(bo);
> +	if (shadow) {
> +		r = ttm_bo_validate(&shadow->tbo, &shadow->placement, &ctx);
>   		if (r)
>   			return r;
>   	}
> @@ -863,14 +865,16 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>    * @vm: requesting vm
>    * @level: the page table level
>    * @immediate: use a immediate update
> - * @bo: pointer to the buffer object pointer
> + * @vmbo: pointer to the buffer object pointer
>    */
>   static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   			       struct amdgpu_vm *vm,
>   			       int level, bool immediate,
> -			       struct amdgpu_bo **bo)
> +			       struct amdgpu_bo_vm **vmbo)
>   {
>   	struct amdgpu_bo_param bp;
> +	struct amdgpu_bo *bo;
> +	struct dma_resv *resv;
>   	int r;
>
>   	memset(&bp, 0, sizeof(bp));
> @@ -881,7 +885,7 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   	bp.domain = amdgpu_bo_get_preferred_pin_domain(adev, bp.domain);
>   	bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>   		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo_vm);
>   	if (vm->use_cpu_for_update)
>   		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>
> @@ -890,26 +894,41 @@ static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>   	if (vm->root.base.bo)
>   		bp.resv = vm->root.base.bo->tbo.base.resv;
>
> -	r = amdgpu_bo_create(adev, &bp, bo);
> +	r = amdgpu_bo_create_vm(adev, &bp, vmbo);
>   	if (r)
>   		return r;
>
> -	if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
> +	bo = &(*vmbo)->bo;
> +	if (vm->is_compute_context && (adev->flags & AMD_IS_APU)) {
> +		(*vmbo)->shadow = NULL;
>   		return 0;
> +	}
>
>   	if (!bp.resv)
> -		WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
> +		WARN_ON(dma_resv_lock(bo->tbo.base.resv,
>   				      NULL));
> -	r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
> +	resv = bp.resv;
> +	memset(&bp, 0, sizeof(bp));
> +	bp.size = amdgpu_vm_bo_size(adev, level);
> +	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
> +	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> +	bp.type = ttm_bo_type_kernel;
> +	bp.resv = bo->tbo.base.resv;
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>
> -	if (!bp.resv)
> -		dma_resv_unlock((*bo)->tbo.base.resv);
> +	r = amdgpu_bo_create(adev, &bp, &(*vmbo)->shadow);
> +
> +	if (!resv)
> +		dma_resv_unlock(bo->tbo.base.resv);
>
>   	if (r) {
> -		amdgpu_bo_unref(bo);
> +		amdgpu_bo_unref(&bo);
>   		return r;
>   	}
>
> +	(*vmbo)->shadow->parent = amdgpu_bo_ref(bo);
> +	amdgpu_bo_add_to_shadow_list((*vmbo)->shadow);
> +
>   	return 0;
>   }
>
> @@ -933,7 +952,8 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			       bool immediate)
>   {
>   	struct amdgpu_vm_pt *entry = cursor->entry;
> -	struct amdgpu_bo *pt;
> +	struct amdgpu_bo *pt_bo;
> +	struct amdgpu_bo_vm *pt;
>   	int r;
>
>   	if (cursor->level < AMDGPU_VM_PTB && !entry->entries) { @@ -957,10 
> +977,11 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   	/* Keep a reference to the root directory to avoid
>   	 * freeing them up in the wrong order.
>   	 */
> -	pt->parent = amdgpu_bo_ref(cursor->parent->base.bo);
> -	amdgpu_vm_bo_base_init(&entry->base, vm, pt);
> +	pt_bo = &pt->bo;
> +	pt_bo->parent = amdgpu_bo_ref(cursor->parent->base.bo);
> +	amdgpu_vm_bo_base_init(&entry->base, vm, pt_bo);
>
> -	r = amdgpu_vm_clear_bo(adev, vm, pt, immediate);
> +	r = amdgpu_vm_clear_bo(adev, vm, pt_bo, immediate);
>   	if (r)
>   		goto error_free_pt;
>
> @@ -968,7 +989,7 @@ static int amdgpu_vm_alloc_pts(struct 
> amdgpu_device *adev,
>
>   error_free_pt:
>   	amdgpu_bo_unref(&pt->shadow);
> -	amdgpu_bo_unref(&pt);
> +	amdgpu_bo_unref(&pt_bo);
>   	return r;
>   }
>
> @@ -979,10 +1000,13 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>    */
>   static void amdgpu_vm_free_table(struct amdgpu_vm_pt *entry)
>   {
> +	struct amdgpu_bo *shadow;
> +
>   	if (entry->base.bo) {
> +		shadow = amdgpu_bo_shadowed(entry->base.bo);
>   		entry->base.bo->vm_bo = NULL;
>   		list_del(&entry->base.vm_status);
> -		amdgpu_bo_unref(&entry->base.bo->shadow);
> +		amdgpu_bo_unref(&shadow);
>   		amdgpu_bo_unref(&entry->base.bo);
>   	}
>   	kvfree(entry->entries);
> @@ -2674,7 +2698,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   	struct amdgpu_vm_bo_base *bo_base;
>
>   	/* shadow bo doesn't have bo base, its validation needs its parent */
> -	if (bo->parent && bo->parent->shadow == bo)
> +	if (bo->parent && (amdgpu_bo_shadowed(bo->parent) == bo))
>   		bo = bo->parent;
>
>   	for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) { @@ 
> -2843,7 +2867,8 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
>    */
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   {
> -	struct amdgpu_bo *root;
> +	struct amdgpu_bo *root_bo;
> +	struct amdgpu_bo_vm *root;
>   	int r, i;
>
>   	vm->va = RB_ROOT_CACHED;
> @@ -2897,18 +2922,18 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   				false, &root);
>   	if (r)
>   		goto error_free_delayed;
> -
> -	r = amdgpu_bo_reserve(root, true);
> +	root_bo = &root->bo;
> +	r = amdgpu_bo_reserve(root_bo, true);
>   	if (r)
>   		goto error_free_root;
>
> -	r = dma_resv_reserve_shared(root->tbo.base.resv, 1);
> +	r = dma_resv_reserve_shared(root_bo->tbo.base.resv, 1);
>   	if (r)
>   		goto error_unreserve;
>
> -	amdgpu_vm_bo_base_init(&vm->root.base, vm, root);
> +	amdgpu_vm_bo_base_init(&vm->root.base, vm, root_bo);
>
> -	r = amdgpu_vm_clear_bo(adev, vm, root, false);
> +	r = amdgpu_vm_clear_bo(adev, vm, root_bo, false);
>   	if (r)
>   		goto error_unreserve;
>
> @@ -2935,8 +2960,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   	amdgpu_bo_unreserve(vm->root.base.bo);
>
>   error_free_root:
> -	amdgpu_bo_unref(&vm->root.base.bo->shadow);
> -	amdgpu_bo_unref(&vm->root.base.bo);
> +	amdgpu_bo_unref(&root->shadow);
> +	amdgpu_bo_unref(&root_bo);
>   	vm->root.base.bo = NULL;
>
>   error_free_delayed:
> @@ -3078,7 +3103,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	}
>
>   	/* Free the shadow bo for compute VM */
> -	amdgpu_bo_unref(&vm->root.base.bo->shadow);
> +	amdgpu_bo_unref(&to_amdgpu_bo_vm(vm->root.base.bo)->shadow);
>
>   	if (pasid)
>   		vm->pasid = pasid;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index a83a646759c5..c9ef025c43f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -41,10 +41,7 @@ static int amdgpu_vm_sdma_map_table(struct amdgpu_bo *table)
>   	if (r)
>   		return r;
>
> -	if (table->shadow)
> -		r = amdgpu_ttm_alloc_gart(&table->shadow->tbo);
> -
> -	return r;
> +	return amdgpu_ttm_alloc_gart(&to_amdgpu_bo_vm(table)->shadow->tbo);

Here you also need to check if shadow isn't NULL.

I think it would be simpler if you make the parameter to amdgpu_vm_sdma_map_table a vmbo in the first place.

>   }
>
>   /**
> @@ -201,17 +198,20 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>   				 uint64_t addr, unsigned count, uint32_t incr,
>   				 uint64_t flags)
>   {
> +	struct amdgpu_bo *shadow;
>   	enum amdgpu_ib_pool_type pool = p->immediate ? AMDGPU_IB_POOL_IMMEDIATE
>   		: AMDGPU_IB_POOL_DELAYED;
>   	unsigned int i, ndw, nptes;
>   	uint64_t *pte;
>   	int r;
>
> +	shadow = amdgpu_bo_shadowed(bo);

Same for amdgpu_vm_sdma_map_table(), just make the parameter a vmbo in the first place.

Apart from that patch looks good to me.

Christian.

>   	/* Wait for PD/PT moves to be completed */
>   	r = amdgpu_sync_fence(&p->job->sync, bo->tbo.moving);
>   	if (r)
>   		return r;
>
> +
>   	do {
>   		ndw = p->num_dw_left;
>   		ndw -= p->job->ibs->length_dw;
> @@ -238,8 +238,8 @@ static int amdgpu_vm_sdma_update(struct 
> amdgpu_vm_update_params *p,
>
>   		if (!p->pages_addr) {
>   			/* set page commands needed */
> -			if (bo->shadow)
> -				amdgpu_vm_sdma_set_ptes(p, bo->shadow, pe, addr,
> +			if (shadow)
> +				amdgpu_vm_sdma_set_ptes(p, shadow, pe, addr,
>   							count, incr, flags);
>   			amdgpu_vm_sdma_set_ptes(p, bo, pe, addr, count,
>   						incr, flags);
> @@ -248,7 +248,7 @@ static int amdgpu_vm_sdma_update(struct 
> amdgpu_vm_update_params *p,
>
>   		/* copy commands needed */
>   		ndw -= p->adev->vm_manager.vm_pte_funcs->copy_pte_num_dw *
> -			(bo->shadow ? 2 : 1);
> +			(shadow ? 2 : 1);
>
>   		/* for padding */
>   		ndw -= 7;
> @@ -263,8 +263,8 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>   			pte[i] |= flags;
>   		}
>
> -		if (bo->shadow)
> -			amdgpu_vm_sdma_copy_ptes(p, bo->shadow, pe, nptes);
> +		if (shadow)
> +			amdgpu_vm_sdma_copy_ptes(p, shadow, pe, nptes);
>   		amdgpu_vm_sdma_copy_ptes(p, bo, pe, nptes);
>
>   		pe += nptes * 8;
> --
> 2.31.1
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 5/6] drm/amdgpu: remove unused code

[Nirmoy Das]

Remove unused code related to shadow BO.

v2: removing shadow bo ptr from base class.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 29 ----------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 -----
 2 files changed, 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 89ba612a5080..15cee49f11e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -638,35 +638,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	return r;
 }

-int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
-			    unsigned long size,
-			    struct amdgpu_bo *bo)
-{
-	struct amdgpu_bo_param bp;
-	int r;
-
-	if (bo->shadow)
-		return 0;
-
-	memset(&bp, 0, sizeof(bp));
-	bp.size = size;
-	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
-	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-	bp.type = ttm_bo_type_kernel;
-	bp.resv = bo->tbo.base.resv;
-	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
-
-	r = amdgpu_bo_create(adev, &bp, &bo->shadow);
-	if (!r) {
-		bo->shadow->parent = amdgpu_bo_ref(bo);
-		mutex_lock(&adev->shadow_list_lock);
-		list_add_tail(&bo->shadow->shadow_list, &adev->shadow_list);
-		mutex_unlock(&adev->shadow_list_lock);
-	}
-
-	return r;
-}
-
 /**
  * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
  * @adev: amdgpu device object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 9afccf6c66f2..fa75251148be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -104,9 +104,6 @@ struct amdgpu_bo {
 	struct amdgpu_vm_bo_base	*vm_bo;
 	/* Constant after initialization */
 	struct amdgpu_bo		*parent;
-	struct amdgpu_bo		*shadow;
-
-

 #ifdef CONFIG_MMU_NOTIFIER
 	struct mmu_interval_notifier	notifier;
@@ -300,9 +297,6 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 			struct amdgpu_bo_vm **ubo_ptr);
 void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
 			   void **cpu_addr);
-int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
-			    unsigned long size,
-			    struct amdgpu_bo *bo);
 int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
 void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
 void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
--
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx