[PATCH] drm/amdgpu: Adjust the VM size based on system memory size

[Zhang, Jerry (Junwei)]

On 08/22/2018 05:25 AM, Felix Kuehling wrote:
> Set the VM size based on system memory size between the ASIC-specific
> limits given by min_vm_size and max_bits. GFXv9 GPUs will keep their
> default VM size of 256TB (48 bit). Only older GPUs will adjust VM size
> depending on system memory size.
>
> This makes more VM space available for ROCm applications on GFXv8 GPUs
> that want to map all available VRAM and system memory in their SVM
> address space.
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 ++++++++++++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 +-
>   2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 662e8a3..48971cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2482,28 +2482,46 @@ static uint32_t amdgpu_vm_get_block_size(uint64_t vm_size)
>    * amdgpu_vm_adjust_size - adjust vm size, block size and fragment size
>    *
>    * @adev: amdgpu_device pointer
> - * @vm_size: the default vm size if it's set auto
> + * @min_vm_size: the minimum vm size in GB if it's set auto
>    * @fragment_size_default: Default PTE fragment size
>    * @max_level: max VMPT level
>    * @max_bits: max address space size in bits
>    *
>    */
> -void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t vm_size,
> +void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>   			   uint32_t fragment_size_default, unsigned max_level,
>   			   unsigned max_bits)
>   {
> +	unsigned int max_size = 1 << (max_bits - 30);
> +	unsigned int vm_size;
>   	uint64_t tmp;
>
>   	/* adjust vm size first */
>   	if (amdgpu_vm_size != -1) {
> -		unsigned max_size = 1 << (max_bits - 30);
> -
>   		vm_size = amdgpu_vm_size;
>   		if (vm_size > max_size) {
>   			dev_warn(adev->dev, "VM size (%d) too large, max is %u GB\n",
>   				 amdgpu_vm_size, max_size);
>   			vm_size = max_size;
>   		}
> +	} else {
> +		struct sysinfo si;
> +		unsigned int phys_ram_gb;
> +
> +		/* Optimal VM size depends on the amount of physical
> +		 * RAM available. Underlying requirements and
> +		 * assumptions:
> +		 *
> +		 *  - Need to map system memory and VRAM from all GPUs
> +		 *     - VRAM from other GPUs not known here
> +		 *     - Assume VRAM <= system memory
> +		 *  - On GFX8 and older, VM space can be segmented for
> +		 *    different MTYPEs
> +		 *  - Need to allow room for fragmentation, guard pages etc.
> +		 */
> +		si_meminfo(&si);
> +		phys_ram_gb = ((uint64_t)si.totalram * si.mem_unit) >> 30;
> +		vm_size = min(max(phys_ram_gb * 3, min_vm_size), max_size);

The patch looks good for me.
Curious about the constant "3", is that a experiment value? or any verification for that?

Regards,
Jerry

>   	}
>
>   	adev->vm_manager.max_pfn = (uint64_t)vm_size << 18;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 1162c2b..ab1d23e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -345,7 +345,7 @@ struct amdgpu_bo_va_mapping *amdgpu_vm_bo_lookup_mapping(struct amdgpu_vm *vm,
>   void amdgpu_vm_bo_trace_cs(struct amdgpu_vm *vm, struct ww_acquire_ctx *ticket);
>   void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>   		      struct amdgpu_bo_va *bo_va);
> -void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t vm_size,
> +void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>   			   uint32_t fragment_size_default, unsigned max_level,
>   			   unsigned max_bits);
>   int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp);
>