[PATCH] drm/amdgpu: don't limit gtt size on apus
Christian König
ckoenig.leichtzumerken at gmail.com
Wed Jan 6 13:05:54 UTC 2021
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.
>
>
> > 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
>
> >>>>>
> >>
>
> _______________________________________________
> 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
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210106/5101f948/attachment.htm>
More information about the amd-gfx
mailing list