All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
@ 2018-01-09 16:16 Tvrtko Ursulin
  2018-01-09 16:16 ` [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore Tvrtko Ursulin
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-01-09 16:16 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Make sure busyness is correctly reported when PMU is enabled after the
engine is already busy with a single long batch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/perf_pmu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 45e2f6148453..e1f449d48808 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -157,6 +157,41 @@ single(int gem_fd, const struct intel_execution_engine2 *e, bool busy)
 	gem_quiescent_gpu(gem_fd);
 }
 
+static void
+busy_start(int gem_fd, const struct intel_execution_engine2 *e)
+{
+	unsigned long slept;
+	igt_spin_t *spin;
+	uint64_t val;
+	int fd;
+
+	/*
+	 * Defeat the busy stats delayed disable, we need to guarantee we are
+	 * the first user.
+	 */
+	if (gem_has_execlists(gem_fd))
+		sleep(2);
+
+	spin = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
+
+	/*
+	 * Sleep for a bit after making the engine busy to make sure the PMU
+	 * gets enabled when the batch is already running.
+	 */
+	usleep(500000);
+
+	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
+
+	slept = measured_usleep(batch_duration_ns / 1000);
+	val = pmu_read_single(fd);
+
+	igt_spin_batch_free(gem_fd, spin);
+	close(fd);
+
+	assert_within_epsilon(val, slept, tolerance);
+	gem_quiescent_gpu(gem_fd);
+}
+
 static void log_busy(int fd, unsigned int num_engines, uint64_t *val)
 {
 	char buf[1024];
@@ -1164,6 +1199,13 @@ igt_main
 		 */
 		igt_subtest_f("multi-client-%s", e->name)
 			multi_client(fd, e);
+
+		/**
+		 * Check that reported usage is correct when PMU is enabled
+		 * after the batch is running.
+		 */
+		igt_subtest_f("busy-start-%s", e->name)
+			busy_start(fd, e);
 	}
 
 	/**
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore
  2018-01-09 16:16 [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Tvrtko Ursulin
@ 2018-01-09 16:16 ` Tvrtko Ursulin
  2018-01-09 21:39   ` Chris Wilson
  2018-01-09 16:59 ` ✓ Fi.CI.BAT: success for series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-01-09 16:16 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

While developing a fix for an accounting hole in busy stats we realized
lite-restore is a potential edge case which would be interesting to check
is properly handled.

It is unfortnately quite timing sensitive to hit lite-restore in the
fashion test needs, so downside of this test is that it sufferes from a
high rate of false negatives.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/perf_pmu.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index e1f449d48808..7db5059d202e 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -192,6 +192,84 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
 	gem_quiescent_gpu(gem_fd);
 }
 
+/*
+ * This test has a potentially low rate of catching the issue it is trying to
+ * catch. Or in other words, quite high rate of false negative successes. We
+ * will depend on the CI systems running it a lot to detect issues.
+ */
+static void
+busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
+{
+	unsigned long slept;
+	igt_spin_t *spin[2];
+	uint64_t val, val2;
+	bool execlists;
+	uint32_t ctx;
+	int fd;
+
+	ctx = gem_context_create(gem_fd);
+
+	if (gem_has_execlists(gem_fd)) {
+		/*
+		 * Defeat the busy stats delayed disable, we need to guarantee
+		 * we are the first user.
+		 */
+		execlists = true;
+		sleep(2);
+	} else {
+		execlists = false;
+	}
+
+	/*
+	 * Submit two contexts, with a pause in between targeting the ELSP
+	 * re-submission in execlists mode. Make sure busyness is correctly
+	 * reported with the engine busy, and after the engine went idle.
+	 */
+	spin[0] = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
+	usleep(500000);
+	spin[1] = __igt_spin_batch_new(gem_fd, ctx, e2ring(gem_fd, e), 0);
+
+	/*
+	 * Open PMU as fast as possible after the second spin batch in attempt
+	 * to be faster than the driver handling lite-restore.
+	 */
+	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
+
+	slept = measured_usleep(batch_duration_ns / 1000);
+	val = pmu_read_single(fd);
+
+	igt_spin_batch_end(spin[0]);
+	igt_spin_batch_end(spin[1]);
+
+	gem_sync(gem_fd, spin[0]->handle);
+	gem_sync(gem_fd, spin[1]->handle);
+
+	/* Wait for context complete to avoid accounting residual busyness. */
+	if (execlists)
+		usleep(100000);
+
+	val2 = pmu_read_single(fd);
+	usleep(batch_duration_ns / 1000);
+	val2 = pmu_read_single(fd) - val2;
+
+	igt_info("busy=%lu idle=%lu\n", val, val2);
+
+	igt_spin_batch_free(gem_fd, spin[0]);
+	igt_spin_batch_free(gem_fd, spin[1]);
+
+	close(fd);
+
+	gem_context_destroy(gem_fd, ctx);
+
+	assert_within_epsilon(val, slept, tolerance);
+	if (execlists)
+		igt_assert_eq(val2, 0);
+	else
+		assert_within_epsilon(val2, 0.0, tolerance);
+
+	gem_quiescent_gpu(gem_fd);
+}
+
 static void log_busy(int fd, unsigned int num_engines, uint64_t *val)
 {
 	char buf[1024];
@@ -1206,6 +1284,13 @@ igt_main
 		 */
 		igt_subtest_f("busy-start-%s", e->name)
 			busy_start(fd, e);
+
+		/**
+		 * Check that reported usage is correct when PMU is enabled
+		 * after two batches are running.
+		 */
+		igt_subtest_f("busy-double-start-%s", e->name)
+			busy_double_start(fd, e);
 	}
 
 	/**
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
  2018-01-09 16:16 [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Tvrtko Ursulin
  2018-01-09 16:16 ` [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore Tvrtko Ursulin
@ 2018-01-09 16:59 ` Patchwork
  2018-01-09 18:57 ` ✗ Fi.CI.IGT: warning " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-01-09 16:59 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
URL   : https://patchwork.freedesktop.org/series/36201/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
834321a5d76a16783000441a02d7e79e72be9cc9 tools: Cannonlake port clock programming

with latest DRM-Tip kernel build CI_DRM_3614
27c4212cc2b1 drm-tip: 2018y-01m-09d-14h-51m-00s UTC integration manifest

Testlist changes:
+igt@perf_pmu@busy-double-start-bcs0
+igt@perf_pmu@busy-double-start-rcs0
+igt@perf_pmu@busy-double-start-vcs0
+igt@perf_pmu@busy-double-start-vcs1
+igt@perf_pmu@busy-double-start-vecs0
+igt@perf_pmu@busy-start-bcs0
+igt@perf_pmu@busy-start-rcs0
+igt@perf_pmu@busy-start-vcs0
+igt@perf_pmu@busy-start-vcs1
+igt@perf_pmu@busy-start-vecs0

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> PASS       (fi-elk-e7500) fdo#103989 +1

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:423s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:429s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:492s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:282s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:487s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:488s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:483s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:467s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:279s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:517s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:401s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:415s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:414s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:468s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:500s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:503s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:585s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:432s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:512s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:530s
fi-skl-6700k2    total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:434s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:534s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:402s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y2        total:288  pass:258  dwarn:3   dfail:0   fail:0   skip:27  time:533s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:473s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_758/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ✗ Fi.CI.IGT: warning for series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
  2018-01-09 16:16 [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Tvrtko Ursulin
  2018-01-09 16:16 ` [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore Tvrtko Ursulin
  2018-01-09 16:59 ` ✓ Fi.CI.BAT: success for series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Patchwork
@ 2018-01-09 18:57 ` Patchwork
  2018-01-09 21:28 ` [PATCH i-g-t 1/2] " Chris Wilson
  2018-01-11 13:21 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy (rev3) Patchwork
  4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-01-09 18:57 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
URL   : https://patchwork.freedesktop.org/series/36201/
State : warning

== Summary ==

Test gem_eio:
        Subgroup in-flight:
                pass       -> DMESG-WARN (shard-snb) fdo#104058
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623
Test kms_plane_lowres:
        Subgroup pipe-a-tiling-none:
                pass       -> SKIP       (shard-snb)
Test kms_pwrite_crc:
                pass       -> SKIP       (shard-snb)
Test gem_tiled_swapping:
        Subgroup non-threaded:
                pass       -> INCOMPLETE (shard-hsw) fdo#104218
Test kms_cursor_crc:
        Subgroup cursor-256x256-suspend:
                incomplete -> PASS       (shard-hsw) fdo#103375

fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-hsw        total:2701 pass:1530 dwarn:1   dfail:0   fail:10  skip:1159 time:8822s
shard-snb        total:2723 pass:1313 dwarn:2   dfail:0   fail:11  skip:1397 time:7985s
Blacklisted hosts:
shard-apl        total:2723 pass:1684 dwarn:1   dfail:0   fail:34  skip:1003 time:13641s
shard-kbl        total:2723 pass:1759 dwarn:43  dfail:2   fail:42  skip:877 time:10695s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_758/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
  2018-01-09 16:16 [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2018-01-09 18:57 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-01-09 21:28 ` Chris Wilson
  2018-01-10 10:42   ` Tvrtko Ursulin
  2018-01-11 13:21 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy (rev3) Patchwork
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2018-01-09 21:28 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

Quoting Tvrtko Ursulin (2018-01-09 16:16:20)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Make sure busyness is correctly reported when PMU is enabled after the
> engine is already busy with a single long batch.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/perf_pmu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index 45e2f6148453..e1f449d48808 100644
> --- a/tests/perf_pmu.c
> +++ b/tests/perf_pmu.c
> @@ -157,6 +157,41 @@ single(int gem_fd, const struct intel_execution_engine2 *e, bool busy)
>         gem_quiescent_gpu(gem_fd);
>  }
>  
> +static void
> +busy_start(int gem_fd, const struct intel_execution_engine2 *e)
> +{
> +       unsigned long slept;
> +       igt_spin_t *spin;
> +       uint64_t val;
> +       int fd;
> +
> +       /*
> +        * Defeat the busy stats delayed disable, we need to guarantee we are
> +        * the first user.
> +        */
> +       if (gem_has_execlists(gem_fd))
> +               sleep(2);

I don't have a better idea than sleep, but I don't like tying this to
execlists. Make the sleep unconditional for now. Is there anyway we can
export the knowledge of the implementation through the perf api?
Different counters, or now different attrs for different busy-stats?

> +
> +       spin = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
> +
> +       /*
> +        * Sleep for a bit after making the engine busy to make sure the PMU
> +        * gets enabled when the batch is already running.
> +        */
> +       usleep(500000);

Just a request for 500e3.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore
  2018-01-09 16:16 ` [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore Tvrtko Ursulin
@ 2018-01-09 21:39   ` Chris Wilson
  2018-01-10 10:37     ` Tvrtko Ursulin
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2018-01-09 21:39 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx

Quoting Tvrtko Ursulin (2018-01-09 16:16:21)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> While developing a fix for an accounting hole in busy stats we realized
> lite-restore is a potential edge case which would be interesting to check
> is properly handled.
> 
> It is unfortnately quite timing sensitive to hit lite-restore in the
> fashion test needs, so downside of this test is that it sufferes from a
> high rate of false negatives.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/perf_pmu.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index e1f449d48808..7db5059d202e 100644
> --- a/tests/perf_pmu.c
> +++ b/tests/perf_pmu.c
> @@ -192,6 +192,84 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
>         gem_quiescent_gpu(gem_fd);
>  }
>  
> +/*
> + * This test has a potentially low rate of catching the issue it is trying to
> + * catch. Or in other words, quite high rate of false negative successes. We
> + * will depend on the CI systems running it a lot to detect issues.
> + */
> +static void
> +busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
> +{
> +       unsigned long slept;
> +       igt_spin_t *spin[2];
> +       uint64_t val, val2;
> +       bool execlists;
> +       uint32_t ctx;
> +       int fd;
> +
> +       ctx = gem_context_create(gem_fd);
> +
> +       if (gem_has_execlists(gem_fd)) {
> +               /*
> +                * Defeat the busy stats delayed disable, we need to guarantee
> +                * we are the first user.
> +                */
> +               execlists = true;
> +               sleep(2);
> +       } else {
> +               execlists = false;
> +       }
> +
> +       /*
> +        * Submit two contexts, with a pause in between targeting the ELSP
> +        * re-submission in execlists mode. Make sure busyness is correctly
> +        * reported with the engine busy, and after the engine went idle.
> +        */
> +       spin[0] = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
> +       usleep(500000);
> +       spin[1] = __igt_spin_batch_new(gem_fd, ctx, e2ring(gem_fd, e), 0);

If you want to avoid the second spinner, you can resubmit the first with
a second context.

> +       /*
> +        * Open PMU as fast as possible after the second spin batch in attempt
> +        * to be faster than the driver handling lite-restore.
> +        */
> +       fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
> +
> +       slept = measured_usleep(batch_duration_ns / 1000);
> +       val = pmu_read_single(fd);
> +
> +       igt_spin_batch_end(spin[0]);
> +       igt_spin_batch_end(spin[1]);
> +
> +       gem_sync(gem_fd, spin[0]->handle);
> +       gem_sync(gem_fd, spin[1]->handle);
> +
> +       /* Wait for context complete to avoid accounting residual busyness. */
> +       if (execlists)
> +               usleep(100000);

Then you want a gem_quiescent_gpu() instead of the gem_sync()? Though
I'm scratching my head here as to what you mean. If there's an issue
here, won't we see it in our other measurements as well?

> +       val2 = pmu_read_single(fd);
> +       usleep(batch_duration_ns / 1000);
> +       val2 = pmu_read_single(fd) - val2;

But the engine is idle at this point? Why sleep for execlists and not
legacy? Isn't the execlists case where we know that just by waiting for
idle (gem_quiescent_gpu() not an arbitrary usleep) then the pmu counter
is idle. It's legacy where we would have to wait for an sample interval
following gpu idle to be sure the pmu is idle.

I think I half understand what you want to assert (an idle gpu after
being busy, reports idle?) but I'm not entirely sure. :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore
  2018-01-09 21:39   ` Chris Wilson
@ 2018-01-10 10:37     ` Tvrtko Ursulin
  2018-01-10 13:45       ` Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-01-10 10:37 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, Intel-gfx


On 09/01/2018 21:39, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-09 16:16:21)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> While developing a fix for an accounting hole in busy stats we realized
>> lite-restore is a potential edge case which would be interesting to check
>> is properly handled.
>>
>> It is unfortnately quite timing sensitive to hit lite-restore in the
>> fashion test needs, so downside of this test is that it sufferes from a
>> high rate of false negatives.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/perf_pmu.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 85 insertions(+)
>>
>> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
>> index e1f449d48808..7db5059d202e 100644
>> --- a/tests/perf_pmu.c
>> +++ b/tests/perf_pmu.c
>> @@ -192,6 +192,84 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
>>          gem_quiescent_gpu(gem_fd);
>>   }
>>   
>> +/*
>> + * This test has a potentially low rate of catching the issue it is trying to
>> + * catch. Or in other words, quite high rate of false negative successes. We
>> + * will depend on the CI systems running it a lot to detect issues.
>> + */
>> +static void
>> +busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
>> +{
>> +       unsigned long slept;
>> +       igt_spin_t *spin[2];
>> +       uint64_t val, val2;
>> +       bool execlists;
>> +       uint32_t ctx;
>> +       int fd;
>> +
>> +       ctx = gem_context_create(gem_fd);
>> +
>> +       if (gem_has_execlists(gem_fd)) {
>> +               /*
>> +                * Defeat the busy stats delayed disable, we need to guarantee
>> +                * we are the first user.
>> +                */
>> +               execlists = true;
>> +               sleep(2);
>> +       } else {
>> +               execlists = false;
>> +       }
>> +
>> +       /*
>> +        * Submit two contexts, with a pause in between targeting the ELSP
>> +        * re-submission in execlists mode. Make sure busyness is correctly
>> +        * reported with the engine busy, and after the engine went idle.
>> +        */
>> +       spin[0] = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
>> +       usleep(500000);
>> +       spin[1] = __igt_spin_batch_new(gem_fd, ctx, e2ring(gem_fd, e), 0);
> 
> If you want to avoid the second spinner, you can resubmit the first with
> a second context.

I know you like that but I prefer the pedestrian way. :)

>> +       /*
>> +        * Open PMU as fast as possible after the second spin batch in attempt
>> +        * to be faster than the driver handling lite-restore.
>> +        */
>> +       fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
>> +
>> +       slept = measured_usleep(batch_duration_ns / 1000);
>> +       val = pmu_read_single(fd);
>> +
>> +       igt_spin_batch_end(spin[0]);
>> +       igt_spin_batch_end(spin[1]);
>> +
>> +       gem_sync(gem_fd, spin[0]->handle);
>> +       gem_sync(gem_fd, spin[1]->handle);
>> +
>> +       /* Wait for context complete to avoid accounting residual busyness. */
>> +       if (execlists)
>> +               usleep(100000);
> 
> Then you want a gem_quiescent_gpu() instead of the gem_sync()? Though
> I'm scratching my head here as to what you mean. If there's an issue
> here, won't we see it in our other measurements as well?

Primarily because we want to measure idle after busy here and not in 
other tests, *and*, with execlists time between user interrupt and 
context save counts as busyness. So without this delay it is possible to 
observe a few micro-second of engine busyness in the below check.

>> +       val2 = pmu_read_single(fd);
>> +       usleep(batch_duration_ns / 1000);
>> +       val2 = pmu_read_single(fd) - val2;
> 
> But the engine is idle at this point? Why sleep for execlists and not
> legacy? Isn't the execlists case where we know that just by waiting for
> idle (gem_quiescent_gpu() not an arbitrary usleep) then the pmu counter
> is idle. It's legacy where we would have to wait for an sample interval
> following gpu idle to be sure the pmu is idle.

Hm legacy and sampling.. I think we don't have to wait since there is no 
context save there is the idleness comes straight with the user 
interrupt, or no?

> I think I half understand what you want to assert (an idle gpu after
> being busy, reports idle?) but I'm not entirely sure. :)

Yep!

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
  2018-01-09 21:28 ` [PATCH i-g-t 1/2] " Chris Wilson
@ 2018-01-10 10:42   ` Tvrtko Ursulin
  2018-01-10 13:38     ` Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-01-10 10:42 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin, Intel-gfx


On 09/01/2018 21:28, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-09 16:16:20)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Make sure busyness is correctly reported when PMU is enabled after the
>> engine is already busy with a single long batch.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/perf_pmu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>
>> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
>> index 45e2f6148453..e1f449d48808 100644
>> --- a/tests/perf_pmu.c
>> +++ b/tests/perf_pmu.c
>> @@ -157,6 +157,41 @@ single(int gem_fd, const struct intel_execution_engine2 *e, bool busy)
>>          gem_quiescent_gpu(gem_fd);
>>   }
>>   
>> +static void
>> +busy_start(int gem_fd, const struct intel_execution_engine2 *e)
>> +{
>> +       unsigned long slept;
>> +       igt_spin_t *spin;
>> +       uint64_t val;
>> +       int fd;
>> +
>> +       /*
>> +        * Defeat the busy stats delayed disable, we need to guarantee we are
>> +        * the first user.
>> +        */
>> +       if (gem_has_execlists(gem_fd))
>> +               sleep(2);
> 
> I don't have a better idea than sleep, but I don't like tying this to
> execlists. Make the sleep unconditional for now. Is there anyway we can
> export the knowledge of the implementation through the perf api?
> Different counters, or now different attrs for different busy-stats?

I can't come up with any reasonable idea. Best hope I have is that we 
could also expose busyness via sysfs one day and then I would move this 
test out of the PMU specific ones to a new home.

To sleep unconditionally I am also torn because the less backend 
knowledge and variability in the tests the better, but also I would 
prefer not to waste time when not necessary.

>> +
>> +       spin = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
>> +
>> +       /*
>> +        * Sleep for a bit after making the engine busy to make sure the PMU
>> +        * gets enabled when the batch is already running.
>> +        */
>> +       usleep(500000);
> 
> Just a request for 500e3.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks! Shard run shows it is catching the current issue fine.

Do you plan to respin your fix so the other subtest passes as well?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
  2018-01-10 10:42   ` Tvrtko Ursulin
@ 2018-01-10 13:38     ` Chris Wilson
  2018-01-11  8:46       ` [PATCH i-g-t v2 " Tvrtko Ursulin
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2018-01-10 13:38 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx

Quoting Tvrtko Ursulin (2018-01-10 10:42:34)
> 
> On 09/01/2018 21:28, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-01-09 16:16:20)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Make sure busyness is correctly reported when PMU is enabled after the
> >> engine is already busy with a single long batch.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   tests/perf_pmu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 42 insertions(+)
> >>
> >> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> >> index 45e2f6148453..e1f449d48808 100644
> >> --- a/tests/perf_pmu.c
> >> +++ b/tests/perf_pmu.c
> >> @@ -157,6 +157,41 @@ single(int gem_fd, const struct intel_execution_engine2 *e, bool busy)
> >>          gem_quiescent_gpu(gem_fd);
> >>   }
> >>   
> >> +static void
> >> +busy_start(int gem_fd, const struct intel_execution_engine2 *e)
> >> +{
> >> +       unsigned long slept;
> >> +       igt_spin_t *spin;
> >> +       uint64_t val;
> >> +       int fd;
> >> +
> >> +       /*
> >> +        * Defeat the busy stats delayed disable, we need to guarantee we are
> >> +        * the first user.
> >> +        */
> >> +       if (gem_has_execlists(gem_fd))
> >> +               sleep(2);
> > 
> > I don't have a better idea than sleep, but I don't like tying this to
> > execlists. Make the sleep unconditional for now. Is there anyway we can
> > export the knowledge of the implementation through the perf api?
> > Different counters, or now different attrs for different busy-stats?
> 
> I can't come up with any reasonable idea. Best hope I have is that we 
> could also expose busyness via sysfs one day and then I would move this 
> test out of the PMU specific ones to a new home.
> 
> To sleep unconditionally I am also torn because the less backend 
> knowledge and variability in the tests the better, but also I would 
> prefer not to waste time when not necessary.

Consider it a motivator to export enough information to avoid
assumptions :) A test is only as fast as the slowest machine ;)

> >> +
> >> +       spin = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
> >> +
> >> +       /*
> >> +        * Sleep for a bit after making the engine busy to make sure the PMU
> >> +        * gets enabled when the batch is already running.
> >> +        */
> >> +       usleep(500000);
> > 
> > Just a request for 500e3.
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Thanks! Shard run shows it is catching the current issue fine.
> 
> Do you plan to respin your fix so the other subtest passes as well?

I'm open to suggestions. It was only intended as a quick fix, if you
have a better idea for the api, be my guest. But a quick patch to get us
to a stable state from which we can improve isn't a horrible idea.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore
  2018-01-10 10:37     ` Tvrtko Ursulin
@ 2018-01-10 13:45       ` Chris Wilson
  2018-01-11  8:46         ` [PATCH i-g-t v2 " Tvrtko Ursulin
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2018-01-10 13:45 UTC (permalink / raw)
  To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx

Quoting Tvrtko Ursulin (2018-01-10 10:37:51)
> 
> On 09/01/2018 21:39, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-01-09 16:16:21)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> While developing a fix for an accounting hole in busy stats we realized
> >> lite-restore is a potential edge case which would be interesting to check
> >> is properly handled.
> >>
> >> It is unfortnately quite timing sensitive to hit lite-restore in the
> >> fashion test needs, so downside of this test is that it sufferes from a
> >> high rate of false negatives.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   tests/perf_pmu.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 85 insertions(+)
> >>
> >> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> >> index e1f449d48808..7db5059d202e 100644
> >> --- a/tests/perf_pmu.c
> >> +++ b/tests/perf_pmu.c
> >> @@ -192,6 +192,84 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
> >>          gem_quiescent_gpu(gem_fd);
> >>   }
> >>   
> >> +/*
> >> + * This test has a potentially low rate of catching the issue it is trying to
> >> + * catch. Or in other words, quite high rate of false negative successes. We
> >> + * will depend on the CI systems running it a lot to detect issues.
> >> + */
> >> +static void
> >> +busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
> >> +{
> >> +       unsigned long slept;
> >> +       igt_spin_t *spin[2];
> >> +       uint64_t val, val2;
> >> +       bool execlists;
> >> +       uint32_t ctx;
> >> +       int fd;
> >> +
> >> +       ctx = gem_context_create(gem_fd);
> >> +
> >> +       if (gem_has_execlists(gem_fd)) {
> >> +               /*
> >> +                * Defeat the busy stats delayed disable, we need to guarantee
> >> +                * we are the first user.
> >> +                */
> >> +               execlists = true;
> >> +               sleep(2);
> >> +       } else {
> >> +               execlists = false;
> >> +       }
> >> +
> >> +       /*
> >> +        * Submit two contexts, with a pause in between targeting the ELSP
> >> +        * re-submission in execlists mode. Make sure busyness is correctly
> >> +        * reported with the engine busy, and after the engine went idle.
> >> +        */
> >> +       spin[0] = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
> >> +       usleep(500000);
> >> +       spin[1] = __igt_spin_batch_new(gem_fd, ctx, e2ring(gem_fd, e), 0);
> > 
> > If you want to avoid the second spinner, you can resubmit the first with
> > a second context.
> 
> I know you like that but I prefer the pedestrian way. :)
> 
> >> +       /*
> >> +        * Open PMU as fast as possible after the second spin batch in attempt
> >> +        * to be faster than the driver handling lite-restore.
> >> +        */
> >> +       fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
> >> +
> >> +       slept = measured_usleep(batch_duration_ns / 1000);
> >> +       val = pmu_read_single(fd);
> >> +
> >> +       igt_spin_batch_end(spin[0]);
> >> +       igt_spin_batch_end(spin[1]);
> >> +
> >> +       gem_sync(gem_fd, spin[0]->handle);
> >> +       gem_sync(gem_fd, spin[1]->handle);
> >> +
> >> +       /* Wait for context complete to avoid accounting residual busyness. */
> >> +       if (execlists)
> >> +               usleep(100000);
> > 
> > Then you want a gem_quiescent_gpu() instead of the gem_sync()? Though
> > I'm scratching my head here as to what you mean. If there's an issue
> > here, won't we see it in our other measurements as well?
> 
> Primarily because we want to measure idle after busy here and not in 
> other tests, *and*, with execlists time between user interrupt and 
> context save counts as busyness. So without this delay it is possible to 
> observe a few micro-second of engine busyness in the below check.

Ok, so with a gem_quiescent_gpu() we shouldn't need an arbitrary sleep,
as we are stating the gpu is then idle (ELSP drained etc).

> >> +       val2 = pmu_read_single(fd);
> >> +       usleep(batch_duration_ns / 1000);
> >> +       val2 = pmu_read_single(fd) - val2;
> > 
> > But the engine is idle at this point? Why sleep for execlists and not
> > legacy? Isn't the execlists case where we know that just by waiting for
> > idle (gem_quiescent_gpu() not an arbitrary usleep) then the pmu counter
> > is idle. It's legacy where we would have to wait for an sample interval
> > following gpu idle to be sure the pmu is idle.
> 
> Hm legacy and sampling.. I think we don't have to wait since there is no 
> context save there is the idleness comes straight with the user 
> interrupt, or no?

Hmm. Right. Since the sample after completion doesn't add any time to
the counter, you only need to ensure that pmu_read_single() is after the
seqno completion (i.e. gem_sync) to assert it should increment anymore.

Ok. I think you can just remove the 

> >> +       gem_sync(gem_fd, spin[0]->handle);
> >> +       gem_sync(gem_fd, spin[1]->handle);
> >> +
> >> +       /* Wait for context complete to avoid accounting residual busyness. */
> >> +       if (execlists)
> >> +               usleep(100000);

with a gem_quiescent_gpu() and everybody is happy.

With that tweak,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
though I'd be happy if the initial sleep(2) was unconditional (pending
suitable information exported from the kernel).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH i-g-t v2 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy
  2018-01-10 13:38     ` Chris Wilson
@ 2018-01-11  8:46       ` Tvrtko Ursulin
  0 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-01-11  8:46 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Make sure busyness is correctly reported when PMU is enabled after the
engine is already busy with a single long batch.

v2:
 * Make the sleep unconditional and use scientific notiation for large
   constants. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/perf_pmu.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 45e2f6148453..e1653b0367b8 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -157,6 +157,40 @@ single(int gem_fd, const struct intel_execution_engine2 *e, bool busy)
 	gem_quiescent_gpu(gem_fd);
 }
 
+static void
+busy_start(int gem_fd, const struct intel_execution_engine2 *e)
+{
+	unsigned long slept;
+	igt_spin_t *spin;
+	uint64_t val;
+	int fd;
+
+	/*
+	 * Defeat the busy stats delayed disable, we need to guarantee we are
+	 * the first user.
+	 */
+	sleep(2);
+
+	spin = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
+
+	/*
+	 * Sleep for a bit after making the engine busy to make sure the PMU
+	 * gets enabled when the batch is already running.
+	 */
+	usleep(500e3);
+
+	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
+
+	slept = measured_usleep(batch_duration_ns / 1000);
+	val = pmu_read_single(fd);
+
+	igt_spin_batch_free(gem_fd, spin);
+	close(fd);
+
+	assert_within_epsilon(val, slept, tolerance);
+	gem_quiescent_gpu(gem_fd);
+}
+
 static void log_busy(int fd, unsigned int num_engines, uint64_t *val)
 {
 	char buf[1024];
@@ -1164,6 +1198,13 @@ igt_main
 		 */
 		igt_subtest_f("multi-client-%s", e->name)
 			multi_client(fd, e);
+
+		/**
+		 * Check that reported usage is correct when PMU is enabled
+		 * after the batch is running.
+		 */
+		igt_subtest_f("busy-start-%s", e->name)
+			busy_start(fd, e);
 	}
 
 	/**
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH i-g-t v2 2/2] tests/perf_pmu: Exercise busy stats and lite-restore
  2018-01-10 13:45       ` Chris Wilson
@ 2018-01-11  8:46         ` Tvrtko Ursulin
  0 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2018-01-11  8:46 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

While developing a fix for an accounting hole in busy stats we realized
lite-restore is a potential edge case which would be interesting to check
is properly handled.

It is unfortnately quite timing sensitive to hit lite-restore in the
fashion test needs, so downside of this test is that it sufferes from a
high rate of false negatives.

v2:
 * Make the sleep unconditional and use scientific notiation for large
   constants. (Chris Wilson)
 * Use gem_quiscent_gpu instead of gem_sync+usleep to ensure context
   complete was received under execlists. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/perf_pmu.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index e1653b0367b8..2f7d33414a53 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -191,6 +191,71 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
 	gem_quiescent_gpu(gem_fd);
 }
 
+/*
+ * This test has a potentially low rate of catching the issue it is trying to
+ * catch. Or in other words, quite high rate of false negative successes. We
+ * will depend on the CI systems running it a lot to detect issues.
+ */
+static void
+busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
+{
+	unsigned long slept;
+	igt_spin_t *spin[2];
+	uint64_t val, val2;
+	uint32_t ctx;
+	int fd;
+
+	ctx = gem_context_create(gem_fd);
+
+	/*
+	 * Defeat the busy stats delayed disable, we need to guarantee we are
+	 * the first user.
+	 */
+	sleep(2);
+
+	/*
+	 * Submit two contexts, with a pause in between targeting the ELSP
+	 * re-submission in execlists mode. Make sure busyness is correctly
+	 * reported with the engine busy, and after the engine went idle.
+	 */
+	spin[0] = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
+	usleep(500e3);
+	spin[1] = __igt_spin_batch_new(gem_fd, ctx, e2ring(gem_fd, e), 0);
+
+	/*
+	 * Open PMU as fast as possible after the second spin batch in attempt
+	 * to be faster than the driver handling lite-restore.
+	 */
+	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
+
+	slept = measured_usleep(batch_duration_ns / 1000);
+	val = pmu_read_single(fd);
+
+	igt_spin_batch_end(spin[0]);
+	igt_spin_batch_end(spin[1]);
+
+	/* Wait for GPU idle to verify PMU reports idle. */
+	gem_quiescent_gpu(gem_fd);
+
+	val2 = pmu_read_single(fd);
+	usleep(batch_duration_ns / 1000);
+	val2 = pmu_read_single(fd) - val2;
+
+	igt_info("busy=%lu idle=%lu\n", val, val2);
+
+	igt_spin_batch_free(gem_fd, spin[0]);
+	igt_spin_batch_free(gem_fd, spin[1]);
+
+	close(fd);
+
+	gem_context_destroy(gem_fd, ctx);
+
+	assert_within_epsilon(val, slept, tolerance);
+	igt_assert_eq(val2, 0);
+
+	gem_quiescent_gpu(gem_fd);
+}
+
 static void log_busy(int fd, unsigned int num_engines, uint64_t *val)
 {
 	char buf[1024];
@@ -1205,6 +1270,13 @@ igt_main
 		 */
 		igt_subtest_f("busy-start-%s", e->name)
 			busy_start(fd, e);
+
+		/**
+		 * Check that reported usage is correct when PMU is enabled
+		 * after two batches are running.
+		 */
+		igt_subtest_f("busy-double-start-%s", e->name)
+			busy_double_start(fd, e);
 	}
 
 	/**
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy (rev3)
  2018-01-09 16:16 [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2018-01-09 21:28 ` [PATCH i-g-t 1/2] " Chris Wilson
@ 2018-01-11 13:21 ` Patchwork
  4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-01-11 13:21 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy (rev3)
URL   : https://patchwork.freedesktop.org/series/36201/
State : failure

== Summary ==

IGT patchset tested on top of latest successful build
d37369c7146a2ceb332592297d311d501c1c748e Revert "build: make meson more official" damage

with latest DRM-Tip kernel build CI_DRM_3621
d9bbaa0c5203 drm-tip: 2018y-01m-11d-12h-31m-18s UTC integration manifest

Testlist changes:
+igt@perf_pmu@busy-double-start-bcs0
+igt@perf_pmu@busy-double-start-rcs0
+igt@perf_pmu@busy-double-start-vcs0
+igt@perf_pmu@busy-double-start-vcs1
+igt@perf_pmu@busy-double-start-vecs0
+igt@perf_pmu@busy-start-bcs0
+igt@perf_pmu@busy-start-rcs0
+igt@perf_pmu@busy-start-vcs0
+igt@perf_pmu@busy-start-vcs1
+igt@perf_pmu@busy-start-vecs0

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test gem_ringfill:
        Subgroup basic-default:
                pass       -> INCOMPLETE (fi-cnl-y2)
Test kms_chamelium:
        Subgroup dp-edid-read:
                dmesg-warn -> PASS       (fi-kbl-7500u) fdo#102505

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:410s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:414s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:359s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:451s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:262s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:461s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:469s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:432s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:423s
fi-cnl-y2        total:142  pass:130  dwarn:0   dfail:0   fail:0   skip:11 
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:245s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:501s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:382s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:392s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:404s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:449s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:403s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:458s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:492s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:443s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:494s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:488s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:414s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:497s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:519s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:484s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:465s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:423s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:526s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:391s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:563s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:452s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_767/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-01-11 13:21 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09 16:16 [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Tvrtko Ursulin
2018-01-09 16:16 ` [PATCH i-g-t 2/2] tests/perf_pmu: Exercise busy stats and lite-restore Tvrtko Ursulin
2018-01-09 21:39   ` Chris Wilson
2018-01-10 10:37     ` Tvrtko Ursulin
2018-01-10 13:45       ` Chris Wilson
2018-01-11  8:46         ` [PATCH i-g-t v2 " Tvrtko Ursulin
2018-01-09 16:59 ` ✓ Fi.CI.BAT: success for series starting with [1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy Patchwork
2018-01-09 18:57 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-01-09 21:28 ` [PATCH i-g-t 1/2] " Chris Wilson
2018-01-10 10:42   ` Tvrtko Ursulin
2018-01-10 13:38     ` Chris Wilson
2018-01-11  8:46       ` [PATCH i-g-t v2 " Tvrtko Ursulin
2018-01-11 13:21 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy (rev3) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.