-----Original Message-----
From: Koenig, Christian
Sent: Tuesday, August 15, 2017 4:48 PM
To: He, Roger <Hongbo.He at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amd/amdgpu: expose fragment size as module parameter
Am 15.08.2017 um 10:36 schrieb Roger He:
> Change-Id: I70e4ea94b8520e19cfee5ba6c9a0ecf1ee3f5f1a
> Signed-off-by: Roger He <Hongbo.He at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +++++--
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 1 -
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 1 -
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 1 -
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
> 8 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d2aaad7..957bd2b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -97,6 +97,7 @@ extern int amdgpu_bapm;
> extern int amdgpu_deep_color;
> extern int amdgpu_vm_size;
> extern int amdgpu_vm_block_size;
> +extern int amdgpu_vm_fragment_size;
> extern int amdgpu_vm_fault_stop;
> extern int amdgpu_vm_debug;
> extern int amdgpu_vm_update_mode;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index dd1dc87..44c66a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1077,6 +1077,14 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
> amdgpu_gtt_size = -1;
> }
>
> + /* make sense only for GFX8 and previous ASICs
> + * valid rang is between 4 and 9 inclusive
> + */
> + if (amdgpu_vm_fragment_size > 9 || amdgpu_vm_fragment_size < 4) {
> + dev_warn(adev->dev, "valid rang is between 4 and 9\n");
> + amdgpu_vm_fragment_size = 4;
> + }
> +
> amdgpu_check_vm_size(adev);
>
> amdgpu_check_block_size(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2cdf844..d9522a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -94,6 +94,7 @@ unsigned amdgpu_ip_block_mask = 0xffffffff;
> int amdgpu_bapm = -1;
> int amdgpu_deep_color = 0;
> int amdgpu_vm_size = -1;
> +int amdgpu_vm_fragment_size = 4;
> int amdgpu_vm_block_size = -1;
> int amdgpu_vm_fault_stop = 0;
> int amdgpu_vm_debug = 0;
> @@ -184,6 +185,9 @@ module_param_named(deep_color, amdgpu_deep_color, int, 0444);
> MODULE_PARM_DESC(vm_size, "VM address space size in gigabytes (default 64GB)");
> module_param_named(vm_size, amdgpu_vm_size, int, 0444);
>
> +MODULE_PARM_DESC(vm_fragment_size, "VM fragment size in bits (4, 5,
> +etc. 4 = 64K (default), Max 9 = 2M)");
> +module_param_named(vm_fragment_size, amdgpu_vm_fragment_size, int,
> +0444);
> +
> MODULE_PARM_DESC(vm_block_size, "VM page table size in bits (default depending on vm_size)");
> module_param_named(vm_block_size, amdgpu_vm_block_size, int, 0444);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 4ad04cd..85ef4d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2437,8 +2437,11 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size)
> else
> adev->vm_manager.block_size = amdgpu_vm_block_size;
>
> - DRM_INFO("vm size is %llu GB, block size is %u-bit\n",
> - adev->vm_manager.vm_size, adev->vm_manager.block_size);
> + adev->vm_manager.fragment_size = amdgpu_vm_fragment_size;
> +
> + DRM_INFO("vm size is %llu GB, block size is %u-bit, fragment size is %u-bit\n",
> + adev->vm_manager.vm_size, adev->vm_manager.block_size,
> + adev->vm_manager.fragment_size);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index dcb053f..56218ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -815,7 +815,6 @@ static int gmc_v6_0_sw_init(void *handle)
> return r;
>
> amdgpu_vm_adjust_size(adev, 64);
> - adev->vm_manager.fragment_size = 4;
> adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18;
>
> adev->mc.mc_mask = 0xffffffffffULL; diff --git
> a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 2ac9afa..7f5eb02 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -951,7 +951,6 @@ static int gmc_v7_0_sw_init(void *handle)
> * Max GPUVM size for cayman and SI is 40 bits.
> */
> amdgpu_vm_adjust_size(adev, 64);
> - adev->vm_manager.fragment_size = 4;
> adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18;
>
> /* Set the internal MC address mask diff --git
> a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 27c70d8..1ffba0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1049,7 +1049,6 @@ static int gmc_v8_0_sw_init(void *handle)
> * Max GPUVM size for cayman and SI is 40 bits.
> */
> amdgpu_vm_adjust_size(adev, 64);
> - adev->vm_manager.fragment_size = 4;
> adev->vm_manager.max_pfn = adev->vm_manager.vm_size << 18;
>
> /* Set the internal MC address mask diff --git
> a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index f721b4f..150da9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -544,9 +544,9 @@ static int gmc_v9_0_sw_init(void *handle)
> adev->vm_manager.fragment_size = 9;
> } else {
> /* vm_size is 64GB for legacy 2-level page support*/
> + amdgpu_vm_fragment_size = 9;
Don't adjust the global parameter here.
Better use the default handling to amdgpu_vm_adjust_size() like you had in the last version of the patch.
For Vega10 fragment handling works a bit differently, the L1 works like
GFX8 but the L2 is currently fixed to 2M.
So different fragment sizes still make sense for Vega10, you should just not change the hardware setup of the L2.
[Roger]: which register do you mean here?
Thanks
Roger(Hongbo.He)
> amdgpu_vm_adjust_size(adev, 64);
> adev->vm_manager.num_level = 1;
> - adev->vm_manager.fragment_size = 9;
> }
> break;
> case CHIP_VEGA10:
