diff mbox series

[i-g-t,3/4] i915_pm_freq_api: Add some basic SLPC igt tests

Message ID 20230425162405.1730513-4-vinay.belgaumkar@intel.com (mailing list archive)
State New, archived
Headers show
Series tests/slpc: Add basic IGT test | expand

Commit Message

Vinay Belgaumkar April 25, 2023, 4:24 p.m. UTC 
Validate basic api for GT freq control. Also test
interaction with GT reset. We skip rps tests with
SLPC enabled, this will re-introduce some coverage.
SLPC selftests are already covering some other workload
related scenarios.

v2: Rename test (Rodrigo)
v3: Review comments (Ashutosh)
v4: Skip when SLPC is disabled. Check for enable_guc is
not sufficient as kernel config may have it but the
platform doesn't actually support it.
v5: Use the updated SLPC helper
v6: Check for guc enabled as well as slpc debugfs

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 tests/i915/i915_pm_freq_api.c | 153 ++++++++++++++++++++++++++++++++++
 tests/meson.build             |   1 +
 2 files changed, 154 insertions(+)
 create mode 100644 tests/i915/i915_pm_freq_api.c

Comments

Dixit, Ashutosh April 26, 2023, 8:40 p.m. UTC | #1 
On Tue, 25 Apr 2023 09:24:04 -0700, Vinay Belgaumkar wrote:
>
> diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
> new file mode 100644
> index 00000000..17adacbc
> --- /dev/null
> +++ b/tests/i915/i915_pm_freq_api.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <stdlib.h>
> +#include <sys/stat.h>
> +#include <sys/syscall.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "drmtest.h"

The series is merged but before merging I removed all the #include's above,
they are not needed.

> +#include "i915/gem.h"
> +#include "igt_sysfs.h"
> +#include "igt.h"

Thanks.
--
Ashutosh
Kamil Konieczny April 27, 2023, 4:24 p.m. UTC | #2 
Hi Ashutosh,

On 2023-04-26 at 13:40:28 -0700, Dixit, Ashutosh wrote:
> On Tue, 25 Apr 2023 09:24:04 -0700, Vinay Belgaumkar wrote:
> >
> > diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
> > new file mode 100644
> > index 00000000..17adacbc
> > --- /dev/null
> > +++ b/tests/i915/i915_pm_freq_api.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#include <dirent.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <inttypes.h>
> > +#include <stdlib.h>
> > +#include <sys/stat.h>
> > +#include <sys/syscall.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include "drmtest.h"
> 
> The series is merged but before merging I removed all the #include's above,
> they are not needed.

Please do not do this, at least send it to trybot and look at
GitLab.Pipeline status. There are platforms where compilation 
failed due to missing fcntl.h, see
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/137

It is fixed now with
https://patchwork.freedesktop.org/series/117047/

Regards,
Kamil

> 
> > +#include "i915/gem.h"
> > +#include "igt_sysfs.h"
> > +#include "igt.h"
> 
> Thanks.
> --
> Ashutosh
Dixit, Ashutosh April 27, 2023, 4:40 p.m. UTC | #3 
On Thu, 27 Apr 2023 09:24:58 -0700, Kamil Konieczny wrote:
>
> Hi Ashutosh,
>
> On 2023-04-26 at 13:40:28 -0700, Dixit, Ashutosh wrote:
> > On Tue, 25 Apr 2023 09:24:04 -0700, Vinay Belgaumkar wrote:
> > >
> > > diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
> > > new file mode 100644
> > > index 00000000..17adacbc
> > > --- /dev/null
> > > +++ b/tests/i915/i915_pm_freq_api.c
> > > @@ -0,0 +1,153 @@
> > > +// SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2023 Intel Corporation
> > > + */
> > > +
> > > +#include <dirent.h>
> > > +#include <errno.h>
> > > +#include <fcntl.h>
> > > +#include <inttypes.h>
> > > +#include <stdlib.h>
> > > +#include <sys/stat.h>
> > > +#include <sys/syscall.h>
> > > +#include <sys/types.h>
> > > +#include <unistd.h>
> > > +
> > > +#include "drmtest.h"
> >
> > The series is merged but before merging I removed all the #include's above,
> > they are not needed.
>
> Please do not do this, at least send it to trybot and look at
> GitLab.Pipeline status. There are platforms where compilation
> failed due to missing fcntl.h, see
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/137
>
> It is fixed now with
> https://patchwork.freedesktop.org/series/117047/

Hi Kamil, agreed, sorry about that. Lesson learnt. Thanks for fixing it quickly.

Ashutosh
diff mbox series

Patch

diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
new file mode 100644
index 00000000..17adacbc
--- /dev/null
+++ b/tests/i915/i915_pm_freq_api.c
@@ -0,0 +1,153 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "drmtest.h"
+#include "i915/gem.h"
+#include "igt_sysfs.h"
+#include "igt.h"
+
+IGT_TEST_DESCRIPTION("Test SLPC freq API");
+/*
+ * Too many intermediate components and steps before freq is adjusted
+ * Specially if workload is under execution, so let's wait 100 ms.
+ */
+#define ACT_FREQ_LATENCY_US 100000
+
+static uint32_t get_freq(int dirfd, uint8_t id)
+{
+	uint32_t val;
+
+	igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
+
+	return val;
+}
+
+static int set_freq(int dirfd, uint8_t id, uint32_t val)
+{
+	return igt_sysfs_rps_printf(dirfd, id, "%u", val);
+}
+
+static void test_freq_basic_api(int dirfd, int gt)
+{
+	uint32_t rpn, rp0, rpe;
+
+	/* Save frequencies */
+	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
+	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
+	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
+	igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
+
+	/*
+	 * Negative bound tests
+	 * RPn is the floor
+	 * RP0 is the ceiling
+	 */
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn - 1) < 0);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
+
+	/* Assert min requests are respected from rp0 to rpn */
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0) > 0);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rp0);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpe) > 0);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpe);
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
+
+	/* Assert max requests are respected from rpn to rp0 */
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpe) > 0);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpe);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rp0);
+
+}
+
+static void test_reset(int i915, int dirfd, int gt)
+{
+	uint32_t rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
+	int fd;
+
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);
+	usleep(ACT_FREQ_LATENCY_US);
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
+
+	/* Manually trigger a GT reset */
+	fd = igt_debugfs_gt_open(i915, gt, "reset", O_WRONLY);
+	igt_require(fd >= 0);
+	igt_ignore_warn(write(fd, "1\n", 2));
+	close(fd);
+
+	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
+	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
+}
+
+igt_main
+{
+	int i915 = -1;
+	uint32_t *stash_min, *stash_max;
+
+	igt_fixture {
+		int num_gts, dirfd, gt;
+
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
+		/* i915_pm_rps already covers execlist path */
+		igt_require_f(gem_using_guc_submission(i915) &&
+			      i915_is_slpc_enabled(i915),
+			      "This test is supported only with SLPC enabled\n");
+
+		num_gts = igt_sysfs_get_num_gt(i915);
+		stash_min = (uint32_t*)malloc(sizeof(uint32_t) * num_gts);
+		stash_max = (uint32_t*)malloc(sizeof(uint32_t) * num_gts);
+
+		/* Save curr min and max across GTs */
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
+			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
+			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
+		}
+	}
+
+	igt_describe("Test basic API for controlling min/max GT frequency");
+	igt_subtest_with_dynamic_f("freq-basic-api") {
+		int dirfd, gt;
+
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt)
+			igt_dynamic_f("gt%u", gt)
+				test_freq_basic_api(dirfd, gt);
+	}
+
+	igt_describe("Test basic freq API works after a reset");
+	igt_subtest_with_dynamic_f("freq-reset") {
+		int dirfd, gt;
+
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt)
+			igt_dynamic_f("gt%u", gt)
+				test_reset(i915, dirfd, gt);
+	}
+
+	igt_fixture {
+		int dirfd, gt;
+		/* Restore frequencies */
+		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
+			igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, stash_max[gt]) > 0);
+			igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, stash_min[gt]) > 0);
+		}
+		close(i915);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index da31e782..46109f10 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -202,6 +202,7 @@  i915_progs = [
 	'gem_workarounds',
 	'i915_fb_tiling',
 	'i915_getparams_basic',
+	'i915_pm_freq_api',
 	'i915_hangman',
 	'i915_hwmon',
 	'i915_module_load',