[4/5] drm/amdgpu: don't use ttm_bo_move_ttm in amdgpu_ttm_bind v2

Submitted by Christian König on Oct. 27, 2017, 2:43 p.m.

Details

Message ID 1509115410-2557-4-git-send-email-deathsimple@vodafone.de
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Christian König Oct. 27, 2017, 2:43 p.m.
From: Christian König <christian.koenig@amd.com>

Just allocate the GART space and fill it.

This prevents forcing the BO to be idle.

v2: don't unbind/bind at all, just fill the allocated GART space

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index eead348..06871a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -878,10 +878,12 @@  static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
+	struct amdgpu_ttm_tt *gtt = (void*)bo->ttm;
 	struct ttm_mem_reg tmp;
 
 	struct ttm_placement placement;
 	struct ttm_place placements;
+	uint64_t flags;
 	int r;
 
 	if (bo->mem.mem_type != TTM_PL_TT ||
@@ -903,14 +905,21 @@  int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 	if (unlikely(r))
 		return r;
 
-	r = ttm_bo_move_ttm(bo, true, false, &tmp);
-	if (unlikely(r))
+	flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
+	gtt->offset = (u64)tmp.start << PAGE_SHIFT;
+	r = amdgpu_gart_bind(adev, gtt->offset, bo->ttm->num_pages,
+			     bo->ttm->pages, gtt->ttm.dma_address, flags);
+	if (unlikely(r)) {
 		ttm_bo_mem_put(bo, &tmp);
-	else
-		bo->offset = (bo->mem.start << PAGE_SHIFT) +
-			bo->bdev->man[bo->mem.mem_type].gpu_offset;
+		return r;
+	}
 
-	return r;
+	ttm_bo_mem_put(bo, &bo->mem);
+	bo->mem = tmp;
+	bo->offset = (bo->mem.start << PAGE_SHIFT) +
+		bo->bdev->man[bo->mem.mem_type].gpu_offset;
+
+	return 0;
 }
 
 int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo)

Comments

On 2017年10月27日 22:43, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Just allocate the GART space and fill it.
>
> This prevents forcing the BO to be idle.
>
> v2: don't unbind/bind at all, just fill the allocated GART space
Could you explain what 'unbind/bind' is? My old understanding is 'bind' 
is to filling gart space, isn't it?

one comment in line...
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 +++++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index eead348..06871a9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -878,10 +878,12 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
>   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>   {
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
> +	struct amdgpu_ttm_tt *gtt = (void*)bo->ttm;
>   	struct ttm_mem_reg tmp;
>   
>   	struct ttm_placement placement;
>   	struct ttm_place placements;
> +	uint64_t flags;
>   	int r;
>   
>   	if (bo->mem.mem_type != TTM_PL_TT ||
> @@ -903,14 +905,21 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>   	if (unlikely(r))
>   		return r;
>   
> -	r = ttm_bo_move_ttm(bo, true, false, &tmp);
> -	if (unlikely(r))
> +	flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
> +	gtt->offset = (u64)tmp.start << PAGE_SHIFT;
> +	r = amdgpu_gart_bind(adev, gtt->offset, bo->ttm->num_pages,
> +			     bo->ttm->pages, gtt->ttm.dma_address, flags);
> +	if (unlikely(r)) {
>   		ttm_bo_mem_put(bo, &tmp);
> -	else
> -		bo->offset = (bo->mem.start << PAGE_SHIFT) +
> -			bo->bdev->man[bo->mem.mem_type].gpu_offset;
> +		return r;
> +	}
>   
> -	return r;
> +	ttm_bo_mem_put(bo, &bo->mem);
The old node doesn't need to wait before put it? if another user 
allocate in same place immediately, doesn't it result in problem?

Regards,
David Zhou
> +	bo->mem = tmp;
> +	bo->offset = (bo->mem.start << PAGE_SHIFT) +
> +		bo->bdev->man[bo->mem.mem_type].gpu_offset;
> +
> +	return 0;
>   }
>   
>   int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo)
Am 30.10.2017 um 04:25 schrieb Chunming Zhou:
>
>
> On 2017年10月27日 22:43, Christian König wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> Just allocate the GART space and fill it.
>>
>> This prevents forcing the BO to be idle.
>>
>> v2: don't unbind/bind at all, just fill the allocated GART space
> Could you explain what 'unbind/bind' is? My old understanding is 
> 'bind' is to filling gart space, isn't it?

Bind means that TTM is telling us that a ttm_tt (which represent a bunch 
of system pages) is going to be used by the GPU.

Mapping the TTM to GART space is just one thing we could do during that 
callback, but that is purely optional.

>
> one comment in line...
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 21 +++++++++++++++------
>>   1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index eead348..06871a9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -878,10 +878,12 @@ static int amdgpu_ttm_backend_bind(struct 
>> ttm_tt *ttm,
>>   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>>   {
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>> +    struct amdgpu_ttm_tt *gtt = (void*)bo->ttm;
>>       struct ttm_mem_reg tmp;
>>         struct ttm_placement placement;
>>       struct ttm_place placements;
>> +    uint64_t flags;
>>       int r;
>>         if (bo->mem.mem_type != TTM_PL_TT ||
>> @@ -903,14 +905,21 @@ int amdgpu_ttm_alloc_gart(struct 
>> ttm_buffer_object *bo)
>>       if (unlikely(r))
>>           return r;
>>   -    r = ttm_bo_move_ttm(bo, true, false, &tmp);
>> -    if (unlikely(r))
>> +    flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
>> +    gtt->offset = (u64)tmp.start << PAGE_SHIFT;
>> +    r = amdgpu_gart_bind(adev, gtt->offset, bo->ttm->num_pages,
>> +                 bo->ttm->pages, gtt->ttm.dma_address, flags);
>> +    if (unlikely(r)) {
>>           ttm_bo_mem_put(bo, &tmp);
>> -    else
>> -        bo->offset = (bo->mem.start << PAGE_SHIFT) +
>> - bo->bdev->man[bo->mem.mem_type].gpu_offset;
>> +        return r;
>> +    }
>>   -    return r;
>> +    ttm_bo_mem_put(bo, &bo->mem);
> The old node doesn't need to wait before put it? if another user 
> allocate in same place immediately, doesn't it result in problem?

Good point, but no that isn't a problem.

When we go into this branch we know that the old node didn't had any 
GART space allocated to it, so we don't need to wait for anything using 
this GART space.

Regards,
Christian.

>
> Regards,
> David Zhou
>> +    bo->mem = tmp;
>> +    bo->offset = (bo->mem.start << PAGE_SHIFT) +
>> +        bo->bdev->man[bo->mem.mem_type].gpu_offset;
>> +
>> +    return 0;
>>   }
>>     int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo)
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx