[PATCH] drm/amdgpu: don't limit gtt size on apus
Joshua Ashton
joshua at froggi.es
Wed Jan 6 13:17:28 UTC 2021
On 1/6/21 1:05 PM, Christian König wrote:
> Am 06.01.21 um 14:02 schrieb Bas Nieuwenhuizen:
>>
>>
>> On Wed, Jan 6, 2021 at 1:54 PM Christian König
>> <christian.koenig at amd.com <mailto:christian.koenig at amd.com>> 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 <mailto: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.
>>
>>
>> For Vulkan we (both RADV and AMDVLK) use GTT as the total size. Usage
>> in modern games is essentially "bindless" so there is no way to track
>> at a per-submission level what memory needs to be resident. (and even
>> with tracking applications are allowed to use all the memory in a
>> single draw call, which would be unsplittable anyway ...)
>
> Yeah, that is a really good point.
>
> The issue is that we need some limitation since 3/4 of system memory is
> way to much and the max texture size test in piglit can cause a system
> crash.
>
> The alternative is a better OOM handling, so that an application which
> uses to much system memory through the driver stack has a more likely
> chance to get killed. Cause currently that is either X or Wayland :(
>
> Christian.
As I understand it, what is being exposed right now is essentially
max(vram size, 3GiB) limited by 3/4ths of the memory. Previously, before
the revert what was being taken was just max(3GiB, 3/4ths).
If you had < 3GiB of system memory that seems like a bit of an issue
that could easily leat to OOM to me?
Are you hitting on something smaller than 3/4ths right now? I remember
the source commit mentioned they only had 1GiB of system memory
available, so that could be possible if you had a carveout of < 786MiB...
- Joshie 🐸✨
>
>>
>>
>> > 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
>> <mailto: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
>> <mailto: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
>> <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>
>>
>> >>>>>
>> >>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
More information about the amd-gfx
mailing list