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

[Guo, Yejun]

No problem, I'll add into utest in another patch.

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Thursday, September 25, 2014 2:55 PM
To: Guo, Yejun
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] fix issue to create cl image from libva with non-zero offset

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