Re: [PATCH i-g-t v4 5/6] tests/gem_gpgpu_fill: Add width/height/x/y command line args

["Grzegorzek, Dominik" <dominik.grzegorzek@intel.com>]

On Thu, 2024-11-21 at 13:33 +0100, Zbigniew Kempczyński wrote:
> I've noticed shaders/pipelines have limitation to work on 16B
> boundaries (due to SIMD16). So to play with different surface sizes
> and offsets add W/H/X/Y switches.
> 
> There's no problem at all not all sizes/offsets are supported as we
> would like to have, we use gpgpu fill to verify compute workload so
> if we won't notice gpu hang that's fine.

This sentence sounds a bit unclear to me. May I ask you to rephrase that
that paragraph in both xe and gem patches.

Anyway it is:
Reviewed-by: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> ---
>  tests/intel/gem_gpgpu_fill.c | 62 ++++++++++++++++++++++++++----------
>  1 file changed, 46 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/intel/gem_gpgpu_fill.c b/tests/intel/gem_gpgpu_fill.c
> index 36c60e75f0..ec34e95844 100644
> --- a/tests/intel/gem_gpgpu_fill.c
> +++ b/tests/intel/gem_gpgpu_fill.c
> @@ -68,6 +68,10 @@
>  #define COLOR_4C	0x4c
>  
>  static bool dump_surface;
> +static uint32_t surfwidth = WIDTH;
> +static uint32_t surfheight = HEIGHT;
> +static uint32_t start_x;
> +static uint32_t start_y;
>  
>  typedef struct {
>  	int drm_fd;
> @@ -103,17 +107,20 @@ create_buf(data_t *data, int width, int height, uint8_t color, uint32_t region)
>  	return buf;
>  }
>  
> -static void buf_check(uint8_t *ptr, int x, int y, uint8_t color)
> +static void buf_check(uint8_t *ptr, int width, int x, int y, uint8_t color)
>  {
>  	uint8_t val;
>  
> -	val = ptr[y * WIDTH + x];
> +	val = ptr[y * width + x];
>  	igt_assert_f(val == color,
>  		     "Expected 0x%02x, found 0x%02x at (%d,%d)\n",
>  		     color, val, x, y);
>  }
>  
> -static void gpgpu_fill(data_t *data, igt_fillfunc_t fill, uint32_t region)
> +static void gpgpu_fill(data_t *data, igt_fillfunc_t fill, uint32_t region,
> +		       uint32_t surf_width, uint32_t surf_height,
> +		       uint32_t x, uint32_t y,
> +		       uint32_t width, uint32_t height)
>  {
>  	struct intel_buf *buf;
>  	uint8_t *ptr;
> @@ -123,17 +130,17 @@ static void gpgpu_fill(data_t *data, igt_fillfunc_t fill, uint32_t region)
>  	ptr = gem_mmap__device_coherent(data->drm_fd, buf->handle, 0,
>  					buf->surface[0].size, PROT_READ);
>  
> -	for (i = 0; i < WIDTH; i++)
> -		for (j = 0; j < HEIGHT; j++)
> -			buf_check(ptr, i, j, COLOR_88);
> +	for (i = 0; i < surf_width; i++)
> +		for (j = 0; j < surf_height; j++)
> +			buf_check(ptr, surf_width, i, j, COLOR_88);
>  
>  	fill(data->drm_fd, buf, 0, 0, WIDTH / 2, HEIGHT / 2, COLOR_4C);
>  
>  	if (dump_surface) {
> -		for (j = 0; j < HEIGHT; j++) {
> +		for (j = 0; j < surf_height; j++) {
>  			igt_info("[%04x] ", j);
> -			for (i = 0; i < WIDTH; i++) {
> -				igt_info("%02x", ptr[j * HEIGHT + i]);
> +			for (i = 0; i < surf_width; i++) {
> +				igt_info("%02x", ptr[j * surf_height + i]);
>  				if (i % 4 == 3)
>  					igt_info(" ");
>  			}
> @@ -141,12 +148,13 @@ static void gpgpu_fill(data_t *data, igt_fillfunc_t fill, uint32_t region)
>  		}
>  	}
>  
> -	for (i = 0; i < WIDTH; i++)
> -		for (j = 0; j < HEIGHT; j++)
> -			if (i < WIDTH / 2 && j < HEIGHT / 2)
> -				buf_check(ptr, i, j, COLOR_4C);
> +	for (i = 0; i < surf_width; i++)
> +		for (j = 0; j < surf_height; j++)
> +			if (i >= x && i < width + x &&
> +			    j >= y && j < height + y)
> +				buf_check(ptr, surf_width, i, j, COLOR_4C);
>  			else
> -				buf_check(ptr, i, j, COLOR_88);
> +				buf_check(ptr, surf_height, i, j, COLOR_88);
>  
>  	munmap(ptr, buf->surface[0].size);
>  }
> @@ -157,6 +165,18 @@ static int opt_handler(int opt, int opt_index, void *data)
>  	case 'd':
>  		dump_surface = true;
>  		break;
> +	case 'W':
> +		surfwidth = atoi(optarg);
> +		break;
> +	case 'H':
> +		surfheight = atoi(optarg);
> +		break;
> +	case 'X':
> +		start_x = atoi(optarg);
> +		break;
> +	case 'Y':
> +		start_y = atoi(optarg);
> +		break;
>  	default:
>  		return IGT_OPT_HANDLER_ERROR;
>  	}
> @@ -167,10 +187,14 @@ static int opt_handler(int opt, int opt_index, void *data)
>  
>  const char *help_str =
>  	"  -d\tDump surface\n"
> +	"  -W\tWidth (default 64)\n"
> +	"  -H\tHeight (default 64)\n"
> +	"  -X\tX start (aligned to 4)\n"
> +	"  -Y\tY start (aligned to 1)\n"
>  	;
>  
>  
> -igt_main_args("d", NULL, help_str, opt_handler, NULL)
> +igt_main_args("dW:H:X:Y:", NULL, help_str, opt_handler, NULL)
>  {
>  	data_t data = {0, };
>  	igt_fillfunc_t fill_fn = NULL;
> @@ -193,6 +217,8 @@ igt_main_args("d", NULL, help_str, opt_handler, NULL)
>  		region_set = get_memory_region_set(region_info,
>  						   I915_SYSTEM_MEMORY,
>  						   I915_DEVICE_MEMORY);
> +
> +		start_x = ALIGN(start_x, 16);
>  	}
>  
>  	igt_subtest_with_dynamic("basic") {
> @@ -203,7 +229,11 @@ igt_main_args("d", NULL, help_str, opt_handler, NULL)
>  			uint32_t id = igt_collection_get_value(region, 0);
>  
>  			igt_dynamic(name)
> -				gpgpu_fill(&data, fill_fn, id);
> +				gpgpu_fill(&data, fill_fn, id,
> +					   surfwidth, surfheight,
> +					   start_x, start_y,
> +					   surfwidth / 2,
> +					   surfheight / 2);
>  
>  			free(name);
>  		}