All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
@ 2022-01-17 23:43 Jonathan Kim
  2022-01-18  2:17 ` Chen, Guchun
  2022-01-18  7:29 ` Christian König
  0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Kim @ 2022-01-17 23:43 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Jonathan Kim, Christian.Koenig, Guchun.chen

Move the debug sdma vram bounce buffer GART map on device init after when
GART is ready to avoid warnings and non-access to SDMA.

Also move bounce buffer tear down after the memory manager has flushed
queued work for safety.

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 --------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index da3348fa7b0e..099460d15258 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
 	if (r)
 		goto init_failed;
 
+	/* GTT bounce buffer for debug vram access over sdma. */
+	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
+				AMDGPU_GEM_DOMAIN_GTT,
+				&adev->mman.sdma_access_bo, NULL,
+				&adev->mman.sdma_access_ptr))
+		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
+
 	/*
 	 * retired pages will be loaded from eeprom and reserved here,
 	 * it should be called after amdgpu_device_ip_hw_init_phase2  since
@@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 	}
 	adev->shutdown = true;
 
+	/* remove debug vram sdma access bounce buffer. */
+	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
+					&adev->mman.sdma_access_ptr);
+
 	/* make sure IB test finished before entering exclusive mode
 	 * to avoid preemption on IB test
 	 * */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b489cd8abe31..6178ae7ba624 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		return r;
 	}
 
-	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
-				AMDGPU_GEM_DOMAIN_GTT,
-				&adev->mman.sdma_access_bo, NULL,
-				adev->mman.sdma_access_ptr))
-		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
-
 	return 0;
 }
 
@@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 	ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
 	ttm_device_fini(&adev->mman.bdev);
 	adev->mman.initialized = false;
-	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
-					&adev->mman.sdma_access_ptr);
 	DRM_INFO("amdgpu: ttm finalized\n");
 }
 
-- 
2.25.1


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

* RE: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
  2022-01-17 23:43 [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer Jonathan Kim
@ 2022-01-18  2:17 ` Chen, Guchun
  2022-01-18  7:29 ` Christian König
  1 sibling, 0 replies; 6+ messages in thread
From: Chen, Guchun @ 2022-01-18  2:17 UTC (permalink / raw)
  To: Kim, Jonathan, amd-gfx; +Cc: Kuehling, Felix, Koenig, Christian

[Public]

Thanks for your quick fix, Jonathan.

I wonder if it's better to encapsulate the bo creation into a function and put the function definition in amdgpu_sdma or amdgpu_ttm?

Regards,
Guchun

-----Original Message-----
From: Kim, Jonathan <Jonathan.Kim@amd.com> 
Sent: Tuesday, January 18, 2022 7:43 AM
To: amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>
Subject: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer

Move the debug sdma vram bounce buffer GART map on device init after when GART is ready to avoid warnings and non-access to SDMA.

Also move bounce buffer tear down after the memory manager has flushed queued work for safety.

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 --------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index da3348fa7b0e..099460d15258 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
 	if (r)
 		goto init_failed;
 
+	/* GTT bounce buffer for debug vram access over sdma. */
+	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
+				AMDGPU_GEM_DOMAIN_GTT,
+				&adev->mman.sdma_access_bo, NULL,
+				&adev->mman.sdma_access_ptr))
+		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
+
 	/*
 	 * retired pages will be loaded from eeprom and reserved here,
 	 * it should be called after amdgpu_device_ip_hw_init_phase2  since @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 	}
 	adev->shutdown = true;
 
+	/* remove debug vram sdma access bounce buffer. */
+	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
+					&adev->mman.sdma_access_ptr);
+
 	/* make sure IB test finished before entering exclusive mode
 	 * to avoid preemption on IB test
 	 * */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b489cd8abe31..6178ae7ba624 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		return r;
 	}
 
-	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
-				AMDGPU_GEM_DOMAIN_GTT,
-				&adev->mman.sdma_access_bo, NULL,
-				adev->mman.sdma_access_ptr))
-		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
-
 	return 0;
 }
 
@@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 	ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
 	ttm_device_fini(&adev->mman.bdev);
 	adev->mman.initialized = false;
-	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
-					&adev->mman.sdma_access_ptr);
 	DRM_INFO("amdgpu: ttm finalized\n");
 }
 
--
2.25.1

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

* Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
  2022-01-17 23:43 [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer Jonathan Kim
  2022-01-18  2:17 ` Chen, Guchun
@ 2022-01-18  7:29 ` Christian König
  2022-01-18  8:09   ` Chen, Guchun
  1 sibling, 1 reply; 6+ messages in thread
From: Christian König @ 2022-01-18  7:29 UTC (permalink / raw)
  To: Jonathan Kim, amd-gfx; +Cc: Felix.Kuehling, Guchun.chen

Am 18.01.22 um 00:43 schrieb Jonathan Kim:
> Move the debug sdma vram bounce buffer GART map on device init after when
> GART is ready to avoid warnings and non-access to SDMA.

Well that doesn't seem to make sense the GART is initialized by the code 
around the allocation so that should work fine.

Freeing the BO indeed needs to be moved up, but that can still be in the 
same function.

Also please don't move TTM related code outside of the TTM code in amdgpu.

Regards,
Christian.

>
> Also move bounce buffer tear down after the memory manager has flushed
> queued work for safety.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 --------
>   2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index da3348fa7b0e..099460d15258 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>   	if (r)
>   		goto init_failed;
>   
> +	/* GTT bounce buffer for debug vram access over sdma. */
> +	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> +				AMDGPU_GEM_DOMAIN_GTT,
> +				&adev->mman.sdma_access_bo, NULL,
> +				&adev->mman.sdma_access_ptr))
> +		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
> +
>   	/*
>   	 * retired pages will be loaded from eeprom and reserved here,
>   	 * it should be called after amdgpu_device_ip_hw_init_phase2  since
> @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
>   	}
>   	adev->shutdown = true;
>   
> +	/* remove debug vram sdma access bounce buffer. */
> +	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> +					&adev->mman.sdma_access_ptr);
> +
>   	/* make sure IB test finished before entering exclusive mode
>   	 * to avoid preemption on IB test
>   	 * */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index b489cd8abe31..6178ae7ba624 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   		return r;
>   	}
>   
> -	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> -				AMDGPU_GEM_DOMAIN_GTT,
> -				&adev->mman.sdma_access_bo, NULL,
> -				adev->mman.sdma_access_ptr))
> -		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
> -
>   	return 0;
>   }
>   
> @@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   	ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
>   	ttm_device_fini(&adev->mman.bdev);
>   	adev->mman.initialized = false;
> -	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> -					&adev->mman.sdma_access_ptr);
>   	DRM_INFO("amdgpu: ttm finalized\n");
>   }
>   


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

* RE: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
  2022-01-18  7:29 ` Christian König
@ 2022-01-18  8:09   ` Chen, Guchun
  2022-01-18  8:13     ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Chen, Guchun @ 2022-01-18  8:09 UTC (permalink / raw)
  To: Koenig, Christian, Kim, Jonathan, amd-gfx; +Cc: Kuehling, Felix

[Public]

Hi Christian,

Re: Well that doesn't seem to make sense the GART is initialized by the code around the allocation so that should work fine.

Below is the calltrace during driver probe. When binding the page(SDMA bo) into gart table, there is a check by gart.ready, that will be set to be true later on in gmc_v10_0_hw_init. So a calltrace is observed.

[    3.381510]  amdgpu_ttm_gart_bind+0x80/0xc0 [amdgpu]
[    3.381580]  amdgpu_ttm_alloc_gart+0x158/0x1c0 [amdgpu]
[    3.381647]  amdgpu_bo_create_reserved+0x136/0x1e0 [amdgpu]
[    3.381714]  ? amdgpu_ttm_debugfs_init+0x120/0x120 [amdgpu]
[    3.381782]  amdgpu_bo_create_kernel+0x17/0x80 [amdgpu]
[    3.381849]  amdgpu_ttm_init.cold+0x174/0x18e [amdgpu]
[    3.381951]  ? vprintk_default+0x1d/0x20
[    3.381955]  ? printk+0x58/0x6f
[    3.381957]  amdgpu_bo_init.cold+0x4b/0x53 [amdgpu]
[    3.382052]  gmc_v10_0_sw_init+0x304/0x490 [amdgpu]

Regards,
Guchun

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com> 
Sent: Tuesday, January 18, 2022 3:30 PM
To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer

Am 18.01.22 um 00:43 schrieb Jonathan Kim:
> Move the debug sdma vram bounce buffer GART map on device init after 
> when GART is ready to avoid warnings and non-access to SDMA.

Well that doesn't seem to make sense the GART is initialized by the code around the allocation so that should work fine.

Freeing the BO indeed needs to be moved up, but that can still be in the same function.

Also please don't move TTM related code outside of the TTM code in amdgpu.

Regards,
Christian.

>
> Also move bounce buffer tear down after the memory manager has flushed 
> queued work for safety.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 --------
>   2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index da3348fa7b0e..099460d15258 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>   	if (r)
>   		goto init_failed;
>   
> +	/* GTT bounce buffer for debug vram access over sdma. */
> +	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> +				AMDGPU_GEM_DOMAIN_GTT,
> +				&adev->mman.sdma_access_bo, NULL,
> +				&adev->mman.sdma_access_ptr))
> +		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
> +
>   	/*
>   	 * retired pages will be loaded from eeprom and reserved here,
>   	 * it should be called after amdgpu_device_ip_hw_init_phase2  since 
> @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
>   	}
>   	adev->shutdown = true;
>   
> +	/* remove debug vram sdma access bounce buffer. */
> +	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> +					&adev->mman.sdma_access_ptr);
> +
>   	/* make sure IB test finished before entering exclusive mode
>   	 * to avoid preemption on IB test
>   	 * */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index b489cd8abe31..6178ae7ba624 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   		return r;
>   	}
>   
> -	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> -				AMDGPU_GEM_DOMAIN_GTT,
> -				&adev->mman.sdma_access_bo, NULL,
> -				adev->mman.sdma_access_ptr))
> -		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
> -
>   	return 0;
>   }
>   
> @@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   	ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
>   	ttm_device_fini(&adev->mman.bdev);
>   	adev->mman.initialized = false;
> -	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> -					&adev->mman.sdma_access_ptr);
>   	DRM_INFO("amdgpu: ttm finalized\n");
>   }
>   

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

* Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
  2022-01-18  8:09   ` Chen, Guchun
@ 2022-01-18  8:13     ` Christian König
  2022-01-18  8:18       ` Chen, Guchun
  0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2022-01-18  8:13 UTC (permalink / raw)
  To: Chen, Guchun, Kim, Jonathan, amd-gfx; +Cc: Kuehling, Felix

That check is utterly nonsense and should probably be removed.

What needs to be checked is if the GART ptr is available and that should 
certainly be the case at this point.

Christian.

Am 18.01.22 um 09:09 schrieb Chen, Guchun:
> [Public]
>
> Hi Christian,
>
> Re: Well that doesn't seem to make sense the GART is initialized by the code around the allocation so that should work fine.
>
> Below is the calltrace during driver probe. When binding the page(SDMA bo) into gart table, there is a check by gart.ready, that will be set to be true later on in gmc_v10_0_hw_init. So a calltrace is observed.
>
> [    3.381510]  amdgpu_ttm_gart_bind+0x80/0xc0 [amdgpu]
> [    3.381580]  amdgpu_ttm_alloc_gart+0x158/0x1c0 [amdgpu]
> [    3.381647]  amdgpu_bo_create_reserved+0x136/0x1e0 [amdgpu]
> [    3.381714]  ? amdgpu_ttm_debugfs_init+0x120/0x120 [amdgpu]
> [    3.381782]  amdgpu_bo_create_kernel+0x17/0x80 [amdgpu]
> [    3.381849]  amdgpu_ttm_init.cold+0x174/0x18e [amdgpu]
> [    3.381951]  ? vprintk_default+0x1d/0x20
> [    3.381955]  ? printk+0x58/0x6f
> [    3.381957]  amdgpu_bo_init.cold+0x4b/0x53 [amdgpu]
> [    3.382052]  gmc_v10_0_sw_init+0x304/0x490 [amdgpu]
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Tuesday, January 18, 2022 3:30 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
>
> Am 18.01.22 um 00:43 schrieb Jonathan Kim:
>> Move the debug sdma vram bounce buffer GART map on device init after
>> when GART is ready to avoid warnings and non-access to SDMA.
> Well that doesn't seem to make sense the GART is initialized by the code around the allocation so that should work fine.
>
> Freeing the BO indeed needs to be moved up, but that can still be in the same function.
>
> Also please don't move TTM related code outside of the TTM code in amdgpu.
>
> Regards,
> Christian.
>
>> Also move bounce buffer tear down after the memory manager has flushed
>> queued work for safety.
>>
>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 --------
>>    2 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index da3348fa7b0e..099460d15258 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>>    	if (r)
>>    		goto init_failed;
>>    
>> +	/* GTT bounce buffer for debug vram access over sdma. */
>> +	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> +				AMDGPU_GEM_DOMAIN_GTT,
>> +				&adev->mman.sdma_access_bo, NULL,
>> +				&adev->mman.sdma_access_ptr))
>> +		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> +
>>    	/*
>>    	 * retired pages will be loaded from eeprom and reserved here,
>>    	 * it should be called after amdgpu_device_ip_hw_init_phase2  since
>> @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
>>    	}
>>    	adev->shutdown = true;
>>    
>> +	/* remove debug vram sdma access bounce buffer. */
>> +	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>> +					&adev->mman.sdma_access_ptr);
>> +
>>    	/* make sure IB test finished before entering exclusive mode
>>    	 * to avoid preemption on IB test
>>    	 * */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index b489cd8abe31..6178ae7ba624 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>    		return r;
>>    	}
>>    
>> -	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> -				AMDGPU_GEM_DOMAIN_GTT,
>> -				&adev->mman.sdma_access_bo, NULL,
>> -				adev->mman.sdma_access_ptr))
>> -		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> -
>>    	return 0;
>>    }
>>    
>> @@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>>    	ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
>>    	ttm_device_fini(&adev->mman.bdev);
>>    	adev->mman.initialized = false;
>> -	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>> -					&adev->mman.sdma_access_ptr);
>>    	DRM_INFO("amdgpu: ttm finalized\n");
>>    }
>>    


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

* RE: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer
  2022-01-18  8:13     ` Christian König
@ 2022-01-18  8:18       ` Chen, Guchun
  0 siblings, 0 replies; 6+ messages in thread
From: Chen, Guchun @ 2022-01-18  8:18 UTC (permalink / raw)
  To: Koenig, Christian, Kim, Jonathan, amd-gfx; +Cc: Kuehling, Felix

[Public]

Okay... in amdgpu_gart_bind, the check of gart.ptr is also present, so it's safe.

Regards,
Guchun

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com> 
Sent: Tuesday, January 18, 2022 4:14 PM
To: Chen, Guchun <Guchun.Chen@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer

That check is utterly nonsense and should probably be removed.

What needs to be checked is if the GART ptr is available and that should certainly be the case at this point.

Christian.

Am 18.01.22 um 09:09 schrieb Chen, Guchun:
> [Public]
>
> Hi Christian,
>
> Re: Well that doesn't seem to make sense the GART is initialized by the code around the allocation so that should work fine.
>
> Below is the calltrace during driver probe. When binding the page(SDMA bo) into gart table, there is a check by gart.ready, that will be set to be true later on in gmc_v10_0_hw_init. So a calltrace is observed.
>
> [    3.381510]  amdgpu_ttm_gart_bind+0x80/0xc0 [amdgpu]
> [    3.381580]  amdgpu_ttm_alloc_gart+0x158/0x1c0 [amdgpu]
> [    3.381647]  amdgpu_bo_create_reserved+0x136/0x1e0 [amdgpu]
> [    3.381714]  ? amdgpu_ttm_debugfs_init+0x120/0x120 [amdgpu]
> [    3.381782]  amdgpu_bo_create_kernel+0x17/0x80 [amdgpu]
> [    3.381849]  amdgpu_ttm_init.cold+0x174/0x18e [amdgpu]
> [    3.381951]  ? vprintk_default+0x1d/0x20
> [    3.381955]  ? printk+0x58/0x6f
> [    3.381957]  amdgpu_bo_init.cold+0x4b/0x53 [amdgpu]
> [    3.382052]  gmc_v10_0_sw_init+0x304/0x490 [amdgpu]
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Tuesday, January 18, 2022 3:30 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; 
> amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Chen, Guchun 
> <Guchun.Chen@amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of 
> debug vram access bounce buffer
>
> Am 18.01.22 um 00:43 schrieb Jonathan Kim:
>> Move the debug sdma vram bounce buffer GART map on device init after 
>> when GART is ready to avoid warnings and non-access to SDMA.
> Well that doesn't seem to make sense the GART is initialized by the code around the allocation so that should work fine.
>
> Freeing the BO indeed needs to be moved up, but that can still be in the same function.
>
> Also please don't move TTM related code outside of the TTM code in amdgpu.
>
> Regards,
> Christian.
>
>> Also move bounce buffer tear down after the memory manager has 
>> flushed queued work for safety.
>>
>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 --------
>>    2 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index da3348fa7b0e..099460d15258 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>>    	if (r)
>>    		goto init_failed;
>>    
>> +	/* GTT bounce buffer for debug vram access over sdma. */
>> +	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> +				AMDGPU_GEM_DOMAIN_GTT,
>> +				&adev->mman.sdma_access_bo, NULL,
>> +				&adev->mman.sdma_access_ptr))
>> +		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> +
>>    	/*
>>    	 * retired pages will be loaded from eeprom and reserved here,
>>    	 * it should be called after amdgpu_device_ip_hw_init_phase2  
>> since @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
>>    	}
>>    	adev->shutdown = true;
>>    
>> +	/* remove debug vram sdma access bounce buffer. */
>> +	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>> +					&adev->mman.sdma_access_ptr);
>> +
>>    	/* make sure IB test finished before entering exclusive mode
>>    	 * to avoid preemption on IB test
>>    	 * */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index b489cd8abe31..6178ae7ba624 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>    		return r;
>>    	}
>>    
>> -	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> -				AMDGPU_GEM_DOMAIN_GTT,
>> -				&adev->mman.sdma_access_bo, NULL,
>> -				adev->mman.sdma_access_ptr))
>> -		DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> -
>>    	return 0;
>>    }
>>    
>> @@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>>    	ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
>>    	ttm_device_fini(&adev->mman.bdev);
>>    	adev->mman.initialized = false;
>> -	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>> -					&adev->mman.sdma_access_ptr);
>>    	DRM_INFO("amdgpu: ttm finalized\n");
>>    }
>>    

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

end of thread, other threads:[~2022-01-18  8:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-17 23:43 [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer Jonathan Kim
2022-01-18  2:17 ` Chen, Guchun
2022-01-18  7:29 ` Christian König
2022-01-18  8:09   ` Chen, Guchun
2022-01-18  8:13     ` Christian König
2022-01-18  8:18       ` Chen, Guchun

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.