public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>,
	igt-dev@lists.freedesktop.org,
	Chris Wilson <chris.p.wilson@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation
Date: Thu, 12 Mar 2020 15:28:20 +0200	[thread overview]
Message-ID: <3cb5c86f-af13-5243-00fd-55192022933d@intel.com> (raw)
In-Reply-To: <20200310190024.6627-1-umesh.nerlige.ramappa@intel.com>

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

  parent reply	other threads:[~2020-03-12 13:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-10 19:00 [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation Umesh Nerlige Ramappa
2020-03-11  9:21 ` [igt-dev] ✓ Fi.CI.BAT: success for test/perf: Add test to verify OA TLB invalidation (rev2) Patchwork
2020-03-11 20:39 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2020-03-12 13:28 ` Lionel Landwerlin [this message]
  -- strict thread matches above, loose matches on Subject: below --
2020-03-14  0:00 [igt-dev] [PATCH i-g-t] test/perf: Add test to verify OA TLB invalidation Umesh Nerlige Ramappa
2020-03-14 10:36 ` Lionel Landwerlin
2020-03-13 17:42 Umesh Nerlige Ramappa
2020-03-13 17:51 ` Chris Wilson
2020-03-16  9:52 ` 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3cb5c86f-af13-5243-00fd-55192022933d@intel.com \
    --to=lionel.g.landwerlin@intel.com \
    --cc=chris.p.wilson@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=umesh.nerlige.ramappa@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox