Re: [PATCH 4/4] tests/intel/xe_oa: Cleanup blocking and polling tests

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 17 Jul 2026 14:00:32 -0700, Umesh Nerlige Ramappa wrote:
>
> 1) Drop set_kernel_timer since we don't support it anymore.
> 2) When running all tests, a failure in blocking/polling tests breaks
>    all subsequent tests. Move __perf_close before the asserts.

Hmm, this should, in the future, be handled by having some sort of an
exit/assert handler, which will close the stream in the event of an
assert. Yeah this is annoying.

> 3) We cannot guarantee latency when reading OA reports as there are no
>    guarantees how long it takes for reports to land in memory w.r.t
>    any running workloads. Since OA reports are mostly used for
>    post-processing, drop the latency checks in these tests.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  tests/intel/xe_oa.c | 153 +++++---------------------------------------
>  1 file changed, 17 insertions(+), 136 deletions(-)
>
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 4f652b187bae..9ebb18690ec7 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -2127,8 +2127,6 @@ get_time(void)
>   * kernelspace.
>   */
>  static void test_blocking(uint64_t requested_oa_period,
> -			  bool set_kernel_hrtimer,
> -			  uint64_t kernel_hrtimer,
>			  const struct drm_xe_oa_unit *oau)
>  {
>	int oa_exponent = max_oa_exponent_for_period_lte(requested_oa_period);
> @@ -2142,24 +2140,9 @@ static void test_blocking(uint64_t requested_oa_period,
>	int64_t user_ns, kernel_ns;
>	int64_t tick_ns = 1000000000 / sysconf(_SC_CLK_TCK);
>	int64_t test_duration_ns = tick_ns * 100;
> -	int max_iterations = (test_duration_ns / oa_period) + 2;
> -	int n_extra_iterations = 0;
>	int perf_fd;
> -
> -	/* It's a bit tricky to put a lower limit here, but we expect a
> -	 * relatively low latency for seeing reports, while we don't currently
> -	 * give any control over this in the api.
> -	 *
> -	 * We assume a maximum latency of 6 millisecond to deliver a POLLIN and
> -	 * read() after a new sample is written (46ms per iteration) considering
> -	 * the knowledge that that the driver uses a 200Hz hrtimer (5ms period)
> -	 * to check for data and giving some time to read().
> -	 */
> -	int min_iterations = (test_duration_ns / (oa_period + kernel_hrtimer + kernel_hrtimer / 5));
>	int64_t start, end;
> -	int n = 0;
>	struct intel_xe_perf_metric_set *test_set = oa_unit_metric_set(oau);
> -	size_t format_size = get_oa_format(test_set->perf_oa_format).size;
>
>	ADD_PROPS(props, idx, SAMPLE_OA, true);
>	ADD_PROPS(props, idx, OA_METRIC_SET, test_set->perf_oa_metrics_set);
> @@ -2174,13 +2157,8 @@ static void test_blocking(uint64_t requested_oa_period,
>	perf_fd = __perf_open(drm_fd, &param, true /* prevent_pm */);
>          set_fd_flags(perf_fd, O_CLOEXEC);
>
> -	times(&start_times);
> -
> -	igt_debug("tick length = %dns, test duration = %"PRIu64"ns, min iter. = %d,"
> -		  " estimated max iter. = %d, oa_period = %s\n",
> -		  (int)tick_ns, test_duration_ns,
> -		  min_iterations, max_iterations,
> -		  pretty_print_oa_period(oa_period));
> +	igt_debug("tick length = %dns, test duration = %"PRIu64"ns, oa_period = %s\n",
> +		  (int)tick_ns, test_duration_ns, pretty_print_oa_period(oa_period));
>
>	/* In the loop we perform blocking polls while the HW is sampling at
>	 * ~25Hz, with the expectation that we spend most of our time blocked
> @@ -2204,44 +2182,26 @@ static void test_blocking(uint64_t requested_oa_period,
>	 * the error delta.
>	 */
>	start = get_time();
> +	times(&start_times);
>	do_ioctl(perf_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
> -	for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) {
> -		bool timer_report_read = false;
> -		bool non_timer_report_read = false;
> +	while (((end = get_time()) - start) < test_duration_ns) {
>		int ret;
>
>		while ((ret = read(perf_fd, buf, sizeof(buf))) < 0 &&
>		       (errno == EINTR || errno == EIO))
>			;
>		igt_assert_lt(0, ret);
> -
> -		for (int offset = 0; offset < ret; offset += format_size) {
> -			uint32_t *report = (void *)(buf + offset);
> -
> -			if (oa_report_is_periodic(report))
> -				timer_report_read = true;
> -			else
> -				non_timer_report_read = true;
> -		}
> -
> -		if (non_timer_report_read && !timer_report_read)
> -			n_extra_iterations++;
> -
> -		n++;
>	}
> -
>	times(&end_times);
>
> +	__perf_close(perf_fd);
> +
>	/* Using nanosecond units is fairly silly here, given the tick in-
>	 * precision - ah well, it's consistent with the get_time() units.
>	 */
>	user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
>	kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
>
> -	igt_debug("%d blocking reads during test with %"PRIu64" Hz OA sampling (expect no more than %d)\n",
> -		  n, NSEC_PER_SEC / oa_period, max_iterations);
> -	igt_debug("%d extra iterations seen, not related to periodic sampling (e.g. context switches)\n",
> -		  n_extra_iterations);
>	igt_debug("time in userspace = %"PRIu64"ns (+-%dns) (start utime = %d, end = %d)\n",
>		  user_ns, (int)tick_ns,
>		  (int)start_times.tms_utime, (int)end_times.tms_utime);
> @@ -2249,20 +2209,7 @@ static void test_blocking(uint64_t requested_oa_period,
>		  kernel_ns, (int)tick_ns,
>		  (int)start_times.tms_stime, (int)end_times.tms_stime);
>
> -	/* With completely broken blocking (but also not returning an error) we
> -	 * could end up with an open loop,
> -	 */
> -	igt_assert_lte(n, (max_iterations + n_extra_iterations));
> -
> -	/* Make sure the driver is reporting new samples with a reasonably
> -	 * low latency...
> -	 */
> -	igt_assert_lt((min_iterations + n_extra_iterations), n);
> -
> -	if (!set_kernel_hrtimer)
> -		igt_assert(kernel_ns <= (test_duration_ns / 100ull));
> -
> -	__perf_close(perf_fd);
> +	igt_assert(kernel_ns <= (test_duration_ns / 100ull));

This check (and the identical check in the polling test) is also sort of
arbitrary. Should we remove even this? Anyway, it's ok to leave if it is
passing everywhere. Therefore lgtm:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>