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 E4638CCD183 for ; Mon, 13 Oct 2025 20:32:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9850110E51E; Mon, 13 Oct 2025 20:32:19 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="HxFS6X7L"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id DA08910E51E for ; Mon, 13 Oct 2025 20:32:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1760387539; x=1791923539; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=fsPumpxgYdQQNlM+cYV1M5Bx+e14Evs5Cz/Alq4qcH8=; b=HxFS6X7LbN34vJBl/Q/es+zFZ3ldG7NnDxnEGC/HW3m+iYtFHiyH6dSH 77kDD+mk1+HNwn+2JjHGVYcDOzmTg8tzCEKxHaxFNSA0Iwy4oXghi8Bvo 6mR+OgkrpOQpQphdg4YvEyntv8cElGy+ZKkwzYKaDV0aR4lWr+Jx+v77t vrYrU4ohyGiBPf5PiLxpbKBnwewDd/KWGlAGuvhiz1Ttt8JxATEKkLPxT QYeHqSrTeHCEfKiWNge3+kTZYCUJLPNmQg93CFGXTVqCHBmYEIZDIWZeG 36gDwvd4WJMxmtN6VmajhUArJ4c8LlDZ/T+EZHFDSXfdmMiBqia/ixhtG g==; X-CSE-ConnectionGUID: Hzv8umUYQhepgX1nxSBPlQ== X-CSE-MsgGUID: rtiC4vrfQ72WV+OOVoDU8A== X-IronPort-AV: E=McAfee;i="6800,10657,11581"; a="73987058" X-IronPort-AV: E=Sophos;i="6.19,226,1754982000"; d="scan'208";a="73987058" Received: from fmviesa002.fm.intel.com ([10.60.135.142]) by orvoesa104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2025 13:32:19 -0700 X-CSE-ConnectionGUID: Wh2I0PjTRkqg/yrjDmvmtg== X-CSE-MsgGUID: wFTo4IDnQTe/dYn/ieHi3g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.19,226,1754982000"; d="scan'208";a="205390776" Received: from unknown (HELO adixit-MOBL3.intel.com) ([10.241.243.158]) by fmviesa002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2025 13:32:18 -0700 Date: Mon, 13 Oct 2025 13:32:11 -0700 Message-ID: <878qhewnjo.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: Subject: Re: [PATCH i-g-t 06/18] tests/intel/xe_oa: Add for_each_oa_unit In-Reply-To: References: <20251008211806.625154-1-ashutosh.dixit@intel.com> <20251008211806.625154-7-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/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 Thu, 09 Oct 2025 09:40:29 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > On Wed, Oct 08, 2025 at 04:31:23PM -0700, Umesh Nerlige Ramappa wrote: > > On Wed, Oct 08, 2025 at 02:17:53PM -0700, Ashutosh Dixit wrote: > >> Add a for_each_oa_unit iterator, which eliminates code duplication when > >> iterating over OA units obtained from OA unit query. > >> > >> Signed-off-by: Ashutosh Dixit > >> --- > >> tests/intel/xe_oa.c | 21 +++++++++++---------- > >> 1 file changed, 11 insertions(+), 10 deletions(-) > >> > >> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c > >> index 012f5929ce..bc71bfcd30 100644 > >> --- a/tests/intel/xe_oa.c > >> +++ b/tests/intel/xe_oa.c > >> @@ -477,18 +477,21 @@ static struct drm_xe_engine_class_instance *oa_unit_engine(struct drm_xe_oa_unit > >> return !oau ? NULL : oau->num_engines ? &oau->eci[random() % oau->num_engines] : NULL; > >> } > >> > >> +#define for_each_oa_unit(qoa, oau, poau, i) \ > >> + for (i = 0, qoa = xe_oa_units(drm_fd), poau = (u8 *)&qoa->oa_units[0]; \ > >> + oau = (struct drm_xe_oa_unit *)poau, i < qoa->num_oa_units; \ > >> + poau += sizeof(*oau) + oau->num_engines * sizeof(oau->eci[0]), i++) > >> + > >> static struct drm_xe_oa_unit *oa_unit_by_id(int fd, int n) > >> { > >> - struct drm_xe_query_oa_units *qoa = xe_oa_units(fd); > >> + struct drm_xe_query_oa_units *qoa; > >> struct drm_xe_oa_unit *oau; > >> u8 *poau; > >> + int i; > >> > >> - poau = (u8 *)&qoa->oa_units[0]; > >> - for (int i = 0; i < qoa->num_oa_units; i++) { > >> - oau = (struct drm_xe_oa_unit *)poau; > >> + for_each_oa_unit(qoa, oau, poau, i) { > >> if (oau->oa_unit_id == n) > >> return oau; > >> - poau += sizeof(*oau) + oau->num_engines * sizeof(oau->eci[0]); > >> } > >> > >> return NULL; > >> @@ -496,16 +499,14 @@ static struct drm_xe_oa_unit *oa_unit_by_id(int fd, int n) > >> > >> static struct drm_xe_oa_unit *oa_unit_by_type(int fd, int t) > >> { > >> - struct drm_xe_query_oa_units *qoa = xe_oa_units(fd); > >> + struct drm_xe_query_oa_units *qoa; > >> struct drm_xe_oa_unit *oau; > >> u8 *poau; > >> + int i; > >> > >> - poau = (u8 *)&qoa->oa_units[0]; > >> - for (int i = 0; i < qoa->num_oa_units; i++) { > >> - oau = (struct drm_xe_oa_unit *)poau; > >> + for_each_oa_unit(qoa, oau, poau, i) { > >> if (oau->oa_unit_type == t) > >> return oau; > >> - poau += sizeof(*oau) + oau->num_engines * sizeof(oau->eci[0]); > >> } > >> > >> return NULL; > > > > If it's possible to keep the interface for iterators simple, I would go > > for that. i, poau can be hidden from the caller. Something like this: > > > > // NOTE: not compiled/tested > > > > static u32 xe_oa_first_unit(int fd, struct drm_xe_oa_unit **oau) > > { > > struct drm_xe_query_oa_units *qoa = xe_oa_units(fd); > > *oau = &qoa->oa_units[0]; > > > > return qoa->num_oa_units; > > } > > > > static void xe_oa_next_unit(struct drm_xe_oa_unit **oau) > > { > > struct drm_xe_oa_unit *poau = *oau; > > > > return (u8 *)poau + sizeof(*poau) + poau->num_engines * sizeof(poau->eci[0]); > > Not return, but update *oau. > > *oau = (u8 *)poau + sizeof(*poau) + poau->num_engines * sizeof(poau->eci[0]); In v2, I have implemented something not identical, but very close to this. But thanks for this suggestion, it has resulted in some cleaned up code. Ashutosh > > > } > > > > #define for_each_oa_unit(fd, oau) \ > > for (u32 i = 0, num = xe_oa_first_unit(fd, &oau); \ > > oau && i < num; \ > > i++, xe_oa_next_unit(&oau)) > > > > static const struct drm_xe_oa_unit *oa_unit_by_id(int fd, int n) > > { > > struct drm_xe_oa_unit *oau; > > > > for_each_oa_unit(fd, oau) { > > if (oau->oa_unit_id == n) > > return oau; > > } > > > > return NULL; > > } > > > > static const struct drm_xe_oa_unit *oa_unit_by_type(int fd, int t) > > { > > struct drm_xe_oa_unit *oau; > > > > for_each_oa_unit(fd, oau) { > > if (oau->oa_unit_type == t) > > return oau; > > } > > > > return NULL; > > } > > > > Thanks, > > Umesh > > > > > > > >> -- > >> 2.48.1 > >>