[PATCH 1/4] drm/amdgpu: expose amdgpu_bo_create_shadow()

[PATCH 1/4] drm/amdgpu: expose amdgpu_bo_create_shadow()

[Nirmoy Das]

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

[PATCH 2/4] drm/amdgpu: create shadow bo using amdgpu_bo_create_shadow()

[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 | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f95bcda8463f..bc302548dfba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -873,8 +873,7 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	bp->bo_ptr_size = sizeof(struct amdgpu_bo);
 	if (vm->use_cpu_for_update)
 		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;
 	if (vm->root.base.bo)
@@ -2844,12 +2843,24 @@ 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;
 
+	if (!(vm_context == AMDGPU_VM_CONTEXT_COMPUTE) &&
+	    (!vm->root.base.bo || vm->root.base.bo->shadow)) {
+		if (!bp.resv)
+			WARN_ON(dma_resv_lock(root->tbo.base.resv,
+					      NULL));
+		r = amdgpu_bo_create_shadow(adev, bp.size, root);
+
+		if (!bp.resv)
+			dma_resv_unlock(root->tbo.base.resv);
+
+		if (r)
+			amdgpu_bo_unref(&root);
+	}
+
 	r = amdgpu_bo_reserve(root, true);
 	if (r)
 		goto error_free_root;
-- 
2.31.1

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

Re: [PATCH 2/4] drm/amdgpu: create shadow bo using amdgpu_bo_create_shadow()

[Christian König]

Am 21.04.21 um 18:35 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().

As far as I can see that won't work correctly.

Now you only create the shadow for the root BO, but not for anything else.

Christian.

>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f95bcda8463f..bc302548dfba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -873,8 +873,7 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>   	if (vm->use_cpu_for_update)
>   		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;
>   	if (vm->root.base.bo)
> @@ -2844,12 +2843,24 @@ 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;
>   
> +	if (!(vm_context == AMDGPU_VM_CONTEXT_COMPUTE) &&
> +	    (!vm->root.base.bo || vm->root.base.bo->shadow)) {
> +		if (!bp.resv)
> +			WARN_ON(dma_resv_lock(root->tbo.base.resv,
> +					      NULL));
> +		r = amdgpu_bo_create_shadow(adev, bp.size, root);
> +
> +		if (!bp.resv)
> +			dma_resv_unlock(root->tbo.base.resv);
> +
> +		if (r)
> +			amdgpu_bo_unref(&root);
> +	}
> +
>   	r = amdgpu_bo_reserve(root, true);
>   	if (r)
>   		goto error_free_root;

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

Re: [PATCH 2/4] drm/amdgpu: create shadow bo using amdgpu_bo_create_shadow()

[Nirmoy]

On 4/22/21 9:08 AM, Christian König wrote:
> Am 21.04.21 um 18:35 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().
>
> As far as I can see that won't work correctly.
>
> Now you only create the shadow for the root BO, but not for anything 
> else.


Yes, I just realized, I have to take care of all amdgpu_vm_bo_param() 
callers.


Thanks,

Nirmoy

>
> Christian.
>
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 19 +++++++++++++++----
>>   1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index f95bcda8463f..bc302548dfba 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -873,8 +873,7 @@ static void amdgpu_vm_bo_param(struct 
>> amdgpu_device *adev, struct amdgpu_vm *vm,
>>       bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>>       if (vm->use_cpu_for_update)
>>           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;
>>       if (vm->root.base.bo)
>> @@ -2844,12 +2843,24 @@ 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;
>>   +    if (!(vm_context == AMDGPU_VM_CONTEXT_COMPUTE) &&
>> +        (!vm->root.base.bo || vm->root.base.bo->shadow)) {
>> +        if (!bp.resv)
>> +            WARN_ON(dma_resv_lock(root->tbo.base.resv,
>> +                          NULL));
>> +        r = amdgpu_bo_create_shadow(adev, bp.size, root);
>> +
>> +        if (!bp.resv)
>> +            dma_resv_unlock(root->tbo.base.resv);
>> +
>> +        if (r)
>> +            amdgpu_bo_unref(&root);
>> +    }
>> +
>>       r = amdgpu_bo_reserve(root, true);
>>       if (r)
>>           goto error_free_root;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/4] drm/amdgpu: cleanup amdgpu_bo_create()

[Nirmoy Das]

Remove shadow bo related code as vm code is creating shadow bo
using proper API.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9cdeb20fb6cd..4256cbfec5eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -661,10 +661,7 @@ int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
  * @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.
+ * Creates an &amdgpu_bo buffer object.
  *
  * Returns:
  * 0 for success or a negative error code on failure.
@@ -673,30 +670,7 @@ 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;
+	return  amdgpu_bo_do_create(adev, bp, bo_ptr);
 }

 /**
--
2.31.1

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

[PATCH 4/4] drm/amdgpu: remove AMDGPU_GEM_CREATE_SHADOW flag

[Nirmoy Das]

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 4256cbfec5eb..2d9bd0d4f81c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -638,8 +638,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);
@@ -692,7 +691,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_do_create(adev, bp, &bo_ptr);
 	if (r)
@@ -1566,7 +1564,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