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 534C5C021BB for ; Mon, 24 Feb 2025 20:11:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0CFF010E34C; Mon, 24 Feb 2025 20:11:43 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="X+b2Zdl0"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by gabe.freedesktop.org (Postfix) with ESMTPS id 900A910E362 for ; Mon, 24 Feb 2025 20:11:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1740427902; x=1771963902; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=7iy0x5K0jQvy7hVTGR87QPBsy6UtcyTN+vLKB/hsIXU=; b=X+b2Zdl0Ei/jmeeYE0ojAM8o4J3QXtHbve2iCwZbPTa/cgok99kXktjF JsBfVr2HgnI4XucVMmLrOSWzD13O9Ev4ARnIBNXf3pwmEBROpshXK7Wbk iYWdCH3maowKTR/qNU181E9KZD8qoHyl2EC8XBSPxbWnVZwxZPjn0Zfnq DEgZ75m3V66geJNCQ3s1HawPkNiPFIFN56SQvs6CbaJx9nad5KCq1zE3X oe3vMQtBhnGHDjdTJacFe8QB0/rhMD5nQLkbvt2Xrqb/AiwmMdkZgbp6p zT1nOEg3dde9BZ/HhRvQHlw60ntPpYA6Jbx7hn5ZD017cpq6DyDxHTbsK Q==; X-CSE-ConnectionGUID: QceUeitLTEuT6/diasNyWw== X-CSE-MsgGUID: rwcnpxNUSaK59qgSgvLecg== X-IronPort-AV: E=McAfee;i="6700,10204,11355"; a="44031015" X-IronPort-AV: E=Sophos;i="6.13,312,1732608000"; d="scan'208";a="44031015" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Feb 2025 12:11:39 -0800 X-CSE-ConnectionGUID: 3eHkjk2qRiya5s5i1rWhOQ== X-CSE-MsgGUID: 8cl11kYbTSWengrG47TlIA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,312,1732608000"; d="scan'208";a="120780657" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.142]) by fmviesa005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Feb 2025 12:11:38 -0800 Date: Mon, 24 Feb 2025 12:11:37 -0800 Message-ID: <85seo3je6e.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: Subject: Re: [PATCH i-g-t CI run 06/14] tests/intel/xe_oa: Rewrite the polling small buf test In-Reply-To: <20250218202812.1679653-7-umesh.nerlige.ramappa@intel.com> References: <20250218202812.1679653-1-umesh.nerlige.ramappa@intel.com> <20250218202812.1679653-7-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/28.2 (x86_64-redhat-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 Tue, 18 Feb 2025 12:28:04 -0800, Umesh Nerlige Ramappa wrote: > Hi Umesh, > Use mmio reads as a side-channel to determine if reports are available > and ensure that poll will return with POLLIN set. Then provide a small > buffer to force ENOSPC error. Then poll with a timeout of 0 to check if > POLLIN is still set. Will need a reason for doing this here. But see below. > > Signed-off-by: Umesh Nerlige Ramappa > --- > tests/intel/xe_oa.c | 64 +++++++++++++++++++++++++-------------------- > 1 file changed, 35 insertions(+), 29 deletions(-) > > diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c > index aaf92308a..5792ffec2 100644 > --- a/tests/intel/xe_oa.c > +++ b/tests/intel/xe_oa.c > @@ -2216,7 +2216,6 @@ static void test_polling(uint64_t requested_oa_period, > */ > static void test_polling_small_buf(void) > { > - int oa_exponent = max_oa_exponent_for_period_lte(40 * 1000); /* 40us */ > uint64_t properties[] = { > DRM_XE_OA_PROPERTY_OA_UNIT_ID, 0, > > @@ -2226,50 +2225,57 @@ static void test_polling_small_buf(void) > /* OA unit configuration */ > DRM_XE_OA_PROPERTY_OA_METRIC_SET, default_test_set->perf_oa_metrics_set, > DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(default_test_set->perf_oa_format), > - DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, oa_exponent, > + DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, oa_exponent_default, > DRM_XE_OA_PROPERTY_OA_DISABLED, true, > }; > struct intel_xe_oa_open_prop param = { > .num_properties = ARRAY_SIZE(properties) / 2, > .properties_ptr = to_user_pointer(properties), > }; > - uint32_t test_duration = 80 * 1000 * 1000; > - int sample_size = get_oa_format(default_test_set->perf_oa_format).size; > - int n_expected_reports = test_duration / oa_exponent_to_ns(oa_exponent); > - int n_expect_read_bytes = n_expected_reports * sample_size; > - struct timespec ts = {}; > - int n_bytes_read = 0; > - uint32_t n_polls = 0; > + int report_size = get_oa_format(default_test_set->perf_oa_format).size; > + u32 oa_tail, prev_tail; > + struct pollfd pollfd; > + uint8_t buf[10]; > + int ret, i = 0; > + > + intel_register_access_init(&mmio_data, > + igt_device_get_pci_device(drm_fd), 0); > > stream_fd = __perf_open(drm_fd, ¶m, true /* prevent_pm */); > set_fd_flags(stream_fd, O_CLOEXEC | O_NONBLOCK); > - do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0); > - > - while (igt_nsec_elapsed(&ts) < test_duration) { > - struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN }; > > - ppoll(&pollfd, 1, NULL, NULL); > - if (pollfd.revents & POLLIN) { > - uint8_t buf[1024]; > - int ret; > +#define OAG_OATAILPTR (0xdb04) > + /* Save the current tail */ > + prev_tail = oa_tail = intel_register_read(&mmio_data, OAG_OATAILPTR); > > - ret = read(stream_fd, buf, sizeof(buf)); > - if (ret >= 0) > - n_bytes_read += ret; > - } > + /* Kickstart the capture */ > + do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0); > > - n_polls++; > + /* Wait for 5 reports */ Wait for 5 reports or 10 ms ? > + while ((oa_tail - prev_tail) < (5 * report_size)) { > + usleep(1000); > + oa_tail = intel_register_read(&mmio_data, OAG_OATAILPTR); > + if (i++ > 10) So on slow platforms we might not get any reports in 10 ms? The idea here should be to not have any timing dependence? So if we want to wait for 5 reports, just wait for 5 reports? We tried doing this for the mmap OA buffer: see mmap_wait_for_periodic_reports(), the function waits indefinitely. So if this is done I am not sure if the intel_register_read() approach is needed (but I didn't think of doing that :). But I guess we can use it to see when there are N reports available. Longer term it would be nice to have a centralized function wait_for_n_reports(int n) or something like that which different tests can use. > + break; > } > > - igt_info("Read %d expected %d (%.2f%% of the expected number), polls=%u\n", > - n_bytes_read, n_expect_read_bytes, > - n_bytes_read * 100.0f / n_expect_read_bytes, > - n_polls); > + intel_register_access_fini(&mmio_data); > > - __perf_close(stream_fd); > + /* Just read one report and expect ENOSPC */ > + pollfd.fd = stream_fd; > + pollfd.events = POLLIN; > + poll(&pollfd, 1, 1000); > + igt_assert(pollfd.revents & POLLIN); Is the assumption here that the kernel timer is firing every 5 ms (so if we've waited for 10 ms POLLIN must be set since the timer is firing every 5 ms)? I am not sure if that 5 ms is uapi. Or is it? Actually I was thinking of changing that 5 ms time or changing the timer to a delayed work. > + errno = 0; > + ret = read(stream_fd, buf, sizeof(buf)); > + igt_assert_eq(ret, -1); > + igt_assert_eq(errno, ENOSPC); This part looks ok, it's uapi. > > - igt_assert(abs(n_expect_read_bytes - n_bytes_read) < > - 0.20 * n_expect_read_bytes); > + /* Poll with 0 timeout and expect POLLIN flag to be set */ > + poll(&pollfd, 1, 0); > + igt_assert(pollfd.revents & POLLIN); > + > + __perf_close(stream_fd); How about just reading N reports using a small buffer for this test, however long it takes? N can 5 or 10. Thanks. -- Ashutosh PS: how about separating out the patches which currently have R-b into a separate series and merging them first?