[PATCH 1/2] drm/amdgpu: linear validate first then bind to GART

[PATCH 1/2] drm/amdgpu: linear validate first then bind to GART

[Christian König]

From: Christian König <christian.koenig@amd.com>

For VM emulation for old UVD/VCE we need to validate the BO with linear
VRAM flag set first and then eventually bind it to GART.

Validating with linear VRAM flag set can move the BO to GART making
UVD/VCE read/write from an unbound GART BO.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 52dd78ee8fd0..0c52295e74d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1583,14 +1583,14 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser,
 	if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket)
 		return -EINVAL;
 
-	r = amdgpu_ttm_bind(&(*bo)->tbo, &(*bo)->tbo.mem);
-	if (unlikely(r))
-		return r;
-
-	if ((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)
-		return 0;
+	if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)) {
+		(*bo)->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+		amdgpu_ttm_placement_from_domain(*bo, (*bo)->allowed_domains);
+		r = ttm_bo_validate(&(*bo)->tbo, &(*bo)->placement, false,
+				    false);
+		if (r)
+			return r;
+	}
 
-	(*bo)->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
-	amdgpu_ttm_placement_from_domain(*bo, (*bo)->allowed_domains);
-	return ttm_bo_validate(&(*bo)->tbo, &(*bo)->placement, false, false);
+	return amdgpu_ttm_bind(&(*bo)->tbo, &(*bo)->tbo.mem);
 }
-- 
2.11.0

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

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

[Christian König]

From: Christian König <christian.koenig@amd.com>

While binding BOs to GART we need to allow a bit overcommit in the GTT
domain. Otherwise we can never use the full GART space when GART size=GTT size.

Signed-off-by: Christian König <christian.koenig@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);
 }
-- 
2.11.0

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

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

[Chunming Zhou]

On 2017年10月16日 17:26, Christian König wrote:
> From: Christian König <christian.koenig@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?

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@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);
>   }

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

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

[Christian König]

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@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.

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@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);
>>   }
>

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

RE: [PATCH 1/2] drm/amdgpu: linear validate first then bind to GART

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Monday, October 16, 2017 5:27 AM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH 1/2] drm/amdgpu: linear validate first then bind to GART
> 
> From: Christian König <christian.koenig@amd.com>
> 
> For VM emulation for old UVD/VCE we need to validate the BO with linear
> VRAM flag set first and then eventually bind it to GART.
> 
> Validating with linear VRAM flag set can move the BO to GART making
> UVD/VCE read/write from an unbound GART BO.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Cc: Stable?

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 52dd78ee8fd0..0c52295e74d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1583,14 +1583,14 @@ int amdgpu_cs_find_mapping(struct
> amdgpu_cs_parser *parser,
>  	if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket)
>  		return -EINVAL;
> 
> -	r = amdgpu_ttm_bind(&(*bo)->tbo, &(*bo)->tbo.mem);
> -	if (unlikely(r))
> -		return r;
> -
> -	if ((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)
> -		return 0;
> +	if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS))
> {
> +		(*bo)->flags |=
> AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
> +		amdgpu_ttm_placement_from_domain(*bo, (*bo)-
> >allowed_domains);
> +		r = ttm_bo_validate(&(*bo)->tbo, &(*bo)->placement, false,
> +				    false);
> +		if (r)
> +			return r;
> +	}
> 
> -	(*bo)->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
> -	amdgpu_ttm_placement_from_domain(*bo, (*bo)-
> >allowed_domains);
> -	return ttm_bo_validate(&(*bo)->tbo, &(*bo)->placement, false,
> false);
> +	return amdgpu_ttm_bind(&(*bo)->tbo, &(*bo)->tbo.mem);
>  }
> --
> 2.11.0
> 
> _______________________________________________
> 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 2/2] drm/amdgpu: allow GTT overcommit during bind

[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@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.

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@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);
>>>   }
>>
>

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

Re: [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@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@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);
>>>>   }
>>>
>>
>

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

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

[Chunming Zhou]

On 2017年10月17日 15:46, Christian König wrote:
> 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@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.
If in memory pressure, I think we could allocate memory over the mgr 
limitation.
For example, the limitation is 1024M, the BO allocation is 800M, when 
the second counting, the available will be -576M, since available is 
unsigned, if another process allocation(800M) is coming at this moment, 
the bo_create is still successful, the binding will be fail. But the 
total allocation will be over the mgr limitation.

Regards,
David Zhou
>
> 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@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);
>>>>>   }
>>>>
>>>
>>
>

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

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

[Christian König]

Am 17.10.2017 um 10:15 schrieb Chunming Zhou:
>
>
> On 2017年10月17日 15:46, Christian König wrote:
>> 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@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.
> If in memory pressure, I think we could allocate memory over the mgr 
> limitation.

> For example, the limitation is 1024M, the BO allocation is 800M, when 
> the second counting, the available will be -576M,

Yes, correct so far. That's why I've added the extra check in 
amdgpu_gtt_mgr_usage().

> since available is unsigned

available is an atomic64_t and that is signed IIRC.

We could cast mem->num_pages to signed as well to be double sure, but as 
far as I can see that should work.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> 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@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);
>>>>>>   }
>>>>>
>>>>
>>>
>>
>

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

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

[Zhu, Rex]

Patch is 
Tested-by: Rex Zhu <Rex.Zhu@amd.com>

Best Regards
Rex


-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Christian König
Sent: Tuesday, October 17, 2017 4:28 PM
To: Zhou, David(ChunMing); amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: allow GTT overcommit during bind

Am 17.10.2017 um 10:15 schrieb Chunming Zhou:
>
>
> On 2017年10月17日 15:46, Christian König wrote:
>> 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@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.
> If in memory pressure, I think we could allocate memory over the mgr 
> limitation.

> For example, the limitation is 1024M, the BO allocation is 800M, when 
> the second counting, the available will be -576M,

Yes, correct so far. That's why I've added the extra check in amdgpu_gtt_mgr_usage().

> since available is unsigned

available is an atomic64_t and that is signed IIRC.

We could cast mem->num_pages to signed as well to be double sure, but as far as I can see that should work.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> 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@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);
>>>>>>   }
>>>>>
>>>>
>>>
>>
>

_______________________________________________
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 2/2] drm/amdgpu: allow GTT overcommit during bind

[Chunming Zhou]

On 2017年10月17日 16:27, Christian König wrote:
> Am 17.10.2017 um 10:15 schrieb Chunming Zhou:
>>
>>
>> On 2017年10月17日 15:46, Christian König wrote:
>>> 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@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.
>> If in memory pressure, I think we could allocate memory over the mgr 
>> limitation.
>
>> For example, the limitation is 1024M, the BO allocation is 800M, when 
>> the second counting, the available will be -576M,
>
> Yes, correct so far. That's why I've added the extra check in 
> amdgpu_gtt_mgr_usage().
>
>> since available is unsigned
>
> available is an atomic64_t and that is signed IIRC.
>
> We could cast mem->num_pages to signed as well to be double sure, but 
> as far as I can see that should work.
I agree above, but why you cut some part of mine:" if another process 
allocation(800M) is coming at this moment, the bo_create is still 
successful, the binding will be fail. But the total allocation will be 
over the mgr limitation."
What do you think of you cut?

Regards,
David Zhou

>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>>>
>>> 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@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);
>>>>>>>   }
>>>>>>
>>>>>
>>>>
>>>
>>
>

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

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

[Chunming Zhou]

On 2017年10月17日 16:34, Chunming Zhou wrote:
>
>
> On 2017年10月17日 16:27, Christian König wrote:
>> Am 17.10.2017 um 10:15 schrieb Chunming Zhou:
>>>
>>>
>>> On 2017年10月17日 15:46, Christian König wrote:
>>>> 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@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.
>>> If in memory pressure, I think we could allocate memory over the mgr 
>>> limitation.
>>
>>> For example, the limitation is 1024M, the BO allocation is 800M, 
>>> when the second counting, the available will be -576M,
>>
>> Yes, correct so far. That's why I've added the extra check in 
>> amdgpu_gtt_mgr_usage().
>>
>>> since available is unsigned
>>
>> available is an atomic64_t and that is signed IIRC.
>>
>> We could cast mem->num_pages to signed as well to be double sure, but 
>> as far as I can see that should work.
> I agree above, but why you cut some part of mine:" if another process 
> allocation(800M) is coming at this moment, the bo_create is still 
> successful, the binding will be fail. But the total allocation will be 
> over the mgr limitation."
> What do you think of you cut?
Ah, sorry, atomic64_t is singed, so the checking will fail, this case 
will not happen.

Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

>
> Regards,
> David Zhou
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> David Zhou
>>>>
>>>> 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@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);
>>>>>>>>   }
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
> _______________________________________________
> 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