All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow()
@ 2021-04-22 15:39 Nirmoy Das
  2021-04-22 15:39 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init() Nirmoy Das
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Nirmoy Das @ 2021-04-22 15:39 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Exposed amdgpu_bo_create_shadow() will be needed
for amdgpu_vm handling.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 1345f7eba011..9cdeb20fb6cd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -625,9 +625,9 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 	return r;
 }
 
-static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
-				   unsigned long size,
-				   struct amdgpu_bo *bo)
+int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
+			    unsigned long size,
+			    struct amdgpu_bo *bo)
 {
 	struct amdgpu_bo_param bp;
 	int r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 973c88bdf37b..e0ec48d6a3fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -271,6 +271,9 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 			  struct amdgpu_bo_user **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

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

* [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init()
  2021-04-22 15:39 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
@ 2021-04-22 15:39 ` Nirmoy Das
  2021-04-22 16:13   ` Felix Kuehling
  2021-04-22 15:40 ` [PATCH 3/6] drm/amdgpu: remove unused vm context flags Nirmoy Das
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nirmoy Das @ 2021-04-22 15:39 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Currently only way to create compute vm is through
amdgpu_vm_make_compute(). So vm_context isn't required
anymore for amdgpu_vm_init().

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 16 +++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 +--
 3 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 39ee88d29cca..07e8a7c28561 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1114,7 +1114,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 		dev_warn(adev->dev, "No more PASIDs available!");
 		pasid = 0;
 	}
-	r = amdgpu_vm_init(adev, &fpriv->vm, AMDGPU_VM_CONTEXT_GFX, pasid);
+
+	r = amdgpu_vm_init(adev, &fpriv->vm, pasid);
 	if (r)
 		goto error_pasid;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f95bcda8463f..577148a4ffaa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2782,8 +2782,7 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
  * Returns:
  * 0 for success, error for failure.
  */
-int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-		   int vm_context, u32 pasid)
+int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
 {
 	struct amdgpu_bo_param bp;
 	struct amdgpu_bo *root;
@@ -2817,16 +2816,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	vm->pte_support_ats = false;
 	vm->is_compute_context = false;
 
-	if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE) {
-		vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
-						AMDGPU_VM_USE_CPU_FOR_COMPUTE);
+	vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
+				    AMDGPU_VM_USE_CPU_FOR_GFX);
 
-		if (adev->asic_type == CHIP_RAVEN)
-			vm->pte_support_ats = true;
-	} else {
-		vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
-						AMDGPU_VM_USE_CPU_FOR_GFX);
-	}
 	DRM_DEBUG_DRIVER("VM update mode is %s\n",
 			 vm->use_cpu_for_update ? "CPU" : "SDMA");
 	WARN_ONCE((vm->use_cpu_for_update &&
@@ -2844,8 +2836,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	vm->evicting = false;
 
 	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, &bp);
-	if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE)
-		bp.flags &= ~AMDGPU_GEM_CREATE_SHADOW;
 	r = amdgpu_bo_create(adev, &bp, &root);
 	if (r)
 		goto error_free_delayed;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 848e175e99ff..7f07acae447b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -379,8 +379,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev);
 void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
 
 long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout);
-int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-		   int vm_context, u32 pasid);
+int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid);
 int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid);
 void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
 void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
-- 
2.31.1

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

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

* [PATCH 3/6] drm/amdgpu: remove unused vm context flags
  2021-04-22 15:39 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
  2021-04-22 15:39 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init() Nirmoy Das
@ 2021-04-22 15:40 ` Nirmoy Das
  2021-04-22 16:14   ` Felix Kuehling
  2021-04-22 15:40 ` [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow() Nirmoy Das
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nirmoy Das @ 2021-04-22 15:40 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Remove unused AMDGPU_VM_CONTEXT_GFX and AMDGPU_VM_CONTEXT_COMPUTE
flags.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 7f07acae447b..6a9dcedfcf89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -121,9 +121,6 @@ struct amdgpu_bo_list_entry;
 /* max vmids dedicated for process */
 #define AMDGPU_VM_MAX_RESERVED_VMID	1
 
-#define AMDGPU_VM_CONTEXT_GFX 0
-#define AMDGPU_VM_CONTEXT_COMPUTE 1
-
 /* See vm_update_mode */
 #define AMDGPU_VM_USE_CPU_FOR_GFX (1 << 0)
 #define AMDGPU_VM_USE_CPU_FOR_COMPUTE (1 << 1)
-- 
2.31.1

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

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

* [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow()
  2021-04-22 15:39 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
  2021-04-22 15:39 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init() Nirmoy Das
  2021-04-22 15:40 ` [PATCH 3/6] drm/amdgpu: remove unused vm context flags Nirmoy Das
@ 2021-04-22 15:40 ` Nirmoy Das
  2021-04-23  7:55   ` Christian König
  2021-04-22 15:40 ` [PATCH 5/6] drm/amdgpu: cleanup amdgpu_bo_create() Nirmoy Das
  2021-04-22 15:40 ` [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag Nirmoy Das
  4 siblings, 1 reply; 12+ messages in thread
From: Nirmoy Das @ 2021-04-22 15:40 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Shadow BOs are only needed for vm code so call amdgpu_bo_create_shadow()
directly instead of depending on amdgpu_bo_create().

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 577148a4ffaa..bb5506ff80dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -850,35 +850,63 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
 }
 
 /**
- * amdgpu_vm_bo_param - fill in parameters for PD/PT allocation
+ * amdgpu_vm_pt_create - create bo for PD/PT
  *
  * @adev: amdgpu_device pointer
  * @vm: requesting vm
  * @level: the page table level
  * @immediate: use a immediate update
- * @bp: resulting BO allocation parameters
+ * @bo: pointer to the buffer object pointer
  */
-static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
+			       struct amdgpu_vm *vm,
 			       int level, bool immediate,
-			       struct amdgpu_bo_param *bp)
+			       struct amdgpu_bo **bo)
 {
-	memset(bp, 0, sizeof(*bp));
+	struct amdgpu_bo_param bp;
+	bool create_shadow = false;
+	int r;
 
-	bp->size = amdgpu_vm_bo_size(adev, level);
-	bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
-	bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
-	bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
-	bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
+	memset(&bp, 0, sizeof(bp));
+
+	bp.size = amdgpu_vm_bo_size(adev, level);
+	bp.byte_align = AMDGPU_GPU_PAGE_SIZE;
+	bp.domain = AMDGPU_GEM_DOMAIN_VRAM;
+	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);
 	if (vm->use_cpu_for_update)
-		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
 	else if (!vm->root.base.bo || vm->root.base.bo->shadow)
-		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
-	bp->type = ttm_bo_type_kernel;
-	bp->no_wait_gpu = immediate;
+		create_shadow = true;
+
+	bp.type = ttm_bo_type_kernel;
+	bp.no_wait_gpu = immediate;
 	if (vm->root.base.bo)
-		bp->resv = vm->root.base.bo->tbo.base.resv;
+		bp.resv = vm->root.base.bo->tbo.base.resv;
+
+	r = amdgpu_bo_create(adev, &bp, bo);
+	if (r)
+		return r;
+
+	if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
+		return 0;
+
+	if (!bp.resv)
+		WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
+				      NULL));
+	r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
+
+	if (!bp.resv)
+		dma_resv_unlock((*bo)->tbo.base.resv);
+
+	if (r) {
+		amdgpu_bo_unref(bo);
+		return r;
+	}
+
+	return 0;
 }
 
 /**
@@ -901,7 +929,6 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 			       bool immediate)
 {
 	struct amdgpu_vm_pt *entry = cursor->entry;
-	struct amdgpu_bo_param bp;
 	struct amdgpu_bo *pt;
 	int r;
 
@@ -919,9 +946,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
 	if (entry->base.bo)
 		return 0;
 
-	amdgpu_vm_bo_param(adev, vm, cursor->level, immediate, &bp);
-
-	r = amdgpu_bo_create(adev, &bp, &pt);
+	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt);
 	if (r)
 		return r;
 
@@ -2784,7 +2809,6 @@ 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_param bp;
 	struct amdgpu_bo *root;
 	int r, i;
 
@@ -2835,8 +2859,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
 	mutex_init(&vm->eviction_lock);
 	vm->evicting = false;
 
-	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, &bp);
-	r = amdgpu_bo_create(adev, &bp, &root);
+	r = amdgpu_vm_pt_create(adev, vm, adev->vm_manager.root_level,
+				false, &root);
 	if (r)
 		goto error_free_delayed;
 
-- 
2.31.1

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

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

* [PATCH 5/6] drm/amdgpu: cleanup amdgpu_bo_create()
  2021-04-22 15:39 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
                   ` (2 preceding siblings ...)
  2021-04-22 15:40 ` [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow() Nirmoy Das
@ 2021-04-22 15:40 ` Nirmoy Das
  2021-04-22 15:40 ` [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag Nirmoy Das
  4 siblings, 0 replies; 12+ messages in thread
From: Nirmoy Das @ 2021-04-22 15:40 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Remove shadow bo related code as vm code is creating shadow bo
using proper API. Without shadow bo code, amdgpu_bo_create() is basically
a wrapper around amdgpu_bo_do_create(). So rename amdgpu_bo_do_create()
to amdgpu_bo_create().

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9cdeb20fb6cd..39f88e4a8eb5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -515,7 +515,18 @@ bool amdgpu_bo_support_uswc(u64 bo_flags)
 #endif
 }
 
-static int amdgpu_bo_do_create(struct amdgpu_device *adev,
+/**
+ * amdgpu_bo_create - create an &amdgpu_bo buffer object
+ * @adev: amdgpu device object
+ * @bp: parameters to be used for the buffer object
+ * @bo_ptr: pointer to the buffer object pointer
+ *
+ * Creates an &amdgpu_bo buffer object.
+ *
+ * Returns:
+ * 0 for success or a negative error code on failure.
+ */
+int amdgpu_bo_create(struct amdgpu_device *adev,
 			       struct amdgpu_bo_param *bp,
 			       struct amdgpu_bo **bo_ptr)
 {
@@ -644,7 +655,7 @@ int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 	bp.resv = bo->tbo.base.resv;
 	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
 
-	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
+	r = amdgpu_bo_create(adev, &bp, &bo->shadow);
 	if (!r) {
 		bo->shadow->parent = amdgpu_bo_ref(bo);
 		mutex_lock(&adev->shadow_list_lock);
@@ -655,50 +666,6 @@ int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 	return r;
 }
 
-/**
- * amdgpu_bo_create - create an &amdgpu_bo buffer object
- * @adev: amdgpu device object
- * @bp: parameters to be used for the buffer object
- * @bo_ptr: pointer to the buffer object pointer
- *
- * Creates an &amdgpu_bo buffer object; and if requested, also creates a
- * shadow object.
- * Shadow object is used to backup the original buffer object, and is always
- * in GTT.
- *
- * Returns:
- * 0 for success or a negative error code on failure.
- */
-int amdgpu_bo_create(struct amdgpu_device *adev,
-		     struct amdgpu_bo_param *bp,
-		     struct amdgpu_bo **bo_ptr)
-{
-	u64 flags = bp->flags;
-	int r;
-
-	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
-
-	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
-	if (r)
-		return r;
-
-	if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & AMD_IS_APU)) {
-		if (!bp->resv)
-			WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
-							NULL));
-
-		r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
-
-		if (!bp->resv)
-			dma_resv_unlock((*bo_ptr)->tbo.base.resv);
-
-		if (r)
-			amdgpu_bo_unref(bo_ptr);
-	}
-
-	return r;
-}
-
 /**
  * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
  * @adev: amdgpu device object
@@ -720,7 +687,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 
 	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
 	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
-	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
+	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
 		return r;
 
-- 
2.31.1

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

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

* [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag
  2021-04-22 15:39 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
                   ` (3 preceding siblings ...)
  2021-04-22 15:40 ` [PATCH 5/6] drm/amdgpu: cleanup amdgpu_bo_create() Nirmoy Das
@ 2021-04-22 15:40 ` Nirmoy Das
  2021-04-23  7:57   ` Christian König
  4 siblings, 1 reply; 12+ messages in thread
From: Nirmoy Das @ 2021-04-22 15:40 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Remove unused AMDGPU_GEM_CREATE_SHADOW flag.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +----
 include/uapi/drm/amdgpu_drm.h              | 2 --
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 39f88e4a8eb5..da6d4ee0a132 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -649,8 +649,7 @@ int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 	memset(&bp, 0, sizeof(bp));
 	bp.size = size;
 	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
-	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC |
-		AMDGPU_GEM_CREATE_SHADOW;
+	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);
@@ -685,7 +684,6 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 	struct amdgpu_bo *bo_ptr;
 	int r;
 
-	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
 	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
 	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
@@ -1559,7 +1557,6 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
 	amdgpu_bo_print_flag(m, bo, NO_CPU_ACCESS);
 	amdgpu_bo_print_flag(m, bo, CPU_GTT_USWC);
 	amdgpu_bo_print_flag(m, bo, VRAM_CLEARED);
-	amdgpu_bo_print_flag(m, bo, SHADOW);
 	amdgpu_bo_print_flag(m, bo, VRAM_CONTIGUOUS);
 	amdgpu_bo_print_flag(m, bo, VM_ALWAYS_VALID);
 	amdgpu_bo_print_flag(m, bo, EXPLICIT_SYNC);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 8b832f7458f2..9169df7fadee 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -119,8 +119,6 @@ extern "C" {
 #define AMDGPU_GEM_CREATE_CPU_GTT_USWC		(1 << 2)
 /* Flag that the memory should be in VRAM and cleared */
 #define AMDGPU_GEM_CREATE_VRAM_CLEARED		(1 << 3)
-/* Flag that create shadow bo(GTT) while allocating vram bo */
-#define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
 /* Flag that allocating the BO should use linear VRAM */
 #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
 /* Flag that BO is always valid in this VM */
-- 
2.31.1

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

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

* Re: [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init()
  2021-04-22 15:39 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init() Nirmoy Das
@ 2021-04-22 16:13   ` Felix Kuehling
  0 siblings, 0 replies; 12+ messages in thread
From: Felix Kuehling @ 2021-04-22 16:13 UTC (permalink / raw)
  To: Nirmoy Das, Christian.Koenig; +Cc: amd-gfx


Am 2021-04-22 um 11:39 a.m. schrieb Nirmoy Das:
> Currently only way to create compute vm is through
> amdgpu_vm_make_compute(). So vm_context isn't required
> anymore for amdgpu_vm_init().
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

I believe you can also remove the AMDGPU_VM_CONTEXT_* #defines from
amdgpu_vm.h. With that fixed, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 16 +++-------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 +--
>  3 files changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 39ee88d29cca..07e8a7c28561 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1114,7 +1114,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>  		dev_warn(adev->dev, "No more PASIDs available!");
>  		pasid = 0;
>  	}
> -	r = amdgpu_vm_init(adev, &fpriv->vm, AMDGPU_VM_CONTEXT_GFX, pasid);
> +
> +	r = amdgpu_vm_init(adev, &fpriv->vm, pasid);
>  	if (r)
>  		goto error_pasid;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f95bcda8463f..577148a4ffaa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2782,8 +2782,7 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
>   * Returns:
>   * 0 for success, error for failure.
>   */
> -int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -		   int vm_context, u32 pasid)
> +int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>  {
>  	struct amdgpu_bo_param bp;
>  	struct amdgpu_bo *root;
> @@ -2817,16 +2816,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  	vm->pte_support_ats = false;
>  	vm->is_compute_context = false;
>  
> -	if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE) {
> -		vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
> -						AMDGPU_VM_USE_CPU_FOR_COMPUTE);
> +	vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
> +				    AMDGPU_VM_USE_CPU_FOR_GFX);
>  
> -		if (adev->asic_type == CHIP_RAVEN)
> -			vm->pte_support_ats = true;
> -	} else {
> -		vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
> -						AMDGPU_VM_USE_CPU_FOR_GFX);
> -	}
>  	DRM_DEBUG_DRIVER("VM update mode is %s\n",
>  			 vm->use_cpu_for_update ? "CPU" : "SDMA");
>  	WARN_ONCE((vm->use_cpu_for_update &&
> @@ -2844,8 +2836,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  	vm->evicting = false;
>  
>  	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, &bp);
> -	if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE)
> -		bp.flags &= ~AMDGPU_GEM_CREATE_SHADOW;
>  	r = amdgpu_bo_create(adev, &bp, &root);
>  	if (r)
>  		goto error_free_delayed;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 848e175e99ff..7f07acae447b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -379,8 +379,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev);
>  void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
>  
>  long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout);
> -int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -		   int vm_context, u32 pasid);
> +int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid);
>  int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid);
>  void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
>  void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 3/6] drm/amdgpu: remove unused vm context flags
  2021-04-22 15:40 ` [PATCH 3/6] drm/amdgpu: remove unused vm context flags Nirmoy Das
@ 2021-04-22 16:14   ` Felix Kuehling
  0 siblings, 0 replies; 12+ messages in thread
From: Felix Kuehling @ 2021-04-22 16:14 UTC (permalink / raw)
  To: Nirmoy Das, Christian.Koenig; +Cc: amd-gfx

Am 2021-04-22 um 11:40 a.m. schrieb Nirmoy Das:
> Remove unused AMDGPU_VM_CONTEXT_GFX and AMDGPU_VM_CONTEXT_COMPUTE
> flags.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

I saw this too late. Ignore my comment on the previous patch. Both
patches 2 and 3 are

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 7f07acae447b..6a9dcedfcf89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -121,9 +121,6 @@ struct amdgpu_bo_list_entry;
>  /* max vmids dedicated for process */
>  #define AMDGPU_VM_MAX_RESERVED_VMID	1
>  
> -#define AMDGPU_VM_CONTEXT_GFX 0
> -#define AMDGPU_VM_CONTEXT_COMPUTE 1
> -
>  /* See vm_update_mode */
>  #define AMDGPU_VM_USE_CPU_FOR_GFX (1 << 0)
>  #define AMDGPU_VM_USE_CPU_FOR_COMPUTE (1 << 1)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow()
  2021-04-22 15:40 ` [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow() Nirmoy Das
@ 2021-04-23  7:55   ` Christian König
  2021-04-23  9:32     ` Nirmoy
  0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2021-04-23  7:55 UTC (permalink / raw)
  To: Nirmoy Das; +Cc: felix.kuehling, amd-gfx

Am 22.04.21 um 17:40 schrieb Nirmoy Das:
> Shadow BOs are only needed for vm code so call amdgpu_bo_create_shadow()
> directly instead of depending on amdgpu_bo_create().
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 +++++++++++++++++---------
>   1 file changed, 47 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 577148a4ffaa..bb5506ff80dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -850,35 +850,63 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   }
>   
>   /**
> - * amdgpu_vm_bo_param - fill in parameters for PD/PT allocation
> + * amdgpu_vm_pt_create - create bo for PD/PT
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requesting vm
>    * @level: the page table level
>    * @immediate: use a immediate update
> - * @bp: resulting BO allocation parameters
> + * @bo: pointer to the buffer object pointer
>    */
> -static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
> +			       struct amdgpu_vm *vm,
>   			       int level, bool immediate,
> -			       struct amdgpu_bo_param *bp)
> +			       struct amdgpu_bo **bo)
>   {
> -	memset(bp, 0, sizeof(*bp));
> +	struct amdgpu_bo_param bp;

> +	bool create_shadow = false;

As far as I can see this variable is only set and never used.

Please clean that up, apart from that looks good to me.

Christian.

> +	int r;
>   
> -	bp->size = amdgpu_vm_bo_size(adev, level);
> -	bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
> -	bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
> -	bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
> -	bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> +	memset(&bp, 0, sizeof(bp));
> +
> +	bp.size = amdgpu_vm_bo_size(adev, level);
> +	bp.byte_align = AMDGPU_GPU_PAGE_SIZE;
> +	bp.domain = AMDGPU_GEM_DOMAIN_VRAM;
> +	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);
>   	if (vm->use_cpu_for_update)
> -		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>   	else if (!vm->root.base.bo || vm->root.base.bo->shadow)
> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
> -	bp->type = ttm_bo_type_kernel;
> -	bp->no_wait_gpu = immediate;
> +		create_shadow = true;
> +
> +	bp.type = ttm_bo_type_kernel;
> +	bp.no_wait_gpu = immediate;
>   	if (vm->root.base.bo)
> -		bp->resv = vm->root.base.bo->tbo.base.resv;
> +		bp.resv = vm->root.base.bo->tbo.base.resv;
> +
> +	r = amdgpu_bo_create(adev, &bp, bo);
> +	if (r)
> +		return r;
> +
> +	if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
> +		return 0;
> +
> +	if (!bp.resv)
> +		WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
> +				      NULL));
> +	r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
> +
> +	if (!bp.resv)
> +		dma_resv_unlock((*bo)->tbo.base.resv);
> +
> +	if (r) {
> +		amdgpu_bo_unref(bo);
> +		return r;
> +	}
> +
> +	return 0;
>   }
>   
>   /**
> @@ -901,7 +929,6 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   			       bool immediate)
>   {
>   	struct amdgpu_vm_pt *entry = cursor->entry;
> -	struct amdgpu_bo_param bp;
>   	struct amdgpu_bo *pt;
>   	int r;
>   
> @@ -919,9 +946,7 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   	if (entry->base.bo)
>   		return 0;
>   
> -	amdgpu_vm_bo_param(adev, vm, cursor->level, immediate, &bp);
> -
> -	r = amdgpu_bo_create(adev, &bp, &pt);
> +	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt);
>   	if (r)
>   		return r;
>   
> @@ -2784,7 +2809,6 @@ 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_param bp;
>   	struct amdgpu_bo *root;
>   	int r, i;
>   
> @@ -2835,8 +2859,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>   	mutex_init(&vm->eviction_lock);
>   	vm->evicting = false;
>   
> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, &bp);
> -	r = amdgpu_bo_create(adev, &bp, &root);
> +	r = amdgpu_vm_pt_create(adev, vm, adev->vm_manager.root_level,
> +				false, &root);
>   	if (r)
>   		goto error_free_delayed;
>   

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

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

* Re: [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag
  2021-04-22 15:40 ` [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag Nirmoy Das
@ 2021-04-23  7:57   ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2021-04-23  7:57 UTC (permalink / raw)
  To: Nirmoy Das; +Cc: felix.kuehling, amd-gfx

Am 22.04.21 um 17:40 schrieb Nirmoy Das:
> Remove unused AMDGPU_GEM_CREATE_SHADOW flag.

Please add "Userspace as never allowed to use this." to the commit message.

Christian.

>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +----
>   include/uapi/drm/amdgpu_drm.h              | 2 --
>   2 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 39f88e4a8eb5..da6d4ee0a132 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -649,8 +649,7 @@ int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
>   	memset(&bp, 0, sizeof(bp));
>   	bp.size = size;
>   	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
> -	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> -		AMDGPU_GEM_CREATE_SHADOW;
> +	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);
> @@ -685,7 +684,6 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
>   	struct amdgpu_bo *bo_ptr;
>   	int r;
>   
> -	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>   	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
>   	r = amdgpu_bo_create(adev, bp, &bo_ptr);
>   	if (r)
> @@ -1559,7 +1557,6 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
>   	amdgpu_bo_print_flag(m, bo, NO_CPU_ACCESS);
>   	amdgpu_bo_print_flag(m, bo, CPU_GTT_USWC);
>   	amdgpu_bo_print_flag(m, bo, VRAM_CLEARED);
> -	amdgpu_bo_print_flag(m, bo, SHADOW);
>   	amdgpu_bo_print_flag(m, bo, VRAM_CONTIGUOUS);
>   	amdgpu_bo_print_flag(m, bo, VM_ALWAYS_VALID);
>   	amdgpu_bo_print_flag(m, bo, EXPLICIT_SYNC);
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 8b832f7458f2..9169df7fadee 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -119,8 +119,6 @@ extern "C" {
>   #define AMDGPU_GEM_CREATE_CPU_GTT_USWC		(1 << 2)
>   /* Flag that the memory should be in VRAM and cleared */
>   #define AMDGPU_GEM_CREATE_VRAM_CLEARED		(1 << 3)
> -/* Flag that create shadow bo(GTT) while allocating vram bo */
> -#define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
>   /* Flag that allocating the BO should use linear VRAM */
>   #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
>   /* Flag that BO is always valid in this VM */

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

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

* Re: [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow()
  2021-04-23  7:55   ` Christian König
@ 2021-04-23  9:32     ` Nirmoy
  0 siblings, 0 replies; 12+ messages in thread
From: Nirmoy @ 2021-04-23  9:32 UTC (permalink / raw)
  To: Christian König, Nirmoy Das; +Cc: felix.kuehling, amd-gfx


On 4/23/21 9:55 AM, Christian König wrote:
> Am 22.04.21 um 17:40 schrieb Nirmoy Das:
>> Shadow BOs are only needed for vm code so call amdgpu_bo_create_shadow()
>> directly instead of depending on amdgpu_bo_create().
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 +++++++++++++++++---------
>>   1 file changed, 47 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 577148a4ffaa..bb5506ff80dd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -850,35 +850,63 @@ static int amdgpu_vm_clear_bo(struct 
>> amdgpu_device *adev,
>>   }
>>     /**
>> - * amdgpu_vm_bo_param - fill in parameters for PD/PT allocation
>> + * amdgpu_vm_pt_create - create bo for PD/PT
>>    *
>>    * @adev: amdgpu_device pointer
>>    * @vm: requesting vm
>>    * @level: the page table level
>>    * @immediate: use a immediate update
>> - * @bp: resulting BO allocation parameters
>> + * @bo: pointer to the buffer object pointer
>>    */
>> -static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct 
>> amdgpu_vm *vm,
>> +static int amdgpu_vm_pt_create(struct amdgpu_device *adev,
>> +                   struct amdgpu_vm *vm,
>>                      int level, bool immediate,
>> -                   struct amdgpu_bo_param *bp)
>> +                   struct amdgpu_bo **bo)
>>   {
>> -    memset(bp, 0, sizeof(*bp));
>> +    struct amdgpu_bo_param bp;
>
>> +    bool create_shadow = false;
>
> As far as I can see this variable is only set and never used.
>
> Please clean that up, apart from that looks good to me.


Sorry I missed that :/ . Re-sending!


>
> Christian.
>
>> +    int r;
>>   -    bp->size = amdgpu_vm_bo_size(adev, level);
>> -    bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
>> -    bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
>> -    bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
>> -    bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>> +    memset(&bp, 0, sizeof(bp));
>> +
>> +    bp.size = amdgpu_vm_bo_size(adev, level);
>> +    bp.byte_align = AMDGPU_GPU_PAGE_SIZE;
>> +    bp.domain = AMDGPU_GEM_DOMAIN_VRAM;
>> +    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);
>>       if (vm->use_cpu_for_update)
>> -        bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>> +        bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>>       else if (!vm->root.base.bo || vm->root.base.bo->shadow)
>> -        bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>> -    bp->type = ttm_bo_type_kernel;
>> -    bp->no_wait_gpu = immediate;
>> +        create_shadow = true;
>> +
>> +    bp.type = ttm_bo_type_kernel;
>> +    bp.no_wait_gpu = immediate;
>>       if (vm->root.base.bo)
>> -        bp->resv = vm->root.base.bo->tbo.base.resv;
>> +        bp.resv = vm->root.base.bo->tbo.base.resv;
>> +
>> +    r = amdgpu_bo_create(adev, &bp, bo);
>> +    if (r)
>> +        return r;
>> +
>> +    if (vm->is_compute_context && (adev->flags & AMD_IS_APU))
>> +        return 0;
>> +
>> +    if (!bp.resv)
>> +        WARN_ON(dma_resv_lock((*bo)->tbo.base.resv,
>> +                      NULL));
>> +    r = amdgpu_bo_create_shadow(adev, bp.size, *bo);
>> +
>> +    if (!bp.resv)
>> +        dma_resv_unlock((*bo)->tbo.base.resv);
>> +
>> +    if (r) {
>> +        amdgpu_bo_unref(bo);
>> +        return r;
>> +    }
>> +
>> +    return 0;
>>   }
>>     /**
>> @@ -901,7 +929,6 @@ static int amdgpu_vm_alloc_pts(struct 
>> amdgpu_device *adev,
>>                      bool immediate)
>>   {
>>       struct amdgpu_vm_pt *entry = cursor->entry;
>> -    struct amdgpu_bo_param bp;
>>       struct amdgpu_bo *pt;
>>       int r;
>>   @@ -919,9 +946,7 @@ static int amdgpu_vm_alloc_pts(struct 
>> amdgpu_device *adev,
>>       if (entry->base.bo)
>>           return 0;
>>   -    amdgpu_vm_bo_param(adev, vm, cursor->level, immediate, &bp);
>> -
>> -    r = amdgpu_bo_create(adev, &bp, &pt);
>> +    r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt);
>>       if (r)
>>           return r;
>>   @@ -2784,7 +2809,6 @@ 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_param bp;
>>       struct amdgpu_bo *root;
>>       int r, i;
>>   @@ -2835,8 +2859,8 @@ int amdgpu_vm_init(struct amdgpu_device 
>> *adev, struct amdgpu_vm *vm, u32 pasid)
>>       mutex_init(&vm->eviction_lock);
>>       vm->evicting = false;
>>   -    amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, 
>> false, &bp);
>> -    r = amdgpu_bo_create(adev, &bp, &root);
>> +    r = amdgpu_vm_pt_create(adev, vm, adev->vm_manager.root_level,
>> +                false, &root);
>>       if (r)
>>           goto error_free_delayed;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag
  2021-04-23 11:17 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
@ 2021-04-23 11:17 ` Nirmoy Das
  0 siblings, 0 replies; 12+ messages in thread
From: Nirmoy Das @ 2021-04-23 11:17 UTC (permalink / raw)
  To: Christian.Koenig; +Cc: felix.kuehling, Nirmoy Das, amd-gfx

Remove unused AMDGPU_GEM_CREATE_SHADOW flag.
Userspace is never allowed to use this.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +----
 include/uapi/drm/amdgpu_drm.h              | 2 --
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 39f88e4a8eb5..da6d4ee0a132 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -649,8 +649,7 @@ int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 	memset(&bp, 0, sizeof(bp));
 	bp.size = size;
 	bp.domain = AMDGPU_GEM_DOMAIN_GTT;
-	bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC |
-		AMDGPU_GEM_CREATE_SHADOW;
+	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);
@@ -685,7 +684,6 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 	struct amdgpu_bo *bo_ptr;
 	int r;
 
-	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
 	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
 	r = amdgpu_bo_create(adev, bp, &bo_ptr);
 	if (r)
@@ -1559,7 +1557,6 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
 	amdgpu_bo_print_flag(m, bo, NO_CPU_ACCESS);
 	amdgpu_bo_print_flag(m, bo, CPU_GTT_USWC);
 	amdgpu_bo_print_flag(m, bo, VRAM_CLEARED);
-	amdgpu_bo_print_flag(m, bo, SHADOW);
 	amdgpu_bo_print_flag(m, bo, VRAM_CONTIGUOUS);
 	amdgpu_bo_print_flag(m, bo, VM_ALWAYS_VALID);
 	amdgpu_bo_print_flag(m, bo, EXPLICIT_SYNC);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 8b832f7458f2..9169df7fadee 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -119,8 +119,6 @@ extern "C" {
 #define AMDGPU_GEM_CREATE_CPU_GTT_USWC		(1 << 2)
 /* Flag that the memory should be in VRAM and cleared */
 #define AMDGPU_GEM_CREATE_VRAM_CLEARED		(1 << 3)
-/* Flag that create shadow bo(GTT) while allocating vram bo */
-#define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
 /* Flag that allocating the BO should use linear VRAM */
 #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
 /* Flag that BO is always valid in this VM */
-- 
2.31.1

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

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

end of thread, other threads:[~2021-04-23 11:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 15:39 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
2021-04-22 15:39 ` [PATCH 2/6] drm/amdgpu: cleanup amdgpu_vm_init() Nirmoy Das
2021-04-22 16:13   ` Felix Kuehling
2021-04-22 15:40 ` [PATCH 3/6] drm/amdgpu: remove unused vm context flags Nirmoy Das
2021-04-22 16:14   ` Felix Kuehling
2021-04-22 15:40 ` [PATCH 4/6] create shadow bo using amdgpu_bo_create_shadow() Nirmoy Das
2021-04-23  7:55   ` Christian König
2021-04-23  9:32     ` Nirmoy
2021-04-22 15:40 ` [PATCH 5/6] drm/amdgpu: cleanup amdgpu_bo_create() Nirmoy Das
2021-04-22 15:40 ` [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag Nirmoy Das
2021-04-23  7:57   ` Christian König
2021-04-23 11:17 [PATCH 1/6] drm/amdgpu: expose amdgpu_bo_create_shadow() Nirmoy Das
2021-04-23 11:17 ` [PATCH 6/6] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag Nirmoy Das

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.