[igt-dev] [PATCH i-g-t 4/5] xe/xe_sysfs_timeslice: Verify timeslice_duration

[Dandamudi, Priyanka]

> -----Original Message-----
> From: Kumar, Janga Rahul <janga.rahul.kumar at intel.com>
> Sent: 27 June 2023 05:09 PM
> To: Dandamudi, Priyanka <priyanka.dandamudi at intel.com>; Upadhyay, Tejas
> <tejas.upadhyay at intel.com>; igt-dev at lists.freedesktop.org
> Subject: RE: [PATCH i-g-t 4/5] xe/xe_sysfs_timeslice: Verify timeslice_duration
> 
> 
> 
> > -----Original Message-----
> > From: Dandamudi, Priyanka <priyanka.dandamudi at intel.com>
> > Sent: 25 June 2023 18:00
> > To: Kumar, Janga Rahul <janga.rahul.kumar at intel.com>; Upadhyay, Tejas
> > <tejas.upadhyay at intel.com>; igt-dev at lists.freedesktop.org; Dandamudi,
> > Priyanka <priyanka.dandamudi at intel.com>
> > Subject: [PATCH i-g-t 4/5] xe/xe_sysfs_timeslice: Verify
> > timeslice_duration
> >
> > From: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> >
> > Basic tests idempotent and invalid are added to verify
> > timeslice_duration for each engine.
> > It verifies whether parameter value within in the range.
> >
> > Cc: Janga Rahul Kumar <janga.rahul.kumar at intel.com>
> > Cc: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> > ---
> >  tests/meson.build             |   1 +
> >  tests/xe/xe_sysfs_timeslice.c | 138
> > ++++++++++++++++++++++++++++++++++
> >  2 files changed, 139 insertions(+)
> >  create mode 100644 tests/xe/xe_sysfs_timeslice.c
> >
> > diff --git a/tests/meson.build b/tests/meson.build index
> > ea6c9239..1474a494
> > 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -272,6 +272,7 @@ xe_progs = [
> >  	'xe_spin_batch',
> >  	'xe_sysfs_defaults',
> >  	'xe_sysfs_preempt_timeout',
> > +	'xe_sysfs_timeslice',
> >  ]
> >
> >  msm_progs = [
> > diff --git a/tests/xe/xe_sysfs_timeslice.c
> > b/tests/xe/xe_sysfs_timeslice.c new file mode 100644 index
> > 00000000..94bed383
> > --- /dev/null
> > +++ b/tests/xe/xe_sysfs_timeslice.c
> > @@ -0,0 +1,138 @@
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > +obtaining a
> > + * copy of this software and associated documentation files (the
> > +"Software"),
> > + * to deal in the Software without restriction, including without
> > +limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > +sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom
> > +the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> > +the next
> > + * paragraph) shall be included in all copies or substantial portions
> > +of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > +EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > +MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > EVENT
> > +SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES OR
> > +OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > +ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER
> > +DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#include <dirent.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <unistd.h>
> > +
> > +#include "igt.h"
> > +#include "igt_sysfs.h"
> > +
> > +#include "xe_drm.h"
> > +#include "xe/xe_query.h"
> > +
> > +/**
> > + * TEST: xe sysfs timeslice
> > + * Run type: FULL
> > + *
> > + * SUBTEST: idempotent
> > + * Description: Test to check whether the timelslice_duration_us
> > +parameter
> > reports the values set.
> > + * Test category: SysMan
> > + *
> > + * SUBTEST: invalid
> > + * Description: Test to check if timeslice_duration parameter rejects
> > + any
> > unrepresentable intervals.
> > + * Test category: SysMan
> > + */
> > +
> > +#define ATTR "timeslice_duration_us"
> > +#define ATTR_MIN "timeslice_duration_min"
> > +#define ATTR_MAX "timeslice_duration_max"
> > +
> > +static void set_timeslice_duration(int engine, unsigned int value) {
> > +	unsigned int delay;
> > +
> > +	igt_assert_lte(0, igt_sysfs_printf(engine, ATTR, "%u", value));
> > +	igt_sysfs_scanf(engine, ATTR, "%u", &delay);
> > +	igt_assert_eq(delay, value);
> > +}
> > +
> > +static void test_idempotent(int xe, int engine) {
> > +	unsigned int delays[] = { 1, 1000, 1234, 10000000 };
> > +	unsigned int saved;
> > +
> > +	igt_assert(igt_sysfs_scanf(engine, ATTR, "%u", &saved) == 1);
> > +	igt_debug("Initial %s:%u\n", ATTR, saved);
> > +
> > +	for (int i = 0; i < ARRAY_SIZE(delays); i++)
> > +		set_timeslice_duration(engine, delays[i]);
> > +
> > +	set_timeslice_duration(engine, saved); }
> > +
> > +static void test_invalid(int xe, int engine) {
> Modify this to sysfs_range(min_max)_invalid(int xe, int engine_fd,  const char
> *attr, const char *attr_min, const char *attr_max) and move this to lib and reuse
> it in other tests.
I modified entire tests and will send the change in next patch series.
> 
> > +	unsigned int saved, set;
> > +	unsigned int min, max;
> > +
> > +	igt_sysfs_scanf(engine, ATTR_MAX, "%u", &max);
> > +	igt_sysfs_scanf(engine, ATTR_MIN, "%u", &min);
> > +
> > +	igt_assert(igt_sysfs_scanf(engine, ATTR, "%u", &saved) == 1);
> > +	igt_debug("Initial %s:%u\n", ATTR, saved);
> > +
> > +	igt_sysfs_printf(engine, ATTR, "%d", max+1);
> > +	igt_sysfs_scanf(engine, ATTR, "%u", &set);
> > +	igt_assert_eq(set, saved);
> > +
> > +	igt_sysfs_printf(engine, ATTR, "%d", min-1);
> > +	igt_sysfs_scanf(engine, ATTR, "%u", &set);
> > +	igt_assert_eq(set, saved);
> > +}
> > +
> > +igt_main
> > +{
> > +	static const struct {
> > +		const char *name;
> > +		void (*fn)(int, int);
> > +	} tests[] = {
> > +		{ "idempotent", test_idempotent },
> > +		{ "invalid", test_invalid },
> > +		{ }
> > +	};
> > +	int xe = -1;
> > +	int sys;
> > +	int gt;
> > +
> > +	igt_fixture {
> > +		xe = drm_open_driver(DRIVER_XE);
> > +		xe_device_get(xe);
> > +
> > +		sys = igt_sysfs_open(xe);
> > +		igt_require(sys != -1);
> > +	}
> > +	for (typeof(*tests) *t = tests; t->name; t++)
> > +		igt_subtest_with_dynamic(t->name) {
> > +			xe_for_each_gt(xe, gt) {
> > +				int engines = -1;
> > +				char buf[100];
> > +
> > +				sprintf(buf, "device/gt%d/engines", gt);
> > +				engines = openat(sys, buf, O_RDONLY);
> For variables being used for storing fd's, Pls append "_fd" to improve readability.
> 
> Thanks,
> Rahul
Will make that change.
Thanks,
Priyanka
> > +				igt_require(engines != -1);
> > +
> > +				igt_sysfs_engines(xe, engines, ATTR, t->fn);
> > +
> > +				close(engines);
> > +			}
> > +		}
> > +
> > +	igt_fixture {
> > +		close(sys);
> > +		xe_device_put(xe);
> > +		close(xe);
> > +	}
> > +}
> > +
> > --
> > 2.25.1