On Wed, Jan 6, 2021 at 1:54 PM Christian König <[email protected]> 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 <[email protected]> > 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 ...) > > 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 <[email protected]> > >>>>> --- > >>>>> 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 > >>>>> [email protected] > >>>>> > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/amd-gfx >
_______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
