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

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

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

Signed-off-by: Felix Kuehling <Felix.Kuehling@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);
 	}
 
 	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

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

[Felix Kuehling]

This patch is meant for amd-staging-drm-next. It's part of my effort to
reduce differences between kfd-staging and upstream and eventually
replace all our memory manager hacks with upstream solutions.

This commit should only affect GFXv8 and older. It should not have any
negative impact, except bigger GPUVM page tables on systems with lots of
memory. Conversely, this could actually allow making the GPUVM page
tables smaller on systems with little memory, if we wanted to take
advantage of that opportunity.

The current minimum is 64GB, which I believe means 32KB page tables and
page directories. The smallest VM size that makes sense with 2-levels of
4KB page tables would be 1GB. If we really want to go that small, I
should also factor the local VRAM size into the equation.

Regards,
  Felix


On 2018-08-21 05:25 PM, 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@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);
>  	}
>  
>  	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

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

[Christian König]

Am 21.08.2018 um 23:45 schrieb Felix Kuehling:
> This patch is meant for amd-staging-drm-next. It's part of my effort to
> reduce differences between kfd-staging and upstream and eventually
> replace all our memory manager hacks with upstream solutions.
>
> This commit should only affect GFXv8 and older. It should not have any
> negative impact, except bigger GPUVM page tables on systems with lots of
> memory. Conversely, this could actually allow making the GPUVM page
> tables smaller on systems with little memory, if we wanted to take
> advantage of that opportunity.
>
> The current minimum is 64GB, which I believe means 32KB page tables and
> page directories. The smallest VM size that makes sense with 2-levels of
> 4KB page tables would be 1GB. If we really want to go that small, I
> should also factor the local VRAM size into the equation.
>
> Regards,
>    Felix
>
>
> On 2018-08-21 05:25 PM, 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@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;

Looks good to me, but I would make sure that round that up before 
shifting it.

>> +		vm_size = min(max(phys_ram_gb * 3, min_vm_size), max_size);

Mhm, "phys_ram_gb * 3"? Maybe add a comment with the rational for that.

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Felix Kuehling]

On 2018-08-22 02:55 AM, Christian König wrote:
> Am 21.08.2018 um 23:45 schrieb Felix Kuehling:
[snip]
>> +    } 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;
>
> Looks good to me, but I would make sure that round that up before
> shifting it.

Hmm, we used to round up. I just removed it because we were told it's
not necessary.

But I guess rounding up to the next power of two increases the available
VM size without increasing page table size. So we should take it if we
can get it for free. I'll reintroduce rounding up.

>
>>> +        vm_size = min(max(phys_ram_gb * 3, min_vm_size), max_size);
>
> Mhm, "phys_ram_gb * 3"? Maybe add a comment with the rational for that.

Well, the long comment above was meant to justify the factor 3. Maybe I
didn't make that clear enough. 1x is system memory itself, 2x is a wild
guess of VRAM on all GPUs. 3x is room for a second aperture for MTYPE
control, fragmentation and guard pages.

Regards,
  Felix

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Christian König]

Am 22.08.2018 um 19:40 schrieb Felix Kuehling:
> On 2018-08-22 02:55 AM, Christian König wrote:
>> Am 21.08.2018 um 23:45 schrieb Felix Kuehling:
> [snip]
>>> +    } 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;
>> Looks good to me, but I would make sure that round that up before
>> shifting it.
> Hmm, we used to round up. I just removed it because we were told it's
> not necessary.
>
> But I guess rounding up to the next power of two increases the available
> VM size without increasing page table size. So we should take it if we
> can get it for free. I'll reintroduce rounding up.

No, that wasn't what I meant. Rounding up to the next power of two is 
indeed not necessary.

What I meant is when the installed system memory is 3.9GB we only take 
into account 3GB here because we always truncate.

>>>> +        vm_size = min(max(phys_ram_gb * 3, min_vm_size), max_size);
>> Mhm, "phys_ram_gb * 3"? Maybe add a comment with the rational for that.
> Well, the long comment above was meant to justify the factor 3. Maybe I
> didn't make that clear enough. 1x is system memory itself, 2x is a wild
> guess of VRAM on all GPUs. 3x is room for a second aperture for MTYPE
> control, fragmentation and guard pages.

Ah! Yeah that wasn't obvious.

Christian.

>
> Regards,
>    Felix
>
>> 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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [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@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);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx