On Thu, Jan 16, 2025 at 4:42 PM Mario Limonciello <superm1@xxxxxxxxxx> wrote:
>
> On 1/16/2025 15:32, Alex Deucher wrote:
> > On Thu, Jan 16, 2025 at 1:29 PM Mario Limonciello <superm1@xxxxxxxxxx> wrote:
> >>
> >> From: Mario Limonciello <mario.limonciello@xxxxxxx>
> >>
> >> When not set `gttsize` module parameter by default will get the
> >> value to use for the GTT pool from the TTM page limit, which is
> >> set by a separate module parameter.
> >>
> >> This inevitably leads to people not sure which one to set when they
> >> want more addressable memory for the GPU, and you'll end up seeing
> >> instructions online saying to set both.
> >>
> >> Drop the amdgpu module parameter and related code for it. This way
> >> if users want to change the amount of addressable memory they can
> >> change it solely in TTM.
> >>
> >
> > I suspect we probably want to keep it for compatibility with users
> > that still use this option to change their GTT size.
>
> Could we mark it as deprecated? Perhaps something like this:
>
> 0) Change kdoc
> 1) Some messaging to show it being deprecated
>
> if (amdgpu_gttsize != -1)
> drm_warn(dev, "Configuring gttsize via module parameter is deprecated,
> please use ttm.ttm_page_limit");
>
> 2) Some messaging to show information if inconsistent values have been
> set against TTM
>
> if (amdgpu_gttsize != ttm_page_limit()
> drm_warn(dev, "GTT size has been set as %lu but TTM size has been set
> as %lu, this is unusual");
>
> Then after the "next" LTS release is declared we can pull it out entirely.
>
Seems reasonable to me.
Alex
> >
> > Alex
> >
> >
> >> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 -------
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ---------
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 ++-------
> >> 4 files changed, 2 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index 5e55a44f9eef..e728974114bb 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -164,7 +164,6 @@ extern int amdgpu_modeset;
> >> extern unsigned int amdgpu_vram_limit;
> >> extern int amdgpu_vis_vram_limit;
> >> extern int amdgpu_gart_size;
> >> -extern int amdgpu_gtt_size;
> >> extern int amdgpu_moverate;
> >> extern int amdgpu_audio;
> >> extern int amdgpu_disp_priority;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 3774d12ebc4a..1b62b843e5fb 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -2031,13 +2031,6 @@ static int amdgpu_device_check_arguments(struct amdgpu_device *adev)
> >> amdgpu_gart_size = -1;
> >> }
> >>
> >> - if (amdgpu_gtt_size != -1 && amdgpu_gtt_size < 32) {
> >> - /* gtt size must be greater or equal to 32M */
> >> - dev_warn(adev->dev, "gtt size (%d) too small\n",
> >> - amdgpu_gtt_size);
> >> - amdgpu_gtt_size = -1;
> >> - }
> >> -
> >> /* valid range is between 4 and 9 inclusive */
> >> if (amdgpu_vm_fragment_size != -1 &&
> >> (amdgpu_vm_fragment_size > 9 || amdgpu_vm_fragment_size < 4)) {
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> index b911653dd8b6..62b09c518665 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> @@ -141,7 +141,6 @@ enum AMDGPU_DEBUG_MASK {
> >> unsigned int amdgpu_vram_limit = UINT_MAX;
> >> int amdgpu_vis_vram_limit;
> >> int amdgpu_gart_size = -1; /* auto */
> >> -int amdgpu_gtt_size = -1; /* auto */
> >> int amdgpu_moverate = -1; /* auto */
> >> int amdgpu_audio = -1;
> >> int amdgpu_disp_priority;
> >> @@ -279,14 +278,6 @@ module_param_named(vis_vramlimit, amdgpu_vis_vram_limit, int, 0444);
> >> MODULE_PARM_DESC(gartsize, "Size of kernel GART to setup in megabytes (32, 64, etc., -1=auto)");
> >> module_param_named(gartsize, amdgpu_gart_size, uint, 0600);
> >>
> >> -/**
> >> - * DOC: gttsize (int)
> >> - * Restrict the size of GTT domain (for userspace use) in MiB for testing.
> >> - * The default is -1 (Use 1/2 RAM, minimum value is 3GB).
> >> - */
> >> -MODULE_PARM_DESC(gttsize, "Size of the GTT userspace domain in megabytes (-1 = auto)");
> >> -module_param_named(gttsize, amdgpu_gtt_size, int, 0600);
> >> -
> >> /**
> >> * DOC: moverate (int)
> >> * Set maximum buffer migration rate in MB/s. The default is -1 (8 MB/s).
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >> index e6fc89440210..e3fee81d8646 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >> @@ -1957,13 +1957,8 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> >> DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
> >> (unsigned int)(adev->gmc.real_vram_size / (1024 * 1024)));
> >>
> >> - /* Compute GTT size, either based on TTM limit
> >> - * or whatever the user passed on module init.
> >> - */
> >> - if (amdgpu_gtt_size == -1)
> >> - gtt_size = ttm_tt_pages_limit() << PAGE_SHIFT;
> >> - else
> >> - gtt_size = (uint64_t)amdgpu_gtt_size << 20;
> >> + /* Compute GTT size, based on TTM limit */
> >> + gtt_size = ttm_tt_pages_limit() << PAGE_SHIFT;
> >>
> >> /* Initialize GTT memory pool */
> >> r = amdgpu_gtt_mgr_init(adev, gtt_size);
> >> --
> >> 2.48.0
> >>
>
