Agreed. Good suggestion. I will refine the patch. -----Original Message----- From: Guo, Yejun Sent: Friday, September 18, 2015 11:47 To: Zhao, Yakui; Weng, Chuanbo Cc: [email protected] Subject: RE: [Beignet] [PATCH] Add extension clCreateMemObjectFromFdIntel to create cl memory object by external buffer object's fd.
According to the rules described in https://www.khronos.org/registry/cl/extensions/template.txt, we can choose the function name as clCreateBufferFromPrimeFdINTEL/clCreateImageFromPrimeFdINTEL and the struct name as cl_mem_object_desc_intel. -----Original Message----- From: Beignet [mailto:[email protected]] On Behalf Of Zhao Yakui Sent: Friday, September 18, 2015 10:58 AM To: Weng, Chuanbo Cc: [email protected] Subject: Re: [Beignet] [PATCH] Add extension clCreateMemObjectFromFdIntel to create cl memory object by external buffer object's fd. On 09/18/2015 10:39 AM, Chuanbo Weng wrote: > Before this patch, Beignet can only import external bo by its handle > using clCreateBufferFromLibvaIntel/clCreateImageFromLibvaIntel. > Render node is the first choice of access gpu in currect Beignet > implementation, it causes DRM_IOCTL_GEM_OPEN fail. So it's necessary > to add this extension to support buffer sharing between different > libraries. The Fd is not restricted to Intel GFX object. Can we rename it as clCreateBufferFromPrimeFD/clCreateImageFromPrimeFD? Otherwise it will cause the confusion that FD only can come from Intel GFX object. > > Signed-off-by: Chuanbo Weng<[email protected]> > --- > include/CL/cl_intel.h | 24 +++++++++++++ > src/cl_api.c | 42 +++++++++++++++++++++++ > src/cl_driver.h | 6 ++++ > src/cl_driver_defs.c | 2 ++ > src/cl_mem.c | 88 > ++++++++++++++++++++++++++++++++++++++++++++++++ > src/cl_mem.h | 13 +++++++ > src/intel/intel_driver.c | 51 ++++++++++++++++++++++++++-- > 7 files changed, 223 insertions(+), 3 deletions(-) > > diff --git a/include/CL/cl_intel.h b/include/CL/cl_intel.h index > 28bcb62..50262cb 100644 > --- a/include/CL/cl_intel.h > +++ b/include/CL/cl_intel.h > @@ -133,6 +133,30 @@ typedef CL_API_ENTRY cl_int (CL_API_CALL > *clGetMemObjectFdIntel_fn)( > cl_mem /* Memory Obejct */, > int* /* returned fd */); > > +/* Create memory object from external buffer object by fd */ typedef > +struct _cl_mem_object_desc { > + int fd; > + cl_mem_object_type type; > + int size; > + struct { > + cl_image_format fmt; > + uint32_t offset; > + uint32_t width; > + uint32_t height; > + uint32_t row_pitch; > + } image_info; > +} cl_mem_object_desc; > + > +extern CL_API_ENTRY cl_mem CL_API_CALL > +clCreateMemObjectFromFdIntel(cl_context /* context */, > + const cl_mem_object_desc * /* desc */, > + cl_int * /* errcode_ret */); > + > +typedef CL_API_ENTRY cl_mem (CL_API_CALL *clCreateMemObjectFromFdIntel_fn)( > + cl_context /* context */, > + const cl_mem_object_desc * /* desc */, > + cl_int * /* errcode_ret */); > + > #ifdef __cplusplus > } > #endif > diff --git a/src/cl_api.c b/src/cl_api.c index dbbcbb0..b879a12 100644 > --- a/src/cl_api.c > +++ b/src/cl_api.c > @@ -3187,6 +3187,7 @@ internal_clGetExtensionFunctionAddress(const char > *func_name) > EXTFUNC(clCreateBufferFromLibvaIntel) > EXTFUNC(clCreateImageFromLibvaIntel) > EXTFUNC(clGetMemObjectFdIntel) > + EXTFUNC(clCreateMemObjectFromFdIntel) > return NULL; > } > > @@ -3355,3 +3356,44 @@ clGetMemObjectFdIntel(cl_context context, > error: > return err; > } > + > +cl_mem > +clCreateMemObjectFromFdIntel(cl_context context, > + const cl_mem_object_desc* desc, > + cl_int *errorcode_ret) { > + cl_mem mem = NULL; > + cl_int err = CL_SUCCESS; > + CHECK_CONTEXT (context); > + > + if (!desc) { > + err = CL_INVALID_VALUE; > + goto error; > + } > + > + //create buffer object > + if(desc->type == CL_MEM_OBJECT_BUFFER){ > + mem = cl_mem_new_buffer_from_fd(context, desc->fd, > +desc->size,&err); > + } > + /* Create image object from fd. > + * We just support creating CL_MEM_OBJECT_IMAGE2D image object now. > + * Other image type will be supported later if necessary. > + */ > + else if(desc->type == CL_MEM_OBJECT_IMAGE2D){ > + mem = cl_mem_new_image_from_fd(context, > + desc->fd, desc->size, > + desc->image_info.offset, > + desc->image_info.width, > desc->image_info.height, > + desc->image_info.fmt, > +desc->image_info.row_pitch, &err); > + } > + else{ > + err = CL_INVALID_ARG_VALUE; > + goto error; > + } > + > +error: > + if (errorcode_ret) > + *errorcode_ret = err; > + return mem; > +} > diff --git a/src/cl_driver.h b/src/cl_driver.h index 1ab4dff..369c24c > 100644 > --- a/src/cl_driver.h > +++ b/src/cl_driver.h > @@ -381,6 +381,12 @@ extern cl_buffer_get_fd_cb *cl_buffer_get_fd; > typedef int (cl_buffer_get_tiling_align_cb)(cl_context ctx, uint32_t > tiling_mode, uint32_t dim); > extern cl_buffer_get_tiling_align_cb *cl_buffer_get_tiling_align; > > +typedef cl_buffer (cl_buffer_get_buffer_from_fd_cb)(cl_context ctx, > +int fd, int size); extern cl_buffer_get_buffer_from_fd_cb > +*cl_buffer_get_buffer_from_fd; > + > +typedef cl_buffer (cl_buffer_get_image_from_fd_cb)(cl_context ctx, > +int fd, int size, struct _cl_mem_image *image); extern > +cl_buffer_get_image_from_fd_cb *cl_buffer_get_image_from_fd; > + > /* Get the device id */ > typedef int (cl_driver_get_device_id_cb)(void); > extern cl_driver_get_device_id_cb *cl_driver_get_device_id; diff > --git a/src/cl_driver_defs.c b/src/cl_driver_defs.c index > b77acdc..d25fd5d 100644 > --- a/src/cl_driver_defs.c > +++ b/src/cl_driver_defs.c > @@ -53,6 +53,8 @@ LOCAL cl_buffer_get_buffer_from_libva_cb > *cl_buffer_get_buffer_from_libva = NULL > LOCAL cl_buffer_get_image_from_libva_cb *cl_buffer_get_image_from_libva = > NULL; > LOCAL cl_buffer_get_fd_cb *cl_buffer_get_fd = NULL; > LOCAL cl_buffer_get_tiling_align_cb *cl_buffer_get_tiling_align = > NULL; > +LOCAL cl_buffer_get_buffer_from_fd_cb *cl_buffer_get_buffer_from_fd = > +NULL; LOCAL cl_buffer_get_image_from_fd_cb > +*cl_buffer_get_image_from_fd = NULL; > > /* cl_khr_gl_sharing */ > LOCAL cl_gl_acquire_texture_cb *cl_gl_acquire_texture = NULL; diff > --git a/src/cl_mem.c b/src/cl_mem.c index b5671bd..420bb78 100644 > --- a/src/cl_mem.c > +++ b/src/cl_mem.c > @@ -2097,3 +2097,91 @@ cl_mem_get_fd(cl_mem mem, > err = CL_INVALID_OPERATION; > return err; > } > + > +LOCAL cl_mem cl_mem_new_buffer_from_fd(cl_context ctx, > + int fd, > + int buffer_sz, > + cl_int* errcode) { > + cl_int err = CL_SUCCESS; > + cl_mem mem = NULL; > + > + mem = cl_mem_allocate(CL_MEM_BUFFER_TYPE, ctx, 0, 0, CL_FALSE, > + NULL,&err); if (mem == NULL || err != CL_SUCCESS) > + goto error; > + > + mem->bo = cl_buffer_get_buffer_from_fd(ctx, fd, buffer_sz); if > + (mem->bo == NULL) { > + err = CL_MEM_OBJECT_ALLOCATION_FAILURE; > + goto error; > + } > + mem->size = buffer_sz; > + > +exit: > + if (errcode) > + *errcode = err; > + return mem; > + > +error: > + cl_mem_delete(mem); > + mem = NULL; > + goto exit; > +} > + > +LOCAL cl_mem cl_mem_new_image_from_fd(cl_context ctx, > + int fd, int image_sz, > + size_t offset, > + size_t width, size_t height, > + cl_image_format fmt, > + size_t row_pitch, > + cl_int *errcode) { > + cl_int err = CL_SUCCESS; > + cl_mem mem = NULL; > + struct _cl_mem_image *image = NULL; > + uint32_t intel_fmt, bpp; > + > + /* Get the size of each pixel */ > + if (UNLIKELY((err = cl_image_byte_per_pixel(&fmt,&bpp)) != CL_SUCCESS)) > + goto error; > + > + intel_fmt = cl_image_get_intel_format(&fmt); if (intel_fmt == > + INTEL_UNSUPPORTED_FORMAT) { > + err = CL_IMAGE_FORMAT_NOT_SUPPORTED; > + goto error; > + } > + > + mem = cl_mem_allocate(CL_MEM_IMAGE_TYPE, ctx, 0, 0, 0, NULL,&err); > + if (mem == NULL || err != CL_SUCCESS) { > + err = CL_OUT_OF_HOST_MEMORY; > + goto error; > + } > + > + image = cl_mem_image(mem); > + > + mem->bo = cl_buffer_get_image_from_fd(ctx, fd, image_sz, image); > + > + image->w = width; > + image->h = height; > + image->image_type = CL_MEM_OBJECT_IMAGE2D; image->depth = 2; > + image->fmt = fmt; image->intel_fmt = intel_fmt; image->bpp = bpp; > + image->row_pitch = row_pitch; image->slice_pitch = 0; // NOTE: > + tiling of image is set in cl_buffer_get_image_from_fd(). > + image->tile_x = 0; > + image->tile_y = 0; > + image->offset = offset; > + > +exit: > + if (errcode) > + *errcode = err; > + return mem; > + > +error: > + cl_mem_delete(mem); > + mem = NULL; > + goto exit; > +} > diff --git a/src/cl_mem.h b/src/cl_mem.h index e027f15..739f107 100644 > --- a/src/cl_mem.h > +++ b/src/cl_mem.h > @@ -293,8 +293,21 @@ extern cl_mem cl_mem_new_libva_image(cl_context ctx, > cl_image_format fmt, > size_t row_pitch, > cl_int *errcode); > + > extern cl_int cl_mem_get_fd(cl_mem mem, int* fd); > > +extern cl_mem cl_mem_new_buffer_from_fd(cl_context ctx, > + int fd, > + int buffer_sz, > + cl_int* errcode); > + > +extern cl_mem cl_mem_new_image_from_fd(cl_context ctx, > + int fd, int image_sz, > + size_t offset, > + size_t width, size_t height, > + cl_image_format fmt, > + size_t row_pitch, > + cl_int *errcode); > > #endif /* __CL_MEM_H__ */ > > diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c index > 507c910..d19f721 100644 > --- a/src/intel/intel_driver.c > +++ b/src/intel/intel_driver.c > @@ -369,7 +369,7 @@ intel_driver_unlock_hardware(intel_driver_t *driver) > } > > LOCAL dri_bo* > -intel_driver_share_buffer(intel_driver_t *driver, const char *sname, > uint32_t name) > +intel_driver_share_buffer_from_name(intel_driver_t *driver, const > +char *sname, uint32_t name) > { > dri_bo *bo = intel_bo_gem_create_from_name(driver->bufmgr, > sname, @@ -381,6 > +381,19 @@ intel_driver_share_buffer(intel_driver_t *driver, const > +char *sname, uint32_t na > return bo; > } > > +LOCAL dri_bo* > +intel_driver_share_buffer_from_fd(intel_driver_t *driver, int fd, int > +size) { > + dri_bo *bo = drm_intel_bo_gem_create_from_prime(driver->bufmgr, > + fd, > + size); > + if (bo == NULL) { > + fprintf(stderr, "drm_intel_bo_gem_create_from_prime create bo(size %d) > from fd %d failed: %s\n", fd, size, strerror(errno)); > + return NULL; > + } > + return bo; > +} > + > LOCAL uint32_t > intel_driver_shared_name(intel_driver_t *driver, dri_bo *bo) > { > @@ -695,7 +708,7 @@ cl_buffer intel_share_buffer_from_libva(cl_context ctx, > { > drm_intel_bo *intel_bo; > > - intel_bo = intel_driver_share_buffer((intel_driver_t *)ctx->drv, > "shared from libva", bo_name); > + intel_bo = intel_driver_share_buffer_from_name((intel_driver_t > + *)ctx->drv, "shared from libva", bo_name); > > if (intel_bo == NULL) > return NULL; > @@ -713,7 +726,37 @@ cl_buffer intel_share_image_from_libva(cl_context ctx, > drm_intel_bo *intel_bo; > uint32_t intel_tiling, intel_swizzle_mode; > > - intel_bo = intel_driver_share_buffer((intel_driver_t *)ctx->drv, > "shared from libva", bo_name); > + intel_bo = intel_driver_share_buffer_from_name((intel_driver_t > + *)ctx->drv, "shared from libva", bo_name); > + > + > + drm_intel_bo_get_tiling(intel_bo,&intel_tiling,&intel_swizzle_mode); > + image->tiling = get_cl_tiling(intel_tiling); > + > + return (cl_buffer)intel_bo; > +} > + > +cl_buffer intel_share_buffer_from_fd(cl_context ctx, > + int fd, > + int buffer_size) { > + drm_intel_bo *intel_bo; > + > + intel_bo = intel_driver_share_buffer_from_fd((intel_driver_t > + *)ctx->drv, fd, buffer_size); > + > + if (intel_bo == NULL) > + return NULL; > + > + return (cl_buffer)intel_bo; > +} > + > +cl_buffer intel_share_image_from_fd(cl_context ctx, > + int fd, > + int image_size, > + struct _cl_mem_image *image) { > + drm_intel_bo *intel_bo; > + uint32_t intel_tiling, intel_swizzle_mode; > + > + intel_bo = intel_driver_share_buffer_from_fd((intel_driver_t > + *)ctx->drv, fd, image_size); > > drm_intel_bo_get_tiling(intel_bo,&intel_tiling,&intel_swizzle_mode); > image->tiling = get_cl_tiling(intel_tiling); @@ -870,5 +913,7 @@ > intel_setup_callbacks(void) > cl_buffer_wait_rendering = (cl_buffer_wait_rendering_cb *) > drm_intel_bo_wait_rendering; > cl_buffer_get_fd = (cl_buffer_get_fd_cb *) > drm_intel_bo_gem_export_to_prime; > cl_buffer_get_tiling_align = (cl_buffer_get_tiling_align_cb > *)intel_buffer_get_tiling_align; > + cl_buffer_get_buffer_from_fd = (cl_buffer_get_buffer_from_fd_cb *) > + intel_share_buffer_from_fd; cl_buffer_get_image_from_fd = > + (cl_buffer_get_image_from_fd_cb *) intel_share_image_from_fd; > intel_set_gpgpu_callbacks(intel_get_device_id()); > } _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
