[Beignet] [PATCH V2] refine code to separate the usage of data and image2d_from_buffer

Luo, Xionghu xionghu.luo at intel.com
Mon Oct 12 23:44:16 PDT 2015


This patch LGTM.
Thanks.

Luo Xionghu
Best Regards

-----Original Message-----
From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Guo Yejun
Sent: Saturday, October 10, 2015 8:30 AM
To: beignet at lists.freedesktop.org
Cc: Guo, Yejun
Subject: [Beignet] [PATCH V2] refine code to separate the usage of data and image2d_from_buffer

currently, 'void* data' has two meanings: the pointer from application, and the buffer to create image2d from. It makes the code a little complex when supporting userptr for image. So, add a new function parameter to separate the two meanings.

V2: fix when HAS_USERPTR is not enabled
Signed-off-by: Guo Yejun <yejun.guo at intel.com>
---
 src/cl_mem.c    | 61 ++++++++++++++++++++++++++++++++++-----------------------
 src/cl_mem.h    |  1 +
 src/cl_mem_gl.c |  2 +-
 3 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/src/cl_mem.c b/src/cl_mem.c index 0358555..fcef2fa 100644
--- a/src/cl_mem.c
+++ b/src/cl_mem.c
@@ -232,7 +232,8 @@ cl_mem_allocate(enum cl_mem_type type,
                 cl_mem_flags flags,
                 size_t sz,
                 cl_int is_tiled,
-                void *host_ptr,
+                void *host_ptr,         //pointer from application
+                cl_mem buffer,          //image2D from buffer
                 cl_int *errcode)
 {
   cl_buffer_mgr bufmgr = NULL;
@@ -281,6 +282,7 @@ cl_mem_allocate(enum cl_mem_type type,
     assert(bufmgr);
 
 #ifdef HAS_USERPTR
+    uint8_t bufCreated = 0;
     if (ctx->device->host_unified_memory) {
       int page_size = getpagesize();
       int cacheline_size = 0;
@@ -298,6 +300,7 @@ cl_mem_allocate(enum cl_mem_type type,
               mem->is_userptr = 1;
               size_t aligned_sz = ALIGN((mem->offset + sz), page_size);
               mem->bo = cl_buffer_alloc_userptr(bufmgr, "CL userptr memory object", aligned_host_ptr, aligned_sz, 0);
+              bufCreated = 1;
             }
           }
         }
@@ -307,20 +310,24 @@ cl_mem_allocate(enum cl_mem_type type,
           mem->host_ptr = internal_host_ptr;
           mem->is_userptr = 1;
           mem->bo = cl_buffer_alloc_userptr(bufmgr, "CL userptr memory object", internal_host_ptr, alignedSZ, 0);
+          bufCreated = 1;
         }
       }
     }
 
-    if(type == CL_MEM_IMAGE_TYPE && host_ptr && ((cl_mem)host_ptr)->magic == CL_MAGIC_MEM_HEADER) {
+    if(type == CL_MEM_IMAGE_TYPE && buffer != NULL) {
       // if the image if created from buffer, should use the bo directly to share same bo.
-      mem->bo = ((cl_mem)host_ptr)->bo;
+      mem->bo = buffer->bo;
       cl_mem_image(mem)->is_image_from_buffer = 1;
-    } else  if (!mem->is_userptr)
+      bufCreated = 1;
+    }
+
+    if (!bufCreated)
       mem->bo = cl_buffer_alloc(bufmgr, "CL memory object", sz, alignment);  #else
-    if(type == CL_MEM_IMAGE_TYPE && host_ptr && ((cl_mem)host_ptr)->magic == CL_MAGIC_MEM_HEADER) {
+    if(type == CL_MEM_IMAGE_TYPE && buffer != NULL) {
       // if the image if created from buffer, should use the bo directly to share same bo.
-      mem->bo = ((cl_mem)host_ptr)->bo;
+      mem->bo = buffer->bo;
       cl_mem_image(mem)->is_image_from_buffer = 1;
     } else
       mem->bo = cl_buffer_alloc(bufmgr, "CL memory object", sz, alignment); @@ -436,7 +443,7 @@ cl_mem_new_buffer(cl_context ctx,
   sz = ALIGN(sz, 4);
 
   /* Create the buffer in video memory */
-  mem = cl_mem_allocate(CL_MEM_BUFFER_TYPE, ctx, flags, sz, CL_FALSE, data, &err);
+  mem = cl_mem_allocate(CL_MEM_BUFFER_TYPE, ctx, flags, sz, CL_FALSE, 
+ data, NULL, &err);
   if (mem == NULL || err != CL_SUCCESS)
     goto error;
 
@@ -704,7 +711,8 @@ _cl_mem_new_image(cl_context ctx,
                   size_t depth,
                   size_t pitch,
                   size_t slice_pitch,
-                  void *data,
+                  void *data,           //pointer from application
+                  cl_mem buffer,        //for image2D from buffer
                   cl_int *errcode_ret)
 {
   cl_int err = CL_SUCCESS;
@@ -768,7 +776,7 @@ _cl_mem_new_image(cl_context ctx,
       h = (w + ctx->device->image2d_max_width - 1) / ctx->device->image2d_max_width;
       w = w > ctx->device->image2d_max_width ? ctx->device->image2d_max_width : w;
       tiling = CL_NO_TILE;
-    } else if(image_type == CL_MEM_OBJECT_IMAGE2D && data && ((cl_mem)data)->magic == CL_MAGIC_MEM_HEADER) {
+    } else if(image_type == CL_MEM_OBJECT_IMAGE2D && buffer != NULL) {
       tiling = CL_NO_TILE;
     } else if (cl_driver_get_ver(ctx->drv) != 6) {
       /* Pick up tiling mode (we do only linear on SNB) */ @@ -782,7 +790,7 @@ _cl_mem_new_image(cl_context ctx,
     if (UNLIKELY(w > ctx->device->image2d_max_width)) DO_IMAGE_ERROR;
     if (UNLIKELY(h > ctx->device->image2d_max_height)) DO_IMAGE_ERROR;
     if (UNLIKELY(data && min_pitch > pitch)) DO_IMAGE_ERROR;
-    if (UNLIKELY(!data && pitch != 0)) DO_IMAGE_ERROR;
+    if (UNLIKELY(!data && pitch != 0 && buffer == NULL)) 
+ DO_IMAGE_ERROR;
 
     depth = 1;
   } else if (image_type == CL_MEM_OBJECT_IMAGE3D || @@ -831,10 +839,10 @@ _cl_mem_new_image(cl_context ctx,
   }
 
   sz = aligned_pitch * aligned_h * depth;
-  if(image_type == CL_MEM_OBJECT_IMAGE2D && data && ((cl_mem)data)->magic == CL_MAGIC_MEM_HEADER)  {
+  if (image_type == CL_MEM_OBJECT_IMAGE2D && buffer != NULL)  {
     //image 2d created from buffer: per spec, the buffer sz maybe larger than the image 2d.
-    if( ((cl_mem)data)->size >= sz)
-      sz = ((cl_mem)data)->size;
+    if (buffer->size >= sz)
+      sz = buffer->size;
     else {
       err = CL_INVALID_IMAGE_SIZE;
       goto error;
@@ -850,10 +858,13 @@ _cl_mem_new_image(cl_context ctx,
     sz = aligned_pitch * aligned_h * depth;
   }
 
-  if (image_type != CL_MEM_OBJECT_IMAGE1D_BUFFER)
-    mem = cl_mem_allocate(CL_MEM_IMAGE_TYPE, ctx, flags, sz, tiling != CL_NO_TILE, data, &err);
-  else {
-    mem = cl_mem_allocate(CL_MEM_BUFFER1D_IMAGE_TYPE, ctx, flags, sz, tiling != CL_NO_TILE, NULL, &err);
+  if (image_type != CL_MEM_OBJECT_IMAGE1D_BUFFER) {
+    if (image_type == CL_MEM_OBJECT_IMAGE2D && buffer != NULL)
+      mem = cl_mem_allocate(CL_MEM_IMAGE_TYPE, ctx, flags, sz, tiling != CL_NO_TILE, NULL, buffer, &err);
+    else
+      mem = cl_mem_allocate(CL_MEM_IMAGE_TYPE, ctx, flags, sz, tiling 
+ != CL_NO_TILE, data, NULL, &err);  } else {
+    mem = cl_mem_allocate(CL_MEM_BUFFER1D_IMAGE_TYPE, ctx, flags, sz, 
+ tiling != CL_NO_TILE, NULL, NULL, &err);
     if (mem != NULL && err == CL_SUCCESS) {
       struct _cl_mem_buffer1d_image *buffer1d_image = (struct _cl_mem_buffer1d_image *)mem;
       buffer1d_image->size = origin_width;; @@ -863,7 +874,7 @@ _cl_mem_new_image(cl_context ctx,
   if (mem == NULL || err != CL_SUCCESS)
     goto error;
 
-  if(!(image_type == CL_MEM_OBJECT_IMAGE2D && data && ((cl_mem)data)->magic == CL_MAGIC_MEM_HEADER))  {
+  if(!(image_type == CL_MEM_OBJECT_IMAGE2D && buffer != NULL))  {
     //no need set tiling if image 2d created from buffer since share same bo.
     cl_buffer_set_tiling(mem->bo, tiling, aligned_pitch);
   }
@@ -1005,14 +1016,14 @@ _cl_mem_new_image_from_buffer(cl_context ctx,
     image = _cl_mem_new_image(ctx, flags, image_format, image_desc->image_type,
                              image_desc->image_width, image_desc->image_height, image_desc->image_depth,
                              image_desc->image_row_pitch, image_desc->image_slice_pitch,
-                             image_desc->buffer, errcode_ret);
+                             NULL, image_desc->buffer, errcode_ret);
   } else if (image_desc->image_type == CL_MEM_OBJECT_IMAGE1D_BUFFER) {
     // Per bspec, a image should has a at least 2 line vertical alignment,
     // thus we can't simply attach a buffer to a 1d image surface which has the same size.
     // We have to create a new image, and copy the buffer data to this new image.
     // And replace all the buffer object's reference to this image.
     image = _cl_mem_new_image(ctx, flags, image_format, image_desc->image_type,
-                    mem_buffer->base.size / bpp, 0, 0, 0, 0, NULL, errcode_ret);
+                    mem_buffer->base.size / bpp, 0, 0, 0, 0, NULL, 
+ NULL, errcode_ret);
   }
   else
     assert(0);
@@ -1077,7 +1088,7 @@ cl_mem_new_image(cl_context context,
     return _cl_mem_new_image(context, flags, image_format, image_desc->image_type,
                              image_desc->image_width, image_desc->image_height, image_desc->image_depth,
                              image_desc->image_row_pitch, image_desc->image_slice_pitch,
-                             host_ptr, errcode_ret);
+                             host_ptr, NULL, errcode_ret);
   case CL_MEM_OBJECT_IMAGE2D:
     if(image_desc->buffer)
       return _cl_mem_new_image_from_buffer(context, flags, image_format, @@ -1086,13 +1097,13 @@ cl_mem_new_image(cl_context context,
       return _cl_mem_new_image(context, flags, image_format, image_desc->image_type,
                              image_desc->image_width, image_desc->image_height, image_desc->image_depth,
                              image_desc->image_row_pitch, image_desc->image_slice_pitch,
-                             host_ptr, errcode_ret);
+                             host_ptr, NULL, errcode_ret);
   case CL_MEM_OBJECT_IMAGE1D_ARRAY:
   case CL_MEM_OBJECT_IMAGE2D_ARRAY:
     return _cl_mem_new_image(context, flags, image_format, image_desc->image_type,
                              image_desc->image_width, image_desc->image_height, image_desc->image_array_size,
                              image_desc->image_row_pitch, image_desc->image_slice_pitch,
-                             host_ptr, errcode_ret);
+                             host_ptr, NULL, errcode_ret);
   case CL_MEM_OBJECT_IMAGE1D_BUFFER:
     return _cl_mem_new_image_from_buffer(context, flags, image_format,
                                          image_desc, errcode_ret); @@ -2069,7 +2080,7 @@ LOCAL cl_mem cl_mem_new_libva_buffer(cl_context ctx,
   cl_int err = CL_SUCCESS;
   cl_mem mem = NULL;
 
-  mem = cl_mem_allocate(CL_MEM_BUFFER_TYPE, ctx, 0, 0, CL_FALSE, NULL, &err);
+  mem = cl_mem_allocate(CL_MEM_BUFFER_TYPE, ctx, 0, 0, CL_FALSE, NULL, 
+ NULL, &err);
   if (mem == NULL || err != CL_SUCCESS)
     goto error;
 
@@ -2114,7 +2125,7 @@ LOCAL cl_mem cl_mem_new_libva_image(cl_context ctx,
     goto error;
   }
 
-  mem = cl_mem_allocate(CL_MEM_IMAGE_TYPE, ctx, 0, 0, 0, NULL, &err);
+  mem = cl_mem_allocate(CL_MEM_IMAGE_TYPE, ctx, 0, 0, 0, NULL, NULL, 
+ &err);
   if (mem == NULL || err != CL_SUCCESS) {
     err = CL_OUT_OF_HOST_MEMORY;
     goto error;
diff --git a/src/cl_mem.h b/src/cl_mem.h index 17830bf..4970a75 100644
--- a/src/cl_mem.h
+++ b/src/cl_mem.h
@@ -272,6 +272,7 @@ cl_mem_allocate(enum cl_mem_type type,
                 size_t sz,
                 cl_int is_tiled,
                 void *host_ptr,
+                cl_mem buffer,
                 cl_int *errcode);
 
 void
diff --git a/src/cl_mem_gl.c b/src/cl_mem_gl.c index be9eedf..b0b2c1b 100644
--- a/src/cl_mem_gl.c
+++ b/src/cl_mem_gl.c
@@ -63,7 +63,7 @@ cl_mem_new_gl_texture(cl_context ctx,
     goto error;
   }
 
-  mem = cl_mem_allocate(CL_MEM_GL_IMAGE_TYPE, ctx, flags, 0, 0, NULL, &err);
+  mem = cl_mem_allocate(CL_MEM_GL_IMAGE_TYPE, ctx, flags, 0, 0, NULL, 
+ NULL, &err);
   if (mem == NULL || err != CL_SUCCESS)
     goto error;
 
--
1.9.1

_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list