From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1FDDBC44515 for ; Mon, 20 Jul 2026 21:08:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8EFF610E1DC; Mon, 20 Jul 2026 21:08:46 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="T6U71ku/"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.11]) by gabe.freedesktop.org (Postfix) with ESMTPS id E142810E1DC for ; Mon, 20 Jul 2026 21:08:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1784581695; x=1816117695; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=TEy6nN5iDSMuRQ86PGYxKPqdMhd25IpMvILX6ZZhhIY=; b=T6U71ku/NYVS7WwPs5CJIqP5WJXrkFrPnjpqAMlrs0jDo0SpgnyJaqED vouiOHmQUBwbJFSogyx7p+eGwasyrIf52HKsfFsYUMq2Jo0XpZJeKguBi g3w8gjw7I1se0mPzCakct1GEsqDV2trxQr9pBJCj4usqNHNWjzM344xRy qQZQ60A3BQX9qIo6jSM4z1sm1DaqriNErU9q4GXoQC1suOTu283kWrOth aaFDta9s09PmhCMdZkeivLSFnDrEdoOG35Hsty+Tl08/L9EM/6n/Ii2mG imH4a14jTJ3wVVovy5ELFYceQzcOAVWyq/73zwAR+fWHiHS52aStq1rzH Q==; X-CSE-ConnectionGUID: 42htDf74SDOw/S7GKNBAjA== X-CSE-MsgGUID: Fg1cqupMRcajeeOlAdX2iQ== X-IronPort-AV: E=McAfee;i="6800,10657,11852"; a="95768657" X-IronPort-AV: E=Sophos;i="6.25,175,1779174000"; d="scan'208";a="95768657" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jul 2026 14:08:14 -0700 X-CSE-ConnectionGUID: W5vJ7GJSR8iVbQbUANJPmg== X-CSE-MsgGUID: YSjhIT2zQ8SpElIDj4Pruw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,175,1779174000"; d="scan'208";a="295776022" Received: from oodele-mobl.amr.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.65.76]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jul 2026 14:08:13 -0700 Date: Mon, 20 Jul 2026 14:08:11 -0700 Message-ID: <87ecgxxtc4.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: Subject: Re: [PATCH 4/4] tests/intel/xe_oa: Cleanup blocking and polling tests In-Reply-To: <20260717210027.2008255-10-umesh.nerlige.ramappa@intel.com> References: <20260717210027.2008255-6-umesh.nerlige.ramappa@intel.com> <20260717210027.2008255-10-umesh.nerlige.ramappa@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-BeenThere: igt-dev@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development mailing list for IGT GPU Tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" 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 > --- > 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, ¶m, 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