* Re: [PATCH] drm/amdgpu: Adjust the VM size based on system memory size v2
[not found] ` <1535063830-8078-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-24 1:53 ` Zhang, Jerry (Junwei)
2018-08-24 6:31 ` Huang Rui
2018-08-24 7:32 ` Christian König
2 siblings, 0 replies; 7+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-08-24 1:53 UTC (permalink / raw)
To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 08/24/2018 06:37 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.
>
> v2:
> * Clarify comment
> * Round up memory size before >> 30
> * Round up automatic vm_size to power of two
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
Acked-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 32 ++++++++++++++++++++++++++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a6b1126..543db67 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2482,28 +2482,52 @@ 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.
> + *
> + * This adds up to a rough guess of system memory x3.
> + * Round up to power of two to maximize the available
> + * VM size with the given page table size.
> + */
> + si_meminfo(&si);
> + phys_ram_gb = ((uint64_t)si.totalram * si.mem_unit +
> + (1 << 30) - 1) >> 30;
> + vm_size = roundup_pow_of_two(
> + min(max(phys_ram_gb * 3, min_vm_size), max_size));
> }
>
> 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);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Adjust the VM size based on system memory size v2
[not found] ` <1535063830-8078-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
2018-08-24 1:53 ` Zhang, Jerry (Junwei)
@ 2018-08-24 6:31 ` Huang Rui
2018-08-24 7:32 ` Christian König
2 siblings, 0 replies; 7+ messages in thread
From: Huang Rui @ 2018-08-24 6:31 UTC (permalink / raw)
To: Felix Kuehling; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Thu, Aug 23, 2018 at 06:37:10PM -0400, 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.
>
> v2:
> * Clarify comment
> * Round up memory size before >> 30
> * Round up automatic vm_size to power of two
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 32 ++++++++++++++++++++++++++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a6b1126..543db67 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2482,28 +2482,52 @@ 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.
> + *
> + * This adds up to a rough guess of system memory x3.
> + * Round up to power of two to maximize the available
> + * VM size with the given page table size.
> + */
> + si_meminfo(&si);
> + phys_ram_gb = ((uint64_t)si.totalram * si.mem_unit +
> + (1 << 30) - 1) >> 30;
> + vm_size = roundup_pow_of_two(
> + min(max(phys_ram_gb * 3, min_vm_size), max_size));
> }
>
> 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);
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: Adjust the VM size based on system memory size v2
[not found] ` <1535063830-8078-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
2018-08-24 1:53 ` Zhang, Jerry (Junwei)
2018-08-24 6:31 ` Huang Rui
@ 2018-08-24 7:32 ` Christian König
[not found] ` <2acc80ae-64a3-534c-c006-97d905b84933-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2018-08-24 7:32 UTC (permalink / raw)
To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Am 24.08.2018 um 00:37 schrieb Felix Kuehling:
> 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.
>
> v2:
> * Clarify comment
> * Round up memory size before >> 30
> * Round up automatic vm_size to power of two
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 32 ++++++++++++++++++++++++++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a6b1126..543db67 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2482,28 +2482,52 @@ 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.
> + *
> + * This adds up to a rough guess of system memory x3.
> + * Round up to power of two to maximize the available
> + * VM size with the given page table size.
> + */
> + si_meminfo(&si);
> + phys_ram_gb = ((uint64_t)si.totalram * si.mem_unit +
> + (1 << 30) - 1) >> 30;
> + vm_size = roundup_pow_of_two(
> + min(max(phys_ram_gb * 3, min_vm_size), max_size));
The roundup to a power of two here is not 100% correct, but I don't
think it will hurt us much.
For now the patch is Reviewed-by: Christian König
<christian.koenig@amd.com>.
Regards,
Christian.
> }
>
> 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);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread