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

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

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.

- 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%7C0dfe0d4b6f694ef4bd3c08d8b1c9ab0b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637454827005214704%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eT0XmwNPzBC1mODZQSeHLlcHzqU2Dtfb1E3goJEy2lA%3D&reserved=0 
>>>>
>