[igt-dev] [PATCH v2 4/4] tests/xe/xe_sysfs_tile_prop: adds new test to verify per tile vram addr_range

[Upadhyay, Tejas]

> -----Original Message-----
> From: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> Sent: Friday, June 23, 2023 5:20 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; Iddamsetty,
> Aravind <aravind.iddamsetty at intel.com>; Upadhyay, Tejas
> <tejas.upadhyay at intel.com>; Kumar, Janga Rahul
> <janga.rahul.kumar at intel.com>
> Subject: [PATCH v2 4/4] tests/xe/xe_sysfs_tile_prop: adds new test to verify
> per tile vram addr_range
> 
> For each tile the test reads the sysfs entry physical_vram_size_bytes and
> compares the value with vram size exposed from query ioctl.
> 
> v2:
> - Change sysfs entry name. (Tejas)
> - Change test name to xe_sysfs_tile_prop. (Rahul)
> 
> Cc: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
> Cc: Upadhyay <tejas.upadhyay at intel.com>
> Cc: Janga Rahul Kumar <janga.rahul.kumar at intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> ---
>  tests/meson.build             |  1 +
>  tests/xe/xe_sysfs_tile_prop.c | 67 +++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+)
>  create mode 100644 tests/xe/xe_sysfs_tile_prop.c
> 
> diff --git a/tests/meson.build b/tests/meson.build index 61dcc076..569f1d22
> 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -267,6 +267,7 @@ xe_progs = [
>  	'xe_pm',
>  	'xe_prime_self_import',
>  	'xe_query',
> +	'xe_sysfs_tile_prop',
>  	'xe_vm',
>  	'xe_waitfence',
>  ]
> diff --git a/tests/xe/xe_sysfs_tile_prop.c b/tests/xe/xe_sysfs_tile_prop.c new
> file mode 100644 index 00000000..f205970f
> --- /dev/null
> +++ b/tests/xe/xe_sysfs_tile_prop.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +/**
> + * TEST: Verify addr_range of each tiles
> + * SUBTEST: addr_range
> + * Description:
> + *             Read sysfs entry for addr_range and compare with
> + *             calculated addr_range. addr_range should be sum
> + *             vram size and stolen memory.
> + */
> +
> +#include <string.h>
> +#include <sys/time.h>
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +
> +#include "xe_drm.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +
> +static void test_vram_addr_range(int sysfs, int tile_num, u64
> +vram_size) {
> +	int err = -EAGAIN;

Can we skip init here!

> +	u64 addr_range;

May be you want to update this variable name now to match .

> +	char path[40];

Better to use already defined path length or define instead of hardcode.

> +
> +	igt_assert(snprintf(path, sizeof(path),
> "device/tile%d/physical_vram_size_bytes",
> +			    tile_num) < sizeof(path));
> +	while (err == -EAGAIN)

And then use do while here. I don’t see value in checking err at first here. Or am I missing something here!

> +		err = igt_sysfs_scanf(sysfs, path, "%lx", &addr_range);
> +
> +	igt_assert_lt(vram_size, addr_range);

Ok, vram_size here would be without stolen can we add vram_size with stolen and check here?

Thanks,
Tejas
> +}
> +
> +igt_main
> +{
> +	int fd;
> +	int tile, total_tiles;
> +	static int sysfs = -1;
> +	u64 vram_size;
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_XE);
> +		xe_device_get(fd);
> +
> +		sysfs = igt_sysfs_open(fd);
> +		igt_assert(sysfs != -1);
> +	}
> +
> +	igt_subtest("addr_range") {
> +		igt_require(xe_has_vram(fd));
> +		xe_for_each_tile(fd, tile, total_tiles) {
> +			vram_size = xe_vram_size(fd, tile);
> +			test_vram_addr_range(sysfs, tile, vram_size);
> +		}
> +	}
> +
> +	igt_fixture {
> +		close(sysfs);
> +		xe_device_put(fd);
> +		close(fd);
> +	}
> +}
> --
> 2.25.1