[PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

[PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

[Roger He]

since sometimes VRAM size is bigger than system memory

Change-Id: I5b14d18ed7a9f79810cc50c023ac9e240bddf101
Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index cdbdf67..f103ccc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1330,9 +1330,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
 		 (unsigned) (adev->mc.real_vram_size / (1024 * 1024)));
 
-	if (amdgpu_gtt_size == -1)
-		gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
-			       adev->mc.mc_vram_size);
+	if (amdgpu_gtt_size == -1) {
+		struct sysinfo si;
+
+		si_meminfo(&si);
+		gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
+			       adev->mc.mc_vram_size),
+			       ((uint64_t)si.totalram * si.mem_unit));
+	}
 	else
 		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
 	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
-- 
2.7.4

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

Re: [PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

[Christian König]

Am 10.11.2017 um 12:33 schrieb Roger He:
> since sometimes VRAM size is bigger than system memory
>
> Change-Id: I5b14d18ed7a9f79810cc50c023ac9e240bddf101
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index cdbdf67..f103ccc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1330,9 +1330,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   	DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>   		 (unsigned) (adev->mc.real_vram_size / (1024 * 1024)));
>   
> -	if (amdgpu_gtt_size == -1)
> -		gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
> -			       adev->mc.mc_vram_size);
> +	if (amdgpu_gtt_size == -1) {
> +		struct sysinfo si;
> +
> +		si_meminfo(&si);
> +		gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
> +			       adev->mc.mc_vram_size),
> +			       ((uint64_t)si.totalram * si.mem_unit));

TTM starts to try to swap things out when more than 50% of system memory 
are used, 75% is the default hard limit where it starts to block new 
allocations.

So I would go with 75% here as well and code this as:

gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
gtt_size = max(gtt_size, adev->mc.mc_vram_size);
gtt_size = min(gtt_size, ((uint64_t)si.totalram * si.mem_unit) * 75 / 100);


Regards,
Christian.

> +		


> +	}
>   	else
>   		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>   	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);


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

RE: [PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

[He, Roger]

-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] 
Sent: Friday, November 10, 2017 8:02 PM
To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

Am 10.11.2017 um 12:33 schrieb Roger He:
> since sometimes VRAM size is bigger than system memory
>
> Change-Id: I5b14d18ed7a9f79810cc50c023ac9e240bddf101
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index cdbdf67..f103ccc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1330,9 +1330,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   	DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>   		 (unsigned) (adev->mc.real_vram_size / (1024 * 1024)));
>   
> -	if (amdgpu_gtt_size == -1)
> -		gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
> -			       adev->mc.mc_vram_size);
> +	if (amdgpu_gtt_size == -1) {
> +		struct sysinfo si;
> +
> +		si_meminfo(&si);
> +		gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
> +			       adev->mc.mc_vram_size),
> +			       ((uint64_t)si.totalram * si.mem_unit));

TTM starts to try to swap things out when more than 50% of system memory are used, 75% is the default hard limit where it starts to block new allocations.

So I would go with 75% here as well and code this as:

gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20; gtt_size = max(gtt_size, adev->mc.mc_vram_size); gtt_size = min(gtt_size, ((uint64_t)si.totalram * si.mem_unit) * 75 / 100);

[Roger]: but in amdgpu_info_ioctl, for GTT size, reported to UMD already is *3/4
	case AMDGPU_INFO_MEMORY: {
		
		mem.gtt.heap_usage =
			amdgpu_gtt_mgr_usage(&adev->mman.bdev.man[TTM_PL_TT]);
		mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
	
	So is it duplicated  if *75/100 here?


Regards,
Christian.

> +		


> +	}
>   	else
>   		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>   	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> 
> PAGE_SHIFT);


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

Re: [PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

[Christian König]

Am 10.11.2017 um 13:23 schrieb He, Roger:
>
> -----Original Message-----
> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
> Sent: Friday, November 10, 2017 8:02 PM
> To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size
>
> Am 10.11.2017 um 12:33 schrieb Roger He:
>> since sometimes VRAM size is bigger than system memory
>>
>> Change-Id: I5b14d18ed7a9f79810cc50c023ac9e240bddf101
>> Signed-off-by: Roger He <Hongbo.He@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++---
>>    1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index cdbdf67..f103ccc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1330,9 +1330,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>    	DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>>    		 (unsigned) (adev->mc.real_vram_size / (1024 * 1024)));
>>    
>> -	if (amdgpu_gtt_size == -1)
>> -		gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
>> -			       adev->mc.mc_vram_size);
>> +	if (amdgpu_gtt_size == -1) {
>> +		struct sysinfo si;
>> +
>> +		si_meminfo(&si);
>> +		gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
>> +			       adev->mc.mc_vram_size),
>> +			       ((uint64_t)si.totalram * si.mem_unit));
> TTM starts to try to swap things out when more than 50% of system memory are used, 75% is the default hard limit where it starts to block new allocations.
>
> So I would go with 75% here as well and code this as:
>
> gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20; gtt_size = max(gtt_size, adev->mc.mc_vram_size); gtt_size = min(gtt_size, ((uint64_t)si.totalram * si.mem_unit) * 75 / 100);
>
> [Roger]: but in amdgpu_info_ioctl, for GTT size, reported to UMD already is *3/4
> 	case AMDGPU_INFO_MEMORY: {
> 		
> 		mem.gtt.heap_usage =
> 			amdgpu_gtt_mgr_usage(&adev->mman.bdev.man[TTM_PL_TT]);
> 		mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
> 	
> 	So is it duplicated  if *75/100 here?

Good point, but that's only the maximum allocation size of a BO in GTT 
space, not how big GTT space itself is.

And since this is only the available memory and since GTT isn't 
fragmented this is pretty much a complete useless value. We should 
probably change it to something like "total gtt size" - "size of pinned 
objects in gtt".

But this is a separate problem I think,
Christian.

>
>
> Regards,
> Christian.
>
>> +		
>
>> +	}
>>    	else
>>    		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>    	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >>
>> PAGE_SHIFT);
>

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

Re: [PATCH] drm/amd/amdgpu: not allow gtt size exceed system memory size

[Felix Kuehling]

On 2017-11-10 07:02 AM, Christian König wrote:
> Am 10.11.2017 um 12:33 schrieb Roger He:
>> since sometimes VRAM size is bigger than system memory
>>
>> Change-Id: I5b14d18ed7a9f79810cc50c023ac9e240bddf101
>> Signed-off-by: Roger He <Hongbo.He@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index cdbdf67..f103ccc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1330,9 +1330,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>       DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>>            (unsigned) (adev->mc.real_vram_size / (1024 * 1024)));
>>   -    if (amdgpu_gtt_size == -1)
>> -        gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
>> -                   adev->mc.mc_vram_size);
>> +    if (amdgpu_gtt_size == -1) {
>> +        struct sysinfo si;
>> +
>> +        si_meminfo(&si);
>> +        gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
>> +                   adev->mc.mc_vram_size),
>> +                   ((uint64_t)si.totalram * si.mem_unit));
>
> TTM starts to try to swap things out when more than 50% of system
> memory are used, 75% is the default hard limit where it starts to
> block new allocations.
>
> So I would go with 75% here as well and code this as:

I believe those limits can be tweaked in
/sys/devices/virtual/drm/ttm/memory_accounting. Therefore I'd be against
hard-coding strict limits in the driver.

Regards,
  Felix

>
> gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
> gtt_size = max(gtt_size, adev->mc.mc_vram_size);
> gtt_size = min(gtt_size, ((uint64_t)si.totalram * si.mem_unit) * 75 /
> 100);
>
>
> Regards,
> Christian.
>
>> +       
>
>
>> +    }
>>       else
>>           gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>       r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >>
>> PAGE_SHIFT);
>
>
> _______________________________________________
> 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] drm/amd/amdgpu: not allow gtt size exceed system memory size

[Christian König]

Am 13.11.2017 um 23:34 schrieb Felix Kuehling:
> On 2017-11-10 07:02 AM, Christian König wrote:
>> Am 10.11.2017 um 12:33 schrieb Roger He:
>>> since sometimes VRAM size is bigger than system memory
>>>
>>> Change-Id: I5b14d18ed7a9f79810cc50c023ac9e240bddf101
>>> Signed-off-by: Roger He <Hongbo.He@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++---
>>>    1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index cdbdf67..f103ccc 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1330,9 +1330,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>>        DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>>>             (unsigned) (adev->mc.real_vram_size / (1024 * 1024)));
>>>    -    if (amdgpu_gtt_size == -1)
>>> -        gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
>>> -                   adev->mc.mc_vram_size);
>>> +    if (amdgpu_gtt_size == -1) {
>>> +        struct sysinfo si;
>>> +
>>> +        si_meminfo(&si);
>>> +        gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
>>> +                   adev->mc.mc_vram_size),
>>> +                   ((uint64_t)si.totalram * si.mem_unit));
>> TTM starts to try to swap things out when more than 50% of system
>> memory are used, 75% is the default hard limit where it starts to
>> block new allocations.
>>
>> So I would go with 75% here as well and code this as:
> I believe those limits can be tweaked in
> /sys/devices/virtual/drm/ttm/memory_accounting. Therefore I'd be against
> hard-coding strict limits in the driver.

Well, it's not a strict limit. You can still overwrite it with a module 
parameter.

Regards,
Christian.

>
> Regards,
>    Felix
>
>> gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
>> gtt_size = max(gtt_size, adev->mc.mc_vram_size);
>> gtt_size = min(gtt_size, ((uint64_t)si.totalram * si.mem_unit) * 75 /
>> 100);
>>
>>
>> Regards,
>> Christian.
>>
>>> +
>>
>>> +    }
>>>        else
>>>            gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>>        r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >>
>>> PAGE_SHIFT);
>>
>> _______________________________________________
>> 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


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