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

2018-06-27 Thread Zhang, Jerry (Junwei)

On 06/28/2018 12:52 AM, Felix Kuehling wrote:

On 2018-06-26 09:42 PM, Zhang, Jerry (Junwei) wrote:


BTW, kfd2kgd_calls kfd2kgd looks duplicated in amdkfd_gfx_v7/8/9.c
we may initialize it in a common place(at least for common members).
If it has other purpose, please ignore that.


Some of the function pointers in kfd2kgd_calls are HW-specific (static
functions implemented in amdgpu_amdkfd_gfx_v[789].c). Therefore they are
not really duplicates. They may look the same in the source code, but
they'll end up pointing to different functions.


Thanks for your explanation.
I see.

Just a suggestion. In this case, we may add the version for each func
to clarify the different HW support, e.g. kgd_program_sh_mem_settings_v9.
Then the common func would be identified clearly, like alloc_gtt_mem() for all 
HWs.
So do HW-specific func.

(seems to talk too much regardless of this thread, thanks for your discussion)

Jerry



Regards,
   Felix



Jerry



Regards,
Felix


   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..3079ea8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -96,11 +96,16 @@ 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) {
+amdgpu_bo_unreserve(sobj);
+goto out_cleanup;
+}
+r = amdgpu_ttm_alloc_gart(>tbo);
   amdgpu_bo_unreserve(sobj);
   if (r) {
   goto out_cleanup;
   }
+saddr = amdgpu_bo_gpu_offset(sobj);
   bp.domain = ddomain;
   r = amdgpu_bo_create(adev, , );
   if (r) {
@@ -110,11 +115,16 @@ 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) {
+amdgpu_bo_unreserve(sobj);
+goto out_cleanup;
+}
+r = amdgpu_ttm_alloc_gart(>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..7d6a36b 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(_abo->tbo);
+if (unlikely(r != 0)) {
+DRM_ERROR("%p bind failed\n", new_abo);
+goto unpin;
+}
+
   r = reservation_object_get_fences_rcu(new_abo->tbo.resv,
>excl,
 >shared_count,
 >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(>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 79cdbf1..7f7c221 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -257,6 +257,13 @@ int amdgpu_bo_create_reserved(struct
amdgpu_device *adev,
   dev_err(adev->dev, "(%d) kernel bo pin failed\n", r);
   goto error_unreserve;
   }
+
+r = amdgpu_ttm_alloc_gart(&(*bo_ptr)->tbo);
+if (r) {
+dev_err(adev->dev, "%p bind failed\n", *bo_ptr);
+goto error_unpin;
+}
+
   if (gpu_addr)
   *gpu_addr = amdgpu_bo_gpu_offset(*bo_ptr);

@@ -270,6 +277,8 @@ int amdgpu_bo_create_reserved(struct
amdgpu_device *adev,

   return 0;

+error_unpin:
+amdgpu_bo_unpin(*bo_ptr);
   error_unreserve:
   amdgpu_bo_unreserve(*bo_ptr);

@@ -903,12 +912,6 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
*bo, u32 domain,
   goto error;
   }

-r = amdgpu_ttm_alloc_gart(>tbo);
-if (unlikely(r)) {
-dev_err(adev->dev, "%p bind failed\n", bo);
-goto error;
-}
-
   bo->pin_count = 1;

   domain = 

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

2018-06-27 Thread Felix Kuehling
On 2018-06-26 09:42 PM, Zhang, Jerry (Junwei) wrote:
>
> BTW, kfd2kgd_calls kfd2kgd looks duplicated in amdkfd_gfx_v7/8/9.c
> we may initialize it in a common place(at least for common members).
> If it has other purpose, please ignore that.

Some of the function pointers in kfd2kgd_calls are HW-specific (static
functions implemented in amdgpu_amdkfd_gfx_v[789].c). Therefore they are
not really duplicates. They may look the same in the source code, but
they'll end up pointing to different functions.

Regards,
  Felix

>
> Jerry
>
>>
>> Regards,
>>    Felix
>>
>>>   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..3079ea8 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>> @@ -96,11 +96,16 @@ 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) {
>>> +    amdgpu_bo_unreserve(sobj);
>>> +    goto out_cleanup;
>>> +    }
>>> +    r = amdgpu_ttm_alloc_gart(>tbo);
>>>   amdgpu_bo_unreserve(sobj);
>>>   if (r) {
>>>   goto out_cleanup;
>>>   }
>>> +    saddr = amdgpu_bo_gpu_offset(sobj);
>>>   bp.domain = ddomain;
>>>   r = amdgpu_bo_create(adev, , );
>>>   if (r) {
>>> @@ -110,11 +115,16 @@ 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) {
>>> +    amdgpu_bo_unreserve(sobj);
>>> +    goto out_cleanup;
>>> +    }
>>> +    r = amdgpu_ttm_alloc_gart(>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..7d6a36b 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(_abo->tbo);
>>> +    if (unlikely(r != 0)) {
>>> +    DRM_ERROR("%p bind failed\n", new_abo);
>>> +    goto unpin;
>>> +    }
>>> +
>>>   r = reservation_object_get_fences_rcu(new_abo->tbo.resv,
>>> >excl,
>>>     >shared_count,
>>>     >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(>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 79cdbf1..7f7c221 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -257,6 +257,13 @@ int amdgpu_bo_create_reserved(struct
>>> amdgpu_device *adev,
>>>   dev_err(adev->dev, "(%d) kernel bo pin failed\n", r);
>>>   goto error_unreserve;
>>>   }
>>> +
>>> +    r = amdgpu_ttm_alloc_gart(&(*bo_ptr)->tbo);
>>> +    if (r) {
>>> +    dev_err(adev->dev, "%p bind failed\n", *bo_ptr);
>>> +    goto error_unpin;
>>> +    }
>>> +
>>>   if (gpu_addr)
>>>   *gpu_addr = amdgpu_bo_gpu_offset(*bo_ptr);
>>>
>>> @@ -270,6 +277,8 @@ int amdgpu_bo_create_reserved(struct
>>> amdgpu_device *adev,
>>>
>>>   return 0;
>>>
>>> +error_unpin:
>>> +    amdgpu_bo_unpin(*bo_ptr);
>>>   error_unreserve:
>>>   amdgpu_bo_unreserve(*bo_ptr);
>>>
>>> @@ -903,12 +912,6 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
>>> *bo, u32 domain,
>>>   goto error;
>>>   }
>>>
>>> -    r = amdgpu_ttm_alloc_gart(>tbo);
>>> -    if (unlikely(r)) {
>>> -    dev_err(adev->dev, "%p bind failed\n", bo);
>>> -    goto error;
>>> -    }
>>> -
>>>   

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

2018-06-26 Thread Zhang, Jerry (Junwei)

On 06/26/2018 11:53 PM, Felix Kuehling wrote:

Comments inline [FK]


On 2018-06-26 04:35 AM, Junwei Zhang wrote:

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

v2: fix error handling

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 | 14 --
  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| 15 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_test.c  |  5 +
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++--
  8 files changed, 63 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..e3ed08d 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(>tbo);
+   if (r) {
+   dev_err(adev->dev, "%p bind failed\n", bo);
+   goto allocate_mem_kmap_bo_failed;
+   }
+
r = amdgpu_bo_kmap(bo, _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..2c0bc7c 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(>tbo);
+   if (ret) {
+   pr_err("%p bind failed\n", bo);
+   goto kmap_failed;
+   }
+


[FK] I think think in this case we don't need the GART mapping. This
function doesn't query the GPU address of the pinned buffer. The only
reason we pin the BO here is to keep the kernel mapping valid.


Felix, thanks for your comments.
Check the code again, it seems to get GTT bo for kernel execution from user 
event or process.
if so, that's really not needed.
Going to remove it in v3.

BTW, kfd2kgd_calls kfd2kgd looks duplicated in amdkfd_gfx_v7/8/9.c
we may initialize it in a common place(at least for common members).
If it has other purpose, please ignore that.

Jerry



Regards,
   Felix


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..3079ea8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -96,11 +96,16 @@ 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) {
+   amdgpu_bo_unreserve(sobj);
+   goto out_cleanup;
+   }
+   r = amdgpu_ttm_alloc_gart(>tbo);
amdgpu_bo_unreserve(sobj);
if (r) {
goto out_cleanup;
}
+   saddr = amdgpu_bo_gpu_offset(sobj);
bp.domain = ddomain;
r = amdgpu_bo_create(adev, , );
if (r) {
@@ -110,11 +115,16 @@ 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) {
+   amdgpu_bo_unreserve(sobj);
+   goto out_cleanup;
+   }
+   r = amdgpu_ttm_alloc_gart(>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..7d6a36b 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(_abo->tbo);
+   if (unlikely(r != 0)) {
+   DRM_ERROR("%p bind failed\n", new_abo);
+   goto unpin;
+   }
+
r = reservation_object_get_fences_rcu(new_abo->tbo.resv, >excl,
  >shared_count,
  >shared);
diff 

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

2018-06-26 Thread Felix Kuehling
Comments inline [FK]


On 2018-06-26 04:35 AM, Junwei Zhang wrote:
> Instead of calling gart memory on every bo pin,
> allocates it on demand
>
> v2: fix error handling
>
> 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 | 14 --
>  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| 15 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_test.c  |  5 +
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++--
>  8 files changed, 63 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..e3ed08d 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(>tbo);
> + if (r) {
> + dev_err(adev->dev, "%p bind failed\n", bo);
> + goto allocate_mem_kmap_bo_failed;
> + }
> +
>   r = amdgpu_bo_kmap(bo, _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..2c0bc7c 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(>tbo);
> + if (ret) {
> + pr_err("%p bind failed\n", bo);
> + goto kmap_failed;
> + }
> +

[FK] I think think in this case we don't need the GART mapping. This
function doesn't query the GPU address of the pinned buffer. The only
reason we pin the BO here is to keep the kernel mapping valid.

Regards,
  Felix

>   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..3079ea8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> @@ -96,11 +96,16 @@ 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) {
> + amdgpu_bo_unreserve(sobj);
> + goto out_cleanup;
> + }
> + r = amdgpu_ttm_alloc_gart(>tbo);
>   amdgpu_bo_unreserve(sobj);
>   if (r) {
>   goto out_cleanup;
>   }
> + saddr = amdgpu_bo_gpu_offset(sobj);
>   bp.domain = ddomain;
>   r = amdgpu_bo_create(adev, , );
>   if (r) {
> @@ -110,11 +115,16 @@ 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) {
> + amdgpu_bo_unreserve(sobj);
> + goto out_cleanup;
> + }
> + r = amdgpu_ttm_alloc_gart(>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..7d6a36b 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(_abo->tbo);
> + if (unlikely(r != 0)) {
> + DRM_ERROR("%p bind failed\n", new_abo);
> + goto unpin;
> + }
> +
>   r = reservation_object_get_fences_rcu(new_abo->tbo.resv, >excl,
> >shared_count,
> >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,
>   

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

2018-06-26 Thread Junwei Zhang
Instead of calling gart memory on every bo pin,
allocates it on demand

v2: fix error handling

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 | 14 --
 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| 15 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c  |  5 +
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++--
 8 files changed, 63 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..e3ed08d 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(>tbo);
+   if (r) {
+   dev_err(adev->dev, "%p bind failed\n", bo);
+   goto allocate_mem_kmap_bo_failed;
+   }
+
r = amdgpu_bo_kmap(bo, _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..2c0bc7c 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(>tbo);
+   if (ret) {
+   pr_err("%p bind failed\n", bo);
+   goto kmap_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..3079ea8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -96,11 +96,16 @@ 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) {
+   amdgpu_bo_unreserve(sobj);
+   goto out_cleanup;
+   }
+   r = amdgpu_ttm_alloc_gart(>tbo);
amdgpu_bo_unreserve(sobj);
if (r) {
goto out_cleanup;
}
+   saddr = amdgpu_bo_gpu_offset(sobj);
bp.domain = ddomain;
r = amdgpu_bo_create(adev, , );
if (r) {
@@ -110,11 +115,16 @@ 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) {
+   amdgpu_bo_unreserve(sobj);
+   goto out_cleanup;
+   }
+   r = amdgpu_ttm_alloc_gart(>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..7d6a36b 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(_abo->tbo);
+   if (unlikely(r != 0)) {
+   DRM_ERROR("%p bind failed\n", new_abo);
+   goto unpin;
+   }
+
r = reservation_object_get_fences_rcu(new_abo->tbo.resv, >excl,
  >shared_count,
  >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(>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