From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0939110E1A6 for ; Thu, 21 Apr 2022 15:49:41 +0000 (UTC) Date: Thu, 21 Apr 2022 17:49:37 +0200 From: Kamil Konieczny To: igt-dev@lists.freedesktop.org Message-ID: References: <85c51c99da76e3fe286ed28e80fa964e148f6020.1650435058.git.ashutosh.dixit@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [igt-dev] [PATCH i-g-t 2/3] lib/igt_sysfs: Add RPS sysfs helpers List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tvrtko Ursulin Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: Hi Ashutosh, On 2022-04-19 at 23:12:50 -0700, Ashutosh Dixit wrote: > From: Umesh Nerlige Ramappa > > RPS sysfs files exposed by the kernel can either be in per-gt sysfs or in > the per-device legacy sysfs. Add helpers to read/write these files in > either of the two sets of locations. > > v2: Added function descriptions (Kamil) > Separated patch from "lib/igt_sysfs: Add helpers to iterate over GTs" > > Cc: Kamil Konieczny > Cc: Andi Shyti > Cc: Tvrtko Ursulin > Cc: Sujaritha Sundaresan > Signed-off-by: Umesh Nerlige Ramappa > Signed-off-by: Ashutosh Dixit > --- > lib/igt_sysfs.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++ > lib/igt_sysfs.h | 54 +++++++++++++++++++++++++++++ > 2 files changed, 144 insertions(+) > > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c > index b167c0507039..0ac5d2a3d6e5 100644 > --- a/lib/igt_sysfs.c > +++ b/lib/igt_sysfs.c > @@ -54,6 +54,96 @@ > * provides basic support for like igt_sysfs_open(). > */ > > +enum { > + GT, > + RPS, > + > + SYSFS_NUM_TYPES, > +}; > + > +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", > + "gt_idle_freq_mhz", > + "gt_boost_freq_mhz", > + "power/rc6_enable", > + "power/rc6_residency_ms", > + "power/rc6p_residency_ms", > + "power/rc6pp_residency_ms", > + "power/media_rc6_residency_ms", > + }, > + { > + "rps_act_freq_mhz", > + "rps_cur_freq_mhz", > + "rps_min_freq_mhz", > + "rps_max_freq_mhz", > + "rps_RP0_freq_mhz", > + "rps_RP1_freq_mhz", > + "rps_RPn_freq_mhz", > + "rps_idle_freq_mhz", > + "rps_boost_freq_mhz", > + "rc6_enable", > + "rc6_residency_ms", > + "rc6p_residency_ms", > + "rc6pp_residency_ms", > + "media_rc6_residency_ms", > + }, > +}; > + > +/** > + * igt_sysfs_dir_id_to_name: > + * @dir: sysfs directory fd > + * @id: sysfs attribute id > + * > + * Returns attribute name corresponding to attribute id in either the > + * per-gt or legacy per-device sysfs > + * > + * Returns: > + * Attribute name in sysfs > + */ > +const char *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id) > +{ > + igt_assert(id < SYSFS_NUM_ATTR); What about id < 0 ? > + > + if (igt_sysfs_has_attr(dir, i915_attr_name[GT][id])) > + return i915_attr_name[GT][id]; > + else if (igt_sysfs_has_attr(dir, i915_attr_name[RPS][id])) ------- ^ s/else // > + return i915_attr_name[RPS][id]; > + else ------- ^ drop this else here, checkpatch warn: WARNING: else is not generally useful after a break or return #102: FILE: lib/igt_sysfs.c:118: + return i915_attr_name[RPS][id]; + else > + igt_assert_f(0, "Invalid sysfs dir\n"); and align this assert. Rest looks good, with that fixed you can add my r-b tag. Regards, Kamil > +} > + > +/** > + * igt_sysfs_path_id_to_name: > + * @path: sysfs directory path > + * @id: sysfs attribute id > + * > + * Returns attribute name corresponding to attribute id in either the > + * per-gt or legacy per-device sysfs > + * > + * Returns: > + * Attribute name in sysfs > + */ > +const char *igt_sysfs_path_id_to_name(const char *path, enum i915_attr_id id) > +{ > + int dir; > + const char *name; > + > + dir = open(path, O_RDONLY); > + igt_assert(dir); > + > + name = igt_sysfs_dir_id_to_name(dir, id); > + close(dir); > + > + return name; > +} > + > /** > * igt_sysfs_has_attr: > * @dir: sysfs directory fd > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h > index 33317a969619..8e39b8fa9890 100644 > --- a/lib/igt_sysfs.h > +++ b/lib/igt_sysfs.h > @@ -38,11 +38,65 @@ > (dirfd__ = igt_sysfs_gt_open(i915__, gt__)) != -1; \ > close(dirfd__), gt__++) > > +#define igt_sysfs_rps_write(dir, id, data, len) \ > + igt_sysfs_write(dir, igt_sysfs_dir_id_to_name(dir, id), data, len) > + > +#define igt_sysfs_rps_read(dir, id, data, len) \ > + igt_sysfs_read(dir, igt_sysfs_dir_id_to_name(dir, id), data, len) > + > +#define igt_sysfs_rps_set(dir, id, value) \ > + igt_sysfs_set(dir, igt_sysfs_dir_id_to_name(dir, id), value) > + > +#define igt_sysfs_rps_get(dir, id) \ > + igt_sysfs_get(dir, igt_sysfs_dir_id_to_name(dir, id)) > + > +#define igt_sysfs_rps_scanf(dir, id, fmt, ...) \ > + igt_sysfs_scanf(dir, igt_sysfs_dir_id_to_name(dir, id), fmt, ##__VA_ARGS__) > + > +#define igt_sysfs_rps_vprintf(dir, id, fmt, ap) \ > + igt_sysfs_vprintf(dir, igt_sysfs_dir_id_to_name(id), fmt, ap) > + > +#define igt_sysfs_rps_printf(dir, id, fmt, ...) \ > + igt_sysfs_printf(dir, igt_sysfs_dir_id_to_name(dir, id), fmt, ##__VA_ARGS__) > + > +#define igt_sysfs_rps_get_u32(dir, id) \ > + igt_sysfs_get_u32(dir, igt_sysfs_dir_id_to_name(dir, id)) > + > +#define igt_sysfs_rps_set_u32(dir, id, value) \ > + igt_sysfs_set_u32(dir, igt_sysfs_dir_id_to_name(dir, id), value) > + > +#define igt_sysfs_rps_get_boolean(dir, id) \ > + igt_sysfs_get_boolean(dir, igt_sysfs_dir_id_to_name(dir, id)) > + > +#define igt_sysfs_rps_set_boolean(dir, id, value) \ > + igt_sysfs_set_boolean(dir, igt_sysfs_dir_id_to_name(dir, id), value) > + > +enum i915_attr_id { > + RPS_ACT_FREQ_MHZ, > + RPS_CUR_FREQ_MHZ, > + RPS_MIN_FREQ_MHZ, > + RPS_MAX_FREQ_MHZ, > + RPS_RP0_FREQ_MHZ, > + RPS_RP1_FREQ_MHZ, > + RPS_RPn_FREQ_MHZ, > + RPS_IDLE_FREQ_MHZ, > + RPS_BOOST_FREQ_MHZ, > + RC6_ENABLE, > + RC6_RESIDENCY_MS, > + RC6P_RESIDENCY_MS, > + RC6PP_RESIDENCY_MS, > + MEDIA_RC6_RESIDENCY_MS, > + > + SYSFS_NUM_ATTR, > +}; > + > char *igt_sysfs_path(int device, char *path, int pathlen); > 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); > 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); > > int igt_sysfs_read(int dir, const char *attr, void *data, int len); > int igt_sysfs_write(int dir, const char *attr, const void *data, int len); > -- > 2.34.1 >