[Beignet] [PATCH] driver/runtime: get global mem size dynamically

[Pan, Xiuli]

So I have rewrite the code here, and I think we should not using totalgpumem for max_mem_alloc_size, the ret->global_mem_size / 2 will always less or equal to totalgpumem.
Or I may not get your point about the threshold.

ret->global_mem_size = min(totalram/2 , totalgpumem) 
ret->max_mem_alloc_size =  ret->global_mem_size / 2;

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Friday, October 9, 2015 3:05 PM
To: Pan, Xiuli <xiuli.pan at intel.com>
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] driver/runtime: get global mem size dynamically

On Fri, Oct 09, 2015 at 03:56:11PM +0800, Pan Xiuli wrote:
> The gen8 and higher gpu can have more than 2G mem, so we dynamically 
> get the global mem size.
> 
> Signed-off-by: Pan Xiuli <xiuli.pan at intel.com>
> ---
>  src/cl_device_id.c       | 6 +++---
>  src/intel/intel_driver.c | 6 ++++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/cl_device_id.c b/src/cl_device_id.c index 
> 78d2cf4..c3bd35f 100644
> --- a/src/cl_device_id.c
> +++ b/src/cl_device_id.c
> @@ -550,10 +550,10 @@ skl_gt4_break:
>  
>    struct sysinfo info;
>    if (sysinfo(&info) == 0) {
> -    uint64_t two_gb = 2 * 1024 * 1024 * 1024ul; 
> +    uint64_t totalgpumem = ret->global_mem_size;
>      uint64_t totalram = info.totalram * info.mem_unit;
> -    ret->global_mem_size = (totalram > two_gb) ? 
> -                            two_gb : info.totalram;
> +    ret->global_mem_size = (totalram > totalgpumem) ?
> +                            totalgpumem: totalram;
>      ret->max_mem_alloc_size = ret->global_mem_size / 2;

The "/ 2" is a hard coded method to avoid allocate too much memory which is not supported by the platform. Now this patch changes to get aperture size from libdrm, then we don't need to use it any more.

But we may still want to make sure that we don't allocate too much system memory. I think totalram/2 should be a reasonable threshold value.

So the following code may be better:

reg->global_mem_size = min(totalram/2, totalgpumem); max_mem_alloc_size 
reg->= min(global_mem_size/2, totalgpumem);

Furthermore we still need to check carefully whether it's safe to use totalgpumem as the global_mem_size and/or max_mem_alloc_size.

If the test case tests the maximum memory size, and there are some aperture space already allocated by system, then the case may be failed. Before submit the patch, it's better to test it with all the related conformance test cases on all platforms. Or maybe we need to add some cases to test edge conditions into the internal unit test cases.

Thanks,
Zhigang Gong.

>    }
>  
> diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c index 
> 035a103..1f286f7 100644
> --- a/src/intel/intel_driver.c
> +++ b/src/intel/intel_driver.c
> @@ -829,6 +829,12 @@ intel_update_device_info(cl_device_id device)
>    if (IS_CHERRYVIEW(device->device_id))
>      printf(CHV_CONFIG_WARNING);
>  #endif
> +  //We should get the device memory dynamically, also the  //mapablce 
> + mem size usage is unknown. Still use global_mem_size/2  //as 
> + max_mem_alloc_size in cl_get_gt_device.
> +  size_t total_mem,map_mem;
> +  drm_intel_get_aperture_sizes(driver->fd,&map_mem,&total_mem);
> +  device->global_mem_size = (cl_ulong)total_mem;
>  
>    intel_driver_context_destroy(driver);
>    intel_driver_close(driver);
> --
> 2.1.4
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet