[Beignet] [PATCH] fix issue to create cl image from libva with non-zero offset

[Zhigang Gong]

Hi Yejun,

Could you add at least one test case for this type of feature?
Currently, it is the only one which is totally untested.

On Thu, Sep 25, 2014 at 07:45:35AM +0800, Guo Yejun wrote:
> Beignet accepts buffer object name to share data between libva,
> it is supposed to support to create cl image from the bo name
> with a non-zero offset, but it does not work at some platforms.
> 
> The driver calls intel_bo_gem_create_from_name to retrieve the
> dri_bo, and the offset of dri_bo is changed by the non-zero offset.
> At some platforms, the change of the offset has side effect when
> the kernel is executed again and so intel_bo_gem_create_from_name
> is called for the second time.
> 
> So, do not change the offset of dri_bo, but maintain the non-zero
> offset in cl_image, and use the non-zero offset until we fill the
> surface state.
> 
> Signed-off-by: Guo Yejun <yejun.guo at intel.com>
> ---
>  src/cl_driver.h          | 2 +-
>  src/cl_mem.c             | 2 +-
>  src/intel/intel_driver.c | 4 +---
>  src/intel/intel_gpgpu.c  | 8 ++++----
>  4 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/src/cl_driver.h b/src/cl_driver.h
> index 38b4830..05556f7 100644
> --- a/src/cl_driver.h
> +++ b/src/cl_driver.h
> @@ -286,7 +286,7 @@ extern cl_buffer_release_from_texture_cb *cl_buffer_release_from_texture;
>  typedef cl_buffer (cl_buffer_get_buffer_from_libva_cb)(cl_context ctx, unsigned int bo_name, size_t *sz);
>  extern cl_buffer_get_buffer_from_libva_cb *cl_buffer_get_buffer_from_libva;
>  
> -typedef cl_buffer (cl_buffer_get_image_from_libva_cb)(cl_context ctx, unsigned int bo_name, struct _cl_mem_image *image, unsigned int offset);
> +typedef cl_buffer (cl_buffer_get_image_from_libva_cb)(cl_context ctx, unsigned int bo_name, struct _cl_mem_image *image);
>  extern cl_buffer_get_image_from_libva_cb *cl_buffer_get_image_from_libva;
>  
>  /* Unref a buffer and destroy it if no more ref */
> diff --git a/src/cl_mem.c b/src/cl_mem.c
> index 077f1d7..5844217 100644
> --- a/src/cl_mem.c
> +++ b/src/cl_mem.c
> @@ -1890,7 +1890,7 @@ LOCAL cl_mem cl_mem_new_libva_image(cl_context ctx,
>  
>    image = cl_mem_image(mem);
>  
> -  mem->bo = cl_buffer_get_image_from_libva(ctx, bo_name, image, offset);
> +  mem->bo = cl_buffer_get_image_from_libva(ctx, bo_name, image);
>  
>    image->w = width;
>    image->h = height;
> diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c
> index 66f2bcf..93f1ebf 100644
> --- a/src/intel/intel_driver.c
> +++ b/src/intel/intel_driver.c
> @@ -658,15 +658,13 @@ cl_buffer intel_share_buffer_from_libva(cl_context ctx,
>  
>  cl_buffer intel_share_image_from_libva(cl_context ctx,
>                                         unsigned int bo_name,
> -                                       struct _cl_mem_image *image,
> -                                       unsigned int offset)
> +                                       struct _cl_mem_image *image)
>  {
>    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->offset += offset;
>    drm_intel_bo_get_tiling(intel_bo, &intel_tiling, &intel_swizzle_mode);
>    image->tiling = get_cl_tiling(intel_tiling);
>  
> diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c
> index c4b9156..02805dc 100644
> --- a/src/intel/intel_gpgpu.c
> +++ b/src/intel/intel_gpgpu.c
> @@ -860,7 +860,7 @@ intel_gpgpu_bind_image_gen7(intel_gpgpu_t *gpgpu,
>      ss->ss0.surface_array_spacing = 1;
>    }
>    ss->ss0.surface_format = format;
> -  ss->ss1.base_addr = obj_bo->offset;
> +  ss->ss1.base_addr = obj_bo->offset + obj_bo_offset;
>    ss->ss2.width = w - 1;
>  
>    ss->ss2.height = h - 1;
> @@ -877,7 +877,7 @@ intel_gpgpu_bind_image_gen7(intel_gpgpu_t *gpgpu,
>      ss->ss0.tile_walk = I965_TILEWALK_YMAJOR;
>    }
>    ss->ss0.render_cache_rw_mode = 1; /* XXX do we need to set it? */
> -  intel_gpgpu_set_buf_reloc_gen7(gpgpu, index, obj_bo, obj_bo_offset);
> +  intel_gpgpu_set_buf_reloc_gen7(gpgpu, index, obj_bo, obj_bo->offset + obj_bo_offset);
>  
>    assert(index < GEN_MAX_SURFACES);
>  }
> @@ -905,7 +905,7 @@ intel_gpgpu_bind_image_gen75(intel_gpgpu_t *gpgpu,
>      ss->ss0.surface_array_spacing = 1;
>    }
>    ss->ss0.surface_format = format;
> -  ss->ss1.base_addr = obj_bo->offset;
> +  ss->ss1.base_addr = obj_bo->offset + obj_bo_offset;
>    ss->ss2.width = w - 1;
>    ss->ss2.height = h - 1;
>    ss->ss3.depth = depth - 1;
> @@ -925,7 +925,7 @@ intel_gpgpu_bind_image_gen75(intel_gpgpu_t *gpgpu,
>      ss->ss0.tile_walk = I965_TILEWALK_YMAJOR;
>    }
>    ss->ss0.render_cache_rw_mode = 1; /* XXX do we need to set it? */
> -  intel_gpgpu_set_buf_reloc_gen7(gpgpu, index, obj_bo, obj_bo_offset);
> +  intel_gpgpu_set_buf_reloc_gen7(gpgpu, index, obj_bo, obj_bo->offset + obj_bo_offset);
>  
>    assert(index < GEN_MAX_SURFACES);
>  }
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet