[PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Roger He]

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 17bf0ce..d0661907 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		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 * 3/4));
-	}
-	else
+		gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
+			(uint64_t)si.totalram * si.mem_unit * 3/4);
+	} else
 		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
 	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
 	if (r) {
-- 
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: set gtt size according to system memory size only

[Christian König]

Am 29.11.2017 um 10:12 schrieb Roger He:
> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
> Signed-off-by: Roger He <Hongbo.He@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 17bf0ce..d0661907 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   		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 * 3/4));
> -	}
> -	else
> +		gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
> +			(uint64_t)si.totalram * si.mem_unit * 3/4);
> +	} else
>   		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>   	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
>   	if (r) {

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

Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Michel Dänzer]

On 2017-11-29 10:12 AM, Roger He wrote:
> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 17bf0ce..d0661907 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>  		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 * 3/4));
> -	}
> -	else
> +		gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
> +			(uint64_t)si.totalram * si.mem_unit * 3/4);
> +	} else
>  		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>  	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
>  	if (r) {
> 

I'm unable to finish a piglit run (using Mesa on Tonga in a Ryzen 7 1700
system with 16 GB of RAM) with this change. Before, I had

 [drm] amdgpu: 3072M of GTT memory ready.

now it's

 [drm] amdgpu: 10473M of GTT memory ready.

While running piglit, there's lots of

 [TTM] Out of kernel memory

messages, followed by more badness, and eventually the machine becomes
inaccessible via SSH and has to be hard rebooted.


It occurred to me one thing not being taken into account here is that
system memory is also needed for storing the contents of BOs evicted
from VRAM. So I tried subtracting the VRAM size, resulting in

 [drm] amdgpu: 8425M of GTT memory ready.

but the problem still happened. So I tried 1/2 instead of 3/4 of RAM,
resulting in

 [drm] amdgpu: 6982M of GTT memory ready.

and was able to finish a piglit run with that.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Christian König]

Am 07.12.2017 um 18:34 schrieb Michel Dänzer:
> On 2017-11-29 10:12 AM, Roger He wrote:
>> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
>> Signed-off-by: Roger He <Hongbo.He@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 17bf0ce..d0661907 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>   		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 * 3/4));
>> -	}
>> -	else
>> +		gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
>> +			(uint64_t)si.totalram * si.mem_unit * 3/4);
>> +	} else
>>   		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>   	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
>>   	if (r) {
>>
> I'm unable to finish a piglit run (using Mesa on Tonga in a Ryzen 7 1700
> system with 16 GB of RAM) with this change. Before, I had
>
>   [drm] amdgpu: 3072M of GTT memory ready.
>
> now it's
>
>   [drm] amdgpu: 10473M of GTT memory ready.
>
> While running piglit, there's lots of
>
>   [TTM] Out of kernel memory
>
> messages, followed by more badness, and eventually the machine becomes
> inaccessible via SSH and has to be hard rebooted.
>
>
> It occurred to me one thing not being taken into account here is that
> system memory is also needed for storing the contents of BOs evicted
> from VRAM. So I tried subtracting the VRAM size, resulting in
>
>   [drm] amdgpu: 8425M of GTT memory ready.
>
> but the problem still happened. So I tried 1/2 instead of 3/4 of RAM,
> resulting in
>
>   [drm] amdgpu: 6982M of GTT memory ready.
>
> and was able to finish a piglit run with that.

I think I know what is going on here. The max-texture-size keeps 
increasing the texture size as long as it doesn't fails to allocate one.

So the "Out of kernel memory" message is actually the desired effect 
(but we should probably remove the message).

The price question is what happens after that? Those code paths are 
probably not very well tested.

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

Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Michel Dänzer]

[-- Attachment #1: Type: text/plain, Size: 3518 bytes --]

On 2017-12-07 07:07 PM, Christian König wrote:
> Am 07.12.2017 um 18:34 schrieb Michel Dänzer:
>> On 2017-11-29 10:12 AM, Roger He wrote:
>>> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
>>> Signed-off-by: Roger He <Hongbo.He-5C7GfCeVMHo@public.gmane.org>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
>>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 17bf0ce..d0661907 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>>           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 * 3/4));
>>> -    }
>>> -    else
>>> +        gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
>>> +            (uint64_t)si.totalram * si.mem_unit * 3/4);
>>> +    } else
>>>           gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>>       r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >>
>>> PAGE_SHIFT);
>>>       if (r) {
>>>
>> I'm unable to finish a piglit run (using Mesa on Tonga in a Ryzen 7 1700
>> system with 16 GB of RAM) with this change. Before, I had
>>
>>   [drm] amdgpu: 3072M of GTT memory ready.
>>
>> now it's
>>
>>   [drm] amdgpu: 10473M of GTT memory ready.
>>
>> While running piglit, there's lots of
>>
>>   [TTM] Out of kernel memory
>>
>> messages, followed by more badness, and eventually the machine becomes
>> inaccessible via SSH and has to be hard rebooted.
>>
>>
>> It occurred to me one thing not being taken into account here is that
>> system memory is also needed for storing the contents of BOs evicted
>> from VRAM. So I tried subtracting the VRAM size, resulting in
>>
>>   [drm] amdgpu: 8425M of GTT memory ready.
>>
>> but the problem still happened. So I tried 1/2 instead of 3/4 of RAM,
>> resulting in
>>
>>   [drm] amdgpu: 6982M of GTT memory ready.
>>
>> and was able to finish a piglit run with that.
> 
> I think I know what is going on here. The max-texture-size keeps
> increasing the texture size as long as it doesn't fails to allocate one.
> 
> So the "Out of kernel memory" message is actually the desired effect
> (but we should probably remove the message).
> 
> The price question is what happens after that? Those code paths are
> probably not very well tested.

I'm attaching all the related dmesg output I've captured. Basically,
best case is the OOM reaper killing piglit, worst case is no response to
SSH => hard reboot.


Until this becomes more robust, this change should probably be reverted.


On 2017-12-08 02:52 AM, He, Roger wrote:
>                  [TTM] Out of kernel memory
> The direct reason is GTT BO swap out failure which results from more Bo allocation. And  along with that need more acc_size.
> But why swap out failure I am not sure that is expected here for this case, maybe need to investigate.

FWIW, though it's probably not directly related: This happens with the
swap partition (8GB) completely unused.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

[-- Attachment #2: kern.log.gz --]
[-- Type: application/gzip, Size: 131100 bytes --]

[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

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

RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[He, Roger]

-----Original Message-----
From: Michel Dänzer [mailto:michel@daenzer.net] 
Sent: Saturday, December 09, 2017 1:26 AM
To: Koenig, Christian <Christian.Koenig@amd.com>; He, Roger <Hongbo.He@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

On 2017-12-07 07:07 PM, Christian König wrote:
> Am 07.12.2017 um 18:34 schrieb Michel Dänzer:
>> On 2017-11-29 10:12 AM, Roger He wrote:
>>> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
>>> Signed-off-by: Roger He <Hongbo.He@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
>>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 17bf0ce..d0661907 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device 
>>> *adev)
>>>           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 * 3/4));
>>> -    }
>>> -    else
>>> +        gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
>>> +            (uint64_t)si.totalram * si.mem_unit * 3/4);
>>> +    } else
>>>           gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>>       r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> 
>>> PAGE_SHIFT);
>>>       if (r) {
>>>
>> I'm unable to finish a piglit run (using Mesa on Tonga in a Ryzen 7 
>> 1700 system with 16 GB of RAM) with this change. Before, I had
>>
>>   [drm] amdgpu: 3072M of GTT memory ready.
>>
>> now it's
>>
>>   [drm] amdgpu: 10473M of GTT memory ready.
>>
>> While running piglit, there's lots of
>>
>>   [TTM] Out of kernel memory
>>
>> messages, followed by more badness, and eventually the machine 
>> becomes inaccessible via SSH and has to be hard rebooted.
>>
>>
>> It occurred to me one thing not being taken into account here is that 
>> system memory is also needed for storing the contents of BOs evicted 
>> from VRAM. So I tried subtracting the VRAM size, resulting in
>>
>>   [drm] amdgpu: 8425M of GTT memory ready.
>>
>> but the problem still happened. So I tried 1/2 instead of 3/4 of RAM, 
>> resulting in
>>
>>   [drm] amdgpu: 6982M of GTT memory ready.
>>
>> and was able to finish a piglit run with that.
> 
> I think I know what is going on here. The max-texture-size keeps 
> increasing the texture size as long as it doesn't fails to allocate one.
> 
> So the "Out of kernel memory" message is actually the desired effect 
> (but we should probably remove the message).
> 
> The price question is what happens after that? Those code paths are 
> probably not very well tested.

I'm attaching all the related dmesg output I've captured. Basically, best case is the OOM reaper killing piglit, worst case is no response to SSH => hard reboot.


	 Until this becomes more robust, this change should probably be reverted.

I have not got any valuable info from error log.
Please revert it now as quick solution, later let me check the reason. 


Thanks
Roger(Hongbo.He)

On 2017-12-08 02:52 AM, He, Roger wrote:
>                  [TTM] Out of kernel memory The direct reason is GTT 
> BO swap out failure which results from more Bo allocation. And  along with that need more acc_size.
> But why swap out failure I am not sure that is expected here for this case, maybe need to investigate.

FWIW, though it's probably not directly related: This happens with the swap partition (8GB) completely unused.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Michel Dänzer]

On 2017-12-11 03:49 AM, He, Roger wrote:
>> From: Michel Dänzer [mailto:michel@daenzer.net] On 2017-12-07 07:07
>> PM, Christian König wrote:
>>> 
>>> I think I know what is going on here. The max-texture-size keeps
>>>  increasing the texture size as long as it doesn't fails to
>>> allocate one.
>>> 
>>> So the "Out of kernel memory" message is actually the desired
>>> effect (but we should probably remove the message).
>>> 
>>> The price question is what happens after that? Those code paths
>>> are probably not very well tested.
>> 
>> I'm attaching all the related dmesg output I've captured.
>> Basically, best case is the OOM reaper killing piglit, worst case
>> is no response to SSH => hard reboot.

BTW, even when the machine stays responsive via SSH, the GPU hangs.


>> Until this becomes more robust, this change should probably be
>> reverted.
> 
> I have not got any valuable info from error log. Please revert it now
> as quick solution, later let me check the reason.

I'm busy with other stuff, can you revert it?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[He, Roger]

-----Original Message-----
From: Michel Dänzer [mailto:michel@daenzer.net] 
Sent: Monday, December 11, 2017 7:29 PM
To: He, Roger <Hongbo.He@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

On 2017-12-11 03:49 AM, He, Roger wrote:
>> From: Michel Dänzer [mailto:michel@daenzer.net] On 2017-12-07 07:07 
>> PM, Christian König wrote:
>>> 
>>> I think I know what is going on here. The max-texture-size keeps  
>>> increasing the texture size as long as it doesn't fails to allocate 
>>> one.
>>> 
>>> So the "Out of kernel memory" message is actually the desired effect 
>>> (but we should probably remove the message).
>>> 
>>> The price question is what happens after that? Those code paths are 
>>> probably not very well tested.
>> 
>> I'm attaching all the related dmesg output I've captured.
>> Basically, best case is the OOM reaper killing piglit, worst case is 
>> no response to SSH => hard reboot.

BTW, even when the machine stays responsive via SSH, the GPU hangs.


>> Until this becomes more robust, this change should probably be 
>> reverted.
> 
> I have not got any valuable info from error log. Please revert it now 
> as quick solution, later let me check the reason.

	I'm busy with other stuff, can you revert it?

Sorry for until now noticing your mail. And thanks Grodzovsky.Andrey for doing that.

Could you tell me how to duplicate this issue?  Maybe now I can look into it.

Thanks
Roger(Hongbo.He)





-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Michel Dänzer]

On 2017-12-25 09:45 AM, He, Roger wrote:
> 
> Could you tell me how to duplicate this issue?  Maybe now I can look into it.

It happened for me running

 piglit run -x glx-multithread-texture --process-isolation false gpu results/gpu.<date>

using the Mesa radeonsi driver on Tonga in a Ryzen 7 1700 (8 cores, 16
threads) system with 16 GB of RAM.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[He, Roger]

Got it, thank you!

-----Original Message-----
From: Michel Dänzer [mailto:michel@daenzer.net] 
Sent: Wednesday, December 27, 2017 4:58 PM
To: He, Roger <Hongbo.He@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

On 2017-12-25 09:45 AM, He, Roger wrote:
> 
> Could you tell me how to duplicate this issue?  Maybe now I can look into it.

It happened for me running

 piglit run -x glx-multithread-texture --process-isolation false gpu results/gpu.<date>

using the Mesa radeonsi driver on Tonga in a Ryzen 7 1700 (8 cores, 16
threads) system with 16 GB of RAM.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[He, Roger]

----Original Message-----
From: Michel Dänzer [mailto:michel@daenzer.net] 
Sent: Wednesday, December 27, 2017 4:58 PM
To: He, Roger <Hongbo.He@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

On 2017-12-25 09:45 AM, He, Roger wrote:
> 
> Could you tell me how to duplicate this issue?  Maybe now I can look into it.

	 piglit run -x glx-multithread-texture --process-isolation false gpu results/gpu.<date>


on my side, not work with above command:

root@jenkins-MS-7984:/home/jenkins/roger/piglit.0902.release# ./piglit run -x glx-multithread-texture --process-isolation false gpu results/gpu
usage: piglit [-h] [-f CONFIG_FILE] [-n <test name>] [-d] [-t <regex>]
              [-x <regex>] [-b {junit,json}] [-c | -1]
              [-p {glx,x11_egl,wayland,gbm,mixed_glx_egl}] [--valgrind]
              [--dmesg] [--abort-on-monitored-error] [-s]
              [--junit_suffix JUNIT_SUFFIX] [--junit-subtests]
              [-v | -l {quiet,verbose,dummy,http}] [--test-list TEST_LIST]
              [-o] [--deqp-mustpass-list]
              <Profile paths)> [<Profile path(s> ...] <Results Path>
piglit: error: unrecognized arguments: --process-isolation

is it because my piglit is too old?
Or any other command can duplicate the failure as well?


Thanks
Roger(Hongbo.He)
-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Michel Dänzer]

On 2018-01-02 10:57 AM, He, Roger wrote:
> ----Original Message-----
> From: Michel Dänzer [mailto:michel@daenzer.net] 
> Sent: Wednesday, December 27, 2017 4:58 PM
> To: He, Roger <Hongbo.He@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only
> 
> On 2017-12-25 09:45 AM, He, Roger wrote:
>>
>> Could you tell me how to duplicate this issue?  Maybe now I can look into it.
> 
> 	 piglit run -x glx-multithread-texture --process-isolation false gpu results/gpu.<date>
> 
> 
> on my side, not work with above command:
> 
> root@jenkins-MS-7984:/home/jenkins/roger/piglit.0902.release# ./piglit run -x glx-multithread-texture --process-isolation false gpu results/gpu
> usage: piglit [-h] [-f CONFIG_FILE] [-n <test name>] [-d] [-t <regex>]
>               [-x <regex>] [-b {junit,json}] [-c | -1]
>               [-p {glx,x11_egl,wayland,gbm,mixed_glx_egl}] [--valgrind]
>               [--dmesg] [--abort-on-monitored-error] [-s]
>               [--junit_suffix JUNIT_SUFFIX] [--junit-subtests]
>               [-v | -l {quiet,verbose,dummy,http}] [--test-list TEST_LIST]
>               [-o] [--deqp-mustpass-list]
>               <Profile paths)> [<Profile path(s> ...] <Results Path>
> piglit: error: unrecognized arguments: --process-isolation
> 
> is it because my piglit is too old?

Looks like it. Normally, one should always use current Git master of
piglit. If you can't or don't want to do that, you can try with just

 piglit run -x glx-multithread-texture gpu results/gpu.<date>

but it may be less likely to reproduce the issue.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[He, Roger]

                 [TTM] Out of kernel memory
The direct reason is GTT BO swap out failure which results from more Bo allocation. And  along with that need more acc_size.
But why swap out failure I am not sure that is expected here for this case, maybe need to investigate.

Thanks
Roger(Hongbo.He)
-----Original Message-----
From: Michel Dänzer [mailto:michel@daenzer.net] 
Sent: Friday, December 08, 2017 1:34 AM
To: He, Roger <Hongbo.He@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

On 2017-11-29 10:12 AM, Roger He wrote:
> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 17bf0ce..d0661907 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1330,11 +1330,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>  		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 * 3/4));
> -	}
> -	else
> +		gtt_size = max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
> +			(uint64_t)si.totalram * si.mem_unit * 3/4);
> +	} else
>  		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>  	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
>  	if (r) {
> 

I'm unable to finish a piglit run (using Mesa on Tonga in a Ryzen 7 1700 system with 16 GB of RAM) with this change. Before, I had

 [drm] amdgpu: 3072M of GTT memory ready.

now it's

 [drm] amdgpu: 10473M of GTT memory ready.

While running piglit, there's lots of

 [TTM] Out of kernel memory

messages, followed by more badness, and eventually the machine becomes inaccessible via SSH and has to be hard rebooted.


It occurred to me one thing not being taken into account here is that system memory is also needed for storing the contents of BOs evicted from VRAM. So I tried subtracting the VRAM size, resulting in

 [drm] amdgpu: 8425M of GTT memory ready.

but the problem still happened. So I tried 1/2 instead of 3/4 of RAM, resulting in

 [drm] amdgpu: 6982M of GTT memory ready.

and was able to finish a piglit run with that.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[Roger He]

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 17bf0ce..d773c5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1328,13 +1328,19 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 
 	if (amdgpu_gtt_size == -1) {
 		struct sysinfo si;
+		uint64_t sys_mem_size;
 
 		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 * 3/4));
-	}
-	else
+		sys_mem_size = (uint64_t)si.totalram * si.mem_unit;
+		gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
+
+		/* leave 2GB for OS to work with */
+		if (sys_mem_size > (2ULL << 30)) {
+			gtt_size = max(gtt_size, sys_mem_size - (2ULL << 30));
+		} else
+			DRM_INFO("amdgpu: Too small system memory %llu MB\n",
+				sys_mem_size >> 20);
+	} else
 		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
 	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
 	if (r) {
-- 
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: set gtt size according to system memory size only

[Christian König]

Am 28.11.2017 um 10:40 schrieb Roger He:
> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 17bf0ce..d773c5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1328,13 +1328,19 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   
>   	if (amdgpu_gtt_size == -1) {
>   		struct sysinfo si;
> +		uint64_t sys_mem_size;
>   
>   		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 * 3/4));
> -	}
> -	else
> +		sys_mem_size = (uint64_t)si.totalram * si.mem_unit;
> +		gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
> +
> +		/* leave 2GB for OS to work with */
> +		if (sys_mem_size > (2ULL << 30)) {
> +			gtt_size = max(gtt_size, sys_mem_size - (2ULL << 30));
> +		} else

No need for the "{}" here.

> +			DRM_INFO("amdgpu: Too small system memory %llu MB\n",
> +				sys_mem_size >> 20);
> +	} else

I have a preference to stick with the 75% rule similar to how TTM does 
things, but that isn't a hard opinion if you have a good argument.

Regards,
Christian.

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

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

RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

[He, Roger]

-----Original Message-----
From: Koenig, Christian 
Sent: Tuesday, November 28, 2017 6:01 PM
To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only

Am 28.11.2017 um 10:40 schrieb Roger He:
> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 17bf0ce..d773c5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1328,13 +1328,19 @@ int amdgpu_ttm_init(struct amdgpu_device 
> *adev)
>   
>   	if (amdgpu_gtt_size == -1) {
>   		struct sysinfo si;
> +		uint64_t sys_mem_size;
>   
>   		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 * 3/4));
> -	}
> -	else
> +		sys_mem_size = (uint64_t)si.totalram * si.mem_unit;
> +		gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
> +
> +		/* leave 2GB for OS to work with */
> +		if (sys_mem_size > (2ULL << 30)) {
> +			gtt_size = max(gtt_size, sys_mem_size - (2ULL << 30));
> +		} else

No need for the "{}" here.

> +			DRM_INFO("amdgpu: Too small system memory %llu MB\n",
> +				sys_mem_size >> 20);
> +	} else

I have a preference to stick with the 75% rule similar to how TTM does things, but that isn't a hard opinion if you have a good argument.

[Roger]: originally I used the 75% rule as well. But for a special test case, test failed. Anyway, let's keep 75% here since seems it is more reasonable. And for the special test case, will use module parameter to change GTT size temporarily.


Thanks
Roger(Hongbo.He)




Regards,
Christian.

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

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