[Beignet] [PATCH 1/4] drivers: change the buf size to size_t

[Pan, Xiuli]

Thanks, I will send V2 patches.

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Friday, October 23, 2015 10:12 AM
To: Pan, Xiuli <xiuli.pan at intel.com>
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH 1/4] drivers: change the buf size to size_t

On Wed, Oct 14, 2015 at 04:34:04PM +0800, Pan Xiuli wrote:
> The uint32_t size is not enough for coming bigger gpu memory, now GEN9 
> support 4G buffer. Also add assertion for invalid size.
> 
> Signed-off-by: Pan Xiuli <xiuli.pan at intel.com>
> ---
>  src/cl_driver.h         |  2 +-
>  src/intel/intel_gpgpu.c | 13 ++++++++-----
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/src/cl_driver.h b/src/cl_driver.h index 1ab4dff..4ffca09 
> 100644
> --- a/src/cl_driver.h
> +++ b/src/cl_driver.h
> @@ -138,7 +138,7 @@ typedef void (cl_gpgpu_sync_cb)(void*);  extern 
> cl_gpgpu_sync_cb *cl_gpgpu_sync;
>  
>  /* Bind a regular unformatted buffer */ -typedef void 
> (cl_gpgpu_bind_buf_cb)(cl_gpgpu, cl_buffer, uint32_t offset, uint32_t 
> internal_offset, uint32_t size, uint8_t bti);
> +typedef void (cl_gpgpu_bind_buf_cb)(cl_gpgpu, cl_buffer, uint32_t 
> +offset, uint32_t internal_offset, size_t size, uint8_t bti);
>  extern cl_gpgpu_bind_buf_cb *cl_gpgpu_bind_buf;
>  
>  /* bind samplers defined in both kernel and kernel args. */ diff 
> --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c index 
> 901bd98..4650325 100644
> --- a/src/intel/intel_gpgpu.c
> +++ b/src/intel/intel_gpgpu.c
> @@ -86,7 +86,7 @@ typedef void 
> (intel_gpgpu_set_base_address_t)(intel_gpgpu_t *gpgpu);  
> intel_gpgpu_set_base_address_t *intel_gpgpu_set_base_address = NULL;
>  
>  typedef void (intel_gpgpu_setup_bti_t)(intel_gpgpu_t *gpgpu, drm_intel_bo *buf, uint32_t internal_offset,
> -                                       uint32_t size, unsigned char index, uint32_t format);
> +                                       size_t size, unsigned char 
> + index, uint32_t format);
>  intel_gpgpu_setup_bti_t *intel_gpgpu_setup_bti = NULL;
>  
>  
> @@ -1000,8 +1000,9 @@ intel_gpgpu_alloc_constant_buffer(intel_gpgpu_t 
> *gpgpu, uint32_t size, uint8_t b
>  
>  static void
>  intel_gpgpu_setup_bti_gen7(intel_gpgpu_t *gpgpu, drm_intel_bo *buf, uint32_t internal_offset,
> -                                   uint32_t size, unsigned char index, uint32_t format)
> +                                   size_t size, unsigned char index, 
> + uint32_t format)
>  {
> +  assert(size <= (2<<30));
should use 2ul<<30
>    uint32_t s = size - 1;
Please use size_t for s as well.

There are some similar problem in the following functions.

>    surface_heap_t *heap = gpgpu->aux_buf.bo->virtual + gpgpu->aux_offset.surface_heap_offset;
>    gen7_surface_state_t *ss0 = (gen7_surface_state_t *) 
> &heap->surface[index * sizeof(gen7_surface_state_t)]; @@ -1030,8 
> +1031,9 @@ intel_gpgpu_setup_bti_gen7(intel_gpgpu_t *gpgpu, 
> drm_intel_bo *buf, uint32_t int
>  
>  static void
>  intel_gpgpu_setup_bti_gen75(intel_gpgpu_t *gpgpu, drm_intel_bo *buf, uint32_t internal_offset,
> -                                   uint32_t size, unsigned char index, uint32_t format)
> +                                   size_t size, unsigned char index, 
> + uint32_t format)
>  {
> +  assert(size <= (2<<30));
>    uint32_t s = size - 1;
>    surface_heap_t *heap = gpgpu->aux_buf.bo->virtual + gpgpu->aux_offset.surface_heap_offset;
>    gen7_surface_state_t *ss0 = (gen7_surface_state_t *) 
> &heap->surface[index * sizeof(gen7_surface_state_t)]; @@ -1066,8 
> +1068,9 @@ intel_gpgpu_setup_bti_gen75(intel_gpgpu_t *gpgpu, 
> drm_intel_bo *buf, uint32_t in
>  
>  static void
>  intel_gpgpu_setup_bti_gen8(intel_gpgpu_t *gpgpu, drm_intel_bo *buf, uint32_t internal_offset,
> -                                   uint32_t size, unsigned char index, uint32_t format)
> +                                   size_t size, unsigned char index, 
> + uint32_t format)
>  {
> +  assert(size <= (2<<30));
>    uint32_t s = size - 1;
>    surface_heap_t *heap = gpgpu->aux_buf.bo->virtual + gpgpu->aux_offset.surface_heap_offset;
>    gen8_surface_state_t *ss0 = (gen8_surface_state_t *) 
> &heap->surface[index * sizeof(gen8_surface_state_t)]; @@ -1395,7 
> +1398,7 @@ intel_gpgpu_bind_image_gen9(intel_gpgpu_t *gpgpu,
>  
>  static void
>  intel_gpgpu_bind_buf(intel_gpgpu_t *gpgpu, drm_intel_bo *buf, uint32_t offset,
> -                     uint32_t internal_offset, uint32_t size, uint8_t bti)
> +                     uint32_t internal_offset, size_t size, uint8_t 
> + bti)
>  {
>    assert(gpgpu->binded_n < max_buf_n);
>    gpgpu->binded_buf[gpgpu->binded_n] = buf;
> --
> 2.1.4
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet