[Beignet] [PATCH] re-enable userptr with fix: CPU access after GPU finishes the rendering

[Zhigang Gong]

This patch LGTM, just pushed, thanks.

On Tue, Nov 18, 2014 at 04:28:29PM +0800, Guo Yejun wrote:
> 1. the wait logic is integrated into function cl_mem_map/unmap_auto
> 2. use cl_mem_map/unmap_auto for userptr inside clEnqueueRead/WriteBuffer
> 3. do not use cl_buffer_subdata for userptr, use cl_mem_map/memcpy instead
> 
> Signed-off-by: Guo Yejun <yejun.guo at intel.com>
> ---
>  CMakeLists.txt   | 13 ++++++-------
>  src/cl_enqueue.c | 32 +++++++++++++++++++++++++++-----
>  src/cl_mem.c     | 11 ++++++++---
>  3 files changed, 41 insertions(+), 15 deletions(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 3c68187..b14de96 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -121,13 +121,12 @@ IF(DRM_INTEL_FOUND)
>    INCLUDE_DIRECTORIES(${DRM_INTEL_INCLUDE_DIRS})
>    MESSAGE(STATUS "Looking for DRM Intel - found at ${DRM_INTEL_PREFIX} ${DRM_INTEL_VERSION}")
>    #userptr support starts from 2.4.57, but 2.4.58 is the actual stable release
> -  #FIXME userptr has randome fail for some cases, need further investigating.
> -  #IF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> -  #  MESSAGE(STATUS "Enable userptr support")
> -  #  SET(DRM_INTEL_USERPTR "enable")
> -  #ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> -  #  MESSAGE(STATUS "Disable userptr support")
> -  #ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> +  IF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> +    MESSAGE(STATUS "Enable userptr support")
> +    SET(DRM_INTEL_USERPTR "enable")
> +  ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> +    MESSAGE(STATUS "Disable userptr support")
> +  ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
>  ELSE(DRM_INTEL_FOUND)
>    MESSAGE(FATAL_ERROR "Looking for DRM Intel (>= 2.4.52) - not found")
>  ENDIF(DRM_INTEL_FOUND)
> diff --git a/src/cl_enqueue.c b/src/cl_enqueue.c
> index 5798e20..d51592c 100644
> --- a/src/cl_enqueue.c
> +++ b/src/cl_enqueue.c
> @@ -40,8 +40,15 @@ cl_int cl_enqueue_read_buffer(enqueue_data* data)
>      if (cl_buffer_get_subdata(mem->bo, data->offset + buffer->sub_offset,
>  			       data->size, data->ptr) != 0)
>        err = CL_MAP_FAILURE;
> -  } else
> -    memcpy(data->ptr, (char*)mem->host_ptr + data->offset + buffer->sub_offset, data->size);
> +  } else {
> +    void* src_ptr = cl_mem_map_auto(mem, 0);
> +    if (src_ptr == NULL)
> +      err = CL_MAP_FAILURE;
> +    else {
> +      memcpy(data->ptr, (char*)src_ptr + data->offset + buffer->sub_offset, data->size);
> +      cl_mem_unmap_auto(mem);
> +    }
> +  }
>    return err;
>  }
>  
> @@ -99,13 +106,28 @@ error:
>  
>  cl_int cl_enqueue_write_buffer(enqueue_data *data)
>  {
> +  cl_int err = CL_SUCCESS;
>    cl_mem mem = data->mem_obj;
>    assert(mem->type == CL_MEM_BUFFER_TYPE ||
>           mem->type == CL_MEM_SUBBUFFER_TYPE);
>    struct _cl_mem_buffer* buffer = (struct _cl_mem_buffer*)mem;
>  
> -  return cl_buffer_subdata(mem->bo, data->offset + buffer->sub_offset,
> -			   data->size, data->const_ptr);
> +  if (mem->is_userptr) {
> +    void* dst_ptr = cl_mem_map_auto(mem, 1);
> +    if (dst_ptr == NULL)
> +      err = CL_MAP_FAILURE;
> +    else {
> +      memcpy((char*)dst_ptr + data->offset + buffer->sub_offset, data->const_ptr, data->size);
> +      cl_mem_unmap_auto(mem);
> +    }
> +  }
> +  else {
> +    if (cl_buffer_subdata(mem->bo, data->offset + buffer->sub_offset,
> +			   data->size, data->const_ptr) != 0)
> +      err = CL_MAP_FAILURE;
> +  }
> +
> +  return err;
>  }
>  
>  cl_int cl_enqueue_write_buffer_rect(enqueue_data *data)
> @@ -240,7 +262,7 @@ cl_int cl_enqueue_map_buffer(enqueue_data *data)
>    struct _cl_mem_buffer* buffer = (struct _cl_mem_buffer*)mem;
>  
>    if (mem->is_userptr)
> -    ptr = mem->host_ptr;
> +    ptr = cl_mem_map_auto(mem, data->write_map ? 1 : 0);
>    else {
>      if(data->unsync_map == 1)
>        //because using unsync map in clEnqueueMapBuffer, so force use map_gtt here
> diff --git a/src/cl_mem.c b/src/cl_mem.c
> index 0fbd304..65f299f 100644
> --- a/src/cl_mem.c
> +++ b/src/cl_mem.c
> @@ -1818,8 +1818,13 @@ cl_mem_map_auto(cl_mem mem, int write)
>  {
>    if (IS_IMAGE(mem) && cl_mem_image(mem)->tiling != CL_NO_TILE)
>      return cl_mem_map_gtt(mem);
> -  else
> -    return cl_mem_map(mem, write);
> +  else {
> +    if (mem->is_userptr) {
> +      cl_buffer_wait_rendering(mem->bo);
> +      return mem->host_ptr;
> +    }else
> +      return cl_mem_map(mem, write);
> +  }
>  }
>  
>  LOCAL cl_int
> @@ -1829,7 +1834,7 @@ cl_mem_unmap_auto(cl_mem mem)
>      cl_buffer_unmap_gtt(mem->bo);
>      mem->mapped_gtt = 0;
>    }
> -  else
> +  else if (!mem->is_userptr)
>      cl_buffer_unmap(mem->bo);
>    return CL_SUCCESS;
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet