Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_sysfs: Add helpers to iterate over gts

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Thu, 14 Apr 2022 08:18:57 -0700, Kamil Konieczny wrote:
>
> Hi Ashutosh,

Hi Kamil,

> may you change last word in subject from "gts" into "GTs" ?

Done in v2.

> > +static const char *i915_attr_name[SYSFS_NUM_TYPES][SYSFS_NUM_ATTR] = {
> > +	{
> > +		"gt_act_freq_mhz",
> > +		"gt_cur_freq_mhz",
> > +		"gt_min_freq_mhz",
> > +		"gt_max_freq_mhz",
> > +		"gt_RP0_freq_mhz",
> > +		"gt_RP1_freq_mhz",
> > +		"gt_RPn_freq_mhz",
> ------------------- ^ ------ ^
> I suppose it is too late now, but mixing capitals with small
> letters is bad, it should either be all small or proper names,
> so either rpn and mhz or RPn and MHz (MHz for Mega Hertz).

So these are names of sysfs files exposed by the kernel, so these cannot be
changed even in the kernel, they are already uapi.

> These two functions below have no descriptions in c file.
>
> > +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);

I have added these descriptions in v2. Also made some minor changes to
these two functions since what we had was not entirely correct. Please
review.

Also I have separated out the RPS related changes (including the functions
above) into a separate patch since the original patch commit message only
referred to the per-gt sysfs parsing. So now there are two patches:

 - lib/igt_sysfs: Add helpers to iterate over GTs
 - lib/igt_sysfs: Add RPS sysfs helpers

Thanks for reviewing!
--
Ashutosh