Re: [PATCH i-g-t 07/12] lib/igt_drm_fdinfo: Parse unit for engine utilization

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Fri, Apr 05, 2024 at 01:00:51AM -0500, Lucas De Marchi wrote:
>Kernel adds a " ns" at the end of engine utilization. Make sure we parse
>it so we don't fail if there's another suitable unit chosen by the
>driver or another format.
>
>This prepares the ground for xe driver which will use 2 timestamps
>rather than 1 with a different unit, to make sure it's compatible with
>SR-IOV so we don't have to handle the conversion between GPU and CPU
>clock domains.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> lib/igt_drm_fdinfo.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
>diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>index 5f05f210e..1541a62c9 100644
>--- a/lib/igt_drm_fdinfo.c
>+++ b/lib/igt_drm_fdinfo.c
>@@ -63,9 +63,10 @@ static const char *ignore_space(const char *s)
>
> static int parse_engine(const char *name, struct drm_client_fdinfo *info,
> 			const char **name_map, unsigned int map_entries,
>-			uint64_t *val)
>+			uint64_t *val, const char **unit)
> {
> 	const char *p;
>+	char *end_ptr;
> 	size_t name_len;
> 	int found = -1;
> 	unsigned int i;
>@@ -103,8 +104,15 @@ static int parse_engine(const char *name, struct drm_client_fdinfo *info,
> 		}
> 	}
>
>-	if (found >= 0)
>-		*val = strtoull(p, NULL, 10);
>+	if (found < 0)
>+		return found;
>+
>+	*val = strtoull(p, &end_ptr, 10);
>+	if (p == end_ptr)
>+		return -1;
>+
>+	if (unit)
>+		*unit = end_ptr;
>
> 	return found;
> }
>@@ -212,7 +220,7 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> 	while ((l = strtok_r(_buf, "\n", &ctx))) {
> 		uint64_t val = 0;
> 		size_t keylen;
>-		const char *v;
>+		const char *v, *unit;
> 		char *end_ptr;
> 		int idx;
>
>@@ -237,15 +245,15 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> 			strncpy(info->pdev, v, sizeof(info->pdev) - 1);
> 		} else if (strstartswith(l, "drm-engine-capacity-", &keylen)) {
> 			idx = parse_engine(l + keylen, info,
>-					   name_map, map_entries, &val);
>+					   name_map, map_entries, &val, NULL);
> 			if (idx >= 0) {
> 				info->capacity[idx] = val;
> 				num_capacity++;
> 			}
> 		} else if (strstartswith(l, "drm-engine-", &keylen)) {
> 			idx = parse_engine(l + keylen, info,
>-					   name_map, map_entries, &val);
>-			if (idx >= 0) {
>+					   name_map, map_entries, &val, &unit);

Should we ignore spaces in unit and then do a strcmp(unit, "ns")? OR are 
we sure that there is just one space prior to unit here?

Either ways,

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Umesh

>+			if (idx >= 0 && !strcmp(unit, " ns")) {
> 				if (!info->capacity[idx])
> 					info->capacity[idx] = 1;
> 				info->busy[idx] = val;
>-- 
>2.44.0
>