[Beignet] [PATCH] Fix clEnqueueMapImage with CL_MEM_USE_HOST_PTR bug.

Zhigang Gong zhigang.gong at linux.intel.com
Wed Jun 25 00:07:38 PDT 2014


LGTM, will push latter, thanks.

On Wed, Jun 25, 2014 at 11:23:24PM +0800, Yang Rong wrote:
> Should return host row pitch and host slice pitch.
> Also should copy back to image when unmap.
> 
> Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> ---
>  src/cl_api.c     | 54 ++++++++++++++++++++++++++++++------------------------
>  src/cl_enqueue.c | 37 +++++++++++++++++++++++++++++++------
>  src/cl_mem.h     |  2 ++
>  3 files changed, 63 insertions(+), 30 deletions(-)
> 
> diff --git a/src/cl_api.c b/src/cl_api.c
> index 9042243..a3c7432 100644
> --- a/src/cl_api.c
> +++ b/src/cl_api.c
> @@ -2533,7 +2533,9 @@ error:
>    return err;
>  }
>  
> -static cl_int _cl_map_mem(cl_mem mem, void *ptr, void **mem_ptr, size_t offset, size_t size)
> +static cl_int _cl_map_mem(cl_mem mem, void *ptr, void **mem_ptr,
> +                          size_t offset, size_t size,
> +                          const size_t *origin, const size_t *region)
>  {
>    cl_int slot = -1;
>    int err = CL_SUCCESS;
> @@ -2593,6 +2595,15 @@ static cl_int _cl_map_mem(cl_mem mem, void *ptr, void **mem_ptr, size_t offset,
>    mem->mapped_ptr[slot].ptr = *mem_ptr;
>    mem->mapped_ptr[slot].v_ptr = ptr;
>    mem->mapped_ptr[slot].size = size;
> +  if(origin) {
> +    assert(region);
> +    mem->mapped_ptr[slot].origin[0] = origin[0];
> +    mem->mapped_ptr[slot].origin[1] = origin[1];
> +    mem->mapped_ptr[slot].origin[2] = origin[2];
> +    mem->mapped_ptr[slot].region[0] = region[0];
> +    mem->mapped_ptr[slot].region[1] = region[1];
> +    mem->mapped_ptr[slot].region[2] = region[2];
> +  }
>    mem->map_ref++;
>  error:
>    if (err != CL_SUCCESS)
> @@ -2662,7 +2673,7 @@ clEnqueueMapBuffer(cl_command_queue  command_queue,
>        goto error;
>      }
>    }
> -  err = _cl_map_mem(buffer, ptr, &mem_ptr, offset, size);
> +  err = _cl_map_mem(buffer, ptr, &mem_ptr, offset, size, NULL, NULL);
>    if (err != CL_SUCCESS)
>      goto error;
>  
> @@ -2710,13 +2721,6 @@ clEnqueueMapImage(cl_command_queue   command_queue,
>      goto error;
>    }
>  
> -  if (image_slice_pitch)
> -    *image_slice_pitch = image->slice_pitch;
> -  if (image->image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> -    *image_row_pitch = image->slice_pitch;
> -  else
> -    *image_row_pitch = image->row_pitch;
> -
>    if ((map_flags & CL_MAP_READ &&
>         mem->flags & (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_NO_ACCESS)) ||
>        (map_flags & (CL_MAP_WRITE | CL_MAP_WRITE_INVALIDATE_REGION) &&
> @@ -2727,17 +2731,6 @@ clEnqueueMapImage(cl_command_queue   command_queue,
>    }
>  
>    size_t offset = image->bpp*origin[0] + image->row_pitch*origin[1] + image->slice_pitch*origin[2];
> -  size_t size;
> -  if(region[2] == 1) {
> -    if(region[1] == 1)
> -      size = image->bpp * region[0];
> -    else
> -      size = image->row_pitch * (region[1] - 1) + (image->bpp * (origin[0] + region[0]));
> -  } else {
> -    size = image->slice_pitch * (region[2] - 1);
> -    size += image->row_pitch * (origin[1] + region[1]);
> -    size += image->bpp * (origin[0] + region[0]);
> -  }
>  
>    TRY(cl_event_check_waitlist, num_events_in_wait_list, event_wait_list, event, mem->ctx);
>  
> @@ -2746,9 +2739,6 @@ clEnqueueMapImage(cl_command_queue   command_queue,
>    data->mem_obj     = mem;
>    data->origin[0]   = origin[0];  data->origin[1] = origin[1];  data->origin[2] = origin[2];
>    data->region[0]   = region[0];  data->region[1] = region[1];  data->region[2] = region[2];
> -  data->row_pitch   = *image_row_pitch;
> -  if (image_slice_pitch)
> -    data->slice_pitch = *image_slice_pitch;
>    data->ptr         = ptr;
>    data->offset      = offset;
>    data->unsync_map  = 1;
> @@ -2767,10 +2757,26 @@ clEnqueueMapImage(cl_command_queue   command_queue,
>        goto error;
>      }
>    }
> -  err = _cl_map_mem(mem, ptr, &mem_ptr, offset, size);
> +  err = _cl_map_mem(mem, ptr, &mem_ptr, offset, 0, origin, region);
>    if (err != CL_SUCCESS)
>      goto error;
>  
> +  if(mem->flags & CL_MEM_USE_HOST_PTR) {
> +    if (image_slice_pitch)
> +      *image_slice_pitch = image->host_slice_pitch;
> +    if (image->image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> +      *image_row_pitch = image->host_slice_pitch;
> +    else
> +      *image_row_pitch = image->host_row_pitch;
> +  } else {
> +    if (image_slice_pitch)
> +      *image_slice_pitch = image->slice_pitch;
> +    if (image->image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> +      *image_row_pitch = image->slice_pitch;
> +    else
> +      *image_row_pitch = image->row_pitch;
> +  }
> +
>  error:
>    if (errcode_ret)
>      *errcode_ret = err;
> diff --git a/src/cl_enqueue.c b/src/cl_enqueue.c
> index 52c824d..11f1680 100644
> --- a/src/cl_enqueue.c
> +++ b/src/cl_enqueue.c
> @@ -283,6 +283,7 @@ cl_int cl_enqueue_map_image(enqueue_data *data)
>    cl_int err = CL_SUCCESS;
>    cl_mem mem = data->mem_obj;
>    void *ptr = NULL;
> +  size_t row_pitch = 0;
>    CHECK_IMAGE(mem, image);
>  
>    if(data->unsync_map == 1)
> @@ -296,12 +297,16 @@ cl_int cl_enqueue_map_image(enqueue_data *data)
>      goto error;
>    }
>    data->ptr = ptr;
> +  if (image->image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> +    row_pitch = image->slice_pitch;
> +  else
> +    row_pitch = image->row_pitch;
>  
>    if(mem->flags & CL_MEM_USE_HOST_PTR) {
>      assert(mem->host_ptr);
>      cl_mem_copy_image_region(data->origin, data->region,
>                               mem->host_ptr, image->host_row_pitch, image->host_slice_pitch,
> -                             data->ptr, data->row_pitch, data->slice_pitch, image);
> +                             data->ptr, row_pitch, image->slice_pitch, image);
>    }
>  
>  error:
> @@ -311,11 +316,13 @@ error:
>  cl_int cl_enqueue_unmap_mem_object(enqueue_data *data)
>  {
>    cl_int err = CL_SUCCESS;
> -  int i;
> +  int i, j;
>    size_t mapped_size = 0;
> +  size_t origin[3], region[3];
>    void * v_ptr = NULL;
>    void * mapped_ptr = data->ptr;
>    cl_mem memobj = data->mem_obj;
> +  size_t row_pitch = 0;
>  
>    assert(memobj->mapped_ptr_sz >= memobj->map_ref);
>    INVALID_VALUE_IF(!mapped_ptr);
> @@ -324,6 +331,12 @@ cl_int cl_enqueue_unmap_mem_object(enqueue_data *data)
>        memobj->mapped_ptr[i].ptr = NULL;
>        mapped_size = memobj->mapped_ptr[i].size;
>        v_ptr = memobj->mapped_ptr[i].v_ptr;
> +      for(j=0; j<3; j++) {
> +        region[j] = memobj->mapped_ptr[i].region[j];
> +        origin[j] = memobj->mapped_ptr[i].origin[j];
> +        memobj->mapped_ptr[i].region[j] = 0;
> +        memobj->mapped_ptr[i].origin[j] = 0;
> +      }
>        memobj->mapped_ptr[i].size = 0;
>        memobj->mapped_ptr[i].v_ptr = NULL;
>        memobj->map_ref--;
> @@ -334,10 +347,22 @@ cl_int cl_enqueue_unmap_mem_object(enqueue_data *data)
>    INVALID_VALUE_IF(i == memobj->mapped_ptr_sz);
>  
>    if (memobj->flags & CL_MEM_USE_HOST_PTR) {
> -    assert(mapped_ptr >= memobj->host_ptr &&
> -      mapped_ptr + mapped_size <= memobj->host_ptr + memobj->size);
> -    /* Sync the data. */
> -    memcpy(v_ptr, mapped_ptr, mapped_size);
> +    if(memobj->type == CL_MEM_BUFFER_TYPE ||
> +       memobj->type == CL_MEM_SUBBUFFER_TYPE) {
> +      assert(mapped_ptr >= memobj->host_ptr &&
> +        mapped_ptr + mapped_size <= memobj->host_ptr + memobj->size);
> +      /* Sync the data. */
> +      memcpy(v_ptr, mapped_ptr, mapped_size);
> +    } else {
> +      CHECK_IMAGE(memobj, image);
> +      if (image->image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> +        row_pitch = image->slice_pitch;
> +      else
> +        row_pitch = image->row_pitch;
> +      cl_mem_copy_image_region(origin, region, v_ptr, row_pitch, image->slice_pitch,
> +                               memobj->host_ptr, image->host_row_pitch, image->host_slice_pitch,
> +                               image);
> +    }
>    } else {
>      assert(v_ptr == mapped_ptr);
>    }
> diff --git a/src/cl_mem.h b/src/cl_mem.h
> index a2fb851..7e9aa83 100644
> --- a/src/cl_mem.h
> +++ b/src/cl_mem.h
> @@ -55,6 +55,8 @@ typedef struct _cl_mapped_ptr {
>    void * ptr;
>    void * v_ptr;
>    size_t size;
> +  size_t origin[3];  /* mapped origin */
> +  size_t region[3];  /* mapped region */
>  }cl_mapped_ptr;
>  
>  typedef struct _cl_mem_dstr_cb {
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list