From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id F277010E11C for ; Mon, 26 Jun 2023 20:27:17 +0000 (UTC) Date: Mon, 26 Jun 2023 13:24:15 -0700 Message-ID: <87jzvq6kgg.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Upadhyay, Tejas" In-Reply-To: References: <20230623114946.3803540-1-himal.prasad.ghimiray@intel.com> <20230623114946.3803540-2-himal.prasad.ghimiray@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH v2 1/4] lib/igt_sysfs: Add support to query number of tiles List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "igt-dev@lists.freedesktop.org" Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Mon, 26 Jun 2023 03:24:05 -0700, Upadhyay, Tejas wrote: > > > -----Original Message----- > > From: Ghimiray, Himal Prasad > > Sent: Friday, June 23, 2023 5:20 PM > > To: igt-dev@lists.freedesktop.org > > Cc: Ghimiray, Himal Prasad ; Iddamsetty, > > Aravind ; Upadhyay, Tejas > > ; Kumar, Janga Rahul > > > > Subject: [PATCH v2 1/4] lib/igt_sysfs: Add support to query number of tiles > > > > With tile and GT seperation in KMD, we need to know number of tiles > > supported by platform. > > We will need to access tile associated properties from IGT. > > Hence adding iterator for all supported tiles. > > > > v2: > > - Calculate number of tiles once within iterator. (Rahul) > > - Use snprintf instead of sprintf. > > > > Cc: Aravind Iddamsetty > > Cc: Upadhyay > > Cc: Janga Rahul Kumar > > Signed-off-by: Himal Prasad Ghimiray > > --- > > lib/igt_sysfs.c | 30 ++++++++++++++++++++++++++++++ lib/igt_sysfs.h | 8 > > ++++++++ > > 2 files changed, 38 insertions(+) > > > > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c index 35a4faa9..495b0288 100644 > > --- a/lib/igt_sysfs.c > > +++ b/lib/igt_sysfs.c > > @@ -287,6 +287,36 @@ int igt_sysfs_get_num_gt(int device) > > return num_gts; > > } > > > > +/** > > + * igt_sysfs_get_num_tiles: > > + * @device: fd of the device > > + * > > + * Reads number of tiles sysfs entries. > > + * Asserts for at least one tile entry. > > + * > > + * Returns: Number of tiles. > > + */ > > +int igt_sysfs_get_num_tiles(int device) { > > + int num_tiles; > > + char sysfs[48]; > > + char tiledir[96]; > > + > > + num_tiles =0; > > + if (!igt_sysfs_path(device, sysfs, sizeof(sysfs))) > > + return -1; > > + > > + do { > > + igt_assert(snprintf(tiledir, sizeof(tiledir), "%s/device/tile%d", > > + sysfs, num_tiles) < sizeof(tiledir)); > > I feel this is not best way to count tiles, as because of some reason if > we don't create tile sysfs dir , wrong number of tiles will be detected > and then logic to follow for_each_gt/tile. But for now as we don't have > anything supporting in kernel right now to give tile info we can rely on > this. > > Reviewed-by: Tejas Upadhyay I don't see the point of adding temporary solutions like this. Why not add XE_QUERY_CONFIG_TILE_COUNT under drm_xe_query_config in the kernel? > > > + num_tiles++; > > + } while (!access(tiledir, F_OK)); > > + > > + num_tiles--; > > + igt_assert_f(num_tiles > 0, "No tiles sysfs entry is found."); > > + return num_tiles; > > +} > > + > > /** > > * igt_sysfs_write: > > * @dir: directory for the device from igt_sysfs_open() diff --git > > a/lib/igt_sysfs.h b/lib/igt_sysfs.h index 978b6906..de2c9a86 100644 > > --- a/lib/igt_sysfs.h > > +++ b/lib/igt_sysfs.h > > @@ -38,6 +38,11 @@ > > (dirfd__ = igt_sysfs_gt_open(i915__, gt__)) != -1; \ > > close(dirfd__), gt__++) > > > > +#define for_each_sysfs_tile_dirfd(xe__, tile__, tile_cnt__) \ > > + for (tile__ = 0, tile_cnt__ = igt_sysfs_get_num_tiles(xe__); \ > > + tile__ < tile_cnt__; \ > > + ++tile__) Where is dirfd here which should be a fd to an open sysfs directory as in for_each_sysfs_gt_dirfd? > > + > > #define i915_for_each_gt for_each_sysfs_gt_dirfd > > > > #define igt_sysfs_rps_write(dir, id, data, len) \ @@ -73,6 +78,8 @@ #define > > igt_sysfs_rps_set_boolean(dir, id, value) \ > > igt_sysfs_set_boolean(dir, igt_sysfs_dir_id_to_name(dir, id), value) > > > > +#define xe_for_each_tile for_each_sysfs_tile_dirfd > > + > > enum i915_attr_id { > > RPS_ACT_FREQ_MHZ, > > RPS_CUR_FREQ_MHZ, > > @@ -97,6 +104,7 @@ int igt_sysfs_open(int device); char > > *igt_sysfs_gt_path(int device, int gt, char *path, int pathlen); int > > igt_sysfs_gt_open(int device, int gt); int igt_sysfs_get_num_gt(int device); > > +int igt_sysfs_get_num_tiles(int device); > > bool igt_sysfs_has_attr(int dir, const char *attr); const char > > *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id); const char > > *igt_sysfs_path_id_to_name(const char *path, enum i915_attr_id id); > > -- > > 2.25.1 >