[PATCH igt 1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe

[PATCH igt 1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe

[Chris Wilson]

Instead of trying to sleep for 2 evaluations intervals and then assuming
that rc6 is working, poll the rc6 residency instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=104099
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/pm_rc6_residency.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c
index 3f6860199..16f4b1421 100644
--- a/tests/pm_rc6_residency.c
+++ b/tests/pm_rc6_residency.c
@@ -122,9 +122,6 @@ static void measure_residencies(int devid, unsigned int mask,
 	struct residencies end = { };
 	int retry;
 
-	if (!mask)
-		return;
-
 	/*
 	 * Retry in case of counter wrap-around. We simply re-run the
 	 * measurement, since the valid counter range is different on
@@ -168,17 +165,18 @@ static void measure_residencies(int devid, unsigned int mask,
 	res->rc6 += res->rc6p;
 }
 
-static unsigned long rc6_enable_us(void)
+static bool wait_for_rc6(void)
 {
-	/*
-	 * To know how long we need to wait for the device to enter rc6 once
-	 * idle, we need to look at GEN6_RC_EVALUATION_INTERVAL. Currently,
-	 * this is set to 125000 (12500 * 1280ns or 0.16s) on all platforms.
-	 * We must complete at least one EI with activity below the
-	 * per-platform threshold for RC6 to kick. Therefore, we must wait
-	 * at least 2 EI cycles, before we can expect rc6 to start ticking.
-	 */
-	return 2 * 160 * 1000;
+	struct timespec tv = {};
+	unsigned long start, now;
+
+	start = read_rc6_residency("rc6");
+	do {
+		usleep(50);
+		now = read_rc6_residency("rc6");
+	} while (now == start && !igt_seconds_elapsed(&tv));
+
+	return now != start;
 }
 
 igt_main
@@ -198,19 +196,17 @@ igt_main
 
 		/* Make sure rc6 counters are running */
 		igt_drop_caches_set(fd, DROP_IDLE);
-		usleep(rc6_enable_us());
+		igt_require(wait_for_rc6());
 
 		close(fd);
 
 		rc6_enabled = get_rc6_enabled_mask();
-		igt_require(rc6_enabled);
+		igt_require(rc6_enabled & RC6_ENABLED);
 	}
 
 	igt_subtest("rc6-accuracy") {
 		struct residencies res;
 
-		igt_require(rc6_enabled & RC6_ENABLED);
-
 		measure_residencies(devid, rc6_enabled, &res);
 		residency_accuracy(res.rc6, res.duration, "rc6");
 	}
@@ -218,8 +214,7 @@ igt_main
 	igt_subtest("media-rc6-accuracy") {
 		struct residencies res;
 
-		igt_require((rc6_enabled & RC6_ENABLED) &&
-			    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)));
+		igt_require(IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid));
 
 		measure_residencies(devid, rc6_enabled, &res);
 		residency_accuracy(res.media_rc6, res.duration, "media_rc6");
-- 
2.15.1

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

[PATCH igt 2/2] igt/perf_pmu: Replace hard-coded sleep before rc6 with a probe

[Chris Wilson]

Instead of trying to sleep for 2 evaluations intervals and then assuming
that rc6 is working, poll the rc6 residency instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=103929
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/perf_pmu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index e872f4e55..65bc734da 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1008,6 +1008,20 @@ static unsigned long rc6_enable_us(void)
 	return 2 * 160 * 1000;
 }
 
+static bool wait_for_rc6(int fd)
+{
+	struct timespec tv = {};
+	uint64_t start, now;
+
+	start = pmu_read_single(fd);
+	do {
+		usleep(50);
+		now = pmu_read_single(fd);
+	} while (start == now && !igt_seconds_elapsed(&tv));
+
+	return start != now;
+}
+
 static void
 test_rc6(int gem_fd)
 {
@@ -1019,7 +1033,7 @@ test_rc6(int gem_fd)
 	fd = open_pmu(I915_PMU_RC6_RESIDENCY);
 
 	gem_quiescent_gpu(gem_fd);
-	usleep(rc6_enable_us()); /* wait for the rc6 cycle counter to kick in */
+	igt_require(wait_for_rc6(fd));
 
 	/* Go idle and check full RC6. */
 	prev = pmu_read_single(fd);
-- 
2.15.1

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

✓ Fi.CI.BAT: success for series starting with [1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe

[Patchwork]

== Series Details ==

Series: series starting with [1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe
URL   : https://patchwork.freedesktop.org/series/34897/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
ece35d37e6b0afc0ba99f66179279960042c87bc tests/gem_seqno_wrap: Drop gem_seqno_wrap.c

with latest DRM-Tip kernel build CI_DRM_3456
84cd3d972bdd drm-tip: 2017y-12m-05d-08h-41m-59s UTC integration manifest

No testlist changes.

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test kms_chamelium:
        Subgroup dp-crc-fast:
                dmesg-fail -> PASS       (fi-kbl-7500u) fdo#103841
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-hsw-4770) fdo#103375

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:389s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:518s
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:508s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:495s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:481s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:271s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:544s
fi-hsw-4770      total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:23 
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:260s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:395s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:473s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:447s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:491s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:528s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:529s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:591s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:538s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:519s
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:447s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:562s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:422s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:616s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:620s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s

== Logs ==

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

Re: [PATCH igt 2/2] igt/perf_pmu: Replace hard-coded sleep before rc6 with a probe

[Tvrtko Ursulin]

On 05/12/2017 10:56, Chris Wilson wrote:
> Instead of trying to sleep for 2 evaluations intervals and then assuming
> that rc6 is working, poll the rc6 residency instead.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=103929
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/perf_pmu.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index e872f4e55..65bc734da 100644
> --- a/tests/perf_pmu.c
> +++ b/tests/perf_pmu.c
> @@ -1008,6 +1008,20 @@ static unsigned long rc6_enable_us(void)
>   	return 2 * 160 * 1000;
>   }
>   
> +static bool wait_for_rc6(int fd)
> +{
> +	struct timespec tv = {};
> +	uint64_t start, now;
> +
> +	start = pmu_read_single(fd);
> +	do {
> +		usleep(50);

Not needlessly fast?

> +		now = pmu_read_single(fd);
> +	} while (start == now && !igt_seconds_elapsed(&tv));

So up to one second wait.

> +
> +	return start != now;
> +}
> +
>   static void
>   test_rc6(int gem_fd)
>   {
> @@ -1019,7 +1033,7 @@ test_rc6(int gem_fd)
>   	fd = open_pmu(I915_PMU_RC6_RESIDENCY);
>   
>   	gem_quiescent_gpu(gem_fd);
> -	usleep(rc6_enable_us()); /* wait for the rc6 cycle counter to kick in */
> +	igt_require(wait_for_rc6(fd));

Eliminate now unused rc6_enable_us() ?

>   
>   	/* Go idle and check full RC6. */
>   	prev = pmu_read_single(fd);
> 

With dead-code removed:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH igt 1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe

[Tvrtko Ursulin]

On 05/12/2017 10:56, Chris Wilson wrote:
> Instead of trying to sleep for 2 evaluations intervals and then assuming
> that rc6 is working, poll the rc6 residency instead.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104099
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/pm_rc6_residency.c | 33 ++++++++++++++-------------------
>   1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c
> index 3f6860199..16f4b1421 100644
> --- a/tests/pm_rc6_residency.c
> +++ b/tests/pm_rc6_residency.c
> @@ -122,9 +122,6 @@ static void measure_residencies(int devid, unsigned int mask,
>   	struct residencies end = { };
>   	int retry;
>   
> -	if (!mask)
> -		return;
> -
>   	/*
>   	 * Retry in case of counter wrap-around. We simply re-run the
>   	 * measurement, since the valid counter range is different on
> @@ -168,17 +165,18 @@ static void measure_residencies(int devid, unsigned int mask,
>   	res->rc6 += res->rc6p;
>   }
>   
> -static unsigned long rc6_enable_us(void)
> +static bool wait_for_rc6(void)
>   {
> -	/*
> -	 * To know how long we need to wait for the device to enter rc6 once
> -	 * idle, we need to look at GEN6_RC_EVALUATION_INTERVAL. Currently,
> -	 * this is set to 125000 (12500 * 1280ns or 0.16s) on all platforms.
> -	 * We must complete at least one EI with activity below the
> -	 * per-platform threshold for RC6 to kick. Therefore, we must wait
> -	 * at least 2 EI cycles, before we can expect rc6 to start ticking.
> -	 */
> -	return 2 * 160 * 1000;
> +	struct timespec tv = {};
> +	unsigned long start, now;
> +
> +	start = read_rc6_residency("rc6");
> +	do {
> +		usleep(50);
> +		now = read_rc6_residency("rc6");
> +	} while (now == start && !igt_seconds_elapsed(&tv));
> +
> +	return now != start;
>   }
>   
>   igt_main
> @@ -198,19 +196,17 @@ igt_main
>   
>   		/* Make sure rc6 counters are running */
>   		igt_drop_caches_set(fd, DROP_IDLE);
> -		usleep(rc6_enable_us());
> +		igt_require(wait_for_rc6());
>   
>   		close(fd);
>   
>   		rc6_enabled = get_rc6_enabled_mask();
> -		igt_require(rc6_enabled);
> +		igt_require(rc6_enabled & RC6_ENABLED);
>   	}
>   
>   	igt_subtest("rc6-accuracy") {
>   		struct residencies res;
>   
> -		igt_require(rc6_enabled & RC6_ENABLED);
> -
>   		measure_residencies(devid, rc6_enabled, &res);
>   		residency_accuracy(res.rc6, res.duration, "rc6");
>   	}
> @@ -218,8 +214,7 @@ igt_main
>   	igt_subtest("media-rc6-accuracy") {
>   		struct residencies res;
>   
> -		igt_require((rc6_enabled & RC6_ENABLED) &&
> -			    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)));
> +		igt_require(IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid));
>   
>   		measure_residencies(devid, rc6_enabled, &res);
>   		residency_accuracy(res.media_rc6, res.duration, "media_rc6");
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH igt 2/2] igt/perf_pmu: Replace hard-coded sleep before rc6 with a probe

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-12-05 12:05:14)
> 
> On 05/12/2017 10:56, Chris Wilson wrote:
> > Instead of trying to sleep for 2 evaluations intervals and then assuming
> > that rc6 is working, poll the rc6 residency instead.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=103929
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   tests/perf_pmu.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> > index e872f4e55..65bc734da 100644
> > --- a/tests/perf_pmu.c
> > +++ b/tests/perf_pmu.c
> > @@ -1008,6 +1008,20 @@ static unsigned long rc6_enable_us(void)
> >       return 2 * 160 * 1000;
> >   }
> >   
> > +static bool wait_for_rc6(int fd)
> > +{
> > +     struct timespec tv = {};
> > +     uint64_t start, now;
> > +
> > +     start = pmu_read_single(fd);
> > +     do {
> > +             usleep(50);
> 
> Not needlessly fast?

Since we expect the EI to be 160us, I was erring on the side of
optimism. i.e. if the idling took long enough, rc6 would be close to
starting on our return. However, the code is trying to be safe just in
case, for whatever unknown reason, it takes longer. This pair of tests,
I am assuming are only concerned with the accuracy of the counter rather
than investigating kernel/device behaviour. (We should have some other
pm_rc6 tests that try to check that rc6 kicks off in a timely manner,
but we don't publish the expectations for rc6 behaviour. Not sure?)
 
> > +             now = pmu_read_single(fd);
> > +     } while (start == now && !igt_seconds_elapsed(&tv));
> 
> So up to one second wait.

Expectation is that it should only take 320us for it to kick off, so 1s
seems a reasonable upper bound. Maybe 2s?
 
> > +
> > +     return start != now;
> > +}
> > +
> >   static void
> >   test_rc6(int gem_fd)
> >   {
> > @@ -1019,7 +1033,7 @@ test_rc6(int gem_fd)
> >       fd = open_pmu(I915_PMU_RC6_RESIDENCY);
> >   
> >       gem_quiescent_gpu(gem_fd);
> > -     usleep(rc6_enable_us()); /* wait for the rc6 cycle counter to kick in */
> > +     igt_require(wait_for_rc6(fd));
> 
> Eliminate now unused rc6_enable_us() ?

I thought I did, my mistake.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for series starting with [1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe

[Patchwork]

== Series Details ==

Series: series starting with [1/2] igt/pm_rc6_residency: Replace hard-coded sleep before rc6 with a probe
URL   : https://patchwork.freedesktop.org/series/34897/
State : warning

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                fail       -> PASS       (shard-snb) fdo#101623
Test kms_chv_cursor_fail:
        Subgroup pipe-a-128x128-bottom-edge:
                pass       -> SKIP       (shard-hsw)
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-snb) fdo#102707
Test kms_flip:
        Subgroup plain-flip-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368

shard-hsw        total:2605 pass:1487 dwarn:1   dfail:0   fail:10  skip:1106 time:9097s
shard-snb        total:2679 pass:1308 dwarn:2   dfail:0   fail:11  skip:1358 time:8128s
Blacklisted hosts:
shard-apl        total:2657 pass:1654 dwarn:3   dfail:0   fail:22  skip:977 time:13324s
shard-kbl        total:2679 pass:1794 dwarn:2   dfail:0   fail:23  skip:860 time:10853s

== Logs ==

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