[igt-dev] [PATCH i-g-t v7 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes

[Ghimiray, Himal Prasad]

> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit at intel.com>
> Sent: 07 July 2023 06:26
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Kamil Konieczny
> <kamil.konieczny at linux.intel.com>; Iddamsetty, Aravind
> <aravind.iddamsetty at intel.com>; Upadhyay, Tejas
> <tejas.upadhyay at intel.com>; Kumar, Janga Rahul
> <janga.rahul.kumar at intel.com>; Dugast, Francois
> <francois.dugast at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>
> Subject: Re: [PATCH i-g-t v7 2/4] lib/igt_sysfs: Handling gt related sysfs uapi
> changes
> 
> 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.

Sure.

> 
> 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.

Will go ahead with suggested approach.

> 
> 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.

Sure.

> 
> > 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
> >