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

[Pan, Xiuli]

Thank you for your advice, I will remember that.
Thanks
Xiuli Pan

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

LGTM.

Just a reminder, please add some thing to the commit log to indicate the version of this patch and what has been changed in this patch.

Thanks,
Zhigang Gong.

On Fri, Oct 23, 2015 at 01:22:56PM +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 | 19 +++++++++++--------
>  2 files changed, 12 insertions(+), 9 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 
> 60d318a..e96bb95 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,9 +1000,10 @@ 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)
>  {
> -  uint32_t s = size - 1;
> +  assert(size <= (2ul<<30));
> +  size_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)];
>    memset(ss0, 0, sizeof(gen7_surface_state_t)); @@ -1030,9 +1031,10 
> @@ 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)
>  {
> -  uint32_t s = size - 1;
> +  assert(size <= (2ul<<30));
> +  size_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)];
>    memset(ss0, 0, sizeof(gen7_surface_state_t)); @@ -1066,9 +1068,10 
> @@ 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)
>  {
> -  uint32_t s = size - 1;
> +  assert(size <= (2ul<<30));
> +  size_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)];
>    memset(ss0, 0, 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