diff mbox

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

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

Commit Message

Lionel Landwerlin Aug. 4, 2017, 11:20 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

Chris Wilson Aug. 4, 2017, 11:44 a.m. UTC | #1
Quoting Lionel Landwerlin (2017-08-04 12:20:35)
> 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.

If clock frequency is a fundamental invariant required for the test,
set the clock frequency via sysfs:
	/sys/class/drm/card0/gt_(min|max|boost)_freq_mhz
-Chris
Lionel Landwerlin Aug. 4, 2017, 12:56 p.m. UTC | #2
On 04/08/17 12:44, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2017-08-04 12:20:35)
>> 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.
> If clock frequency is a fundamental invariant required for the test,
> set the clock frequency via sysfs:
> 	/sys/class/drm/card0/gt_(min|max|boost)_freq_mhz
> -Chris
>
>

I haven't tried setting the boost value, but settings min&max to the 
maximum the device can do didn't seem to make any difference.
Reading the current frequency file would sometimes report values 200Mhz 
below what was put in the min...
diff mbox

Patch

diff --git a/tests/perf.c b/tests/perf.c
index 5c7a2a34..f33ccffd 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2841,10 +2841,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
@@ -2864,18 +2872,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...
 		 */
@@ -2888,6 +2969,9 @@  test_enable_disable(void)
 	free(buf);
 
 	__perf_close(stream_fd);
+
+	load_helper_stop();
+	load_helper_deinit();
 }
 
 static void