[PATCH 1/2 v2] drm/amdgpu: fix vm size and block size for Vega10 VMPT

[Deucher, Alexander]

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

Alex

> 
>  	/* Set the internal MC address mask
> --
> 1.9.1