[PATCH i-g-t 5/8] tests/intel: replace intel_buf_init_using_handle()

[Zbigniew Kempczyński]

On Fri, Feb 09, 2024 at 06:34:30PM +0000, Matthew Auld wrote:
> Require the original object size, if the caller created the object and
> convert all existing users over.
> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
>  lib/intel_bufops.c                  | 17 ++++++++++-------
>  lib/intel_bufops.h                  | 17 ++++++++++-------
>  tests/intel/gem_set_tiling_vs_blt.c |  7 ++++---
>  3 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
> index 914034af6..7f72f4762 100644
> --- a/lib/intel_bufops.c
> +++ b/lib/intel_bufops.c
> @@ -1011,7 +1011,7 @@ void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
>  }
>  
>  /**
> - * intel_buf_init_using_handle
> + * intel_buf_init_using_handle_and_size
>   * @bops: pointer to buf_ops
>   * @handle: BO handle created by the caller
>   * @buf: pointer to intel_buf structure to be filled
> @@ -1021,6 +1021,7 @@ void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
>   * @alignment: alignment of the stride for linear surfaces
>   * @tiling: surface tiling
>   * @compression: surface compression type
> + * @size: real bo size
>   *
>   * Function configures BO handle within intel_buf structure passed by the caller
>   * (with all its metadata - width, height, ...). Useful if BO was created
> @@ -1029,14 +1030,16 @@ void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
>   * Note: intel_buf_close() can be used because intel_buf is aware it is not
>   * buffer owner so it won't close it underneath.
>   */
> -void intel_buf_init_using_handle(struct buf_ops *bops,
> -				 uint32_t handle,
> -				 struct intel_buf *buf,
> -				 int width, int height, int bpp, int alignment,
> -				 uint32_t req_tiling, uint32_t compression)
> +void intel_buf_init_using_handle_and_size(struct buf_ops *bops,
> +					  uint32_t handle,
> +					  struct intel_buf *buf,
> +					  int width, int height, int bpp, int alignment,
> +					  uint32_t req_tiling, uint32_t compression,
> +					  uint64_t size)
>  {
> +	igt_assert(handle);

I would assert on size either.

>  	__intel_buf_init(bops, handle, buf, width, height, bpp, alignment,
> -			 req_tiling, compression, 0, 0, -1, DEFAULT_PAT_INDEX);
> +			 req_tiling, compression, size, 0, -1, DEFAULT_PAT_INDEX);
>  }
>  
>  /**
> diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
> index 54a143c5b..75ddf5d4c 100644
> --- a/lib/intel_bufops.h
> +++ b/lib/intel_bufops.h
> @@ -154,7 +154,8 @@ void intel_buf_init_using_handle(struct buf_ops *bops,
>  				 uint32_t handle,
>  				 struct intel_buf *buf,
>  				 int width, int height, int bpp, int alignment,
> -				 uint32_t req_tiling, uint32_t compression);
> +				 uint32_t req_tiling, uint32_t compression,
> +				 uint64_t size);

Hmm, is this intel_buf_init_using_handle() prototype or it should be
intel_buf_init_using_handle_and_size()? I'm not sure why this diff
looks so.

>  void intel_buf_init_full(struct buf_ops *bops,
>  			 uint32_t handle,
>  			 struct intel_buf *buf,
> @@ -172,12 +173,14 @@ struct intel_buf *intel_buf_create(struct buf_ops *bops,
>  				   int bpp, int alignment,
>  				   uint32_t req_tiling, uint32_t compression);
>  
> -struct intel_buf *intel_buf_create_using_handle(struct buf_ops *bops,
> -						uint32_t handle,
> -						int width, int height,
> -						int bpp, int alignment,
> -						uint32_t req_tiling,
> -						uint32_t compression);
> +void intel_buf_init_using_handle_and_size(struct buf_ops *bops,
> +					  uint32_t handle,
> +					  struct intel_buf *buf,
> +					  int width, int height,
> +					  int bpp, int alignment,
> +					  uint32_t req_tiling,
> +					  uint32_t compression,
> +					  uint64_t size);
>  
>  struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
>  							 uint32_t handle,
> diff --git a/tests/intel/gem_set_tiling_vs_blt.c b/tests/intel/gem_set_tiling_vs_blt.c
> index e9d5e3c63..9077e0f3f 100644
> --- a/tests/intel/gem_set_tiling_vs_blt.c
> +++ b/tests/intel/gem_set_tiling_vs_blt.c
> @@ -138,9 +138,10 @@ static void do_test(struct buf_ops *bops, uint32_t tiling, unsigned stride,
>  	gem_munmap(ptr, TEST_SIZE);
>  
>  	/* Reuse previously aligned in the gtt object */
> -	intel_buf_init_using_handle(bops, test_buf->handle, test_buf,
> -				    TEST_WIDTH(stride), TEST_HEIGHT(stride), 32,
> -				    0, tiling, I915_COMPRESSION_NONE);
> +	intel_buf_init_using_handle_and_size(bops, test_buf->handle, test_buf,
> +					     TEST_WIDTH(stride), TEST_HEIGHT(stride), 32,
> +					     0, tiling, I915_COMPRESSION_NONE,
> +					     test_buf->bo_size);
>  	igt_assert_eq_u32(intel_buf_size(test_buf), TEST_SIZE);
>  	intel_buf_set_ownership(test_buf, true);
>  	intel_bb_add_intel_buf(ibb, test_buf, false);
> -- 
> 2.43.0
> 

Rest is correct imo:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

--
Zbigniew