Re: [PATCH 2/2] drm/amdgpu: allocate gart memory when it's required

[Zhang, Jerry (Junwei)]

On 06/26/2018 03:42 PM, Michel Dänzer wrote:

On 2018-06-26 08:00 AM, Junwei Zhang wrote:

Instead of calling gart memory on every bo pin,
allocates it on demand

Signed-off-by: Junwei Zhang 

[...]

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 036b6f7..0d1c4be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -194,6 +194,12 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
goto unreserve;
}

+   r = amdgpu_ttm_alloc_gart(&new_abo->tbo);
+   if (unlikely(r != 0)) {
+   DRM_ERROR("%p bind failed\n", new_abo);
+   goto unreserve;
+   }


The error case here also needs to unpin the BO, otherwise we leak the
pin. I suspect the same issue applies to at least some of the other
places modified by this patch.


Aha, I felt into the similar error.

will update v2 and find one more unpin issue for bo creation in patch 3.

Jerry




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

Re: [PATCH 2/2] drm/amdgpu: allocate gart memory when it's required

[Michel Dänzer]

On 2018-06-26 08:00 AM, Junwei Zhang wrote:
> Instead of calling gart memory on every bo pin,
> allocates it on demand
> 
> Signed-off-by: Junwei Zhang 
> 
> [...]
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 036b6f7..0d1c4be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -194,6 +194,12 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
> *crtc,
>   goto unreserve;
>   }
>  
> + r = amdgpu_ttm_alloc_gart(&new_abo->tbo);
> + if (unlikely(r != 0)) {
> + DRM_ERROR("%p bind failed\n", new_abo);
> + goto unreserve;
> + }

The error case here also needs to unpin the BO, otherwise we leak the
pin. I suspect the same issue applies to at least some of the other
places modified by this patch.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: allocate gart memory when it's required

[Junwei Zhang]

Instead of calling gart memory on every bo pin,
allocates it on demand

Signed-off-by: Junwei Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 10 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c|  8 
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 13 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c  |  5 +
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 +--
 8 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 98e3bf8..982cfa5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -280,6 +280,12 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
goto allocate_mem_pin_bo_failed;
}
 
+   r = amdgpu_ttm_alloc_gart(&bo->tbo);
+   if (r) {
+   dev_err(adev->dev, "%p bind failed\n", bo);
+   goto allocate_mem_pin_bo_failed;
+   }
+
r = amdgpu_bo_kmap(bo, &cpu_ptr_tmp);
if (r) {
dev_err(adev->dev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 079af8a..d498b3c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1593,6 +1593,12 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct 
kgd_dev *kgd,
goto pin_failed;
}
 
+   ret = amdgpu_ttm_alloc_gart(&bo->tbo);
+   if (ret) {
+   pr_err("%p bind failed\n", bo);
+   goto pin_failed;
+   }
+
ret = amdgpu_bo_kmap(bo, kptr);
if (ret) {
pr_err("Failed to map bo to kernel. ret %d\n", ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index cb88d7e..1d92a88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -96,11 +96,14 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
if (unlikely(r != 0))
goto out_cleanup;
r = amdgpu_bo_pin(sobj, sdomain);
-   saddr = amdgpu_bo_gpu_offset(sobj);
+   if (r)
+   goto out_cleanup;
+   r = amdgpu_ttm_alloc_gart(&sobj->tbo);
amdgpu_bo_unreserve(sobj);
if (r) {
goto out_cleanup;
}
+   saddr = amdgpu_bo_gpu_offset(sobj);
bp.domain = ddomain;
r = amdgpu_bo_create(adev, &bp, &dobj);
if (r) {
@@ -110,11 +113,14 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
if (unlikely(r != 0))
goto out_cleanup;
r = amdgpu_bo_pin(dobj, ddomain);
-   daddr = amdgpu_bo_gpu_offset(dobj);
+   if (r)
+   goto out_cleanup;
+   r = amdgpu_ttm_alloc_gart(&dobj->tbo);
amdgpu_bo_unreserve(dobj);
if (r) {
goto out_cleanup;
}
+   daddr = amdgpu_bo_gpu_offset(dobj);
 
if (adev->mman.buffer_funcs) {
time = amdgpu_benchmark_do_move(adev, size, saddr, daddr, n);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 036b6f7..0d1c4be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -194,6 +194,12 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
goto unreserve;
}
 
+   r = amdgpu_ttm_alloc_gart(&new_abo->tbo);
+   if (unlikely(r != 0)) {
+   DRM_ERROR("%p bind failed\n", new_abo);
+   goto unreserve;
+   }
+
r = reservation_object_get_fences_rcu(new_abo->tbo.resv, &work->excl,
  &work->shared_count,
  &work->shared);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 462b7a1..cd68a2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -173,6 +173,14 @@ static int amdgpufb_create_pinned_object(struct 
amdgpu_fbdev *rfbdev,
amdgpu_bo_unreserve(abo);
goto out_unref;
}
+
+   ret = amdgpu_ttm_alloc_gart(&abo->tbo);
+   if (ret) {
+   amdgpu_bo_unreserve(abo);
+   dev_err(adev->dev, "%p bind failed\n", abo);
+   goto out_unref;
+   }
+
ret = amdgpu_bo_kmap(abo, NULL);
amdgpu_bo_unreserve(abo);
if (ret) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 86c51f8..407e