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 85A17CD4F4A for ; Mon, 18 May 2026 23:53:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 24CBB10E35E; Mon, 18 May 2026 23:53:24 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="iT7Dhjd3"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.14]) by gabe.freedesktop.org (Postfix) with ESMTPS id 24A4F10E35E for ; Mon, 18 May 2026 23:53:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1779148386; x=1810684386; h=date:message-id:from:to:subject:in-reply-to:references: mime-version; bh=FmogMNysynLuowzIeD3E366QIJyfyrXHZ4vOfofcoig=; b=iT7Dhjd3UbsNhtmpoTh+w0C8AC6An7irtXNpxWmx5PITa4oV4k0aPUX2 9OF6ca3CJmMdYoUkFr2DetpFHd8tV6PRed4jaK+8nf6rIMAJvHGEIRShJ S/eYZ8M5rA30JTU5XN65KFQ7fpHTOjzP1oJuf2w4rh+HwWAo7sAgvCLIi hSvLQkUjtaIlLR4s1Vw5+ZGYe5QCawqJSsMJh2vD1TzInmb22nK6WbvrB B6M0kGBdzKYX6QcbBzNwRKGpvOL/+dNT/G7fDnQsSXFRe6CiI0odddfZQ qtFXiumfl/5aVEl7BenIKIUDZlOxKE4IY0xxEJVjI//1s84WRiOlG2Q1D w==; X-CSE-ConnectionGUID: YCLs9N8UQYiVu/YHsz+tIA== X-CSE-MsgGUID: dAKKY8DsRnCSrbmb8gsloA== X-IronPort-AV: E=McAfee;i="6800,10657,11790"; a="80049003" X-IronPort-AV: E=Sophos;i="6.23,242,1770624000"; d="scan'208";a="80049003" Received: from fmviesa003.fm.intel.com ([10.60.135.143]) by fmvoesa108.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2026 16:53:06 -0700 X-CSE-ConnectionGUID: NLAJ/caNQ6anXIQxrWeV6g== X-CSE-MsgGUID: gOlupZMLS6iYkOwNse6UnQ== X-ExtLoop1: 1 Received: from unknown (HELO adixit-MOBL3.intel.com) ([10.241.243.116]) by fmviesa003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2026 16:53:06 -0700 Date: Mon, 18 May 2026 16:53:04 -0700 Message-ID: <87cxysgtr3.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Kamil Konieczny , Shekhar Chauhan , , Subject: Re: [PATCH v2] lib/intel_wa: Assert on error instead of returning -1 In-Reply-To: <87ecjbgofi.wl-ashutosh.dixit@intel.com> References: <20260514063655.1683322-1-shekhar.chauhan@intel.com> <20260515122408.zd564l5tnhaprha3@kamilkon-DESK.igk.intel.com> <87ecjbgofi.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/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 Sat, 16 May 2026 12:11:13 -0700, Dixit, Ashutosh wrote: > > On Fri, 15 May 2026 05:24:08 -0700, Kamil Konieczny wrote: > > > > Hi Kamil, > > > Hi Shekhar, > > On 2026-05-14 at 12:06:55 +0530, Shekhar Chauhan wrote: > > > igt_has_intel_wa() returns 0, 1, or -1, but there is no point > > > distinguishing the error and no-workaround cases for callers. Replace > > > the error return with igt_assert() and simplify debugfs_file_has_wa() by > > > dropping the igt_debugfs_exists() check, since igt_sysfs_get() returns > > > NULL for missing files anyway. > > > > > > v2: Make the func as bool instead of int. > > > > > > Signed-off-by: Shekhar Chauhan > > > --- > > > lib/intel_wa.c | 19 +++++++++---------- > > > lib/intel_wa.h | 2 +- > > > 2 files changed, 10 insertions(+), 11 deletions(-) > > > > > > diff --git a/lib/intel_wa.c b/lib/intel_wa.c > > > index 727dd6c98..e4fe7b0ee 100644 > > > --- a/lib/intel_wa.c > > > +++ b/lib/intel_wa.c > > > @@ -7,18 +7,18 @@ > > > #include > > > #include > > > > > > +#include "igt_core.h" > > > #include "igt_debugfs.h" > > > #include "igt_sysfs.h" > > > #include "intel_wa.h" > > > #include "xe/xe_query.h" > > > > > > -static int debugfs_file_has_wa(int drm_fd, int debugfs_fd, > > > +static bool debugfs_file_has_wa(int drm_fd, int debugfs_fd, > > > const char *debugfs_name, const char *wa) > > > { > > > char *debugfs_dump; > > > > > > - if (!igt_debugfs_exists(drm_fd, debugfs_name, O_RDONLY)) > > > - return -1; > > > + igt_assert(igt_debugfs_exists(drm_fd, debugfs_name, O_RDONLY)); > > > > > > debugfs_dump = igt_sysfs_get(debugfs_fd, debugfs_name); > > > if (debugfs_dump) { > > > @@ -27,10 +27,10 @@ static int debugfs_file_has_wa(int drm_fd, int debugfs_fd, > > > free(debugfs_dump); > > > > > > if (has_wa) > > > - return 1; > > > + return true; > > > } > > > > > > - return 0; > > > + return false; > > > } > > > > > > /** > > > @@ -38,18 +38,17 @@ static int debugfs_file_has_wa(int drm_fd, int debugfs_fd, > > > * @drm_fd: A drm file descriptor > > > * @check_wa: Workaround to be checked > > > * > > > - * Returns: 0 if no WA, 1 if WA present, -1 on error > > > + * Returns: true if WA present, false otherwise > > > */ > > > -int igt_has_intel_wa(int drm_fd, const char *check_wa) > > > +bool igt_has_intel_wa(int drm_fd, const char *check_wa) > > > { > > > - int ret = 0; > > > + bool ret = 0; > > > int debugfs_fd; > > > unsigned int xe; > > > char name[256]; > > > > > > debugfs_fd = igt_debugfs_dir(drm_fd); > > > - if (debugfs_fd == -1) > > > - return -1; > > > + igt_assert(debugfs_fd >= 0); > > > > This will mean that all other users needs to be updated. > > To date there are no other users of this function, except one in OA. > > > The way of adding any igt_assert/igt_require into lib/ > > make it impossible to reuse them in tools/ and require > > to write same functionality twice, one in lib/ and one > > for any tools which will want to use them. Or to write > > __function() without igt_assert/igt_require. > > Well it was I who suggested doing this. Can you explain your point a bit? > Any tools which use a lib function which uses e.g. igt_assert/igt_require > can also link with lib_igt (which provides igt_assert/igt_require). See > e.g. tools/xe-perf/meson.build. It is not unreasonable for IGT tools to > link with lib_igt. I thought not linking with igt_lib was an unreasonabe > requirement and I removed it for tools/xe-perf. > > If you are saying tools should be able to run without debugfs mounted, > maybe you have a point. In that case we can drop this patch. But even here > maybe IGT tools should run only with debugfs mounted? Also the tool should realize it is calling a function which requires debugfs, and separately check whether or not debugfs is mounted and skip calling the function if it isn't. So I am not sure why we should do anything different, since wa list is exposed through debugfs. > > > > > xe_for_each_gt(drm_fd, xe) { > > > sprintf(name, "gt%d/workarounds", xe); > > > diff --git a/lib/intel_wa.h b/lib/intel_wa.h > > > index 765a5948e..34cafecc4 100644 > > > --- a/lib/intel_wa.h > > > +++ b/lib/intel_wa.h > > > @@ -6,6 +6,6 @@ > > > #ifndef __INTEL_WA_H__ > > > #define __INTEL_WA_H__ > > > > > > -int igt_has_intel_wa(int drm_fd, const char *check_wa); > > > +bool igt_has_intel_wa(int drm_fd, const char *check_wa); > > > > > > #endif /* __INTEL_WA_H__ */ > > > -- > > > 2.53.0 > > >