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 D419AC5B543 for ; Thu, 5 Jun 2025 02:29:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7F40610E86B; Thu, 5 Jun 2025 02:29:48 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="VIkJtoi4"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id DA44F10E855 for ; Thu, 5 Jun 2025 02:29:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1749090587; x=1780626587; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=Uyh4CMFJFYh5UUzkLfL8lon/ueY8K1/A3xdqHJ0XihE=; b=VIkJtoi45Ti5kUUGl+6yEyublhuG5qi85Afbnwko6JB0IwTjRGxWgVBZ hlsOP52xqj5NdvEFZH6wxH84WzpTCerUbPErufkkEdf9GtS3rFv9l95aX c795emWEFEYBGj0h1e0QgVLMfA9qtlcxSnIxcEUru93fdxy5g7U12AKQ8 BwVmbegg1PzRzR1efQJf1TA5z5E3Bwy1XnJ4ezwCdQcfrZfdcFYJ3JdRH YIl2qHutegd9GOWFrhlzhKu9PGXX8ABJvnVQwuNMXWaE6dJxAQjkbKOyI 3GjpNHcJspdc6vp2rzwx/Y9GyWFedjG65a0Y2O4XEUxUmZVxBk3S0b8Q9 A==; X-CSE-ConnectionGUID: hH62Ua8uQ8eAkGyNa720kQ== X-CSE-MsgGUID: h2baZPjKSI+onm/SgTbozg== X-IronPort-AV: E=McAfee;i="6800,10657,11454"; a="62549881" X-IronPort-AV: E=Sophos;i="6.16,210,1744095600"; d="scan'208";a="62549881" Received: from fmviesa010.fm.intel.com ([10.60.135.150]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jun 2025 19:29:46 -0700 X-CSE-ConnectionGUID: z1/COcr/TZ+uf9o/05cFYA== X-CSE-MsgGUID: 2qCclZ76QqSiIc9TzjdRWQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.16,210,1744095600"; d="scan'208";a="145855924" Received: from asinha1-mobl.amr.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.217.164]) by fmviesa010-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jun 2025 19:29:46 -0700 Date: Wed, 04 Jun 2025 19:29:45 -0700 Message-ID: <871pryvs2u.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Harish Chegondi Cc: , Lukasz Laguna , "Adam Miszczak" , Jakub Kolakowski , Kamil Konieczny , Marcin Bernatowicz Subject: Re: [PATCH i-g-t 3/3] tests/intel/xe_eu_stall: Use query IOCTL to check for EU stall support In-Reply-To: <875xhbuoev.wl-ashutosh.dixit@intel.com> References: <0e894b0ab963e0e5b03b200f919e9318fe02de20.1748994982.git.harish.chegondi@intel.com> <875xhbuoev.wl-ashutosh.dixit@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/29.4 (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 Wed, 04 Jun 2025 15:34:16 -0700, Dixit, Ashutosh wrote: > > On Tue, 03 Jun 2025 16:57:36 -0700, Harish Chegondi wrote: > > > > Use EU stall query IOCTL to check if EU stall monitoring feature is > > available on the platform and if it is supported in the driver. > > This would replace the checks in the test with those in the driver. > > > > Cc: Lukasz Laguna > > Cc: Ashutosh Dixit > > Cc: Adam Miszczak > > Cc: Jakub Kolakowski > > Cc: Kamil Konieczny > > Cc: Marcin Bernatowicz > > Signed-off-by: Harish Chegondi > > Resolves: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/5131 > > --- > > tests/intel/xe_eu_stall.c | 55 +++++++++++++++++++++------------------ > > 1 file changed, 30 insertions(+), 25 deletions(-) > > > > diff --git a/tests/intel/xe_eu_stall.c b/tests/intel/xe_eu_stall.c > > index b3a0ee742..4e76018b6 100644 > > --- a/tests/intel/xe_eu_stall.c > > +++ b/tests/intel/xe_eu_stall.c > > @@ -74,6 +74,8 @@ static int stream_fd = -1; > > > > static volatile bool child_is_running = true; > > > > +static struct drm_xe_query_eu_stall *query_eu_stall_data; > > + > > /* > > * EU stall data format for PVC > > */ > > @@ -506,33 +508,12 @@ static void test_eustall(int drm_fd, uint32_t devid, bool blocking_read, int ite > > .properties_ptr = to_user_pointer(properties), > > }; > > > > - struct drm_xe_query_eu_stall *query_eu_stall_data; > > - struct drm_xe_device_query query = { > > - .extensions = 0, > > - .query = DRM_XE_DEVICE_QUERY_EU_STALL, > > - .size = 0, > > - .data = 0, > > - }; > > - > > igt_info("User buffer size: %u\n", p_user); > > if (p_args[0]) > > igt_info("Workload: %s\n", p_args[0]); > > else > > igt_info("Workload: GPGPU fill\n"); > > - > > - igt_assert_eq(igt_ioctl(drm_fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0); > > - igt_assert_neq(query.size, 0); > > - > > - query_eu_stall_data = malloc(query.size); > > - igt_assert(query_eu_stall_data); > > - > > - query.data = to_user_pointer(query_eu_stall_data); > > - igt_assert_eq(igt_ioctl(drm_fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0); > > - > > - igt_assert(query_eu_stall_data->num_sampling_rates > 0); > > - if (p_rate == 0) > > - properties[3] = query_eu_stall_data->sampling_rates[0]; > > - igt_info("Sampling Rate: %lu\n", properties[3]); > > + igt_info("Sampling Rate: %lu\n", p_rate); > > > > stream_fd = eu_stall_open(drm_fd, &props); > > > > @@ -660,18 +641,42 @@ static struct option long_options[] = { > > > > igt_main_args("e:g:o:r:u:w:", long_options, help_str, opt_handler, NULL) > > { > > - int drm_fd; > > + bool blocking_read = true; > > + int drm_fd, ret; > > uint32_t devid; > > struct stat sb; > > - bool blocking_read = true; > > + struct drm_xe_device_query query = { > > + .extensions = 0, > > + .query = DRM_XE_DEVICE_QUERY_EU_STALL, > > + .size = 0, > > + .data = 0, > > + }; > > > > igt_fixture { > > drm_fd = drm_open_driver(DRIVER_XE); > > igt_require_fd(drm_fd); > > devid = intel_get_drm_devid(drm_fd); > > - igt_require(IS_PONTEVECCHIO(devid) || intel_graphics_ver(devid) >= IP_VER(20, 0)); > > + > > igt_require_f(igt_get_gpgpu_fillfunc(devid), "no gpgpu-fill function\n"); > > igt_require_f(!stat(OBSERVATION_PARANOID, &sb), "no observation_paranoid file\n"); > > + > > + ret = igt_ioctl(drm_fd, DRM_IOCTL_XE_DEVICE_QUERY, &query); > > + igt_skip_on_f((ret == -1 && errno == ENODEV), > > + "EU stall monitoring is not available on this platform\n"); > > + igt_skip_on_f((ret == -1 && errno == EINVAL), > > inner brackets not needed... > > > + "EU stall monitoring is not supported in the driver\n"); > > + igt_assert_neq(query.size, 0); > > + > > + query_eu_stall_data = malloc(query.size); > > + igt_assert(query_eu_stall_data); > > + > > + query.data = to_user_pointer(query_eu_stall_data); > > + igt_assert_eq(igt_ioctl(drm_fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0); > > + > > + igt_assert(query_eu_stall_data->num_sampling_rates > 0); > > + if (p_rate == 0) > > + p_rate = query_eu_stall_data->sampling_rates[0]; > > + > > Maybe cleaner to put all this query stuff into a function and calling the > function from the fixture? Also, queries are generally cached in 'struct > xe_device' so we could do the same here? See xe_device_get() and also > xe_query_oa_units_new(). Since that's in the lib, make sure IGT still works > even if the eu_stall query fails (see 851e62faf971). > > Also, another nit, use igt_require or igt_require_f instead of > igt_skip_on_f? Though they are mostly NOT of each other so they are > equivalent, so either is ok. Changes suggested above are optional, but please consider if they are worth making. If you do I'll review again. But for now I'll R-b this as is: Reviewed-by: Ashutosh Dixit Will need to spin another version of this series anyway, since this can't be merged as is due to conflicts. So I'll wait for a v2. Thanks. > > > if (output_file) { > > output = fopen(output_file, "w"); > > igt_require(output); > > -- > > 2.48.1 > >