[igt-dev] [PATCH i-g-t 3/4] lib/i915_blt: Extract init functions for blt_copy_object

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Tue Dec 20 11:51:56 UTC 2022


On Mon, Dec 19, 2022 at 12:49:10PM +0100, Karolina Stolarek wrote:
> gem_ccs and gem_lmem_swapping share a couple of functions. Extract them
> to i915_blt so they are accessible for both tests. Delete local
> definitions.
> 
> Signed-off-by: Karolina Stolarek <karolina.stolarek at intel.com>
> ---
> Karolina's comment: I'm not convinced it's the best place for them.
> 					These functions are more specific to the tests
> 					than lib functions. So, I'd appreciate
> 					suggestions on where I can move them to.
> 
>  lib/i915/i915_blt.c            | 81 ++++++++++++++++++++++++++++++++++
>  lib/i915/i915_blt.h            | 22 +++++++++
>  tests/i915/gem_ccs.c           | 81 ----------------------------------
>  tests/i915/gem_lmem_swapping.c | 36 ---------------
>  4 files changed, 103 insertions(+), 117 deletions(-)
> 
> diff --git a/lib/i915/i915_blt.c b/lib/i915/i915_blt.c
> index 2513dfc0..0f9f7265 100644
> --- a/lib/i915/i915_blt.c
> +++ b/lib/i915/i915_blt.c
> @@ -1053,6 +1053,87 @@ int blt_fast_copy(int i915,
>  	return ret;
>  }
>  
> +void set_geom(struct blt_copy_object *obj, uint32_t pitch,
> +	      int16_t x1, int16_t y1, int16_t x2, int16_t y2,
> +	      uint16_t x_offset, uint16_t y_offset)
> +{
> +	obj->pitch = pitch;
> +	obj->x1 = x1;
> +	obj->y1 = y1;
> +	obj->x2 = x2;
> +	obj->y2 = y2;
> +	obj->x_offset = x_offset;
> +	obj->y_offset = y_offset;
> +}
> +
> +void set_batch(struct blt_copy_batch *batch,
> +	       uint32_t handle, uint64_t size, uint32_t region)
> +{
> +	batch->handle = handle;
> +	batch->size = size;
> +	batch->region = region;
> +}
> +
> +struct blt_copy_object *
> +create_object(int i915, uint32_t region,
> +	      uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs,
> +	      enum tiling_type tiling,
> +	      enum blt_compression compression,
> +	      enum blt_compression_type compression_type,
> +	      bool create_mapping)
> +{
> +	struct blt_copy_object *obj;
> +	uint64_t size = width * height * bpp / 8;
> +	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
> +	uint32_t handle;
> +
> +	obj = calloc(1, sizeof(*obj));
> +
> +	obj->size = size;
> +	igt_assert(__gem_create_in_memory_regions(i915, &handle,
> +						  &size, region) == 0);
> +
> +	set_object(obj, handle, size, region, mocs, tiling,
> +		   compression, compression_type);
> +	set_geom(obj, stride, 0, 0, width, height, 0, 0);
> +
> +	if (create_mapping)
> +		obj->ptr = gem_mmap__device_coherent(i915, handle, 0, size,
> +						     PROT_READ | PROT_WRITE);
> +
> +	return obj;
> +}
> +
> +void destroy_object(int i915, struct blt_copy_object *obj)
> +{
> +	if (obj->ptr)
> +		munmap(obj->ptr, obj->size);
> +
> +	gem_close(i915, obj->handle);
> +	free(obj);
> +}
> +
> +void set_object(struct blt_copy_object *obj,
> +		uint32_t handle, uint64_t size, uint32_t region,
> +		uint8_t mocs, enum tiling_type tiling,
> +		enum blt_compression compression,
> +		enum blt_compression_type compression_type)
> +{
> +	obj->handle = handle;
> +	obj->size = size;
> +	obj->region = region;
> +	obj->mocs = mocs;
> +	obj->tiling = tiling;
> +	obj->compression = compression;
> +	obj->compression_type = compression_type;
> +}
> +
> +void set_blt_object(struct blt_copy_object *obj,
> +		    const struct blt_copy_object *orig)
> +{
> +	memcpy(obj, orig, sizeof(*obj));
> +}
> +

I would add some prefix, set_geom() to blt_set_geom(), etc. to minimize
risk of name collision.

But yes, I agree extracting this from test to library makes sense.

--
Zbigniew

>  /**
>   * blt_surface_fill_rect:
>   * @i915: drm fd
> diff --git a/lib/i915/i915_blt.h b/lib/i915/i915_blt.h
> index f1cf5408..1ef459c8 100644
> --- a/lib/i915/i915_blt.h
> +++ b/lib/i915/i915_blt.h
> @@ -198,6 +198,28 @@ int blt_fast_copy(int i915,
>  		  uint64_t ahnd,
>  		  const struct blt_copy_data *blt);
>  
> +void set_geom(struct blt_copy_object *obj, uint32_t pitch,
> +	      int16_t x1, int16_t y1, int16_t x2, int16_t y2,
> +	      uint16_t x_offset, uint16_t y_offset);
> +void set_batch(struct blt_copy_batch *batch,
> +	       uint32_t handle, uint64_t size, uint32_t region);
> +
> +struct blt_copy_object *
> +create_object(int i915, uint32_t region,
> +	      uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs,
> +	      enum tiling_type tiling,
> +	      enum blt_compression compression,
> +	      enum blt_compression_type compression_type,
> +	      bool create_mapping);
> +void destroy_object(int i915, struct blt_copy_object *obj);
> +void set_object(struct blt_copy_object *obj,
> +		uint32_t handle, uint64_t size, uint32_t region,
> +		uint8_t mocs, enum tiling_type tiling,
> +		enum blt_compression compression,
> +		enum blt_compression_type compression_type);
> +void set_blt_object(struct blt_copy_object *obj,
> +		    const struct blt_copy_object *orig);
> +
>  void blt_surface_info(const char *info,
>  		      const struct blt_copy_object *obj);
>  void blt_surface_fill_rect(int i915, const struct blt_copy_object *obj,
> diff --git a/tests/i915/gem_ccs.c b/tests/i915/gem_ccs.c
> index 4c137c94..30ee60fd 100644
> --- a/tests/i915/gem_ccs.c
> +++ b/tests/i915/gem_ccs.c
> @@ -44,42 +44,6 @@ struct test_config {
>  	bool suspend_resume;
>  };
>  
> -static void set_object(struct blt_copy_object *obj,
> -		       uint32_t handle, uint64_t size, uint32_t region,
> -		       uint8_t mocs, enum tiling_type tiling,
> -		       enum blt_compression compression,
> -		       enum blt_compression_type compression_type)
> -{
> -	obj->handle = handle;
> -	obj->size = size;
> -	obj->region = region;
> -	obj->mocs = mocs;
> -	obj->tiling = tiling;
> -	obj->compression = compression;
> -	obj->compression_type = compression_type;
> -}
> -
> -static void set_geom(struct blt_copy_object *obj, uint32_t pitch,
> -		     int16_t x1, int16_t y1, int16_t x2, int16_t y2,
> -		     uint16_t x_offset, uint16_t y_offset)
> -{
> -	obj->pitch = pitch;
> -	obj->x1 = x1;
> -	obj->y1 = y1;
> -	obj->x2 = x2;
> -	obj->y2 = y2;
> -	obj->x_offset = x_offset;
> -	obj->y_offset = y_offset;
> -}
> -
> -static void set_batch(struct blt_copy_batch *batch,
> -		      uint32_t handle, uint64_t size, uint32_t region)
> -{
> -	batch->handle = handle;
> -	batch->size = size;
> -	batch->region = region;
> -}
> -
>  static void set_object_ext(struct blt_block_copy_object_ext *obj,
>  			   uint8_t compression_format,
>  			   uint16_t surface_width, uint16_t surface_height,
> @@ -105,51 +69,6 @@ static void set_surf_object(struct blt_ctrl_surf_copy_object *obj,
>  	obj->access_type = access_type;
>  }
>  
> -static struct blt_copy_object *
> -create_object(int i915, uint32_t region,
> -	      uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs,
> -	      enum tiling_type tiling,
> -	      enum blt_compression compression,
> -	      enum blt_compression_type compression_type,
> -	      bool create_mapping)
> -{
> -	struct blt_copy_object *obj;
> -	uint64_t size = width * height * bpp / 8;
> -	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
> -	uint32_t handle;
> -
> -	obj = calloc(1, sizeof(*obj));
> -
> -	obj->size = size;
> -	igt_assert(__gem_create_in_memory_regions(i915, &handle,
> -						  &size, region) == 0);
> -
> -	set_object(obj, handle, size, region, mocs, tiling,
> -		   compression, compression_type);
> -	set_geom(obj, stride, 0, 0, width, height, 0, 0);
> -
> -	if (create_mapping)
> -		obj->ptr = gem_mmap__device_coherent(i915, handle, 0, size,
> -						     PROT_READ | PROT_WRITE);
> -
> -	return obj;
> -}
> -
> -static void destroy_object(int i915, struct blt_copy_object *obj)
> -{
> -	if (obj->ptr)
> -		munmap(obj->ptr, obj->size);
> -
> -	gem_close(i915, obj->handle);
> -	free(obj);
> -}
> -
> -static void set_blt_object(struct blt_copy_object *obj,
> -			   const struct blt_copy_object *orig)
> -{
> -	memcpy(obj, orig, sizeof(*obj));
> -}
> -
>  #define PRINT_SURFACE_INFO(name, obj) do { \
>  	if (param.print_surface_info) \
>  		blt_surface_info((name), (obj)); } while (0)
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 8cff35d5..746fbf80 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -76,42 +76,6 @@ struct object {
>  	struct blt_copy_object *blt_obj;
>  };
>  
> -static void set_object(struct blt_copy_object *obj,
> -		       uint32_t handle, uint64_t size, uint32_t region,
> -		       uint8_t mocs, enum tiling_type tiling,
> -		       enum blt_compression compression,
> -		       enum blt_compression_type compression_type)
> -{
> -	obj->handle = handle;
> -	obj->size = size;
> -	obj->region = region;
> -	obj->mocs = mocs;
> -	obj->tiling = tiling;
> -	obj->compression = compression;
> -	obj->compression_type = compression_type;
> -}
> -
> -static void set_geom(struct blt_copy_object *obj, uint32_t pitch,
> -		     int16_t x1, int16_t y1, int16_t x2, int16_t y2,
> -		     uint16_t x_offset, uint16_t y_offset)
> -{
> -	obj->pitch = pitch;
> -	obj->x1 = x1;
> -	obj->y1 = y1;
> -	obj->x2 = x2;
> -	obj->y2 = y2;
> -	obj->x_offset = x_offset;
> -	obj->y_offset = y_offset;
> -}
> -
> -static void set_batch(struct blt_copy_batch *batch,
> -		      uint32_t handle, uint64_t size, uint32_t region)
> -{
> -	batch->handle = handle;
> -	batch->size = size;
> -	batch->region = region;
> -}
> -
>  static void set_object_ext(struct blt_block_copy_object_ext *obj,
>  			   uint8_t compression_format,
>  			   uint16_t surface_width, uint16_t surface_height,
> -- 
> 2.25.1
> 


More information about the igt-dev mailing list