[PATCH] drm/amdgpu: don't limit gtt size on apus

Joshua Ashton joshua at froggi.es
Wed Jan 6 13:06:48 UTC 2021 


On 1/6/21 12:54 PM, Christian König wrote:
> Am 06.01.21 um 13:47 schrieb Joshua Ashton:
>>
>>
>> On 1/6/21 7:52 AM, Christian König wrote:
>>> Am 05.01.21 um 23:31 schrieb Joshua Ashton:
>>>> On 1/5/21 10:10 PM, Alex Deucher wrote:
>>>>> On Tue, Jan 5, 2021 at 5:05 PM Joshua Ashton <joshua at froggi.es> wrote:
>>>>>>
>>>>>> Since commit 24562523688b ("Revert "drm/amd/amdgpu: set gtt size
>>>>>> according to system memory size only""), the GTT size was limited by
>>>>>> 3GiB or VRAM size.
>>>>>
>>>>> The commit in question was to fix a hang with certain tests on APUs.
>>>>> That should be tested again before we re-enable this.  If it is fixed,
>>>>> we should just revert the revert rather than special case dGPUs.
>>>>>
>>>>> Alex
>>>>>
>>>>
>>>> I think the commit before the revert (ba851eed895c) has some 
>>>> fundamental problems:
>>>>
>>>> It was always specifying max(3GiB, 3/4ths RAM) of GTT, even if that 
>>>> wouldn't fit into say, 1GiB or 2GiB of available RAM.
>>>>
>>>> Limiting GTT to min(max(3GiB, VRAM), 3/4ths RAM) size on dGPUs makes 
>>>> sense also and is a sensible limit to avoid silly situations with 
>>>> overallocation and potential OOM.
>>>>
>>>> This patch solves both of those issues.
>>>
>>> No, Alex is right this approach was already tried and it causes 
>>> problems.
>>>
>>> Additional to that why should this be an issue? Even when VRAM is 
>>> very small on APUs we still use 3GiB of GTT.
>>>
>>> Regards,
>>> Christian.
>>
>> The problem is that 3GiB of GTT isn't enough for most modern games.
> 
> You seem to misunderstand what the GTT size means here. This is the 
> amount of memory an application can lock down in a single command 
> submissions.
> 
> It is still possible for the game to use all of system memory for 
> textures etc... it can just happen that some buffers are temporary 
> marked as inaccessible for the GPU.

In Vulkan, command buffers are explicit and the amount of memory the app 
uses is not trackable at a command buffer level due to bindless.

This means that we can't magically split command buffers like in GL if 
too much memory is being used by a single submission.

This means that the only two visible heaps available to AMD APUs in RADV 
right now are the carveout and GTT. As I understand it there is no other 
way to use more memory in APIs with explicit cmd buffering & bindless.

> 
>> My laptop has a 128MiB carveout which is not possible to be configured 
>> in the BIOS so I am stuck with that size without extra kernel 
>> parameters which shouldn't be necessary.
> 
> Did you ran into problems without the parameter?
> 
>>
>> If you dislike the approach of keeping the extra check for dGPUs and 
>> limiting GTT there, then I would say that we should use
>>     gtt_size = 3/4ths system memory
>> for all devices instead of
>>     gtt_size = max(3/4ths system memory, 3GiB)
>> as it was before the revert, as it is problematic on systems with < 
>> 3GiB of system memory.
> 
> Yeah, that's indeed not a good idea.
> 
> Regards,
> Christian.
> 
>>
>> - Joshie 🐸✨
>>
>>>
>>>>
>>>> - Joshie 🐸✨
>>>>
>>>>>
>>>>>>
>>>>>> This is problematic on APUs, especially with a small carveout
>>>>>> which can be as low as a fixed 128MiB, as there would be very a 
>>>>>> limited
>>>>>> 3GiB available for video memory.
>>>>>> This obviously does not meet the demands of modern applications.
>>>>>>
>>>>>> This patch makes it so the GTT size heuristic always uses 3/4ths of
>>>>>> the system memory size on APUs (limiting the size by 3GiB/VRAM size
>>>>>> only on devices with dedicated video memory).
>>>>>>
>>>>>> Fixes: 24562523688b ("Revert drm/amd/amdgpu: set gtt size 
>>>>>> according to
>>>>>> system memory size only")
>>>>>>
>>>>>> Signed-off-by: Joshua Ashton <joshua at froggi.es>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 +++--
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++++---
>>>>>>   2 files changed, 12 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>>> index 72efd579ec5e..a5a41e9272d6 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>>> @@ -192,8 +192,9 @@ module_param_named(gartsize, amdgpu_gart_size, 
>>>>>> uint, 0600);
>>>>>>
>>>>>>   /**
>>>>>>    * DOC: gttsize (int)
>>>>>> - * Restrict the size of GTT domain in MiB for testing. The 
>>>>>> default is -1 (It's VRAM size if 3GB < VRAM < 3/4 RAM,
>>>>>> - * otherwise 3/4 RAM size).
>>>>>> + * Restrict the size of GTT domain in MiB for testing. The 
>>>>>> default is -1 (On APUs this is 3/4th
>>>>>> + * of the system memory; on dGPUs this is 3GiB or VRAM sized, 
>>>>>> whichever is bigger,
>>>>>> + * with an upper bound of 3/4th of system memory.
>>>>>>    */
>>>>>>   MODULE_PARM_DESC(gttsize, "Size of the GTT domain in megabytes 
>>>>>> (-1 = auto)");
>>>>>>   module_param_named(gttsize, amdgpu_gtt_size, int, 0600);
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> index 4d8f19ab1014..294f26f4f310 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> @@ -1865,9 +1865,15 @@ 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->gmc.mc_vram_size),
>>>>>> -                              ((uint64_t)si.totalram * 
>>>>>> si.mem_unit * 3/4));
>>>>>> +               gtt_size = (uint64_t)si.totalram * si.mem_unit * 3/4;
>>>>>> +               /* If we have dedicated memory, limit our GTT size to
>>>>>> +                * 3GiB or VRAM size, whichever is bigger
>>>>>> +                */
>>>>>> +               if (!(adev->flags & AMD_IS_APU)) {
>>>>>> +                       gtt_size = 
>>>>>> min(max(AMDGPU_DEFAULT_GTT_SIZE_MB << 20,
>>>>>> + adev->gmc.mc_vram_size),
>>>>>> +                               gtt_size);
>>>>>> +               }
>>>>>>          }
>>>>>>          else
>>>>>>                  gtt_size = (uint64_t)amdgpu_gtt_size << 20;
>>>>>> -- 
>>>>>> 2.30.0
>>>>>>
>>>>>> _______________________________________________
>>>>>> amd-gfx mailing list
>>>>>> amd-gfx at lists.freedesktop.org
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cchristian.koenig%40amd.com%7C890f3f3bb9144929d52308d8b2413a35%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637455340521793984%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ix5qMEHXC%2BeOly4OlgZ4mbFPIGz37g0JPawHfh412wE%3D&reserved=0 
>>>>>>
>>>
> 

- Joshie 🐸✨

More information about the amd-gfx mailing list