diff mbox

[i-g-t,v5,07/11] tests/perf: make enable-disable more reliable

Message ID 20170831103515.21188-8-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lionel Landwerlin Aug. 31, 2017, 10:35 a.m. UTC
Estimation of the amount of reports can only refer to periodic ones,
as context switch reports completely depend on what happens on the
system. Also generate some load to prevent clock frequency changes to
impact our measurement.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 tests/perf.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 90 insertions(+), 6 deletions(-)

Comments

Matthew Auld Sept. 26, 2017, 5:43 p.m. UTC | #1
On 31 August 2017 at 11:35, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
> Estimation of the amount of reports can only refer to periodic ones,
> as context switch reports completely depend on what happens on the
> system. Also generate some load to prevent clock frequency changes to
> impact our measurement.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  tests/perf.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 6 deletions(-)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 15a43cf8..3b2e1763 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -2796,10 +2796,18 @@ test_enable_disable(void)
>         int n_full_oa_reports = oa_buf_size / report_size;
>         uint64_t fill_duration = n_full_oa_reports * oa_period;
>
> +       load_helper_init();
> +       load_helper_run(HIGH);
> +
>         stream_fd = __perf_open(drm_fd, &param);
>
>         for (int i = 0; i < 5; i++) {
>                 int len;
> +               uint32_t n_periodic_reports;
> +               struct drm_i915_perf_record_header *header;
> +               uint32_t first_timestamp = 0, last_timestamp = 0;
> +               uint32_t last_periodic_report[64];
> +               double tick_per_period;
>
>                 /* Giving enough time for an overflow might help catch whether
>                  * the OA unit has been enabled even if the driver might at
> @@ -2819,18 +2827,91 @@ test_enable_disable(void)
>
>                 nanosleep(&(struct timespec){ .tv_sec = 0,
>                                               .tv_nsec = fill_duration / 2 },
> -                         NULL);
> +                       NULL);
>
> -               while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR)
> -                       ;
> +               n_periodic_reports = 0;
>
> -               igt_assert_neq(len, -1);
> +               /* Because of the race condition between notification of new
> +                * reports and reports landing in memory, we need to rely on
> +                * timestamps to figure whether we've read enough of them.
> +                */
> +               while (((last_timestamp - first_timestamp) * oa_exponent_to_ns(oa_exponent)) <

(last_timestamp - first_timestamp) * oa_period

Reviewed-by: Matthew Auld <matthew.auld@intel.com>
diff mbox

Patch

diff --git a/tests/perf.c b/tests/perf.c
index 15a43cf8..3b2e1763 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2796,10 +2796,18 @@  test_enable_disable(void)
 	int n_full_oa_reports = oa_buf_size / report_size;
 	uint64_t fill_duration = n_full_oa_reports * oa_period;
 
+	load_helper_init();
+	load_helper_run(HIGH);
+
 	stream_fd = __perf_open(drm_fd, &param);
 
 	for (int i = 0; i < 5; i++) {
 		int len;
+		uint32_t n_periodic_reports;
+		struct drm_i915_perf_record_header *header;
+		uint32_t first_timestamp = 0, last_timestamp = 0;
+		uint32_t last_periodic_report[64];
+		double tick_per_period;
 
 		/* Giving enough time for an overflow might help catch whether
 		 * the OA unit has been enabled even if the driver might at
@@ -2819,18 +2827,91 @@  test_enable_disable(void)
 
 		nanosleep(&(struct timespec){ .tv_sec = 0,
 					      .tv_nsec = fill_duration / 2 },
-			  NULL);
+			NULL);
 
-		while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR)
-			;
+		n_periodic_reports = 0;
 
-		igt_assert_neq(len, -1);
+		/* Because of the race condition between notification of new
+		 * reports and reports landing in memory, we need to rely on
+		 * timestamps to figure whether we've read enough of them.
+		 */
+		while (((last_timestamp - first_timestamp) * oa_exponent_to_ns(oa_exponent)) <
+		       (fill_duration / 2)) {
 
-		igt_assert(len > report_size * n_full_oa_reports * 0.45);
-		igt_assert(len < report_size * n_full_oa_reports * 0.55);
+			while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR)
+				;
+
+			igt_assert_neq(len, -1);
+
+			for (int offset = 0; offset < len; offset += header->size) {
+				uint32_t *report;
+				double previous_tick_per_period;
+
+				header = (void *) (buf + offset);
+				report = (void *) (header + 1);
+
+				switch (header->type) {
+				case DRM_I915_PERF_RECORD_OA_REPORT_LOST:
+					break;
+				case DRM_I915_PERF_RECORD_SAMPLE:
+					if (first_timestamp == 0)
+						first_timestamp = report[1];
+					last_timestamp = report[1];
+
+					previous_tick_per_period = tick_per_period;
+
+					if (n_periodic_reports > 0 &&
+					    oa_report_is_periodic(oa_exponent, report)) {
+						tick_per_period =
+							oa_reports_tick_per_period(last_periodic_report,
+										   report);
+
+						if (!double_value_within(tick_per_period,
+									 previous_tick_per_period, 5))
+							igt_debug("clock change!\n");
+
+						igt_debug(" > report ts=%u"
+							  " ts_delta_last_periodic=%8u is_timer=%i ctx_id=%8x gpu_ticks=%u nb_periodic=%u\n",
+							  report[1],
+							  report[1] - last_periodic_report[1],
+							  oa_report_is_periodic(oa_exponent, report),
+							  oa_report_get_ctx_id(report),
+							  report[3] - last_periodic_report[3],
+							  n_periodic_reports);
+
+						memcpy(last_periodic_report, report,
+						       sizeof(last_periodic_report));
+					}
+
+					/* We want to measure only the periodic
+					 * reports, ctx-switch might inflate the
+					 * content of the buffer and skew or
+					 * measurement.
+					 */
+					n_periodic_reports +=
+						oa_report_is_periodic(oa_exponent, report);
+					break;
+				case DRM_I915_PERF_RECORD_OA_BUFFER_LOST:
+					igt_assert(!"unexpected overflow");
+					break;
+				}
+			}
+
+		}
 
 		do_ioctl(stream_fd, I915_PERF_IOCTL_DISABLE, 0);
 
+		igt_debug("%f < %lu < %f\n",
+			  report_size * n_full_oa_reports * 0.45,
+			  n_periodic_reports * report_size,
+			  report_size * n_full_oa_reports * 0.55);
+
+		igt_assert((n_periodic_reports * report_size) >
+			   (report_size * n_full_oa_reports * 0.45));
+		igt_assert((n_periodic_reports * report_size) <
+			   report_size * n_full_oa_reports * 0.55);
+
+
 		/* It's considered an error to read a stream while it's disabled
 		 * since it would block indefinitely...
 		 */
@@ -2843,6 +2924,9 @@  test_enable_disable(void)
 	free(buf);
 
 	__perf_close(stream_fd);
+
+	load_helper_stop();
+	load_helper_fini();
 }
 
 static void