[PATCH 2/2] drm/amdgpu: allow GTT overcommit during bind

[Christian König]

Am 17.10.2017 um 06:11 schrieb Chunming Zhou:
>
>
> On 2017年10月16日 19:40, Christian König wrote:
>> Am 16.10.2017 um 11:42 schrieb Chunming Zhou:
>>>
>>>
>>> On 2017年10月16日 17:26, Christian König wrote:
>>>> From: Christian König <christian.koenig at amd.com>
>>>>
>>>> While binding BOs to GART we need to allow a bit overcommit in the GTT
>>>> domain.
>>> If allowing overcommit, will the new node not over the GART mc 
>>> range? Which is also allowed?
>> No that is checked separately by drm_mm_insert_node_in_range().
>>
>> This is just to cover the case when we have a BO in GTT space which 
>> needs to be bound into the GART table.
> Sorry, I missed that even gart BO is also without node during creating.
> One nitpick, atomic64_sub(mem->num_pages, &mgr->available) will be 
> calculated twice for one gart bo create and pin, which results in 
> available isn't correct.

Yeah, that is true but not as problematic as you think.

The BO is transfered from the old mem object (without GART mapping) to 
the new mem object (with GART mapping). While doing this the old mem 
object is released, so we actually don't leak the memory.

It's just that for a moment the BO is accounted twice, once for the old 
location and once for the new one.

But we had that behavior previously as well, so that is not something 
introduced with this patch.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> David Zhou
>>>>   Otherwise we can never use the full GART space when GART size=GTT 
>>>> size.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 8 +++++---
>>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>> index 0d15eb7d31d7..33535d347734 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>> @@ -169,7 +169,8 @@ static int amdgpu_gtt_mgr_new(struct 
>>>> ttm_mem_type_manager *man,
>>>>       int r;
>>>>         spin_lock(&mgr->lock);
>>>> -    if (atomic64_read(&mgr->available) < mem->num_pages) {
>>>> +    if ((&tbo->mem == mem || tbo->mem.mem_type != TTM_PL_TT) &&
>>>> +        atomic64_read(&mgr->available) < mem->num_pages) {
>>>>           spin_unlock(&mgr->lock);
>>>>           return 0;
>>>>       }
>>>> @@ -244,8 +245,9 @@ static void amdgpu_gtt_mgr_del(struct 
>>>> ttm_mem_type_manager *man,
>>>>   uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man)
>>>>   {
>>>>       struct amdgpu_gtt_mgr *mgr = man->priv;
>>>> +    s64 result = man->size - atomic64_read(&mgr->available);
>>>>   -    return (u64)(man->size - atomic64_read(&mgr->available)) * 
>>>> PAGE_SIZE;
>>>> +    return (result > 0 ? result : 0) * PAGE_SIZE;
>>>>   }
>>>>     /**
>>>> @@ -265,7 +267,7 @@ static void amdgpu_gtt_mgr_debug(struct 
>>>> ttm_mem_type_manager *man,
>>>>       drm_mm_print(&mgr->mm, printer);
>>>>       spin_unlock(&mgr->lock);
>>>>   -    drm_printf(printer, "man size:%llu pages, gtt available:%llu 
>>>> pages, usage:%lluMB\n",
>>>> +    drm_printf(printer, "man size:%llu pages, gtt available:%lld 
>>>> pages, usage:%lluMB\n",
>>>>              man->size, (u64)atomic64_read(&mgr->available),
>>>>              amdgpu_gtt_mgr_usage(man) >> 20);
>>>>   }
>>>
>>
>