All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
@ 2018-09-06  0:28 Felix Kuehling
       [not found] ` <1536193713-4016-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Felix Kuehling @ 2018-09-06  0:28 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Felix Kuehling, kent.russell-5C7GfCeVMHo

The intent of two commits was lost in the last rebase:

810955b drm/amdgpu: Fix acquiring VM on large-BAR systems
b5d21aa drm/amdgpu: Don't use shadow BO for compute context

This commit restores the original behaviour:
* Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
  to allow them to be reused for compute VMs
* Don't create shadow BOs for page tables in compute VMs

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ea5e277..5e7a3de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
  *
  * @adev: amdgpu_device pointer
  * @vm: requesting vm
+ * @level: level in the page table hierarchy
+ * @no_shadow: disable creation of shadow BO for this VM
  * @bp: resulting BO allocation parameters
  */
 static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			       int level, struct amdgpu_bo_param *bp)
+			       int level, bool no_shadow,
+			       struct amdgpu_bo_param *bp)
 {
 	memset(bp, 0, sizeof(*bp));
 
@@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
 	if (vm->use_cpu_for_update)
 		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
-	else
-		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
-			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
+	else if (!no_shadow)
+		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
 	bp->type = ttm_bo_type_kernel;
 	if (vm->root.base.bo)
 		bp->resv = vm->root.base.bo->tbo.resv;
@@ -626,6 +628,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 				  unsigned level, bool ats)
 {
 	unsigned shift = amdgpu_vm_level_shift(adev, level);
+	bool no_shadow = !vm->root.base.bo->shadow;
 	struct amdgpu_bo_param bp;
 	unsigned pt_idx, from, to;
 	int r;
@@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 	saddr = saddr & ((1 << shift) - 1);
 	eaddr = eaddr & ((1 << shift) - 1);
 
-	amdgpu_vm_bo_param(adev, vm, level, &bp);
+	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
 
 	/* walk over the address space and allocate the page tables */
 	for (pt_idx = from; pt_idx <= to; ++pt_idx) {
@@ -2709,6 +2712,7 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
 int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		   int vm_context, unsigned int pasid)
 {
+	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
 	struct amdgpu_bo_param bp;
 	struct amdgpu_bo *root;
 	int r, i;
@@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		  "CPU update of VM recommended only for large BAR system\n");
 	vm->last_update = NULL;
 
-	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
+	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
+			   &bp);
 	r = amdgpu_bo_create(adev, &bp, &root);
 	if (r)
 		goto error_free_sched_entity;
-- 
2.7.4

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

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

* Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
       [not found] ` <1536193713-4016-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
@ 2018-09-06  1:48   ` Zhang, Jerry (Junwei)
  2018-09-06  2:08   ` zhoucm1
  2018-09-06  7:35   ` Christian König
  2 siblings, 0 replies; 6+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-09-06  1:48 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: kent.russell-5C7GfCeVMHo

On 09/06/2018 08:28 AM, Felix Kuehling wrote:
> The intent of two commits was lost in the last rebase:
>
> 810955b drm/amdgpu: Fix acquiring VM on large-BAR systems
> b5d21aa drm/amdgpu: Don't use shadow BO for compute context
>
> This commit restores the original behaviour:
> * Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
>    to allow them to be reused for compute VMs
> * Don't create shadow BOs for page tables in compute VMs
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

Personally making no_shadow -> shadow looks more simple.
anyway, that patches restoring what it's missing.

Acked-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ea5e277..5e7a3de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requesting vm
> + * @level: level in the page table hierarchy
> + * @no_shadow: disable creation of shadow BO for this VM
>    * @bp: resulting BO allocation parameters
>    */
>   static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			       int level, struct amdgpu_bo_param *bp)
> +			       int level, bool no_shadow,
> +			       struct amdgpu_bo_param *bp)
>   {
>   	memset(bp, 0, sizeof(*bp));
>
> @@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>   	if (vm->use_cpu_for_update)
>   		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> -	else
> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
> -			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> +	else if (!no_shadow)
> +		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>   	bp->type = ttm_bo_type_kernel;
>   	if (vm->root.base.bo)
>   		bp->resv = vm->root.base.bo->tbo.resv;
> @@ -626,6 +628,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   				  unsigned level, bool ats)
>   {
>   	unsigned shift = amdgpu_vm_level_shift(adev, level);
> +	bool no_shadow = !vm->root.base.bo->shadow;
>   	struct amdgpu_bo_param bp;
>   	unsigned pt_idx, from, to;
>   	int r;
> @@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   	saddr = saddr & ((1 << shift) - 1);
>   	eaddr = eaddr & ((1 << shift) - 1);
>
> -	amdgpu_vm_bo_param(adev, vm, level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
>
>   	/* walk over the address space and allocate the page tables */
>   	for (pt_idx = from; pt_idx <= to; ++pt_idx) {
> @@ -2709,6 +2712,7 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		   int vm_context, unsigned int pasid)
>   {
> +	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
>   	struct amdgpu_bo_param bp;
>   	struct amdgpu_bo *root;
>   	int r, i;
> @@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		  "CPU update of VM recommended only for large BAR system\n");
>   	vm->last_update = NULL;
>
> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
> +			   &bp);
>   	r = amdgpu_bo_create(adev, &bp, &root);
>   	if (r)
>   		goto error_free_sched_entity;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
       [not found] ` <1536193713-4016-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
  2018-09-06  1:48   ` Zhang, Jerry (Junwei)
@ 2018-09-06  2:08   ` zhoucm1
  2018-09-06  7:35   ` Christian König
  2 siblings, 0 replies; 6+ messages in thread
From: zhoucm1 @ 2018-09-06  2:08 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: kent.russell-5C7GfCeVMHo



On 2018年09月06日 08:28, Felix Kuehling wrote:
> The intent of two commits was lost in the last rebase:
>
> 810955b drm/amdgpu: Fix acquiring VM on large-BAR systems
> b5d21aa drm/amdgpu: Don't use shadow BO for compute context
>
> This commit restores the original behaviour:
> * Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
>    to allow them to be reused for compute VMs
> * Don't create shadow BOs for page tables in compute VMs
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ea5e277..5e7a3de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requesting vm
> + * @level: level in the page table hierarchy
> + * @no_shadow: disable creation of shadow BO for this VM
>    * @bp: resulting BO allocation parameters
>    */
>   static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			       int level, struct amdgpu_bo_param *bp)
> +			       int level, bool no_shadow,
> +			       struct amdgpu_bo_param *bp)
How about adding no_shadow to bp? Which also can consider to be parm of bo.

Regards,
David Zhou
>   {
>   	memset(bp, 0, sizeof(*bp));
>   
> @@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>   	if (vm->use_cpu_for_update)
>   		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> -	else
> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
> -			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> +	else if (!no_shadow)
> +		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>   	bp->type = ttm_bo_type_kernel;
>   	if (vm->root.base.bo)
>   		bp->resv = vm->root.base.bo->tbo.resv;
> @@ -626,6 +628,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   				  unsigned level, bool ats)
>   {
>   	unsigned shift = amdgpu_vm_level_shift(adev, level);
> +	bool no_shadow = !vm->root.base.bo->shadow;
>   	struct amdgpu_bo_param bp;
>   	unsigned pt_idx, from, to;
>   	int r;
> @@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   	saddr = saddr & ((1 << shift) - 1);
>   	eaddr = eaddr & ((1 << shift) - 1);
>   
> -	amdgpu_vm_bo_param(adev, vm, level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
>   
>   	/* walk over the address space and allocate the page tables */
>   	for (pt_idx = from; pt_idx <= to; ++pt_idx) {
> @@ -2709,6 +2712,7 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		   int vm_context, unsigned int pasid)
>   {
> +	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
>   	struct amdgpu_bo_param bp;
>   	struct amdgpu_bo *root;
>   	int r, i;
> @@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		  "CPU update of VM recommended only for large BAR system\n");
>   	vm->last_update = NULL;
>   
> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
> +			   &bp);
>   	r = amdgpu_bo_create(adev, &bp, &root);
>   	if (r)
>   		goto error_free_sched_entity;

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

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

* Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
       [not found] ` <1536193713-4016-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
  2018-09-06  1:48   ` Zhang, Jerry (Junwei)
  2018-09-06  2:08   ` zhoucm1
@ 2018-09-06  7:35   ` Christian König
       [not found]     ` <924d44b9-379e-42dd-b280-c3d52794d7ef-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-09-06  7:35 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: kent.russell-5C7GfCeVMHo

Am 06.09.2018 um 02:28 schrieb Felix Kuehling:
> The intent of two commits was lost in the last rebase:
>
> 810955b drm/amdgpu: Fix acquiring VM on large-BAR systems
> b5d21aa drm/amdgpu: Don't use shadow BO for compute context
>
> This commit restores the original behaviour:
> * Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
>    to allow them to be reused for compute VMs
> * Don't create shadow BOs for page tables in compute VMs
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ea5e277..5e7a3de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requesting vm
> + * @level: level in the page table hierarchy
> + * @no_shadow: disable creation of shadow BO for this VM
>    * @bp: resulting BO allocation parameters
>    */
>   static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			       int level, struct amdgpu_bo_param *bp)
> +			       int level, bool no_shadow,
> +			       struct amdgpu_bo_param *bp)
>   {
>   	memset(bp, 0, sizeof(*bp));
>   
> @@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>   	if (vm->use_cpu_for_update)
>   		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> -	else
> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
> -			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> +	else if (!no_shadow)
> +		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>   	bp->type = ttm_bo_type_kernel;
>   	if (vm->root.base.bo)
>   		bp->resv = vm->root.base.bo->tbo.resv;
> @@ -626,6 +628,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   				  unsigned level, bool ats)
>   {
>   	unsigned shift = amdgpu_vm_level_shift(adev, level);
> +	bool no_shadow = !vm->root.base.bo->shadow;

Please move that logic into amdgpu_vm_bo_param().

Christian.

>   	struct amdgpu_bo_param bp;
>   	unsigned pt_idx, from, to;
>   	int r;
> @@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   	saddr = saddr & ((1 << shift) - 1);
>   	eaddr = eaddr & ((1 << shift) - 1);
>   
> -	amdgpu_vm_bo_param(adev, vm, level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
>   
>   	/* walk over the address space and allocate the page tables */
>   	for (pt_idx = from; pt_idx <= to; ++pt_idx) {
> @@ -2709,6 +2712,7 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		   int vm_context, unsigned int pasid)
>   {
> +	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
>   	struct amdgpu_bo_param bp;
>   	struct amdgpu_bo *root;
>   	int r, i;
> @@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		  "CPU update of VM recommended only for large BAR system\n");
>   	vm->last_update = NULL;
>   
> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
> +			   &bp);
>   	r = amdgpu_bo_create(adev, &bp, &root);
>   	if (r)
>   		goto error_free_sched_entity;

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

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

* RE: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
       [not found]     ` <924d44b9-379e-42dd-b280-c3d52794d7ef-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-09-06 11:46       ` Russell, Kent
       [not found]         ` <BN6PR12MB1618872F1E93ED8AF37E7E8785010-/b2+HYfkarRqaFUXYJa4HgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Russell, Kent @ 2018-09-06 11:46 UTC (permalink / raw)
  To: Koenig, Christian, Kuehling, Felix,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW



-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Thursday, September 06, 2018 3:36 AM
To: Kuehling, Felix <Felix.Kuehling@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Russell, Kent <Kent.Russell@amd.com>
Subject: Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase

Am 06.09.2018 um 02:28 schrieb Felix Kuehling:
> The intent of two commits was lost in the last rebase:
>
> 810955b drm/amdgpu: Fix acquiring VM on large-BAR systems b5d21aa 
> drm/amdgpu: Don't use shadow BO for compute context
>
> This commit restores the original behaviour:
> * Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
>    to allow them to be reused for compute VMs
> * Don't create shadow BOs for page tables in compute VMs
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ea5e277..5e7a3de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>    *
>    * @adev: amdgpu_device pointer
>    * @vm: requesting vm
> + * @level: level in the page table hierarchy
> + * @no_shadow: disable creation of shadow BO for this VM
>    * @bp: resulting BO allocation parameters
>    */
>   static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			       int level, struct amdgpu_bo_param *bp)
> +			       int level, bool no_shadow,
> +			       struct amdgpu_bo_param *bp)
>   {
>   	memset(bp, 0, sizeof(*bp));
>   
> @@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>   	if (vm->use_cpu_for_update)
>   		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> -	else
> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
> -			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> +	else if (!no_shadow)
> +		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>   	bp->type = ttm_bo_type_kernel;
>   	if (vm->root.base.bo)
>   		bp->resv = vm->root.base.bo->tbo.resv; @@ -626,6 +628,7 @@ static 
> int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   				  unsigned level, bool ats)
>   {
>   	unsigned shift = amdgpu_vm_level_shift(adev, level);
> +	bool no_shadow = !vm->root.base.bo->shadow;

Please move that logic into amdgpu_vm_bo_param().
[KR] The issue with that is that in the other call below, we don't have access to the vm context via the vm struct, so we can't move the logic in for both calls.

Christian.

>   	struct amdgpu_bo_param bp;
>   	unsigned pt_idx, from, to;
>   	int r;
> @@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   	saddr = saddr & ((1 << shift) - 1);
>   	eaddr = eaddr & ((1 << shift) - 1);
>   
> -	amdgpu_vm_bo_param(adev, vm, level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
>   
>   	/* walk over the address space and allocate the page tables */
>   	for (pt_idx = from; pt_idx <= to; ++pt_idx) { @@ -2709,6 +2712,7 @@ 
> void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>   int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		   int vm_context, unsigned int pasid)
>   {
> +	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
>   	struct amdgpu_bo_param bp;
>   	struct amdgpu_bo *root;
>   	int r, i;
> @@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		  "CPU update of VM recommended only for large BAR system\n");
>   	vm->last_update = NULL;
>   
> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
> +	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
> +			   &bp);
>   	r = amdgpu_bo_create(adev, &bp, &root);
>   	if (r)
>   		goto error_free_sched_entity;

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

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

* Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
       [not found]         ` <BN6PR12MB1618872F1E93ED8AF37E7E8785010-/b2+HYfkarRqaFUXYJa4HgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-09-06 11:54           ` Christian König
  0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2018-09-06 11:54 UTC (permalink / raw)
  To: Russell, Kent, Kuehling, Felix, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 06.09.2018 um 13:46 schrieb Russell, Kent:
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Thursday, September 06, 2018 3:36 AM
> To: Kuehling, Felix <Felix.Kuehling@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent <Kent.Russell@amd.com>
> Subject: Re: [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase
>
> Am 06.09.2018 um 02:28 schrieb Felix Kuehling:
>> The intent of two commits was lost in the last rebase:
>>
>> 810955b drm/amdgpu: Fix acquiring VM on large-BAR systems b5d21aa
>> drm/amdgpu: Don't use shadow BO for compute context
>>
>> This commit restores the original behaviour:
>> * Don't set AMDGPU_GEM_CREATE_NO_CPU_ACCESS for page directories
>>     to allow them to be reused for compute VMs
>> * Don't create shadow BOs for page tables in compute VMs
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++------
>>    1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index ea5e277..5e7a3de 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -577,10 +577,13 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>>     *
>>     * @adev: amdgpu_device pointer
>>     * @vm: requesting vm
>> + * @level: level in the page table hierarchy
>> + * @no_shadow: disable creation of shadow BO for this VM
>>     * @bp: resulting BO allocation parameters
>>     */
>>    static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>> -			       int level, struct amdgpu_bo_param *bp)
>> +			       int level, bool no_shadow,
>> +			       struct amdgpu_bo_param *bp)
>>    {
>>    	memset(bp, 0, sizeof(*bp));
>>    
>> @@ -595,9 +598,8 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    		AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>>    	if (vm->use_cpu_for_update)
>>    		bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>> -	else
>> -		bp->flags |= AMDGPU_GEM_CREATE_SHADOW |
>> -			AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
>> +	else if (!no_shadow)
>> +		bp->flags |= AMDGPU_GEM_CREATE_SHADOW;
>>    	bp->type = ttm_bo_type_kernel;
>>    	if (vm->root.base.bo)
>>    		bp->resv = vm->root.base.bo->tbo.resv; @@ -626,6 +628,7 @@ static
>> int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>>    				  unsigned level, bool ats)
>>    {
>>    	unsigned shift = amdgpu_vm_level_shift(adev, level);
>> +	bool no_shadow = !vm->root.base.bo->shadow;
> Please move that logic into amdgpu_vm_bo_param().
> [KR] The issue with that is that in the other call below, we don't have access to the vm context via the vm struct, so we can't move the logic in for both calls.

Yeah, but you can look at the root PD (if present) and see if it has a 
shadow BO.

And for the root PD we can clear the flag manually after calling 
amdgpu_vm_bo_param().

Christian.

>
> Christian.
>
>>    	struct amdgpu_bo_param bp;
>>    	unsigned pt_idx, from, to;
>>    	int r;
>> @@ -650,7 +653,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>>    	saddr = saddr & ((1 << shift) - 1);
>>    	eaddr = eaddr & ((1 << shift) - 1);
>>    
>> -	amdgpu_vm_bo_param(adev, vm, level, &bp);
>> +	amdgpu_vm_bo_param(adev, vm, level, no_shadow, &bp);
>>    
>>    	/* walk over the address space and allocate the page tables */
>>    	for (pt_idx = from; pt_idx <= to; ++pt_idx) { @@ -2709,6 +2712,7 @@
>> void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>>    int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    		   int vm_context, unsigned int pasid)
>>    {
>> +	bool no_shadow = (vm_context == AMDGPU_VM_CONTEXT_COMPUTE);
>>    	struct amdgpu_bo_param bp;
>>    	struct amdgpu_bo *root;
>>    	int r, i;
>> @@ -2748,7 +2752,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    		  "CPU update of VM recommended only for large BAR system\n");
>>    	vm->last_update = NULL;
>>    
>> -	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, &bp);
>> +	amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, no_shadow,
>> +			   &bp);
>>    	r = amdgpu_bo_create(adev, &bp, &root);
>>    	if (r)
>>    		goto error_free_sched_entity;

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

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

end of thread, other threads:[~2018-09-06 11:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06  0:28 [PATCH 1/1] drm/amdgpu: Fix compute VM BO params after rebase Felix Kuehling
     [not found] ` <1536193713-4016-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
2018-09-06  1:48   ` Zhang, Jerry (Junwei)
2018-09-06  2:08   ` zhoucm1
2018-09-06  7:35   ` Christian König
     [not found]     ` <924d44b9-379e-42dd-b280-c3d52794d7ef-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-09-06 11:46       ` Russell, Kent
     [not found]         ` <BN6PR12MB1618872F1E93ED8AF37E7E8785010-/b2+HYfkarRqaFUXYJa4HgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-09-06 11:54           ` Christian König

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.