All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Adjust the VM size based on system memory size v2
@ 2018-08-23 22:37 Felix Kuehling
       [not found] ` <1535063830-8078-1-git-send-email-Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Felix Kuehling @ 2018-08-23 22:37 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: 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));
 	}
 
 	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

^ permalink raw reply related	[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: 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

* Re: [PATCH] drm/amdgpu: Adjust the VM size based on system memory size v2
       [not found]     ` <2acc80ae-64a3-534c-c006-97d905b84933-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-08-24 18:59       ` Felix Kuehling
       [not found]         ` <8e91ee0a-d4e1-80a5-a406-331e78b57a23-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Felix Kuehling @ 2018-08-24 18:59 UTC (permalink / raw)
  To: christian.koenig-5C7GfCeVMHo,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 2018-08-24 03:32 AM, Christian König wrote:
> 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>. 
Thanks. Do you mind elaborating on what's not 100% correct here?

Maybe do the min(..., max_size) after roundup_pow_of_two?

Regards,
  Felix
_______________________________________________
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]         ` <8e91ee0a-d4e1-80a5-a406-331e78b57a23-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-24 19:09           ` Christian König
       [not found]             ` <a3c22bc8-7022-4b67-1926-fb5d4722caaf-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2018-08-24 19:09 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 24.08.2018 um 20:59 schrieb Felix Kuehling:
> On 2018-08-24 03:32 AM, Christian König wrote:
>> 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>.
> Thanks. Do you mind elaborating on what's not 100% correct here?

Well it's not a power of two what give optimal root PD sizes.

IIRC the optimal configuration for different vm sizes are:
1GB:    4k/4k
2GB:    8k/4k or 4k/8k
4GB:    8k/8k
6GB:    12K/8k
8GB:    16K/8k or 8k/16k
12GB:    12k/16k
....

As you can see 6GB and 12GB are still using all the allocated PD, but 
are not power of two.

> Maybe do the min(..., max_size) after roundup_pow_of_two?

We would need to increase the vm_size based on the PD size after 
figuring out all the parameters.

But the win is probably not worth the effort.

Regards,
Christian.

>
> Regards,
>    Felix

_______________________________________________
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]             ` <a3c22bc8-7022-4b67-1926-fb5d4722caaf-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-24 19:20               ` Felix Kuehling
  0 siblings, 0 replies; 7+ messages in thread
From: Felix Kuehling @ 2018-08-24 19:20 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Christian König

On 2018-08-24 03:09 PM, Christian König wrote:
> Am 24.08.2018 um 20:59 schrieb Felix Kuehling:
>> On 2018-08-24 03:32 AM, Christian König wrote:
>>> 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>.
>> Thanks. Do you mind elaborating on what's not 100% correct here?
>
> Well it's not a power of two what give optimal root PD sizes.
>
> IIRC the optimal configuration for different vm sizes are:
> 1GB:    4k/4k
> 2GB:    8k/4k or 4k/8k
> 4GB:    8k/8k
> 6GB:    12K/8k
> 8GB:    16K/8k or 8k/16k
> 12GB:    12k/16k
> ....
>
> As you can see 6GB and 12GB are still using all the allocated PD, but
> are not power of two.
Oh, I see. I guess my mistake was to assume the PT/PD sizes were powers
of two. With my change they will be.

>
>> Maybe do the min(..., max_size) after roundup_pow_of_two?
>
> We would need to increase the vm_size based on the PD size after
> figuring out all the parameters.
>
> But the win is probably not worth the effort.
Right. Then I'll go ahead and submit it as is.

Thanks,
  Felix


>
> Regards,
> Christian.
>
>>
>> Regards,
>>    Felix
>
> _______________________________________________
> 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

end of thread, other threads:[~2018-08-24 19:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-23 22:37 [PATCH] drm/amdgpu: Adjust the VM size based on system memory size v2 Felix Kuehling
     [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>
2018-08-24 18:59       ` Felix Kuehling
     [not found]         ` <8e91ee0a-d4e1-80a5-a406-331e78b57a23-5C7GfCeVMHo@public.gmane.org>
2018-08-24 19:09           ` Christian König
     [not found]             ` <a3c22bc8-7022-4b67-1926-fb5d4722caaf-5C7GfCeVMHo@public.gmane.org>
2018-08-24 19:20               ` Felix Kuehling

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.