All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
@ 2020-03-13 17:42 Umesh Nerlige Ramappa
  2020-03-13 17:51 ` Chris Wilson
  2020-03-16  9:52 ` Lionel Landwerlin
  0 siblings, 2 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-03-13 17:42 UTC (permalink / raw)
  To: igt-dev, Chris Wilson, Lionel G Landwerlin

Run 2 blocking tests back to back and compare the number of OA reports
captured. Make sure the number of reports are within an acceptable range
of the expected number of reports.

v2: Drop poll and use blocking IO (Lionel)
v3: Verify number of expected reports for both tests (Lionel)

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 tests/perf.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/tests/perf.c b/tests/perf.c
index 5e818030..b757e307 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2265,6 +2265,92 @@ test_polling(void)
 	__perf_close(stream_fd);
 }
 
+static int
+num_valid_reports_captured(struct drm_i915_perf_open_param *param,
+			   int64_t *duration_ns)
+{
+	uint8_t buf[1024 * 1024];
+	int64_t start, end;
+	int num_reports = 0;
+
+	igt_debug("Expected duration = %lu\n", *duration_ns);
+
+	stream_fd = __perf_open(drm_fd, param, true);
+
+	start = get_time();
+	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
+	for (/* nop */; ((end = get_time()) - start) < *duration_ns; /* nop */) {
+		struct drm_i915_perf_record_header *header;
+		int ret;
+
+		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
+		       errno == EINTR)
+			;
+
+		igt_assert(ret > 0);
+
+		for (int offset = 0; offset < ret; offset += header->size) {
+			header = (void *)(buf + offset);
+
+			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
+				uint32_t *report = (void *)(header + 1);
+
+				if ((report[0] >> OAREPORT_REASON_SHIFT) &
+				    OAREPORT_REASON_TIMER)
+					num_reports++;
+			}
+		}
+	}
+	__perf_close(stream_fd);
+
+	*duration_ns = end - start;
+
+	igt_debug("Actual duration = %lu\n", *duration_ns);
+
+	return num_reports;
+}
+
+static void
+gen12_test_oa_tlb_invalidate(void)
+{
+	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
+	uint64_t properties[] = {
+		DRM_I915_PERF_PROP_SAMPLE_OA, true,
+
+		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
+		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
+		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
+	};
+	struct drm_i915_perf_open_param param = {
+		.flags = I915_PERF_FLAG_FD_CLOEXEC |
+			I915_PERF_FLAG_DISABLED,
+		.num_properties = sizeof(properties) / 16,
+		.properties_ptr = to_user_pointer(properties),
+	};
+	int num_reports1, num_reports2, num_expected_reports;
+	int64_t duration;
+	
+	/* Capture reports for 5 seconds twice and then make sure you get around
+	 * the same number of reports. In the case of failure, the number of
+	 * reports will vary largely since the beginning of the OA buffer
+	 * will have invalid entries.
+	 */
+	duration = 5LL * NSEC_PER_SEC;
+	num_reports1 = num_valid_reports_captured(&param, &duration);
+	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
+	igt_debug("expected num reports = %d\n", num_expected_reports);
+	igt_debug("actual num reports = %d\n", num_reports1);
+	igt_assert(num_reports1 > 0.95 * num_expected_reports);
+
+	duration = 5LL * NSEC_PER_SEC;
+	num_reports2 = num_valid_reports_captured(&param, &duration);
+	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
+	igt_debug("expected num reports = %d\n", num_expected_reports);
+	igt_debug("actual num reports = %d\n", num_reports2);
+	igt_assert(num_reports2 > 0.95 * num_expected_reports);
+}
+
+
 static void
 test_buffer_fill(void)
 {
@@ -4622,6 +4708,12 @@ igt_main
 		gen8_test_single_ctx_render_target_writes_a_counter();
 	}
 
+	igt_describe("Test OA TLB invalidate");
+	igt_subtest("gen12-oa-tlb-invalidate") {
+		igt_require(intel_gen(devid) >= 12);
+		gen12_test_oa_tlb_invalidate();
+	}
+
 	igt_describe("Measure performance for a specific context using OAR in Gen 12");
 	igt_subtest("gen12-unprivileged-single-ctx-counters") {
 		igt_require(intel_gen(devid) >= 12);
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-13 17:42 [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation Umesh Nerlige Ramappa
@ 2020-03-13 17:51 ` Chris Wilson
  2020-03-16  9:52 ` Lionel Landwerlin
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2020-03-13 17:51 UTC (permalink / raw)
  To: Lionel G Landwerlin, Umesh Nerlige Ramappa, igt-dev

Quoting Umesh Nerlige Ramappa (2020-03-13 17:42:35)
> @@ -4622,6 +4708,12 @@ igt_main
>                 gen8_test_single_ctx_render_target_writes_a_counter();
>         }
>  
> +       igt_describe("Test OA TLB invalidate");
> +       igt_subtest("gen12-oa-tlb-invalidate") {
> +               igt_require(intel_gen(devid) >= 12);
> +               gen12_test_oa_tlb_invalidate();
> +       }
> +
>         igt_describe("Measure performance for a specific context using OAR in Gen 12");
>         igt_subtest("gen12-unprivileged-single-ctx-counters") {
>                 igt_require(intel_gen(devid) >= 12);

Something you might like to add is:

	igt_subtest_group {
		igt_fixture igt_require(intel_gen(devid) >= 12);

		igt_describe("Test OA TLB invalidate");
		igt_subtest("gen12-oa-tlb-invalidate")
			gen12_test_oa_tlb_invalidate();

		igt_describe("Measure performance for a specific context using OAR in Gen 12");
		igt_subtest("gen12-unprivileged-single-ctx-counters")
		...
	}

To combat the redundancy and add some visual grouping.
-Chris
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-13 17:42 [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation Umesh Nerlige Ramappa
  2020-03-13 17:51 ` Chris Wilson
@ 2020-03-16  9:52 ` Lionel Landwerlin
  1 sibling, 0 replies; 11+ messages in thread
From: Lionel Landwerlin @ 2020-03-16  9:52 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa, igt-dev, Chris Wilson

On 13/03/2020 19:42, Umesh Nerlige Ramappa wrote:
> Run 2 blocking tests back to back and compare the number of OA reports
> captured. Make sure the number of reports are within an acceptable range
> of the expected number of reports.
>
> v2: Drop poll and use blocking IO (Lionel)
> v3: Verify number of expected reports for both tests (Lionel)
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   tests/perf.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 92 insertions(+)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 5e818030..b757e307 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -2265,6 +2265,92 @@ test_polling(void)
>   	__perf_close(stream_fd);
>   }
>   
> +static int
> +num_valid_reports_captured(struct drm_i915_perf_open_param *param,
> +			   int64_t *duration_ns)
> +{
> +	uint8_t buf[1024 * 1024];
> +	int64_t start, end;
> +	int num_reports = 0;
> +
> +	igt_debug("Expected duration = %lu\n", *duration_ns);
> +
> +	stream_fd = __perf_open(drm_fd, param, true);
> +
> +	start = get_time();
> +	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
> +	for (/* nop */; ((end = get_time()) - start) < *duration_ns; /* nop */) {
> +		struct drm_i915_perf_record_header *header;
> +		int ret;
> +
> +		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
> +		       errno == EINTR)
> +			;
> +
> +		igt_assert(ret > 0);
> +
> +		for (int offset = 0; offset < ret; offset += header->size) {
> +			header = (void *)(buf + offset);
> +
> +			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
> +				uint32_t *report = (void *)(header + 1);
> +
> +				if ((report[0] >> OAREPORT_REASON_SHIFT) &
> +				    OAREPORT_REASON_TIMER)
> +					num_reports++;
> +			}
> +		}
> +	}
> +	__perf_close(stream_fd);
> +
> +	*duration_ns = end - start;
> +
> +	igt_debug("Actual duration = %lu\n", *duration_ns);
> +
> +	return num_reports;
> +}
> +
> +static void
> +gen12_test_oa_tlb_invalidate(void)
> +{
> +	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
> +	uint64_t properties[] = {
> +		DRM_I915_PERF_PROP_SAMPLE_OA, true,
> +
> +		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
> +		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
> +		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
> +	};
> +	struct drm_i915_perf_open_param param = {
> +		.flags = I915_PERF_FLAG_FD_CLOEXEC |
> +			I915_PERF_FLAG_DISABLED,
> +		.num_properties = sizeof(properties) / 16,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +	int num_reports1, num_reports2, num_expected_reports;
> +	int64_t duration;
> +	
> +	/* Capture reports for 5 seconds twice and then make sure you get around
> +	 * the same number of reports. In the case of failure, the number of
> +	 * reports will vary largely since the beginning of the OA buffer
> +	 * will have invalid entries.
> +	 */
> +	duration = 5LL * NSEC_PER_SEC;
> +	num_reports1 = num_valid_reports_captured(&param, &duration);
> +	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
> +	igt_debug("expected num reports = %d\n", num_expected_reports);
> +	igt_debug("actual num reports = %d\n", num_reports1);
> +	igt_assert(num_reports1 > 0.95 * num_expected_reports);
> +
> +	duration = 5LL * NSEC_PER_SEC;
> +	num_reports2 = num_valid_reports_captured(&param, &duration);
> +	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
> +	igt_debug("expected num reports = %d\n", num_expected_reports);
> +	igt_debug("actual num reports = %d\n", num_reports2);
> +	igt_assert(num_reports2 > 0.95 * num_expected_reports);
> +}
> +
> +
>   static void
>   test_buffer_fill(void)
>   {
> @@ -4622,6 +4708,12 @@ igt_main
>   		gen8_test_single_ctx_render_target_writes_a_counter();
>   	}
>   
> +	igt_describe("Test OA TLB invalidate");
> +	igt_subtest("gen12-oa-tlb-invalidate") {
> +		igt_require(intel_gen(devid) >= 12);
> +		gen12_test_oa_tlb_invalidate();
> +	}
> +
>   	igt_describe("Measure performance for a specific context using OAR in Gen 12");
>   	igt_subtest("gen12-unprivileged-single-ctx-counters") {
>   		igt_require(intel_gen(devid) >= 12);


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-14  0:00 Umesh Nerlige Ramappa
@ 2020-03-14 10:36 ` Lionel Landwerlin
  0 siblings, 0 replies; 11+ messages in thread
From: Lionel Landwerlin @ 2020-03-14 10:36 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa, igt-dev, Chris Wilson

On 14/03/2020 02:00, Umesh Nerlige Ramappa wrote:
> Run 2 blocking tests back to back and compare the number of OA reports
> captured. Make sure the number of reports are within an acceptable range
> of the expected number of reports.
>
> v2: Drop poll and use blocking IO (Lionel)
> v3: Verify number of expected reports for both tests (Lionel)
> v4: Group gen12 tests under igt_subtest_group (Chris)
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   tests/perf.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 100 insertions(+), 10 deletions(-)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 5e818030..724f6f80 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -2265,6 +2265,92 @@ test_polling(void)
>   	__perf_close(stream_fd);
>   }
>   
> +static int
> +num_valid_reports_captured(struct drm_i915_perf_open_param *param,
> +			   int64_t *duration_ns)
> +{
> +	uint8_t buf[1024 * 1024];
> +	int64_t start, end;
> +	int num_reports = 0;
> +
> +	igt_debug("Expected duration = %lu\n", *duration_ns);
> +
> +	stream_fd = __perf_open(drm_fd, param, true);
> +
> +	start = get_time();
> +	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
> +	for (/* nop */; ((end = get_time()) - start) < *duration_ns; /* nop */) {
> +		struct drm_i915_perf_record_header *header;
> +		int ret;
> +
> +		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
> +		       errno == EINTR)
> +			;
> +
> +		igt_assert(ret > 0);
> +
> +		for (int offset = 0; offset < ret; offset += header->size) {
> +			header = (void *)(buf + offset);
> +
> +			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
> +				uint32_t *report = (void *)(header + 1);
> +
> +				if ((report[0] >> OAREPORT_REASON_SHIFT) &
> +				    OAREPORT_REASON_TIMER)
> +					num_reports++;
> +			}
> +		}
> +	}
> +	__perf_close(stream_fd);
> +
> +	*duration_ns = end - start;
> +
> +	igt_debug("Actual duration = %lu\n", *duration_ns);
> +
> +	return num_reports;
> +}
> +
> +static void
> +gen12_test_oa_tlb_invalidate(void)
> +{
> +	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
> +	uint64_t properties[] = {
> +		DRM_I915_PERF_PROP_SAMPLE_OA, true,
> +
> +		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
> +		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
> +		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
> +	};
> +	struct drm_i915_perf_open_param param = {
> +		.flags = I915_PERF_FLAG_FD_CLOEXEC |
> +			I915_PERF_FLAG_DISABLED,
> +		.num_properties = sizeof(properties) / 16,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +	int num_reports1, num_reports2, num_expected_reports;
> +	int64_t duration;
> +
> +	/* Capture reports for 5 seconds twice and then make sure you get around
> +	 * the same number of reports. In the case of failure, the number of
> +	 * reports will vary largely since the beginning of the OA buffer
> +	 * will have invalid entries.
> +	 */
> +	duration = 5LL * NSEC_PER_SEC;
> +	num_reports1 = num_valid_reports_captured(&param, &duration);
> +	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
> +	igt_debug("expected num reports = %d\n", num_expected_reports);
> +	igt_debug("actual num reports = %d\n", num_reports1);
> +	igt_assert(num_reports1 > 0.95 * num_expected_reports);
> +
> +	duration = 5LL * NSEC_PER_SEC;
> +	num_reports2 = num_valid_reports_captured(&param, &duration);
> +	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
> +	igt_debug("expected num reports = %d\n", num_expected_reports);
> +	igt_debug("actual num reports = %d\n", num_reports2);
> +	igt_assert(num_reports2 > 0.95 * num_expected_reports);
> +}
> +
> +
>   static void
>   test_buffer_fill(void)
>   {
> @@ -4598,12 +4684,6 @@ igt_main
>   		test_mi_rpc();
>   	}
>   
> -	igt_describe("Test MI REPORT PERF COUNT for Gen 12");
> -	igt_subtest("gen12-mi-rpc") {
> -		igt_require(intel_gen(devid) >= 12);
> -		gen12_test_mi_rpc();
> -	}
> -
>   	igt_subtest("unprivileged-single-ctx-counters") {
>   		igt_require(IS_HASWELL(devid));
>   		hsw_test_single_ctx_counters();
> @@ -4622,10 +4702,20 @@ igt_main
>   		gen8_test_single_ctx_render_target_writes_a_counter();
>   	}
>   
> -	igt_describe("Measure performance for a specific context using OAR in Gen 12");
> -	igt_subtest("gen12-unprivileged-single-ctx-counters") {
> -		igt_require(intel_gen(devid) >= 12);
> -		gen12_test_single_ctx_render_target_writes_a_counter();
> +	igt_subtest_group {
> +		igt_fixture igt_require(intel_gen(devid) >= 12);
> +
> +		igt_describe("Test MI REPORT PERF COUNT for Gen 12");
> +		igt_subtest("gen12-mi-rpc")
> +			gen12_test_mi_rpc();
> +
> +		igt_describe("Test OA TLB invalidate");
> +		igt_subtest("gen12-oa-tlb-invalidate")
> +			gen12_test_oa_tlb_invalidate();
> +
> +		igt_describe("Measure performance for a specific context using OAR in Gen 12");
> +		igt_subtest("gen12-unprivileged-single-ctx-counters")
> +			gen12_test_single_ctx_render_target_writes_a_counter();
>   	}
>   
>   	igt_subtest("rc6-disable")


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
@ 2020-03-14  0:00 Umesh Nerlige Ramappa
  2020-03-14 10:36 ` Lionel Landwerlin
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-03-14  0:00 UTC (permalink / raw)
  To: igt-dev, Lionel G Landwerlin, Chris Wilson

Run 2 blocking tests back to back and compare the number of OA reports
captured. Make sure the number of reports are within an acceptable range
of the expected number of reports.

v2: Drop poll and use blocking IO (Lionel)
v3: Verify number of expected reports for both tests (Lionel)
v4: Group gen12 tests under igt_subtest_group (Chris)

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 tests/perf.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 100 insertions(+), 10 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 5e818030..724f6f80 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2265,6 +2265,92 @@ test_polling(void)
 	__perf_close(stream_fd);
 }
 
+static int
+num_valid_reports_captured(struct drm_i915_perf_open_param *param,
+			   int64_t *duration_ns)
+{
+	uint8_t buf[1024 * 1024];
+	int64_t start, end;
+	int num_reports = 0;
+
+	igt_debug("Expected duration = %lu\n", *duration_ns);
+
+	stream_fd = __perf_open(drm_fd, param, true);
+
+	start = get_time();
+	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
+	for (/* nop */; ((end = get_time()) - start) < *duration_ns; /* nop */) {
+		struct drm_i915_perf_record_header *header;
+		int ret;
+
+		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
+		       errno == EINTR)
+			;
+
+		igt_assert(ret > 0);
+
+		for (int offset = 0; offset < ret; offset += header->size) {
+			header = (void *)(buf + offset);
+
+			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
+				uint32_t *report = (void *)(header + 1);
+
+				if ((report[0] >> OAREPORT_REASON_SHIFT) &
+				    OAREPORT_REASON_TIMER)
+					num_reports++;
+			}
+		}
+	}
+	__perf_close(stream_fd);
+
+	*duration_ns = end - start;
+
+	igt_debug("Actual duration = %lu\n", *duration_ns);
+
+	return num_reports;
+}
+
+static void
+gen12_test_oa_tlb_invalidate(void)
+{
+	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
+	uint64_t properties[] = {
+		DRM_I915_PERF_PROP_SAMPLE_OA, true,
+
+		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
+		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
+		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
+	};
+	struct drm_i915_perf_open_param param = {
+		.flags = I915_PERF_FLAG_FD_CLOEXEC |
+			I915_PERF_FLAG_DISABLED,
+		.num_properties = sizeof(properties) / 16,
+		.properties_ptr = to_user_pointer(properties),
+	};
+	int num_reports1, num_reports2, num_expected_reports;
+	int64_t duration;
+
+	/* Capture reports for 5 seconds twice and then make sure you get around
+	 * the same number of reports. In the case of failure, the number of
+	 * reports will vary largely since the beginning of the OA buffer
+	 * will have invalid entries.
+	 */
+	duration = 5LL * NSEC_PER_SEC;
+	num_reports1 = num_valid_reports_captured(&param, &duration);
+	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
+	igt_debug("expected num reports = %d\n", num_expected_reports);
+	igt_debug("actual num reports = %d\n", num_reports1);
+	igt_assert(num_reports1 > 0.95 * num_expected_reports);
+
+	duration = 5LL * NSEC_PER_SEC;
+	num_reports2 = num_valid_reports_captured(&param, &duration);
+	num_expected_reports = duration / oa_exponent_to_ns(oa_exponent);
+	igt_debug("expected num reports = %d\n", num_expected_reports);
+	igt_debug("actual num reports = %d\n", num_reports2);
+	igt_assert(num_reports2 > 0.95 * num_expected_reports);
+}
+
+
 static void
 test_buffer_fill(void)
 {
@@ -4598,12 +4684,6 @@ igt_main
 		test_mi_rpc();
 	}
 
-	igt_describe("Test MI REPORT PERF COUNT for Gen 12");
-	igt_subtest("gen12-mi-rpc") {
-		igt_require(intel_gen(devid) >= 12);
-		gen12_test_mi_rpc();
-	}
-
 	igt_subtest("unprivileged-single-ctx-counters") {
 		igt_require(IS_HASWELL(devid));
 		hsw_test_single_ctx_counters();
@@ -4622,10 +4702,20 @@ igt_main
 		gen8_test_single_ctx_render_target_writes_a_counter();
 	}
 
-	igt_describe("Measure performance for a specific context using OAR in Gen 12");
-	igt_subtest("gen12-unprivileged-single-ctx-counters") {
-		igt_require(intel_gen(devid) >= 12);
-		gen12_test_single_ctx_render_target_writes_a_counter();
+	igt_subtest_group {
+		igt_fixture igt_require(intel_gen(devid) >= 12);
+
+		igt_describe("Test MI REPORT PERF COUNT for Gen 12");
+		igt_subtest("gen12-mi-rpc")
+			gen12_test_mi_rpc();
+
+		igt_describe("Test OA TLB invalidate");
+		igt_subtest("gen12-oa-tlb-invalidate")
+			gen12_test_oa_tlb_invalidate();
+
+		igt_describe("Measure performance for a specific context using OAR in Gen 12");
+		igt_subtest("gen12-unprivileged-single-ctx-counters")
+			gen12_test_single_ctx_render_target_writes_a_counter();
 	}
 
 	igt_subtest("rc6-disable")
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-10 19:00 Umesh Nerlige Ramappa
@ 2020-03-12 13:28 ` Lionel Landwerlin
  0 siblings, 0 replies; 11+ messages in thread
From: Lionel Landwerlin @ 2020-03-12 13:28 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa, igt-dev, Chris Wilson

On 10/03/2020 21:00, Umesh Nerlige Ramappa wrote:
> Run 2 polling tests back to back and compare the number of OA reports
> captured. Make sure the number of reports are almost same.
>
> v2: Add timeout to poll (Lionel)
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>   tests/perf.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 99 insertions(+)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 5e818030..fea9da1c 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -2265,6 +2265,99 @@ test_polling(void)
>   	__perf_close(stream_fd);
>   }
>   
> +static int
> +num_valid_reports_captured(struct drm_i915_perf_open_param *param)
> +{
> +	uint8_t buf[1024 * 1024];
> +	int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
> +#define DURATION_SEC 5 /* 5 seconds */
> +	int64_t test_duration_ns = tick_ns * DURATION_SEC * 100;


Why do you need ticks? poll work in milliseconds.

Can't we stick on milliseconds or nanoseconds?


> +	int64_t start, end;
> +	int num_reports = 0;
> +
> +	stream_fd = __perf_open(drm_fd, param, true);
> +
> +	igt_debug("tick length = %dns, test duration = %"PRIu64"ns\n",
> +		  (int)tick_ns, test_duration_ns);
> +
> +	start = get_time();
> +	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
> +	for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) {
> +		struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN };
> +		struct drm_i915_perf_record_header *header;
> +		int ret;
> +
> +		/* we do not want to wait longer than the test duration here */
> +		while ((ret = poll(&pollfd, 1, DURATION_SEC * 1000)) < 0 &&
> +		       errno == EINTR)


You need to deduct the remaining time for the poll.

Otherwise you depending on how you're synced with the kernel timeout you 
might capture one more timeout period than what you need for 5s.


You could use ppoll if you want more precise timings.


> +			;
> +		igt_assert_eq(ret, 1);
> +		igt_assert(pollfd.revents & POLLIN);
> +
> +		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
> +		       errno == EINTR)
> +			;
> +
> +		/* poll checks if the tail has advanced on the OA buffer, but
> +		 * does not check if the reports are valid. On read, the driver
> +		 * checks if the reports are valid or not. if none of the
> +		 * reports are valid, it returns EAGAIN. EAGAIN should also
> +		 * suffice to show that the TLB invalidation failed, but we will
> +		 * try for a more concrete check. Ignore read errors here.
> +		 */
> +		if (ret < 0)
> +			continue;
> +
> +		for (int offset = 0; offset < ret; offset += header->size) {
> +			header = (void *)(buf + offset);
> +
> +			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
> +				uint32_t *report = (void *)(header + 1);
> +
> +				if ((report[0] >> OAREPORT_REASON_SHIFT) &
> +				    OAREPORT_REASON_TIMER)
> +					num_reports++;
> +			}
> +		}
> +	}
> +	__perf_close(stream_fd);
> +
> +	return num_reports;
> +}
> +
> +static void
> +gen12_test_oa_tlb_invalidate(void)
> +{
> +	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
> +	uint64_t properties[] = {
> +		DRM_I915_PERF_PROP_SAMPLE_OA, true,
> +
> +		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
> +		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
> +		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
> +	};
> +	struct drm_i915_perf_open_param param = {
> +		.flags = I915_PERF_FLAG_FD_CLOEXEC |
> +			I915_PERF_FLAG_DISABLED |
> +			I915_PERF_FLAG_FD_NONBLOCK,
> +		.num_properties = sizeof(properties) / 16,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +	int num_reports1, num_reports2;
> +	
> +	/* Capture reports for 5 seconds twice and then make sure you get around
> +	 * the same number of reports. In the case of failure, the number of
> +	 * reports will vary largely since the beginning of the OA buffer
> +	 * will have invalid entries.
> +	 */


I think a better approach would be to estimate how many reports you 
should get for 5s and then verify that both captures are within a delta 
of that expectation.

That way if we fail twice the number of reports we should be getting, 
we'll be notified.


-Lionel


> +	num_reports1 = num_valid_reports_captured(&param);
> +	num_reports2 = num_valid_reports_captured(&param);
> +
> +	igt_debug("num_reports1 = %d, num_reports2 = %d\n", num_reports1, num_reports2);
> +	igt_assert(num_reports2 > 0.95 * num_reports1);
> +}
> +
> +
>   static void
>   test_buffer_fill(void)
>   {
> @@ -4622,6 +4715,12 @@ igt_main
>   		gen8_test_single_ctx_render_target_writes_a_counter();
>   	}
>   
> +	igt_describe("Test OA TLB invalidate");
> +	igt_subtest("gen12-oa-tlb-invalidate") {
> +		igt_require(intel_gen(devid) >= 12);
> +		gen12_test_oa_tlb_invalidate();
> +	}
> +
>   	igt_describe("Measure performance for a specific context using OAR in Gen 12");
>   	igt_subtest("gen12-unprivileged-single-ctx-counters") {
>   		igt_require(intel_gen(devid) >= 12);


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-10 17:57   ` Umesh Nerlige Ramappa
@ 2020-03-12 13:18     ` Lionel Landwerlin
  0 siblings, 0 replies; 11+ messages in thread
From: Lionel Landwerlin @ 2020-03-12 13:18 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: igt-dev, Chris Wilson

On 10/03/2020 19:57, Umesh Nerlige Ramappa wrote:
> On Tue, Mar 10, 2020 at 11:02:00AM +0200, Lionel Landwerlin wrote:
>> On 10/03/2020 05:07, Umesh Nerlige Ramappa wrote:
>>> Run 2 polling tests back to back and compare the number of OA reports
>>> captured. Make sure the number of reports are almost same.
>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>> ---
>>>  tests/perf.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 97 insertions(+)
>>>
>>> diff --git a/tests/perf.c b/tests/perf.c
>>> index 5e818030..2394adc4 100644
>>> --- a/tests/perf.c
>>> +++ b/tests/perf.c
>>> @@ -2265,6 +2265,97 @@ test_polling(void)
>>>      __perf_close(stream_fd);
>>>  }
>>> +static int
>>> +num_valid_reports_captured(struct drm_i915_perf_open_param *param)
>>> +{
>>> +    uint8_t buf[1024 * 1024];
>>> +    int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
>>> +    int64_t test_duration_ns = tick_ns * 5 * 100; /* 5 seconds */
>>> +    int64_t start, end;
>>> +    int num_reports = 0;
>>> +
>>> +    stream_fd = __perf_open(drm_fd, param, true);
>>> +
>>> +    igt_debug("tick length = %dns, test duration = %"PRIu64"ns\n",
>>> +          (int)tick_ns, test_duration_ns);
>>> +
>>> +    start = get_time();
>>> +    do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
>>> +    for (/* nop */; ((end = get_time()) - start) < 
>>> test_duration_ns; /* nop */) {
>>> +        struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN };
>>> +        struct drm_i915_perf_record_header *header;
>>> +        int ret;
>>> +
>>> +        while ((ret = poll(&pollfd, 1, -1)) < 0 &&
>>> +               errno == EINTR)
>>> +            ;
>>> +        igt_assert_eq(ret, 1);
>>> +        igt_assert(pollfd.revents & POLLIN);
>>
>>
>> I guess you can drop the poll() if you drop the 
>> I915_PERF_FLAG_FD_NONBLOCK below.
>>
>> That way your reads are blocking.
>>
>>
>> Or if you want more accuracy, you can compute the amount of poll() 
>> timeout based of test_duration_ns and exit the loop earlier.
>>
>
> In the failure case, I actually see zero valid reports in 5 seconds, 
> so the blocking read would block endlessly. Poll will instead return 
> indicating that reports are available (even if reports are invalid).
>
> I think adding a poll timeout based on the test duration is a good 
> idea in case the poll behavior changes in future.
>
>>
>>> +
>>> +        while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
>>> +               errno == EINTR)
>>> +            ;
>>> +
>>> +        /* poll checks if the tail has advanced on the OA buffer, but
>>> +         * does not check if the reports are valid. On read, the 
>>> driver
>>> +         * checks if the reports are valid or not. if none of the
>>> +         * reports are valid, it returns EAGAIN. EAGAIN should also
>>> +         * suffice to show that the TLB invalidation failed, but we 
>>> will
>>> +         * try for a more concrete check. Ignore read errors here.
>>> +         */
>>> +        if (ret < 0)
>>> +            continue;
>>> +
>>> +        for (int offset = 0; offset < ret; offset += header->size) {
>>> +            header = (void *)(buf + offset);
>>> +
>>> +            if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
>>> +                uint32_t *report = (void *)(header + 1);
>>> +
>>> +                if ((report[0] >> OAREPORT_REASON_SHIFT) &
>>> +                    OAREPORT_REASON_TIMER)
>>> +                    num_reports++;
>>> +            }
>>> +        }
>>> +    }
>>> +    __perf_close(stream_fd);
>>> +
>>> +    return num_reports;
>>> +}
>>> +
>>> +static void
>>> +gen12_test_oa_tlb_invalidate(void)
>>> +{
>>> +    int oa_exponent = max_oa_exponent_for_period_lte(30000000);
>>> +    uint64_t properties[] = {
>>> +        DRM_I915_PERF_PROP_SAMPLE_OA, true,
>>> +
>>> +        DRM_I915_PERF_PROP_OA_METRICS_SET, 
>>> test_set->perf_oa_metrics_set,
>>> +        DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
>>> +        DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
>>> +    };
>>> +    struct drm_i915_perf_open_param param = {
>>> +        .flags = I915_PERF_FLAG_FD_CLOEXEC |
>>> +            I915_PERF_FLAG_DISABLED |
>>> +            I915_PERF_FLAG_FD_NONBLOCK,
>>> +        .num_properties = sizeof(properties) / 16,
>>> +        .properties_ptr = to_user_pointer(properties),
>>> +    };
>>> +    int num_reports1, num_reports2;
>>> +
>>> +    /* Capture reports for 5 seconds twice and then make sure you 
>>> get around
>>> +     * the same number of reports. In the case of failure, the 
>>> number of
>>> +     * reports will vary largely since the beginning of the OA buffer
>>> +     * will have invalid entries.
>>> +     */
>>
>>
>> I thought you were also noticing corruption in the data.
>
> I don't see corruption of OA reports. Just missing reports in the 
> beginning of OA buffer. All reports are zeroed out.
>
> When trying the interrupt patches, I saw a random reboot with 
> blocking-with-interrupt (or any of the other subtests that ran 
> multiple cases). My theory was that OA is writing reports to oa buffer 
> memory from previous iteration. That failure/reboot does not occur 
> anymore with this patch (invalidating tlb).
>
> Thanks,
> Umesh


Thanks for the explanation.


-Lionel


>
>>
>> Seems like it would be an easier characterization of the failure than 
>> the number of reports (since the number can vary for a other reasons).
>>
>> What do you think?
>>
>>
>> -Lionel
>>
>>
>>> +    num_reports1 = num_valid_reports_captured(&param);
>>> +    num_reports2 = num_valid_reports_captured(&param);
>>> +
>>> +    igt_debug("num_reports1 = %d, num_reports2 = %d\n", 
>>> num_reports1, num_reports2);
>>> +    igt_assert(num_reports2 > 0.95 * num_reports1);
>>> +}
>>> +
>>> +
>>>  static void
>>>  test_buffer_fill(void)
>>>  {
>>> @@ -4622,6 +4713,12 @@ igt_main
>>> gen8_test_single_ctx_render_target_writes_a_counter();
>>>      }
>>> +    igt_describe("Test OA TLB invalidate");
>>> +    igt_subtest("gen12-oa-tlb-invalidate") {
>>> +        igt_require(intel_gen(devid) >= 12);
>>> +        gen12_test_oa_tlb_invalidate();
>>> +    }
>>> +
>>>      igt_describe("Measure performance for a specific context using 
>>> OAR in Gen 12");
>>>      igt_subtest("gen12-unprivileged-single-ctx-counters") {
>>>          igt_require(intel_gen(devid) >= 12);
>>
>>

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
@ 2020-03-10 19:00 Umesh Nerlige Ramappa
  2020-03-12 13:28 ` Lionel Landwerlin
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-03-10 19:00 UTC (permalink / raw)
  To: igt-dev, Chris Wilson, Lionel G Landwerlin

Run 2 polling tests back to back and compare the number of OA reports
captured. Make sure the number of reports are almost same.

v2: Add timeout to poll (Lionel)

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 tests/perf.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/tests/perf.c b/tests/perf.c
index 5e818030..fea9da1c 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2265,6 +2265,99 @@ test_polling(void)
 	__perf_close(stream_fd);
 }
 
+static int
+num_valid_reports_captured(struct drm_i915_perf_open_param *param)
+{
+	uint8_t buf[1024 * 1024];
+	int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
+#define DURATION_SEC 5 /* 5 seconds */
+	int64_t test_duration_ns = tick_ns * DURATION_SEC * 100;
+	int64_t start, end;
+	int num_reports = 0;
+
+	stream_fd = __perf_open(drm_fd, param, true);
+
+	igt_debug("tick length = %dns, test duration = %"PRIu64"ns\n",
+		  (int)tick_ns, test_duration_ns);
+
+	start = get_time();
+	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
+	for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) {
+		struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN };
+		struct drm_i915_perf_record_header *header;
+		int ret;
+
+		/* we do not want to wait longer than the test duration here */
+		while ((ret = poll(&pollfd, 1, DURATION_SEC * 1000)) < 0 &&
+		       errno == EINTR)
+			;
+		igt_assert_eq(ret, 1);
+		igt_assert(pollfd.revents & POLLIN);
+
+		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
+		       errno == EINTR)
+			;
+
+		/* poll checks if the tail has advanced on the OA buffer, but
+		 * does not check if the reports are valid. On read, the driver
+		 * checks if the reports are valid or not. if none of the
+		 * reports are valid, it returns EAGAIN. EAGAIN should also
+		 * suffice to show that the TLB invalidation failed, but we will
+		 * try for a more concrete check. Ignore read errors here.
+		 */
+		if (ret < 0)
+			continue;
+
+		for (int offset = 0; offset < ret; offset += header->size) {
+			header = (void *)(buf + offset);
+
+			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
+				uint32_t *report = (void *)(header + 1);
+
+				if ((report[0] >> OAREPORT_REASON_SHIFT) &
+				    OAREPORT_REASON_TIMER)
+					num_reports++;
+			}
+		}
+	}
+	__perf_close(stream_fd);
+
+	return num_reports;
+}
+
+static void
+gen12_test_oa_tlb_invalidate(void)
+{
+	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
+	uint64_t properties[] = {
+		DRM_I915_PERF_PROP_SAMPLE_OA, true,
+
+		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
+		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
+		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
+	};
+	struct drm_i915_perf_open_param param = {
+		.flags = I915_PERF_FLAG_FD_CLOEXEC |
+			I915_PERF_FLAG_DISABLED |
+			I915_PERF_FLAG_FD_NONBLOCK,
+		.num_properties = sizeof(properties) / 16,
+		.properties_ptr = to_user_pointer(properties),
+	};
+	int num_reports1, num_reports2;
+	
+	/* Capture reports for 5 seconds twice and then make sure you get around
+	 * the same number of reports. In the case of failure, the number of
+	 * reports will vary largely since the beginning of the OA buffer
+	 * will have invalid entries.
+	 */
+	num_reports1 = num_valid_reports_captured(&param);
+	num_reports2 = num_valid_reports_captured(&param);
+
+	igt_debug("num_reports1 = %d, num_reports2 = %d\n", num_reports1, num_reports2);
+	igt_assert(num_reports2 > 0.95 * num_reports1);
+}
+
+
 static void
 test_buffer_fill(void)
 {
@@ -4622,6 +4715,12 @@ igt_main
 		gen8_test_single_ctx_render_target_writes_a_counter();
 	}
 
+	igt_describe("Test OA TLB invalidate");
+	igt_subtest("gen12-oa-tlb-invalidate") {
+		igt_require(intel_gen(devid) >= 12);
+		gen12_test_oa_tlb_invalidate();
+	}
+
 	igt_describe("Measure performance for a specific context using OAR in Gen 12");
 	igt_subtest("gen12-unprivileged-single-ctx-counters") {
 		igt_require(intel_gen(devid) >= 12);
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-10  9:02 ` Lionel Landwerlin
@ 2020-03-10 17:57   ` Umesh Nerlige Ramappa
  2020-03-12 13:18     ` Lionel Landwerlin
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-03-10 17:57 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: igt-dev, Chris Wilson

On Tue, Mar 10, 2020 at 11:02:00AM +0200, Lionel Landwerlin wrote:
>On 10/03/2020 05:07, Umesh Nerlige Ramappa wrote:
>>Run 2 polling tests back to back and compare the number of OA reports
>>captured. Make sure the number of reports are almost same.
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>>  tests/perf.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 97 insertions(+)
>>
>>diff --git a/tests/perf.c b/tests/perf.c
>>index 5e818030..2394adc4 100644
>>--- a/tests/perf.c
>>+++ b/tests/perf.c
>>@@ -2265,6 +2265,97 @@ test_polling(void)
>>  	__perf_close(stream_fd);
>>  }
>>+static int
>>+num_valid_reports_captured(struct drm_i915_perf_open_param *param)
>>+{
>>+	uint8_t buf[1024 * 1024];
>>+	int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
>>+	int64_t test_duration_ns = tick_ns * 5 * 100; /* 5 seconds */
>>+	int64_t start, end;
>>+	int num_reports = 0;
>>+
>>+	stream_fd = __perf_open(drm_fd, param, true);
>>+
>>+	igt_debug("tick length = %dns, test duration = %"PRIu64"ns\n",
>>+		  (int)tick_ns, test_duration_ns);
>>+
>>+	start = get_time();
>>+	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
>>+	for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) {
>>+		struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN };
>>+		struct drm_i915_perf_record_header *header;
>>+		int ret;
>>+
>>+		while ((ret = poll(&pollfd, 1, -1)) < 0 &&
>>+		       errno == EINTR)
>>+			;
>>+		igt_assert_eq(ret, 1);
>>+		igt_assert(pollfd.revents & POLLIN);
>
>
>I guess you can drop the poll() if you drop the 
>I915_PERF_FLAG_FD_NONBLOCK below.
>
>That way your reads are blocking.
>
>
>Or if you want more accuracy, you can compute the amount of poll() 
>timeout based of test_duration_ns and exit the loop earlier.
>

In the failure case, I actually see zero valid reports in 5 seconds, so 
the blocking read would block endlessly. Poll will instead return 
indicating that reports are available (even if reports are invalid).

I think adding a poll timeout based on the test duration is a good idea 
in case the poll behavior changes in future.

>
>>+
>>+		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
>>+		       errno == EINTR)
>>+			;
>>+
>>+		/* poll checks if the tail has advanced on the OA buffer, but
>>+		 * does not check if the reports are valid. On read, the driver
>>+		 * checks if the reports are valid or not. if none of the
>>+		 * reports are valid, it returns EAGAIN. EAGAIN should also
>>+		 * suffice to show that the TLB invalidation failed, but we will
>>+		 * try for a more concrete check. Ignore read errors here.
>>+		 */
>>+		if (ret < 0)
>>+			continue;
>>+
>>+		for (int offset = 0; offset < ret; offset += header->size) {
>>+			header = (void *)(buf + offset);
>>+
>>+			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
>>+				uint32_t *report = (void *)(header + 1);
>>+
>>+				if ((report[0] >> OAREPORT_REASON_SHIFT) &
>>+				    OAREPORT_REASON_TIMER)
>>+					num_reports++;
>>+			}
>>+		}
>>+	}
>>+	__perf_close(stream_fd);
>>+
>>+	return num_reports;
>>+}
>>+
>>+static void
>>+gen12_test_oa_tlb_invalidate(void)
>>+{
>>+	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
>>+	uint64_t properties[] = {
>>+		DRM_I915_PERF_PROP_SAMPLE_OA, true,
>>+
>>+		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
>>+		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
>>+		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
>>+	};
>>+	struct drm_i915_perf_open_param param = {
>>+		.flags = I915_PERF_FLAG_FD_CLOEXEC |
>>+			I915_PERF_FLAG_DISABLED |
>>+			I915_PERF_FLAG_FD_NONBLOCK,
>>+		.num_properties = sizeof(properties) / 16,
>>+		.properties_ptr = to_user_pointer(properties),
>>+	};
>>+	int num_reports1, num_reports2;
>>+	
>>+	/* Capture reports for 5 seconds twice and then make sure you get around
>>+	 * the same number of reports. In the case of failure, the number of
>>+	 * reports will vary largely since the beginning of the OA buffer
>>+	 * will have invalid entries.
>>+	 */
>
>
>I thought you were also noticing corruption in the data.

I don't see corruption of OA reports. Just missing reports in the 
beginning of OA buffer. All reports are zeroed out.

When trying the interrupt patches, I saw a random reboot with 
blocking-with-interrupt (or any of the other subtests that ran multiple 
cases). My theory was that OA is writing reports to oa buffer memory 
from previous iteration. That failure/reboot does not occur anymore with 
this patch (invalidating tlb).

Thanks,
Umesh

>
>Seems like it would be an easier characterization of the failure than 
>the number of reports (since the number can vary for a other reasons).
>
>What do you think?
>
>
>-Lionel
>
>
>>+	num_reports1 = num_valid_reports_captured(&param);
>>+	num_reports2 = num_valid_reports_captured(&param);
>>+
>>+	igt_debug("num_reports1 = %d, num_reports2 = %d\n", num_reports1, num_reports2);
>>+	igt_assert(num_reports2 > 0.95 * num_reports1);
>>+}
>>+
>>+
>>  static void
>>  test_buffer_fill(void)
>>  {
>>@@ -4622,6 +4713,12 @@ igt_main
>>  		gen8_test_single_ctx_render_target_writes_a_counter();
>>  	}
>>+	igt_describe("Test OA TLB invalidate");
>>+	igt_subtest("gen12-oa-tlb-invalidate") {
>>+		igt_require(intel_gen(devid) >= 12);
>>+		gen12_test_oa_tlb_invalidate();
>>+	}
>>+
>>  	igt_describe("Measure performance for a specific context using OAR in Gen 12");
>>  	igt_subtest("gen12-unprivileged-single-ctx-counters") {
>>  		igt_require(intel_gen(devid) >= 12);
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
  2020-03-10  3:07 Umesh Nerlige Ramappa
@ 2020-03-10  9:02 ` Lionel Landwerlin
  2020-03-10 17:57   ` Umesh Nerlige Ramappa
  0 siblings, 1 reply; 11+ messages in thread
From: Lionel Landwerlin @ 2020-03-10  9:02 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa, igt-dev, Chris Wilson

On 10/03/2020 05:07, Umesh Nerlige Ramappa wrote:
> Run 2 polling tests back to back and compare the number of OA reports
> captured. Make sure the number of reports are almost same.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>   tests/perf.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 97 insertions(+)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 5e818030..2394adc4 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -2265,6 +2265,97 @@ test_polling(void)
>   	__perf_close(stream_fd);
>   }
>   
> +static int
> +num_valid_reports_captured(struct drm_i915_perf_open_param *param)
> +{
> +	uint8_t buf[1024 * 1024];
> +	int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
> +	int64_t test_duration_ns = tick_ns * 5 * 100; /* 5 seconds */
> +	int64_t start, end;
> +	int num_reports = 0;
> +
> +	stream_fd = __perf_open(drm_fd, param, true);
> +
> +	igt_debug("tick length = %dns, test duration = %"PRIu64"ns\n",
> +		  (int)tick_ns, test_duration_ns);
> +
> +	start = get_time();
> +	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
> +	for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) {
> +		struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN };
> +		struct drm_i915_perf_record_header *header;
> +		int ret;
> +
> +		while ((ret = poll(&pollfd, 1, -1)) < 0 &&
> +		       errno == EINTR)
> +			;
> +		igt_assert_eq(ret, 1);
> +		igt_assert(pollfd.revents & POLLIN);


I guess you can drop the poll() if you drop the 
I915_PERF_FLAG_FD_NONBLOCK below.

That way your reads are blocking.


Or if you want more accuracy, you can compute the amount of poll() 
timeout based of test_duration_ns and exit the loop earlier.


> +
> +		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
> +		       errno == EINTR)
> +			;
> +
> +		/* poll checks if the tail has advanced on the OA buffer, but
> +		 * does not check if the reports are valid. On read, the driver
> +		 * checks if the reports are valid or not. if none of the
> +		 * reports are valid, it returns EAGAIN. EAGAIN should also
> +		 * suffice to show that the TLB invalidation failed, but we will
> +		 * try for a more concrete check. Ignore read errors here.
> +		 */
> +		if (ret < 0)
> +			continue;
> +
> +		for (int offset = 0; offset < ret; offset += header->size) {
> +			header = (void *)(buf + offset);
> +
> +			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
> +				uint32_t *report = (void *)(header + 1);
> +
> +				if ((report[0] >> OAREPORT_REASON_SHIFT) &
> +				    OAREPORT_REASON_TIMER)
> +					num_reports++;
> +			}
> +		}
> +	}
> +	__perf_close(stream_fd);
> +
> +	return num_reports;
> +}
> +
> +static void
> +gen12_test_oa_tlb_invalidate(void)
> +{
> +	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
> +	uint64_t properties[] = {
> +		DRM_I915_PERF_PROP_SAMPLE_OA, true,
> +
> +		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
> +		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
> +		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
> +	};
> +	struct drm_i915_perf_open_param param = {
> +		.flags = I915_PERF_FLAG_FD_CLOEXEC |
> +			I915_PERF_FLAG_DISABLED |
> +			I915_PERF_FLAG_FD_NONBLOCK,
> +		.num_properties = sizeof(properties) / 16,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +	int num_reports1, num_reports2;
> +	
> +	/* Capture reports for 5 seconds twice and then make sure you get around
> +	 * the same number of reports. In the case of failure, the number of
> +	 * reports will vary largely since the beginning of the OA buffer
> +	 * will have invalid entries.
> +	 */


I thought you were also noticing corruption in the data.

Seems like it would be an easier characterization of the failure than 
the number of reports (since the number can vary for a other reasons).

What do you think?


-Lionel


> +	num_reports1 = num_valid_reports_captured(&param);
> +	num_reports2 = num_valid_reports_captured(&param);
> +
> +	igt_debug("num_reports1 = %d, num_reports2 = %d\n", num_reports1, num_reports2);
> +	igt_assert(num_reports2 > 0.95 * num_reports1);
> +}
> +
> +
>   static void
>   test_buffer_fill(void)
>   {
> @@ -4622,6 +4713,12 @@ igt_main
>   		gen8_test_single_ctx_render_target_writes_a_counter();
>   	}
>   
> +	igt_describe("Test OA TLB invalidate");
> +	igt_subtest("gen12-oa-tlb-invalidate") {
> +		igt_require(intel_gen(devid) >= 12);
> +		gen12_test_oa_tlb_invalidate();
> +	}
> +
>   	igt_describe("Measure performance for a specific context using OAR in Gen 12");
>   	igt_subtest("gen12-unprivileged-single-ctx-counters") {
>   		igt_require(intel_gen(devid) >= 12);


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
@ 2020-03-10  3:07 Umesh Nerlige Ramappa
  2020-03-10  9:02 ` Lionel Landwerlin
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-03-10  3:07 UTC (permalink / raw)
  To: igt-dev, Chris Wilson, Lionel G Landwerlin

Run 2 polling tests back to back and compare the number of OA reports
captured. Make sure the number of reports are almost same.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 tests/perf.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/tests/perf.c b/tests/perf.c
index 5e818030..2394adc4 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -2265,6 +2265,97 @@ test_polling(void)
 	__perf_close(stream_fd);
 }
 
+static int
+num_valid_reports_captured(struct drm_i915_perf_open_param *param)
+{
+	uint8_t buf[1024 * 1024];
+	int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
+	int64_t test_duration_ns = tick_ns * 5 * 100; /* 5 seconds */
+	int64_t start, end;
+	int num_reports = 0;
+
+	stream_fd = __perf_open(drm_fd, param, true);
+
+	igt_debug("tick length = %dns, test duration = %"PRIu64"ns\n",
+		  (int)tick_ns, test_duration_ns);
+
+	start = get_time();
+	do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0);
+	for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) {
+		struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN };
+		struct drm_i915_perf_record_header *header;
+		int ret;
+
+		while ((ret = poll(&pollfd, 1, -1)) < 0 &&
+		       errno == EINTR)
+			;
+		igt_assert_eq(ret, 1);
+		igt_assert(pollfd.revents & POLLIN);
+
+		while ((ret = read(stream_fd, buf, sizeof(buf))) < 0 &&
+		       errno == EINTR)
+			;
+
+		/* poll checks if the tail has advanced on the OA buffer, but
+		 * does not check if the reports are valid. On read, the driver
+		 * checks if the reports are valid or not. if none of the
+		 * reports are valid, it returns EAGAIN. EAGAIN should also
+		 * suffice to show that the TLB invalidation failed, but we will
+		 * try for a more concrete check. Ignore read errors here.
+		 */
+		if (ret < 0)
+			continue;
+
+		for (int offset = 0; offset < ret; offset += header->size) {
+			header = (void *)(buf + offset);
+
+			if (header->type == DRM_I915_PERF_RECORD_SAMPLE) {
+				uint32_t *report = (void *)(header + 1);
+
+				if ((report[0] >> OAREPORT_REASON_SHIFT) &
+				    OAREPORT_REASON_TIMER)
+					num_reports++;
+			}
+		}
+	}
+	__perf_close(stream_fd);
+
+	return num_reports;
+}
+
+static void
+gen12_test_oa_tlb_invalidate(void)
+{
+	int oa_exponent = max_oa_exponent_for_period_lte(30000000);
+	uint64_t properties[] = {
+		DRM_I915_PERF_PROP_SAMPLE_OA, true,
+
+		DRM_I915_PERF_PROP_OA_METRICS_SET, test_set->perf_oa_metrics_set,
+		DRM_I915_PERF_PROP_OA_FORMAT, test_set->perf_oa_format,
+		DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent,
+	};
+	struct drm_i915_perf_open_param param = {
+		.flags = I915_PERF_FLAG_FD_CLOEXEC |
+			I915_PERF_FLAG_DISABLED |
+			I915_PERF_FLAG_FD_NONBLOCK,
+		.num_properties = sizeof(properties) / 16,
+		.properties_ptr = to_user_pointer(properties),
+	};
+	int num_reports1, num_reports2;
+	
+	/* Capture reports for 5 seconds twice and then make sure you get around
+	 * the same number of reports. In the case of failure, the number of
+	 * reports will vary largely since the beginning of the OA buffer
+	 * will have invalid entries.
+	 */
+	num_reports1 = num_valid_reports_captured(&param);
+	num_reports2 = num_valid_reports_captured(&param);
+
+	igt_debug("num_reports1 = %d, num_reports2 = %d\n", num_reports1, num_reports2);
+	igt_assert(num_reports2 > 0.95 * num_reports1);
+}
+
+
 static void
 test_buffer_fill(void)
 {
@@ -4622,6 +4713,12 @@ igt_main
 		gen8_test_single_ctx_render_target_writes_a_counter();
 	}
 
+	igt_describe("Test OA TLB invalidate");
+	igt_subtest("gen12-oa-tlb-invalidate") {
+		igt_require(intel_gen(devid) >= 12);
+		gen12_test_oa_tlb_invalidate();
+	}
+
 	igt_describe("Measure performance for a specific context using OAR in Gen 12");
 	igt_subtest("gen12-unprivileged-single-ctx-counters") {
 		igt_require(intel_gen(devid) >= 12);
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2020-03-16  9:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 17:42 [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation Umesh Nerlige Ramappa
2020-03-13 17:51 ` Chris Wilson
2020-03-16  9:52 ` Lionel Landwerlin
  -- strict thread matches above, loose matches on Subject: below --
2020-03-14  0:00 Umesh Nerlige Ramappa
2020-03-14 10:36 ` Lionel Landwerlin
2020-03-10 19:00 Umesh Nerlige Ramappa
2020-03-12 13:28 ` Lionel Landwerlin
2020-03-10  3:07 Umesh Nerlige Ramappa
2020-03-10  9:02 ` Lionel Landwerlin
2020-03-10 17:57   ` Umesh Nerlige Ramappa
2020-03-12 13:18     ` Lionel Landwerlin

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.