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

[Christian König <christian.koenig@xxxxxxx>]

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@xxxxxxxxx> 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.

> - 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@xxxxxxxxx>
> > > ---
> > >   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@xxxxxxxxxxxxxxxxxxxxx
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C0dfe0d4b6f694ef4bd3c08d8b1c9ab0b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637454827005214704%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=eT0XmwNPzBC1mODZQSeHLlcHzqU2Dtfb1E3goJEy2lA%3D&amp;reserved=0 

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