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

[Ghimiray, Himal Prasad]

> -----Original Message-----
> From: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> Sent: 26 June 2023 16:30
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: Iddamsetty, Aravind <aravind.iddamsetty at intel.com>; Kumar, Janga
> Rahul <janga.rahul.kumar at intel.com>
> Subject: RE: [PATCH v2 4/4] tests/xe/xe_sysfs_tile_prop: adds new test to
> verify per tile vram addr_range
> 
> 
> 
> > -----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 .
Ok. sure
> 
> > +	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!
Yes not required. Will address 
> 
> > +		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?
Stolen reported by debugfs is only usable stolen and it is being reported only for root tile. So probably using stolen for comparison doesn't make sense.
Wll update the description in test.
> 
> 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