[PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[Chunming Zhou]

gtt_mgr_alloc is called by many places in local driver, while
gtt_mgr_new is called by get_node in ttm.

Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 69ab2ee..8a950a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
 						mem->page_alignment, 0,
 						fpfn, lpfn, sflags, aflags);
+	if (!r)
+		mgr->available -= mem->num_pages;
 	spin_unlock(&mgr->lock);
 
 	if (!r) {
@@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 		spin_unlock(&mgr->lock);
 		return 0;
 	}
-	mgr->available -= mem->num_pages;
 	spin_unlock(&mgr->lock);
 
 	node = kzalloc(sizeof(*node), GFP_KERNEL);
@@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 
 	return 0;
 err_out:
-	spin_lock(&mgr->lock);
-	mgr->available += mem->num_pages;
-	spin_unlock(&mgr->lock);
 
 	return r;
 }
@@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 		return;
 
 	spin_lock(&mgr->lock);
-	if (node->start != AMDGPU_BO_INVALID_OFFSET)
+	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
 		drm_mm_remove_node(node);
-	mgr->available += mem->num_pages;
+		mgr->available += mem->num_pages;
+	}
 	spin_unlock(&mgr->lock);
 
 	kfree(node);
-- 
1.9.1

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

[PATCH 2/3] drm/amdgpu: add gtt print like vram when dump mm table

[Chunming Zhou]

Change-Id: If0474e24e14d237d2d55731871c5ceb11e5a3601
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 7 +++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 8a950a5..85de145 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -138,6 +138,13 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	return r;
 }
 
+void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager *man)
+{
+	struct amdgpu_gtt_mgr *mgr = man->priv;
+
+	seq_printf(m, "man size:%llu pages, gtt available:%llu pages\n",
+		   man->size,	mgr->available);
+}
 /**
  * amdgpu_gtt_mgr_new - allocate a new node
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c3112b6..688056e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1540,6 +1540,8 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 
 #if defined(CONFIG_DEBUG_FS)
 
+extern void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager
+				 *man);
 static int amdgpu_mm_dump_table(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
@@ -1558,6 +1560,8 @@ static int amdgpu_mm_dump_table(struct seq_file *m, void *data)
 			   adev->mman.bdev.man[ttm_pl].size,
 			   (u64)atomic64_read(&adev->vram_usage) >> 20,
 			   (u64)atomic64_read(&adev->vram_vis_usage) >> 20);
+	if (ttm_pl == TTM_PL_TT)
+		amdgpu_gtt_mgr_print(m, &adev->mman.bdev.man[ttm_pl]);
 	return ret;
 }
 
-- 
1.9.1

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

Re: [PATCH 2/3] drm/amdgpu: add gtt print like vram when dump mm table

[Christian König]

Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
> Change-Id: If0474e24e14d237d2d55731871c5ceb11e5a3601
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>

One nit pick below.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 7 +++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 4 ++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 8a950a5..85de145 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -138,6 +138,13 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   	return r;
>   }
>   
> +void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager *man)
> +{
> +	struct amdgpu_gtt_mgr *mgr = man->priv;
> +
> +	seq_printf(m, "man size:%llu pages, gtt available:%llu pages\n",
> +		   man->size,	mgr->available);
> +}
>   /**
>    * amdgpu_gtt_mgr_new - allocate a new node
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c3112b6..688056e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1540,6 +1540,8 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>   
>   #if defined(CONFIG_DEBUG_FS)
>   
> +extern void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager
> +				 *man);
>   static int amdgpu_mm_dump_table(struct seq_file *m, void *data)
>   {
>   	struct drm_info_node *node = (struct drm_info_node *)m->private;
> @@ -1558,6 +1560,8 @@ static int amdgpu_mm_dump_table(struct seq_file *m, void *data)
>   			   adev->mman.bdev.man[ttm_pl].size,
>   			   (u64)atomic64_read(&adev->vram_usage) >> 20,
>   			   (u64)atomic64_read(&adev->vram_vis_usage) >> 20);
> +	if (ttm_pl == TTM_PL_TT)
> +		amdgpu_gtt_mgr_print(m, &adev->mman.bdev.man[ttm_pl]);

We should probably use a switch/case statement here instead of open 
coding it with an if.

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

Regards,
Christian.

>   	return ret;
>   }
>   


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

[PATCH 3/3] drm/amdgpu: move gtt usage statistic to gtt mgr

[Chunming Zhou]

Change-Id: Ifea42c8ae2206143d7e22b35eea537ba9e928fe8
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 ++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  6 ------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 85de145..db7bbde 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -97,6 +97,7 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
 	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 	enum drm_mm_search_flags sflags = DRM_MM_SEARCH_BEST;
 	enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
 	unsigned long fpfn, lpfn;
@@ -124,8 +125,10 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
 						mem->page_alignment, 0,
 						fpfn, lpfn, sflags, aflags);
-	if (!r)
+	if (!r) {
 		mgr->available -= mem->num_pages;
+		atomic64_add(mem->size, &adev->gtt_usage);
+	}
 	spin_unlock(&mgr->lock);
 
 	if (!r) {
@@ -140,10 +143,12 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 
 void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager *man)
 {
+	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 	struct amdgpu_gtt_mgr *mgr = man->priv;
 
-	seq_printf(m, "man size:%llu pages, gtt available:%llu pages\n",
-		   man->size,	mgr->available);
+	seq_printf(m, "man size:%llu pages, gtt available:%llu pages, usage:%lluMB\n",
+		   man->size,	mgr->available,
+		   (u64)atomic64_read(&adev->gtt_usage) >> 20);
 }
 /**
  * amdgpu_gtt_mgr_new - allocate a new node
@@ -214,6 +219,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
 	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 
 	if (!node)
 		return;
@@ -222,6 +228,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
 		drm_mm_remove_node(node);
 		mgr->available += mem->num_pages;
+		atomic64_sub(mem->size, &adev->gtt_usage);
 	}
 	spin_unlock(&mgr->lock);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 3cde1c9..2249eb6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -61,9 +61,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
 
 	if (new_mem) {
 		switch (new_mem->mem_type) {
-		case TTM_PL_TT:
-			atomic64_add(new_mem->size, &adev->gtt_usage);
-			break;
 		case TTM_PL_VRAM:
 			atomic64_add(new_mem->size, &adev->vram_usage);
 			vis_size = amdgpu_get_vis_part_size(adev, new_mem);
@@ -80,9 +77,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
 
 	if (old_mem) {
 		switch (old_mem->mem_type) {
-		case TTM_PL_TT:
-			atomic64_sub(old_mem->size, &adev->gtt_usage);
-			break;
 		case TTM_PL_VRAM:
 			atomic64_sub(old_mem->size, &adev->vram_usage);
 			vis_size = amdgpu_get_vis_part_size(adev, old_mem);
-- 
1.9.1

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

Re: [PATCH 3/3] drm/amdgpu: move gtt usage statistic to gtt mgr

[Zhang, Jerry (Junwei)]

On 04/19/2017 11:50 AM, Chunming Zhou wrote:
> Change-Id: Ifea42c8ae2206143d7e22b35eea537ba9e928fe8
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 ++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  6 ------
>   2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 85de145..db7bbde 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -97,6 +97,7 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   {
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>   	struct drm_mm_node *node = mem->mm_node;
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
>   	enum drm_mm_search_flags sflags = DRM_MM_SEARCH_BEST;
>   	enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
>   	unsigned long fpfn, lpfn;
> @@ -124,8 +125,10 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
>   						mem->page_alignment, 0,
>   						fpfn, lpfn, sflags, aflags);
> -	if (!r)
> +	if (!r) {
>   		mgr->available -= mem->num_pages;
> +		atomic64_add(mem->size, &adev->gtt_usage);
> +	}
>   	spin_unlock(&mgr->lock);
>
>   	if (!r) {
> @@ -140,10 +143,12 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>
>   void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager *man)
>   {
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>
> -	seq_printf(m, "man size:%llu pages, gtt available:%llu pages\n",
> -		   man->size,	mgr->available);
> +	seq_printf(m, "man size:%llu pages, gtt available:%llu pages, usage:%lluMB\n",
> +		   man->size,	mgr->available,
> +		   (u64)atomic64_read(&adev->gtt_usage) >> 20);
>   }
>   /**
>    * amdgpu_gtt_mgr_new - allocate a new node
> @@ -214,6 +219,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
>   {
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>   	struct drm_mm_node *node = mem->mm_node;
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
>
>   	if (!node)
>   		return;
> @@ -222,6 +228,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
>   	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>   		drm_mm_remove_node(node);
>   		mgr->available += mem->num_pages;
> +		atomic64_sub(mem->size, &adev->gtt_usage);
>   	}
>   	spin_unlock(&mgr->lock);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 3cde1c9..2249eb6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -61,9 +61,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
>
>   	if (new_mem) {
>   		switch (new_mem->mem_type) {
> -		case TTM_PL_TT:
> -			atomic64_add(new_mem->size, &adev->gtt_usage);
> -			break;

IMO, we may still need this, since it's updated when bo moves.
For instance, VRAM bo to GTT, it requires to update gtt_usage as well.

>   		case TTM_PL_VRAM:
>   			atomic64_add(new_mem->size, &adev->vram_usage);
>   			vis_size = amdgpu_get_vis_part_size(adev, new_mem);
> @@ -80,9 +77,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
>
>   	if (old_mem) {
>   		switch (old_mem->mem_type) {
> -		case TTM_PL_TT:
> -			atomic64_sub(old_mem->size, &adev->gtt_usage);
> -			break;

Same comment as above.

Jerry
>   		case TTM_PL_VRAM:
>   			atomic64_sub(old_mem->size, &adev->vram_usage);
>   			vis_size = amdgpu_get_vis_part_size(adev, old_mem);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu: move gtt usage statistic to gtt mgr

[Christian König]

Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
> Change-Id: Ifea42c8ae2206143d7e22b35eea537ba9e928fe8
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>

Nice idea for a cleanup, but please do even a few more things:
1. Rebase on not moving the statistics from mgr_new to mgr_alloc.
2. Joing mgr->available and adev->gtt_usage, keeping track of both is 
probably not a good idea.
3. Do the same thing for the VRAM stats as well.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 ++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  6 ------
>   2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 85de145..db7bbde 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -97,6 +97,7 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   {
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>   	struct drm_mm_node *node = mem->mm_node;
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
>   	enum drm_mm_search_flags sflags = DRM_MM_SEARCH_BEST;
>   	enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
>   	unsigned long fpfn, lpfn;
> @@ -124,8 +125,10 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
>   						mem->page_alignment, 0,
>   						fpfn, lpfn, sflags, aflags);
> -	if (!r)
> +	if (!r) {
>   		mgr->available -= mem->num_pages;
> +		atomic64_add(mem->size, &adev->gtt_usage);
> +	}
>   	spin_unlock(&mgr->lock);
>   
>   	if (!r) {
> @@ -140,10 +143,12 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   
>   void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager *man)
>   {
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>   
> -	seq_printf(m, "man size:%llu pages, gtt available:%llu pages\n",
> -		   man->size,	mgr->available);
> +	seq_printf(m, "man size:%llu pages, gtt available:%llu pages, usage:%lluMB\n",
> +		   man->size,	mgr->available,
> +		   (u64)atomic64_read(&adev->gtt_usage) >> 20);
>   }
>   /**
>    * amdgpu_gtt_mgr_new - allocate a new node
> @@ -214,6 +219,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
>   {
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>   	struct drm_mm_node *node = mem->mm_node;
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
>   
>   	if (!node)
>   		return;
> @@ -222,6 +228,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
>   	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>   		drm_mm_remove_node(node);
>   		mgr->available += mem->num_pages;
> +		atomic64_sub(mem->size, &adev->gtt_usage);
>   	}
>   	spin_unlock(&mgr->lock);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 3cde1c9..2249eb6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -61,9 +61,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
>   
>   	if (new_mem) {
>   		switch (new_mem->mem_type) {
> -		case TTM_PL_TT:
> -			atomic64_add(new_mem->size, &adev->gtt_usage);
> -			break;
>   		case TTM_PL_VRAM:
>   			atomic64_add(new_mem->size, &adev->vram_usage);
>   			vis_size = amdgpu_get_vis_part_size(adev, new_mem);
> @@ -80,9 +77,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
>   
>   	if (old_mem) {
>   		switch (old_mem->mem_type) {
> -		case TTM_PL_TT:
> -			atomic64_sub(old_mem->size, &adev->gtt_usage);
> -			break;
>   		case TTM_PL_VRAM:
>   			atomic64_sub(old_mem->size, &adev->vram_usage);
>   			vis_size = amdgpu_get_vis_part_size(adev, old_mem);


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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[Christian König]

Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
> gtt_mgr_alloc is called by many places in local driver, while
> gtt_mgr_new is called by get_node in ttm.

NAK, that can lead to over allocating the address space and we can't 
handle that during suspend/resume.

Regards,
Christian.

>
> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 69ab2ee..8a950a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
>   						mem->page_alignment, 0,
>   						fpfn, lpfn, sflags, aflags);
> +	if (!r)
> +		mgr->available -= mem->num_pages;
>   	spin_unlock(&mgr->lock);
>   
>   	if (!r) {
> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
>   		spin_unlock(&mgr->lock);
>   		return 0;
>   	}
> -	mgr->available -= mem->num_pages;
>   	spin_unlock(&mgr->lock);
>   
>   	node = kzalloc(sizeof(*node), GFP_KERNEL);
> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
>   
>   	return 0;
>   err_out:
> -	spin_lock(&mgr->lock);
> -	mgr->available += mem->num_pages;
> -	spin_unlock(&mgr->lock);
>   
>   	return r;
>   }
> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
>   		return;
>   
>   	spin_lock(&mgr->lock);
> -	if (node->start != AMDGPU_BO_INVALID_OFFSET)
> +	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>   		drm_mm_remove_node(node);
> -	mgr->available += mem->num_pages;
> +		mgr->available += mem->num_pages;
> +	}
>   	spin_unlock(&mgr->lock);
>   
>   	kfree(node);


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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[zhoucm1]

On 2017年04月19日 14:38, Christian König wrote:
> Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
>> gtt_mgr_alloc is called by many places in local driver, while
>> gtt_mgr_new is called by get_node in ttm.
>
> NAK, that can lead to over allocating the address space and we can't 
> handle that during suspend/resume.
I didn't get your mean here.

Describe it first from my side, I found this issue on APU like carrizo, 
vram is little, so gtt is used much more, we always found the gtt cannot 
be used end up and results in Game failed to allocate memory, but from 
dumping mm hole table, there are many free hole and memory.
The root cause is mgr->available statistic is incorrect, which doesn't 
match dump mm table.
I think mgr->available should only be changed when insert/free node.

Regards,
David Zhou
>
> Regards,
> Christian.
>
>>
>> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> index 69ab2ee..8a950a5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>> ttm_mem_type_manager *man,
>>       r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, 
>> mem->num_pages,
>>                           mem->page_alignment, 0,
>>                           fpfn, lpfn, sflags, aflags);
>> +    if (!r)
>> +        mgr->available -= mem->num_pages;
>>       spin_unlock(&mgr->lock);
>>         if (!r) {
>> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct 
>> ttm_mem_type_manager *man,
>>           spin_unlock(&mgr->lock);
>>           return 0;
>>       }
>> -    mgr->available -= mem->num_pages;
>>       spin_unlock(&mgr->lock);
>>         node = kzalloc(sizeof(*node), GFP_KERNEL);
>> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct 
>> ttm_mem_type_manager *man,
>>         return 0;
>>   err_out:
>> -    spin_lock(&mgr->lock);
>> -    mgr->available += mem->num_pages;
>> -    spin_unlock(&mgr->lock);
>>         return r;
>>   }
>> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct 
>> ttm_mem_type_manager *man,
>>           return;
>>         spin_lock(&mgr->lock);
>> -    if (node->start != AMDGPU_BO_INVALID_OFFSET)
>> +    if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>>           drm_mm_remove_node(node);
>> -    mgr->available += mem->num_pages;
>> +        mgr->available += mem->num_pages;
>> +    }
>>       spin_unlock(&mgr->lock);
>>         kfree(node);
>
>

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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[Christian König]

Am 19.04.2017 um 08:52 schrieb zhoucm1:
>
>
> On 2017年04月19日 14:38, Christian König wrote:
>> Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
>>> gtt_mgr_alloc is called by many places in local driver, while
>>> gtt_mgr_new is called by get_node in ttm.
>>
>> NAK, that can lead to over allocating the address space and we can't 
>> handle that during suspend/resume.
> I didn't get your mean here.
>
> Describe it first from my side, I found this issue on APU like 
> carrizo, vram is little, so gtt is used much more, we always found the 
> gtt cannot be used end up and results in Game failed to allocate 
> memory, but from dumping mm hole table, there are many free hole and 
> memory.

In this case you need to increase the gartsize parameter to the kernel 
module. What is the default size chosen for this? Maybe we need to 
adjust that default.

> The root cause is mgr->available statistic is incorrect, which doesn't 
> match dump mm table.

That's actually correct. We don't map all BOs in the GART domain into 
the actual GART table to avoid all the table operations.

> I think mgr->available should only be changed when insert/free node.

No, on suspend/resume we need to be able to add all BOs into the GART 
table. So with your change suspend/resume can potentially fail.

Regards,
Christian.

>
>
> Regards,
> David Zhou
>>
>> Regards,
>> Christian.
>>
>>>
>>> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> index 69ab2ee..8a950a5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>>> ttm_mem_type_manager *man,
>>>       r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, 
>>> mem->num_pages,
>>>                           mem->page_alignment, 0,
>>>                           fpfn, lpfn, sflags, aflags);
>>> +    if (!r)
>>> +        mgr->available -= mem->num_pages;
>>>       spin_unlock(&mgr->lock);
>>>         if (!r) {
>>> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>> ttm_mem_type_manager *man,
>>>           spin_unlock(&mgr->lock);
>>>           return 0;
>>>       }
>>> -    mgr->available -= mem->num_pages;
>>>       spin_unlock(&mgr->lock);
>>>         node = kzalloc(sizeof(*node), GFP_KERNEL);
>>> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>> ttm_mem_type_manager *man,
>>>         return 0;
>>>   err_out:
>>> -    spin_lock(&mgr->lock);
>>> -    mgr->available += mem->num_pages;
>>> -    spin_unlock(&mgr->lock);
>>>         return r;
>>>   }
>>> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct 
>>> ttm_mem_type_manager *man,
>>>           return;
>>>         spin_lock(&mgr->lock);
>>> -    if (node->start != AMDGPU_BO_INVALID_OFFSET)
>>> +    if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>>>           drm_mm_remove_node(node);
>>> -    mgr->available += mem->num_pages;
>>> +        mgr->available += mem->num_pages;
>>> +    }
>>>       spin_unlock(&mgr->lock);
>>>         kfree(node);
>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[zhoucm1]

On 2017年04月19日 14:59, Christian König wrote:
> Am 19.04.2017 um 08:52 schrieb zhoucm1:
>>
>>
>> On 2017年04月19日 14:38, Christian König wrote:
>>> Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
>>>> gtt_mgr_alloc is called by many places in local driver, while
>>>> gtt_mgr_new is called by get_node in ttm.
>>>
>>> NAK, that can lead to over allocating the address space and we can't 
>>> handle that during suspend/resume.
>> I didn't get your mean here.
>>
>> Describe it first from my side, I found this issue on APU like 
>> carrizo, vram is little, so gtt is used much more, we always found 
>> the gtt cannot be used end up and results in Game failed to allocate 
>> memory, but from dumping mm hole table, there are many free hole and 
>> memory.
>
> In this case you need to increase the gartsize parameter to the kernel 
> module. What is the default size chosen for this? Maybe we need to 
> adjust that default.
>
>> The root cause is mgr->available statistic is incorrect, which 
>> doesn't match dump mm table.
>
> That's actually correct. 
There will be a problem here, memory not mapped in mm table won't be 
able to evict by TTM. The root cause is here, we need to find a solution 
for it.
And yes, I agree your other opinions.

Regards,
David Zhou
> We don't map all BOs in the GART domain into the actual GART table to 
> avoid all the table operations.
>
>> I think mgr->available should only be changed when insert/free node.
>
> No, on suspend/resume we need to be able to add all BOs into the GART 
> table. So with your change suspend/resume can potentially fail.
>
> Regards,
> Christian.
>
>>
>>
>> Regards,
>> David Zhou
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>> index 69ab2ee..8a950a5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>>>> ttm_mem_type_manager *man,
>>>>       r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, 
>>>> mem->num_pages,
>>>>                           mem->page_alignment, 0,
>>>>                           fpfn, lpfn, sflags, aflags);
>>>> +    if (!r)
>>>> +        mgr->available -= mem->num_pages;
>>>>       spin_unlock(&mgr->lock);
>>>>         if (!r) {
>>>> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>> ttm_mem_type_manager *man,
>>>>           spin_unlock(&mgr->lock);
>>>>           return 0;
>>>>       }
>>>> -    mgr->available -= mem->num_pages;
>>>>       spin_unlock(&mgr->lock);
>>>>         node = kzalloc(sizeof(*node), GFP_KERNEL);
>>>> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>> ttm_mem_type_manager *man,
>>>>         return 0;
>>>>   err_out:
>>>> -    spin_lock(&mgr->lock);
>>>> -    mgr->available += mem->num_pages;
>>>> -    spin_unlock(&mgr->lock);
>>>>         return r;
>>>>   }
>>>> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct 
>>>> ttm_mem_type_manager *man,
>>>>           return;
>>>>         spin_lock(&mgr->lock);
>>>> -    if (node->start != AMDGPU_BO_INVALID_OFFSET)
>>>> +    if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>>>>           drm_mm_remove_node(node);
>>>> -    mgr->available += mem->num_pages;
>>>> +        mgr->available += mem->num_pages;
>>>> +    }
>>>>       spin_unlock(&mgr->lock);
>>>>         kfree(node);
>>>
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[Christian König]

Am 19.04.2017 um 11:15 schrieb zhoucm1:
>
>
> On 2017年04月19日 14:59, Christian König wrote:
>> Am 19.04.2017 um 08:52 schrieb zhoucm1:
>>>
>>>
>>> On 2017年04月19日 14:38, Christian König wrote:
>>>> Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
>>>>> gtt_mgr_alloc is called by many places in local driver, while
>>>>> gtt_mgr_new is called by get_node in ttm.
>>>>
>>>> NAK, that can lead to over allocating the address space and we 
>>>> can't handle that during suspend/resume.
>>> I didn't get your mean here.
>>>
>>> Describe it first from my side, I found this issue on APU like 
>>> carrizo, vram is little, so gtt is used much more, we always found 
>>> the gtt cannot be used end up and results in Game failed to allocate 
>>> memory, but from dumping mm hole table, there are many free hole and 
>>> memory.
>>
>> In this case you need to increase the gartsize parameter to the 
>> kernel module. What is the default size chosen for this? Maybe we 
>> need to adjust that default.
>>
>>> The root cause is mgr->available statistic is incorrect, which 
>>> doesn't match dump mm table.
>>
>> That's actually correct. 
> There will be a problem here, memory not mapped in mm table won't be 
> able to evict by TTM.

Why do you think so? It's possible that we have a bug somewhere 
regarding this, but at least in theory that shouldn't be the case.

> The root cause is here, we need to find a solution for it.

Yeah, that sounds like the handling is buggy somehow.

Regards,
Christian.

> And yes, I agree your other opinions.
>
> Regards,
> David Zhou
>> We don't map all BOs in the GART domain into the actual GART table to 
>> avoid all the table operations.
>>
>>> I think mgr->available should only be changed when insert/free node.
>>
>> No, on suspend/resume we need to be able to add all BOs into the GART 
>> table. So with your change suspend/resume can potentially fail.
>>
>> Regards,
>> Christian.
>>
>>>
>>>
>>> Regards,
>>> David Zhou
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
>>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>>>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> index 69ab2ee..8a950a5 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>       r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, 
>>>>> mem->num_pages,
>>>>>                           mem->page_alignment, 0,
>>>>>                           fpfn, lpfn, sflags, aflags);
>>>>> +    if (!r)
>>>>> +        mgr->available -= mem->num_pages;
>>>>>       spin_unlock(&mgr->lock);
>>>>>         if (!r) {
>>>>> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>           spin_unlock(&mgr->lock);
>>>>>           return 0;
>>>>>       }
>>>>> -    mgr->available -= mem->num_pages;
>>>>>       spin_unlock(&mgr->lock);
>>>>>         node = kzalloc(sizeof(*node), GFP_KERNEL);
>>>>> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>         return 0;
>>>>>   err_out:
>>>>> -    spin_lock(&mgr->lock);
>>>>> -    mgr->available += mem->num_pages;
>>>>> -    spin_unlock(&mgr->lock);
>>>>>         return r;
>>>>>   }
>>>>> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct 
>>>>> ttm_mem_type_manager *man,
>>>>>           return;
>>>>>         spin_lock(&mgr->lock);
>>>>> -    if (node->start != AMDGPU_BO_INVALID_OFFSET)
>>>>> +    if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>>>>>           drm_mm_remove_node(node);
>>>>> -    mgr->available += mem->num_pages;
>>>>> +        mgr->available += mem->num_pages;
>>>>> +    }
>>>>>       spin_unlock(&mgr->lock);
>>>>>         kfree(node);
>>>>
>>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>

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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[zhoucm1]

On 2017年04月19日 17:40, Christian König wrote:
> Am 19.04.2017 um 11:15 schrieb zhoucm1:
>>
>>
>> On 2017年04月19日 14:59, Christian König wrote:
>>> Am 19.04.2017 um 08:52 schrieb zhoucm1:
>>>>
>>>>
>>>> On 2017年04月19日 14:38, Christian König wrote:
>>>>> Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
>>>>>> gtt_mgr_alloc is called by many places in local driver, while
>>>>>> gtt_mgr_new is called by get_node in ttm.
>>>>>
>>>>> NAK, that can lead to over allocating the address space and we 
>>>>> can't handle that during suspend/resume.
>>>> I didn't get your mean here.
>>>>
>>>> Describe it first from my side, I found this issue on APU like 
>>>> carrizo, vram is little, so gtt is used much more, we always found 
>>>> the gtt cannot be used end up and results in Game failed to 
>>>> allocate memory, but from dumping mm hole table, there are many 
>>>> free hole and memory.
>>>
>>> In this case you need to increase the gartsize parameter to the 
>>> kernel module. What is the default size chosen for this? Maybe we 
>>> need to adjust that default.
>>>
>>>> The root cause is mgr->available statistic is incorrect, which 
>>>> doesn't match dump mm table.
>>>
>>> That's actually correct. 
>> There will be a problem here, memory not mapped in mm table won't be 
>> able to evict by TTM.
>
> Why do you think so? It's possible that we have a bug somewhere 
> regarding this, but at least in theory that shouldn't be the case.
With more thinking and experiments, I think you're right, with my patch, 
gtt mgr will be able to stolen memory without right limitation so that 
game can allocate more than gtt size memory, so game can run.
Will just increase gtt size to 3G by default, and add precise gtt print, 
some our script tool is basing on it.

Thanks,
David Zhou

>
>> The root cause is here, we need to find a solution for it.
>
> Yeah, that sounds like the handling is buggy somehow.
>
> Regards,
> Christian.
>
>> And yes, I agree your other opinions.
>>
>> Regards,
>> David Zhou
>>> We don't map all BOs in the GART domain into the actual GART table 
>>> to avoid all the table operations.
>>>
>>>> I think mgr->available should only be changed when insert/free node.
>>>
>>> No, on suspend/resume we need to be able to add all BOs into the 
>>> GART table. So with your change suspend/resume can potentially fail.
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>>
>>>> Regards,
>>>> David Zhou
>>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>>>
>>>>>> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
>>>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>>>>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>> index 69ab2ee..8a950a5 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>>>>>> ttm_mem_type_manager *man,
>>>>>>       r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, 
>>>>>> mem->num_pages,
>>>>>>                           mem->page_alignment, 0,
>>>>>>                           fpfn, lpfn, sflags, aflags);
>>>>>> +    if (!r)
>>>>>> +        mgr->available -= mem->num_pages;
>>>>>>       spin_unlock(&mgr->lock);
>>>>>>         if (!r) {
>>>>>> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>>> ttm_mem_type_manager *man,
>>>>>>           spin_unlock(&mgr->lock);
>>>>>>           return 0;
>>>>>>       }
>>>>>> -    mgr->available -= mem->num_pages;
>>>>>>       spin_unlock(&mgr->lock);
>>>>>>         node = kzalloc(sizeof(*node), GFP_KERNEL);
>>>>>> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>>> ttm_mem_type_manager *man,
>>>>>>         return 0;
>>>>>>   err_out:
>>>>>> -    spin_lock(&mgr->lock);
>>>>>> -    mgr->available += mem->num_pages;
>>>>>> -    spin_unlock(&mgr->lock);
>>>>>>         return r;
>>>>>>   }
>>>>>> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct 
>>>>>> ttm_mem_type_manager *man,
>>>>>>           return;
>>>>>>         spin_lock(&mgr->lock);
>>>>>> -    if (node->start != AMDGPU_BO_INVALID_OFFSET)
>>>>>> +    if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>>>>>>           drm_mm_remove_node(node);
>>>>>> -    mgr->available += mem->num_pages;
>>>>>> +        mgr->available += mem->num_pages;
>>>>>> +    }
>>>>>>       spin_unlock(&mgr->lock);
>>>>>>         kfree(node);
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>>
>>
>

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

Re: [PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[Christian König]

Am 20.04.2017 um 05:10 schrieb zhoucm1:
>
>
> On 2017年04月19日 17:40, Christian König wrote:
>> Am 19.04.2017 um 11:15 schrieb zhoucm1:
>>>
>>>
>>> On 2017年04月19日 14:59, Christian König wrote:
>>>> Am 19.04.2017 um 08:52 schrieb zhoucm1:
>>>>>
>>>>>
>>>>> On 2017年04月19日 14:38, Christian König wrote:
>>>>>> Am 19.04.2017 um 05:50 schrieb Chunming Zhou:
>>>>>>> gtt_mgr_alloc is called by many places in local driver, while
>>>>>>> gtt_mgr_new is called by get_node in ttm.
>>>>>>
>>>>>> NAK, that can lead to over allocating the address space and we 
>>>>>> can't handle that during suspend/resume.
>>>>> I didn't get your mean here.
>>>>>
>>>>> Describe it first from my side, I found this issue on APU like 
>>>>> carrizo, vram is little, so gtt is used much more, we always found 
>>>>> the gtt cannot be used end up and results in Game failed to 
>>>>> allocate memory, but from dumping mm hole table, there are many 
>>>>> free hole and memory.
>>>>
>>>> In this case you need to increase the gartsize parameter to the 
>>>> kernel module. What is the default size chosen for this? Maybe we 
>>>> need to adjust that default.
>>>>
>>>>> The root cause is mgr->available statistic is incorrect, which 
>>>>> doesn't match dump mm table.
>>>>
>>>> That's actually correct. 
>>> There will be a problem here, memory not mapped in mm table won't be 
>>> able to evict by TTM.
>>
>> Why do you think so? It's possible that we have a bug somewhere 
>> regarding this, but at least in theory that shouldn't be the case.
> With more thinking and experiments, I think you're right, with my 
> patch, gtt mgr will be able to stolen memory without right limitation 
> so that game can allocate more than gtt size memory, so game can run.
> Will just increase gtt size to 3G by default, and add precise gtt 
> print, some our script tool is basing on it.

Wait a second with that. I had more time thinking about it and you might 
indeed have stumbled over something:

1. In amdgpu_evict_flags() we set an upper limit to force directly 
allocation of the BO.
2. In amdgpu_gtt_mgr_alloc() we set the node start to 
AMDGPU_BO_INVALID_OFFSET if we don't map the BO into GART.
3. In amdgpu_ttm_bo_eviction_valuable() we check if the BO in the LRU 
matches the requested limit.

So what happens is that not mapped BOs are never evicted correctly when 
we run out of GART memory during eviction from VRAM!

Give me a second to hack a patch for this.

Regards,
Christian.

>
> Thanks,
> David Zhou
>
>>
>>> The root cause is here, we need to find a solution for it.
>>
>> Yeah, that sounds like the handling is buggy somehow.
>>
>> Regards,
>> Christian.
>>
>>> And yes, I agree your other opinions.
>>>
>>> Regards,
>>> David Zhou
>>>> We don't map all BOs in the GART domain into the actual GART table 
>>>> to avoid all the table operations.
>>>>
>>>>> I think mgr->available should only be changed when insert/free node.
>>>>
>>>> No, on suspend/resume we need to be able to add all BOs into the 
>>>> GART table. So with your change suspend/resume can potentially fail.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>>
>>>>> Regards,
>>>>> David Zhou
>>>>>>
>>>>>> Regards,
>>>>>> Christian.
>>>>>>
>>>>>>>
>>>>>>> Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
>>>>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>>>>> ---
>>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
>>>>>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>>> index 69ab2ee..8a950a5 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>>> @@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>>>>>>> ttm_mem_type_manager *man,
>>>>>>>       r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, 
>>>>>>> mem->num_pages,
>>>>>>>                           mem->page_alignment, 0,
>>>>>>>                           fpfn, lpfn, sflags, aflags);
>>>>>>> +    if (!r)
>>>>>>> +        mgr->available -= mem->num_pages;
>>>>>>>       spin_unlock(&mgr->lock);
>>>>>>>         if (!r) {
>>>>>>> @@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>>>> ttm_mem_type_manager *man,
>>>>>>>           spin_unlock(&mgr->lock);
>>>>>>>           return 0;
>>>>>>>       }
>>>>>>> -    mgr->available -= mem->num_pages;
>>>>>>>       spin_unlock(&mgr->lock);
>>>>>>>         node = kzalloc(sizeof(*node), GFP_KERNEL);
>>>>>>> @@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct 
>>>>>>> ttm_mem_type_manager *man,
>>>>>>>         return 0;
>>>>>>>   err_out:
>>>>>>> -    spin_lock(&mgr->lock);
>>>>>>> -    mgr->available += mem->num_pages;
>>>>>>> -    spin_unlock(&mgr->lock);
>>>>>>>         return r;
>>>>>>>   }
>>>>>>> @@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct 
>>>>>>> ttm_mem_type_manager *man,
>>>>>>>           return;
>>>>>>>         spin_lock(&mgr->lock);
>>>>>>> -    if (node->start != AMDGPU_BO_INVALID_OFFSET)
>>>>>>> +    if (node->start != AMDGPU_BO_INVALID_OFFSET) {
>>>>>>>           drm_mm_remove_node(node);
>>>>>>> -    mgr->available += mem->num_pages;
>>>>>>> +        mgr->available += mem->num_pages;
>>>>>>> +    }
>>>>>>>       spin_unlock(&mgr->lock);
>>>>>>>         kfree(node);
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>
>>>>
>>>
>>
>

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

[PATCH 1/3] drm/amdgpu: fix gtt mgr available statistics

[Chunming Zhou]

gtt_mgr_alloc is called by many places in local driver, while
gtt_mgr_new is called by get_node in ttm.

Change-Id: Ia5a18a3b531a01ad7d47f40e08f778e7b94c048a
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 69ab2ee..8a950a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -124,6 +124,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
 						mem->page_alignment, 0,
 						fpfn, lpfn, sflags, aflags);
+	if (!r)
+		mgr->available -= mem->num_pages;
 	spin_unlock(&mgr->lock);
 
 	if (!r) {
@@ -160,7 +162,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 		spin_unlock(&mgr->lock);
 		return 0;
 	}
-	mgr->available -= mem->num_pages;
 	spin_unlock(&mgr->lock);
 
 	node = kzalloc(sizeof(*node), GFP_KERNEL);
@@ -187,9 +188,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_mem_type_manager *man,
 
 	return 0;
 err_out:
-	spin_lock(&mgr->lock);
-	mgr->available += mem->num_pages;
-	spin_unlock(&mgr->lock);
 
 	return r;
 }
@@ -214,9 +212,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 		return;
 
 	spin_lock(&mgr->lock);
-	if (node->start != AMDGPU_BO_INVALID_OFFSET)
+	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
 		drm_mm_remove_node(node);
-	mgr->available += mem->num_pages;
+		mgr->available += mem->num_pages;
+	}
 	spin_unlock(&mgr->lock);
 
 	kfree(node);
-- 
1.9.1

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

[PATCH 3/3] drm/amdgpu: move gtt usage statistic to gtt mgr

[Chunming Zhou]

Change-Id: Ifea42c8ae2206143d7e22b35eea537ba9e928fe8
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 ++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  6 ------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 4bc1dd6..4b282ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -97,6 +97,7 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
 	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 	enum drm_mm_search_flags sflags = DRM_MM_SEARCH_BEST;
 	enum drm_mm_allocator_flags aflags = DRM_MM_CREATE_DEFAULT;
 	unsigned long fpfn, lpfn;
@@ -124,8 +125,10 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	r = drm_mm_insert_node_in_range_generic(&mgr->mm, node, mem->num_pages,
 						mem->page_alignment, 0,
 						fpfn, lpfn, sflags, aflags);
-	if (!r)
+	if (!r) {
 		mgr->available -= mem->num_pages;
+		atomic64_add(mem->size, &adev->gtt_usage);
+	}
 	spin_unlock(&mgr->lock);
 
 	if (!r) {
@@ -140,9 +143,11 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 
 void amdgpu_gtt_mgr_print(struct seq_file *m, struct ttm_mem_type_manager *man)
 {
+	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 	struct amdgpu_gtt_mgr *mgr = man->priv;
-		seq_printf(m, "man size:%llu pages, gtt available:%llu pages\n",
-			   man->size,	mgr->available);
+		seq_printf(m, "man size:%llu pages, gtt available:%llu pages, usage:%lluMB\n",
+			   man->size,	mgr->available,
+			   (u64)atomic64_read(&adev->gtt_usage) >> 20);
 }
 /**
  * amdgpu_gtt_mgr_new - allocate a new node
@@ -213,6 +218,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
 	struct drm_mm_node *node = mem->mm_node;
+	struct amdgpu_device *adev = amdgpu_ttm_adev(man->bdev);
 
 	if (!node)
 		return;
@@ -221,6 +227,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_mem_type_manager *man,
 	if (node->start != AMDGPU_BO_INVALID_OFFSET) {
 		drm_mm_remove_node(node);
 		mgr->available += mem->num_pages;
+		atomic64_sub(mem->size, &adev->gtt_usage);
 	}
 	spin_unlock(&mgr->lock);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 3cde1c9..2249eb6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -61,9 +61,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
 
 	if (new_mem) {
 		switch (new_mem->mem_type) {
-		case TTM_PL_TT:
-			atomic64_add(new_mem->size, &adev->gtt_usage);
-			break;
 		case TTM_PL_VRAM:
 			atomic64_add(new_mem->size, &adev->vram_usage);
 			vis_size = amdgpu_get_vis_part_size(adev, new_mem);
@@ -80,9 +77,6 @@ static void amdgpu_update_memory_usage(struct amdgpu_device *adev,
 
 	if (old_mem) {
 		switch (old_mem->mem_type) {
-		case TTM_PL_TT:
-			atomic64_sub(old_mem->size, &adev->gtt_usage);
-			break;
 		case TTM_PL_VRAM:
 			atomic64_sub(old_mem->size, &adev->vram_usage);
 			vis_size = amdgpu_get_vis_part_size(adev, old_mem);
-- 
1.9.1

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