AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: miss PRT case when bo update
@ 2020-03-16  7:52 Yintian Tao
  2020-03-16  9:38 ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Yintian Tao @ 2020-03-16  7:52 UTC (permalink / raw)
  To: christian.koenig, Alexander.Deucher; +Cc: amd-gfx, Yintian Tao

Originally, only the PTE valid is taken in consider.
The PRT case is missied when bo update which raise problem.
We need add condition for PRT case.

Signed-off-by: Yintian Tao <yttao@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 73398831196f..7a3e4514a00c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1446,7 +1446,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
 		uint64_t incr, entry_end, pe_start;
 		struct amdgpu_bo *pt;
 
-		if (flags & AMDGPU_PTE_VALID) {
+		if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 			/* make sure that the page tables covering the
 			 * address range are actually allocated
 			 */
@@ -1605,7 +1605,6 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 
 	if (flags & AMDGPU_PTE_VALID) {
 		struct amdgpu_bo *root = vm->root.base.bo;
-
 		if (!dma_fence_is_signaled(vm->last_direct))
 			amdgpu_bo_fence(root, vm->last_direct, true);
 
@@ -1718,7 +1717,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
 			}
 
-		} else if (flags & AMDGPU_PTE_VALID) {
+		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 			addr += bo_adev->vm_manager.vram_base_offset;
 			addr += pfn << PAGE_SHIFT;
 		}
-- 
2.17.1

_______________________________________________
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] drm/amdgpu: miss PRT case when bo update
  2020-03-16  7:52 Yintian Tao
@ 2020-03-16  9:38 ` Christian König
  2020-03-16  9:50   ` Tao, Yintian
  0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2020-03-16  9:38 UTC (permalink / raw)
  To: Yintian Tao, Alexander.Deucher; +Cc: amd-gfx

Am 16.03.20 um 08:52 schrieb Yintian Tao:
> Originally, only the PTE valid is taken in consider.
> The PRT case is missied when bo update which raise problem.
> We need add condition for PRT case.

Good catch, just one style nit pick below.

>
> Signed-off-by: Yintian Tao <yttao@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 73398831196f..7a3e4514a00c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1446,7 +1446,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   		uint64_t incr, entry_end, pe_start;
>   		struct amdgpu_bo *pt;
>   
> -		if (flags & AMDGPU_PTE_VALID) {
> +		if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   			/* make sure that the page tables covering the
>   			 * address range are actually allocated
>   			 */
> @@ -1605,7 +1605,6 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>   
>   	if (flags & AMDGPU_PTE_VALID) {
>   		struct amdgpu_bo *root = vm->root.base.bo;
> -
>   		if (!dma_fence_is_signaled(vm->last_direct))

Please keep this empty line, it is required by the coding style guides.

With that fixed the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

>   			amdgpu_bo_fence(root, vm->last_direct, true);
>   
> @@ -1718,7 +1717,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
>   			}
>   
> -		} else if (flags & AMDGPU_PTE_VALID) {
> +		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   			addr += bo_adev->vm_manager.vram_base_offset;
>   			addr += pfn << PAGE_SHIFT;
>   		}

_______________________________________________
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] drm/amdgpu: miss PRT case when bo update
  2020-03-16  9:38 ` Christian König
@ 2020-03-16  9:50   ` Tao, Yintian
  0 siblings, 0 replies; 6+ messages in thread
From: Tao, Yintian @ 2020-03-16  9:50 UTC (permalink / raw)
  To: Koenig, Christian, Deucher, Alexander; +Cc: amd-gfx@lists.freedesktop.org

Hi Christian

Many thanks for your review

Best Regards
Yintian Tao

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com> 
Sent: 2020年3月16日 17:38
To: Tao, Yintian <Yintian.Tao@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: miss PRT case when bo update

Am 16.03.20 um 08:52 schrieb Yintian Tao:
> Originally, only the PTE valid is taken in consider.
> The PRT case is missied when bo update which raise problem.
> We need add condition for PRT case.

Good catch, just one style nit pick below.

>
> Signed-off-by: Yintian Tao <yttao@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 73398831196f..7a3e4514a00c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1446,7 +1446,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   		uint64_t incr, entry_end, pe_start;
>   		struct amdgpu_bo *pt;
>   
> -		if (flags & AMDGPU_PTE_VALID) {
> +		if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   			/* make sure that the page tables covering the
>   			 * address range are actually allocated
>   			 */
> @@ -1605,7 +1605,6 @@ static int amdgpu_vm_bo_update_mapping(struct 
> amdgpu_device *adev,
>   
>   	if (flags & AMDGPU_PTE_VALID) {
>   		struct amdgpu_bo *root = vm->root.base.bo;
> -
>   		if (!dma_fence_is_signaled(vm->last_direct))

Please keep this empty line, it is required by the coding style guides.

With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

>   			amdgpu_bo_fence(root, vm->last_direct, true);
>   
> @@ -1718,7 +1717,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
>   			}
>   
> -		} else if (flags & AMDGPU_PTE_VALID) {
> +		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   			addr += bo_adev->vm_manager.vram_base_offset;
>   			addr += pfn << PAGE_SHIFT;
>   		}

_______________________________________________
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

* [PATCH] drm/amdgpu: miss PRT case when bo update
@ 2020-03-16 10:00 Yintian Tao
  0 siblings, 0 replies; 6+ messages in thread
From: Yintian Tao @ 2020-03-16 10:00 UTC (permalink / raw)
  To: christian.koenig, Alexander.Deucher; +Cc: amd-gfx, Yintian Tao

Originally, only the PTE valid is taken in consider.
The PRT case is missied when bo update which raise problem.
We need add condition for PRT case.

v2: add PRT condition for amdgpu_vm_bo_update_mapping, too

Signed-off-by: Yintian Tao <yttao@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 73398831196f..87e6085542f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1446,7 +1446,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
 		uint64_t incr, entry_end, pe_start;
 		struct amdgpu_bo *pt;
 
-		if (flags & AMDGPU_PTE_VALID) {
+		if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 			/* make sure that the page tables covering the
 			 * address range are actually allocated
 			 */
@@ -1603,7 +1603,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 		goto error_unlock;
 	}
 
-	if (flags & AMDGPU_PTE_VALID) {
+	if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT) {
 		struct amdgpu_bo *root = vm->root.base.bo;
 
 		if (!dma_fence_is_signaled(vm->last_direct))
@@ -1718,7 +1718,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
 			}
 
-		} else if (flags & AMDGPU_PTE_VALID) {
+		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 			addr += bo_adev->vm_manager.vram_base_offset;
 			addr += pfn << PAGE_SHIFT;
 		}
-- 
2.17.1

_______________________________________________
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

* [PATCH] drm/amdgpu: miss PRT case when bo update
@ 2020-03-16 10:08 Yintian Tao
  2020-03-16 12:17 ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Yintian Tao @ 2020-03-16 10:08 UTC (permalink / raw)
  To: christian.koenig, Alexander.Deucher; +Cc: amd-gfx, Yintian Tao

Originally, only the PTE valid is taken in consider.
The PRT case is missied when bo update which raise problem.
We need add condition for PRT case.

v2: add PRT condition for amdgpu_vm_bo_update_mapping, too
v3: fix one typo error

Signed-off-by: Yintian Tao <yttao@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 73398831196f..6157f6a9dcc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1446,7 +1446,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
 		uint64_t incr, entry_end, pe_start;
 		struct amdgpu_bo *pt;
 
-		if (flags & AMDGPU_PTE_VALID) {
+		if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 			/* make sure that the page tables covering the
 			 * address range are actually allocated
 			 */
@@ -1603,7 +1603,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 		goto error_unlock;
 	}
 
-	if (flags & AMDGPU_PTE_VALID) {
+	if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 		struct amdgpu_bo *root = vm->root.base.bo;
 
 		if (!dma_fence_is_signaled(vm->last_direct))
@@ -1718,7 +1718,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
 			}
 
-		} else if (flags & AMDGPU_PTE_VALID) {
+		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
 			addr += bo_adev->vm_manager.vram_base_offset;
 			addr += pfn << PAGE_SHIFT;
 		}
-- 
2.17.1

_______________________________________________
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] drm/amdgpu: miss PRT case when bo update
  2020-03-16 10:08 Yintian Tao
@ 2020-03-16 12:17 ` Christian König
  0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2020-03-16 12:17 UTC (permalink / raw)
  To: Yintian Tao, Alexander.Deucher; +Cc: amd-gfx

Am 16.03.20 um 11:08 schrieb Yintian Tao:
> Originally, only the PTE valid is taken in consider.
> The PRT case is missied when bo update which raise problem.
> We need add condition for PRT case.
>
> v2: add PRT condition for amdgpu_vm_bo_update_mapping, too

Good point, yes. Feel free to commit it like this, but we might want to 
add a define for (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT) at some time to 
make the code easier to read.

Going to take care of this as a follow up cleanup as soon as the patch 
lands on amd-staging-drm-next.

Regards,
Christian.

> v3: fix one typo error
>
> Signed-off-by: Yintian Tao <yttao@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 73398831196f..6157f6a9dcc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1446,7 +1446,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   		uint64_t incr, entry_end, pe_start;
>   		struct amdgpu_bo *pt;
>   
> -		if (flags & AMDGPU_PTE_VALID) {
> +		if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   			/* make sure that the page tables covering the
>   			 * address range are actually allocated
>   			 */
> @@ -1603,7 +1603,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>   		goto error_unlock;
>   	}
>   
> -	if (flags & AMDGPU_PTE_VALID) {
> +	if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   		struct amdgpu_bo *root = vm->root.base.bo;
>   
>   		if (!dma_fence_is_signaled(vm->last_direct))
> @@ -1718,7 +1718,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
>   			}
>   
> -		} else if (flags & AMDGPU_PTE_VALID) {
> +		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
>   			addr += bo_adev->vm_manager.vram_base_offset;
>   			addr += pfn << PAGE_SHIFT;
>   		}

_______________________________________________
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:[~2020-03-16 12:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-16 10:00 [PATCH] drm/amdgpu: miss PRT case when bo update Yintian Tao
  -- strict thread matches above, loose matches on Subject: below --
2020-03-16 10:08 Yintian Tao
2020-03-16 12:17 ` Christian König
2020-03-16  7:52 Yintian Tao
2020-03-16  9:38 ` Christian König
2020-03-16  9:50   ` Tao, Yintian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox