From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8B0BA10E4DB for ; Fri, 7 Jul 2023 00:56:48 +0000 (UTC) Date: Thu, 06 Jul 2023 17:56:27 -0700 Message-ID: <87edlkczes.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Himal Prasad Ghimiray In-Reply-To: <20230706104500.595707-3-himal.prasad.ghimiray@intel.com> References: <20230706104500.595707-1-himal.prasad.ghimiray@intel.com> <20230706104500.595707-3-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 i-g-t v7 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Upadhyay , igt-dev@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Thu, 06 Jul 2023 03:44:58 -0700, Himal Prasad Ghimiray wrote: > > +/* > + * xe_gt_sysfs_path:: > + * @sysfs: fd of sysfs > + * @gt_id: gt id > + * > + * This function returns the path for gtX directory > + */ > +char *xe_gt_sysfs_path(int sysfs, int gt_id) > +{ > + char tiledir[24]; > + char gtdir[32]; > + char *gt_path; > + int i = 0; > + > + snprintf(tiledir, sizeof(tiledir), "device/tile0/"); > + snprintf(gtdir, sizeof(gtdir), "%s/gt%d", tiledir, gt_id); > + > + while (dir_exists_in_sysfd(sysfs, tiledir)) { > + if (dir_exists_in_sysfd(sysfs, gtdir)) { > + gt_path = malloc(sizeof(gtdir)); > + strcpy(gt_path, gtdir); > + return gt_path; > + } else { > + snprintf(tiledir, sizeof(tiledir), "device/tile%d/", i++); > + snprintf(gtdir, sizeof(gtdir), "%s/gt%d", tiledir, gt_id); > + } > + } > + igt_assert_f(false, "No gt%d dir found in sysfs\n", gt_id); > +} Now that you I see the code (sorry for changing again but we are adding this to the igt lib so functions should have some uniformity), this function should have the same prototype and arguments as igt_sysfs_gt_path(). Also call it xe_sysfs_gt_path. Also because now we have drm_fd available we can find the device (PVC/MTL) so we can use the simpler implementation I mentioned before: switch (devid) { case PVC: return tile0/gt0 or tile1/gt1 default: return tile0/gtN } This function can be extended for future platforms as they arise. Or someone can write a better function to go over the dir tree. I think it should be done similar to going over the directory as is done in hwmon_read/hwmon_write. I don't like the implementation above but for now the approach mentioned above is fine. The above means we will change set_freq()/get_freq() in xe_guc_pc.c to take the drm_fd as the first argument rather than the sysfs fd. So there will be changes throught the file but better do that once so that all future IGT's can follow the same pattern. Also write another function xe_sysfs_gt_open() similar to igt_sysfs_gt_open() or xe_sysfs_tile_open(). I want Kamil to take a look at this series too after he is back. > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h > index 5d584b1c7..0792c644e 100644 > --- a/lib/igt_sysfs.h > +++ b/lib/igt_sysfs.h > @@ -153,6 +153,7 @@ typedef struct igt_sysfs_rw_attr { > } igt_sysfs_rw_attr_t; > > void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw); > +char *xe_gt_sysfs_path(int sysfs, int gt_id); > > void igt_sysfs_engines(int xe, int engines, const char **property, > void (*test)(int, int, const char **)); > -- > 2.25.1 >