On 03/31/2017 12:05 PM, Deucher, Alexander wrote: >> -----Original Message----- >> From: Junwei Zhang [mailto:Jerry.Zhang at amd.com] >> Sent: Thursday, March 30, 2017 10:52 PM >> To: Deucher, Alexander; Koenig, Christian >> Cc: amd-gfx at lists.freedesktop.org; Zhang, Jerry >> Subject: [PATCH 1/2 v2] drm/amdgpu: fix vm size and block size for Vega10 >> VMPT >> >> From: "Zhang, Jerry" <Jerry.Zhang at amd.com> >> >> Signed-off-by: Junwei Zhang <Jerry.Zhang at amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 -------- >> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 15 ++++++++------- >> 2 files changed, 8 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 363d73c..98555fd 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -1041,14 +1041,6 @@ static bool amdgpu_check_pot_argument(int arg) >> >> static void amdgpu_get_block_size(struct amdgpu_device *adev) >> { >> - /* from AI, asic starts to support multiple level VMPT */ >> - if (adev->asic_type >= CHIP_VEGA10) { >> - if (amdgpu_vm_block_size != 9) >> - dev_warn(adev->dev, >> - "Multi-VMPT limits block size to one >> page!\n"); >> - amdgpu_vm_block_size = 9; >> - return; >> - } >> /* defines number of bits in page table versus page directory, >> * a page is 4KB so we have 12 bits offset, minimum 9 bits in the >> * page table and the remaining bits are in the page directory */ >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> index df69aae..081a676 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> @@ -546,6 +546,14 @@ static int gmc_v9_0_sw_init(void *handle) >> } else { >> /* XXX Don't know how to get VRAM type yet. */ >> adev->mc.vram_type = AMDGPU_VRAM_TYPE_HBM; >> + >> + /* >> + * To fulfill 4-level page support, >> + * vm size is 256TB (48bit), maximum size of Vega10, >> + * block size 512 (9bit) >> + */ >> + amdgpu_vm_size = 1U << 18; >> + amdgpu_vm_block_size = 9; > > This won't work. You are still changing the amdgpu_vm_* global variables so if > you have 2 GPUs in the systems, you'll change them both. > >> } >> >> /* This interrupt is VMC page fault.*/ >> @@ -557,13 +565,6 @@ static int gmc_v9_0_sw_init(void *handle) >> if (r) >> return r; >> >> - /* Because of four level VMPTs, vm size is at least 512GB. >> - * The maximum size is 256TB (48bit). >> - */ >> - if (amdgpu_vm_size < 512) { >> - DRM_WARN("VM size is at least 512GB!\n"); >> - amdgpu_vm_size = 512; >> - } >> adev->vm_manager.max_pfn = (uint64_t)amdgpu_vm_size << 18; > > Instead of changing amdgpu_vm_size, change adev->vm_manager.max_pfn directly, > that way you won't be changing the global variable and possibly messing up > other driver instances. The alternative, and cleaner in my opinion, would be > to add new variables to the vm_manager, e.g., adev->vm_manager.size and > adev->vm_manager.block_size, and set them to something reasonable based on the > module parameters and them use them in the code rather than the global variables. Yeah, thanks to point it out. I prepared another patch, please check it too. Jerry > > Alex > >> >> /* Set the internal MC address mask >> -- >> 1.9.1 >