* [Intel-gfx] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power
@ 2020-11-24 23:27 Chris Wilson
2020-11-24 23:27 ` [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter Chris Wilson
2020-11-25 8:56 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power Tvrtko Ursulin
0 siblings, 2 replies; 8+ messages in thread
From: Chris Wilson @ 2020-11-24 23:27 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Chris Wilson
With integrated graphics the TDP is shared between the gpu and the cpu,
knowing the total energy consumed by the package is relevant to
understanding throttling.
v2: Tidy integration by eliminating struct rapl and improve output
formatting.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tools/intel_gpu_top.c | 218 +++++++++++++++++++++++++-----------------
1 file changed, 130 insertions(+), 88 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 86de09aa9..0a49cfecc 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -50,10 +50,12 @@ struct pmu_pair {
};
struct pmu_counter {
- bool present;
+ uint64_t type;
uint64_t config;
unsigned int idx;
struct pmu_pair val;
+ double scale;
+ bool present;
};
struct engine {
@@ -79,8 +81,8 @@ struct engines {
struct pmu_pair ts;
int rapl_fd;
- double rapl_scale;
- const char *rapl_unit;
+ struct pmu_counter r_gpu, r_pkg;
+ unsigned int num_rapl;
int imc_fd;
double imc_reads_scale;
@@ -92,7 +94,6 @@ struct engines {
struct pmu_counter freq_act;
struct pmu_counter irq;
struct pmu_counter rc6;
- struct pmu_counter rapl;
struct pmu_counter imc_reads;
struct pmu_counter imc_writes;
@@ -108,6 +109,109 @@ struct engines {
};
+__attribute__((format(scanf,3,4)))
+static int igt_sysfs_scanf(int dir, const char *attr, const char *fmt, ...)
+{
+ FILE *file;
+ int fd;
+ int ret = -1;
+
+ fd = openat(dir, attr, O_RDONLY);
+ if (fd < 0)
+ return -1;
+
+ file = fdopen(fd, "r");
+ if (file) {
+ va_list ap;
+
+ va_start(ap, fmt);
+ ret = vfscanf(file, fmt, ap);
+ va_end(ap);
+
+ fclose(file);
+ } else {
+ close(fd);
+ }
+
+ return ret;
+}
+
+static int rapl_parse(struct pmu_counter *pmu, const char *str)
+{
+ locale_t locale, oldlocale;
+ bool result = true;
+ char buf[128] = {};
+ int dir;
+
+ dir = open("/sys/devices/power", O_RDONLY);
+ if (dir < 0)
+ return -errno;
+
+ /* Replace user environment with plain C to match kernel format */
+ locale = newlocale(LC_ALL, "C", 0);
+ oldlocale = uselocale(locale);
+
+ result &= igt_sysfs_scanf(dir, "type", "%"PRIu64, &pmu->type) == 1;
+
+ snprintf(buf, sizeof(buf) - 1, "events/energy-%s", str);
+ result &= igt_sysfs_scanf(dir, buf, "event=%"PRIx64, &pmu->config) == 1;
+
+ snprintf(buf, sizeof(buf) - 1, "events/energy-%s.scale", str);
+ result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
+
+ snprintf(buf, sizeof(buf) - 1, "events/energy-%s.unit", str);
+ if (igt_sysfs_scanf(dir, buf, "%127s", buf) == 1 &&
+ strcmp(buf, "Joules"))
+ fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
+ str, buf);
+
+ uselocale(oldlocale);
+ freelocale(locale);
+
+ close(dir);
+
+ if (!result)
+ return -EINVAL;
+
+ if (isnan(pmu->scale) || !pmu->scale)
+ return -ERANGE;
+
+ return 0;
+}
+
+static void
+rapl_open(struct pmu_counter *pmu,
+ const char *domain,
+ struct engines *engines)
+{
+ int fd;
+
+ if (rapl_parse(pmu, domain) < 0)
+ return;
+
+ fd = igt_perf_open_group(pmu->type, pmu->config, engines->rapl_fd);
+ if (fd < 0)
+ return;
+
+ if (engines->rapl_fd == -1)
+ engines->rapl_fd = fd;
+
+ pmu->idx = engines->num_rapl++;
+ pmu->present = true;
+}
+
+static void gpu_power_open(struct pmu_counter *pmu,
+ struct engines *engines)
+{
+ rapl_open(pmu, "gpu", engines);
+}
+
+static void pkg_power_open(struct pmu_counter *pmu,
+ struct engines *engines)
+{
+ rapl_open(pmu, "pkg", engines);
+}
+
static uint64_t
get_pmu_config(int dirfd, const char *name, const char *counter)
{
@@ -357,38 +461,6 @@ static double filename_to_double(const char *filename)
return v;
}
-#define RAPL_ROOT "/sys/devices/power/"
-#define RAPL_EVENT "/sys/devices/power/events/"
-
-static uint64_t rapl_type_id(void)
-{
- return filename_to_u64(RAPL_ROOT "type", 10);
-}
-
-static uint64_t rapl_gpu_power(void)
-{
- return filename_to_u64(RAPL_EVENT "energy-gpu", 0);
-}
-
-static double rapl_gpu_power_scale(void)
-{
- return filename_to_double(RAPL_EVENT "energy-gpu.scale");
-}
-
-static const char *rapl_gpu_power_unit(void)
-{
- char buf[32];
-
- if (filename_to_buf(RAPL_EVENT "energy-gpu.unit",
- buf, sizeof(buf)) == 0)
- if (!strcmp(buf, "Joules"))
- return strdup("Watts");
- else
- return strdup(buf);
- else
- return NULL;
-}
-
#define IMC_ROOT "/sys/devices/uncore_imc/"
#define IMC_EVENT "/sys/devices/uncore_imc/events/"
@@ -517,22 +589,9 @@ static int pmu_init(struct engines *engines)
}
engines->rapl_fd = -1;
- if (!engines->discrete && rapl_type_id()) {
- engines->rapl_scale = rapl_gpu_power_scale();
- engines->rapl_unit = rapl_gpu_power_unit();
- if (!engines->rapl_unit)
- return -1;
-
- engines->rapl.config = rapl_gpu_power();
- if (!engines->rapl.config)
- return -1;
-
- engines->rapl_fd = igt_perf_open(rapl_type_id(),
- engines->rapl.config);
- if (engines->rapl_fd < 0)
- return -1;
-
- engines->rapl.present = true;
+ if (!engines->discrete) {
+ gpu_power_open(&engines->r_gpu, engines);
+ pkg_power_open(&engines->r_pkg, engines);
}
engines->imc_fd = -1;
@@ -614,25 +673,6 @@ static void fill_str(char *buf, unsigned int bufsz, char c, unsigned int num)
*buf = 0;
}
-static uint64_t __pmu_read_single(int fd, uint64_t *ts)
-{
- uint64_t data[2] = { };
- ssize_t len;
-
- len = read(fd, data, sizeof(data));
- assert(len == sizeof(data));
-
- if (ts)
- *ts = data[1];
-
- return data[0];
-}
-
-static uint64_t pmu_read_single(int fd)
-{
- return __pmu_read_single(fd, NULL);
-}
-
static void __update_sample(struct pmu_counter *counter, uint64_t val)
{
counter->val.prev = counter->val.cur;
@@ -653,10 +693,6 @@ static void pmu_sample(struct engines *engines)
engines->ts.prev = engines->ts.cur;
- if (engines->rapl_fd >= 0)
- __update_sample(&engines->rapl,
- pmu_read_single(engines->rapl_fd));
-
if (engines->imc_fd >= 0) {
pmu_read_multi(engines->imc_fd, 2, val);
update_sample(&engines->imc_reads, val);
@@ -677,6 +713,12 @@ static void pmu_sample(struct engines *engines)
update_sample(&engine->sema, val);
update_sample(&engine->wait, val);
}
+
+ if (engines->num_rapl) {
+ pmu_read_multi(engines->rapl_fd, engines->num_rapl, val);
+ update_sample(&engines->r_gpu, val);
+ update_sample(&engines->r_pkg, val);
+ }
}
static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
@@ -1076,14 +1118,14 @@ print_header(const struct igt_device_card *card,
.items = rc6_items,
};
struct cnt_item power_items[] = {
- { &engines->rapl, 4, 2, 1.0, t, engines->rapl_scale, "value",
- "W" },
+ { &engines->r_gpu, 4, 2, 1.0, t, engines->r_gpu.scale, "GPU", "gpu" },
+ { &engines->r_pkg, 4, 2, 1.0, t, engines->r_pkg.scale, "Package", "pkg" },
{ NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "W" },
{ },
};
struct cnt_group power_group = {
.name = "power",
- .display_name = "Power",
+ .display_name = "Power W",
.items = power_items,
};
struct cnt_group *groups[] = {
@@ -1108,17 +1150,17 @@ print_header(const struct igt_device_card *card,
if (lines++ < con_h) {
printf("intel-gpu-top: %s - ", card->card);
- if (!engines->discrete)
- printf("%s/%s MHz; %s%% RC6; %s %s; %s irqs/s\n",
- freq_items[1].buf, freq_items[0].buf,
- rc6_items[0].buf, power_items[0].buf,
- engines->rapl_unit,
- irq_items[0].buf);
- else
- printf("%s/%s MHz; %s%% RC6; %s irqs/s\n",
- freq_items[1].buf, freq_items[0].buf,
- rc6_items[0].buf, irq_items[0].buf);
+ printf("%s/%s MHz; %s%% RC6; ",
+ freq_items[1].buf, freq_items[0].buf,
+ rc6_items[0].buf);
+ if (engines->r_gpu.present) {
+ printf("%s/%s W; ",
+ power_items[0].buf,
+ power_items[1].buf);
+ }
+ printf("%s irqs/s\n", irq_items[0].buf);
}
+
if (lines++ < con_h)
printf("\n");
}
--
2.29.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter
2020-11-24 23:27 [Intel-gfx] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power Chris Wilson
@ 2020-11-24 23:27 ` Chris Wilson
2020-11-25 8:58 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
` (2 more replies)
2020-11-25 8:56 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power Tvrtko Ursulin
1 sibling, 3 replies; 8+ messages in thread
From: Chris Wilson @ 2020-11-24 23:27 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Chris Wilson
Follow the same pattern as rapl for imc, and consolidate everything to
work on struct pmu_counter.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tools/intel_gpu_top.c | 249 ++++++++++++++----------------------------
1 file changed, 82 insertions(+), 167 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 0a49cfecc..9af1c29b6 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -55,6 +55,7 @@ struct pmu_counter {
unsigned int idx;
struct pmu_pair val;
double scale;
+ const char *units;
bool present;
};
@@ -85,17 +86,14 @@ struct engines {
unsigned int num_rapl;
int imc_fd;
- double imc_reads_scale;
- const char *imc_reads_unit;
- double imc_writes_scale;
- const char *imc_writes_unit;
+ struct pmu_counter imc_reads;
+ struct pmu_counter imc_writes;
+ unsigned int num_imc;
struct pmu_counter freq_req;
struct pmu_counter freq_act;
struct pmu_counter irq;
struct pmu_counter rc6;
- struct pmu_counter imc_reads;
- struct pmu_counter imc_writes;
bool discrete;
char *device;
@@ -400,146 +398,93 @@ static struct engines *discover_engines(char *device)
return engines;
}
-static int
-filename_to_buf(const char *filename, char *buf, unsigned int bufsize)
-{
- int fd, err;
- ssize_t ret;
-
- fd = open(filename, O_RDONLY);
- if (fd < 0)
- return -1;
-
- ret = read(fd, buf, bufsize - 1);
- err = errno;
- close(fd);
- if (ret < 1) {
- errno = ret < 0 ? err : ENOMSG;
-
- return -1;
- }
+#define _open_pmu(type, cnt, pmu, fd) \
+({ \
+ int fd__; \
+\
+ fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
+ if (fd__ >= 0) { \
+ if ((fd) == -1) \
+ (fd) = fd__; \
+ (pmu)->present = true; \
+ (pmu)->idx = (cnt)++; \
+ } \
+\
+ fd__; \
+})
- if (ret > 1 && buf[ret - 1] == '\n')
- buf[ret - 1] = '\0';
- else
- buf[ret] = '\0';
+static int imc_parse(struct pmu_counter *pmu, const char *str)
+{
+ locale_t locale, oldlocale;
+ bool result = true;
+ char buf[128] = {};
+ int dir;
- return 0;
-}
+ dir = open("/sys/devices/uncore_imc", O_RDONLY);
+ if (dir < 0)
+ return -errno;
-static uint64_t filename_to_u64(const char *filename, int base)
-{
- char buf[64], *b;
+ /* Replace user environment with plain C to match kernel format */
+ locale = newlocale(LC_ALL, "C", 0);
+ oldlocale = uselocale(locale);
- if (filename_to_buf(filename, buf, sizeof(buf)))
- return 0;
+ result &= igt_sysfs_scanf(dir, "type", "%"PRIu64, &pmu->type) == 1;
- /*
- * Handle both single integer and key=value formats by skipping
- * leading non-digits.
- */
- b = buf;
- while (*b && !isdigit(*b))
- b++;
+ snprintf(buf, sizeof(buf) - 1, "events/%s", str);
+ result &= igt_sysfs_scanf(dir, buf, "event=%"PRIx64, &pmu->config) == 1;
- return strtoull(b, NULL, base);
-}
+ snprintf(buf, sizeof(buf) - 1, "events/%s.scale", str);
+ result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
-static double filename_to_double(const char *filename)
-{
- char *oldlocale;
- char buf[80];
- double v;
+ snprintf(buf, sizeof(buf) - 1, "events/%s.unit", str);
+ result &= igt_sysfs_scanf(dir, buf, "%127s", buf) == 1;
+ pmu->units = strdup(buf);
- if (filename_to_buf(filename, buf, sizeof(buf)))
- return 0;
+ uselocale(oldlocale);
+ freelocale(locale);
- oldlocale = setlocale(LC_ALL, "C");
- v = strtod(buf, NULL);
- setlocale(LC_ALL, oldlocale);
+ close(dir);
- return v;
-}
+ if (!result)
+ return -EINVAL;
-#define IMC_ROOT "/sys/devices/uncore_imc/"
-#define IMC_EVENT "/sys/devices/uncore_imc/events/"
+ if (isnan(pmu->scale) || !pmu->scale)
+ return -ERANGE;
-static uint64_t imc_type_id(void)
-{
- return filename_to_u64(IMC_ROOT "type", 10);
+ return 0;
}
-static uint64_t imc_data_reads(void)
+static void
+imc_open(struct pmu_counter *pmu,
+ const char *domain,
+ struct engines *engines)
{
- return filename_to_u64(IMC_EVENT "data_reads", 0);
-}
+ int fd;
-static double imc_data_reads_scale(void)
-{
- return filename_to_double(IMC_EVENT "data_reads.scale");
-}
+ if (imc_parse(pmu, domain) < 0)
+ return;
-static const char *imc_data_reads_unit(void)
-{
- char buf[32];
+ fd = igt_perf_open_group(pmu->type, pmu->config, engines->imc_fd);
+ if (fd < 0)
+ return;
- if (filename_to_buf(IMC_EVENT "data_reads.unit", buf, sizeof(buf)) == 0)
- return strdup(buf);
- else
- return NULL;
-}
+ if (engines->imc_fd == -1)
+ engines->imc_fd = fd;
-static uint64_t imc_data_writes(void)
-{
- return filename_to_u64(IMC_EVENT "data_writes", 0);
+ pmu->idx = engines->num_imc++;
+ pmu->present = true;
}
-static double imc_data_writes_scale(void)
+static void imc_writes_open(struct pmu_counter *pmu, struct engines *engines)
{
- return filename_to_double(IMC_EVENT "data_writes.scale");
+ imc_open(pmu, "data_writes", engines);
}
-static const char *imc_data_writes_unit(void)
+static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
{
- char buf[32];
-
- if (filename_to_buf(IMC_EVENT "data_writes.unit",
- buf, sizeof(buf)) == 0)
- return strdup(buf);
- else
- return NULL;
+ imc_open(pmu, "data_reads", engines);
}
-#define _open_pmu(type, cnt, pmu, fd) \
-({ \
- int fd__; \
-\
- fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
- if (fd__ >= 0) { \
- if ((fd) == -1) \
- (fd) = fd__; \
- (pmu)->present = true; \
- (pmu)->idx = (cnt)++; \
- } \
-\
- fd__; \
-})
-
-#define _open_imc(cnt, pmu, fd) \
-({ \
- int fd__; \
-\
- fd__ = igt_perf_open_group(imc_type_id(), (pmu)->config, (fd)); \
- if (fd__ >= 0) { \
- if ((fd) == -1) \
- (fd) = fd__; \
- (pmu)->present = true; \
- (pmu)->idx = (cnt)++; \
- } \
-\
- fd__; \
-})
-
static int pmu_init(struct engines *engines)
{
unsigned int i;
@@ -595,38 +540,8 @@ static int pmu_init(struct engines *engines)
}
engines->imc_fd = -1;
- if (imc_type_id()) {
- unsigned int num = 0;
-
- engines->imc_reads_scale = imc_data_reads_scale();
- engines->imc_writes_scale = imc_data_writes_scale();
-
- engines->imc_reads_unit = imc_data_reads_unit();
- if (!engines->imc_reads_unit)
- return -1;
-
- engines->imc_writes_unit = imc_data_writes_unit();
- if (!engines->imc_writes_unit)
- return -1;
-
- engines->imc_reads.config = imc_data_reads();
- if (!engines->imc_reads.config)
- return -1;
-
- engines->imc_writes.config = imc_data_writes();
- if (!engines->imc_writes.config)
- return -1;
-
- fd = _open_imc(num, &engines->imc_reads, engines->imc_fd);
- if (fd < 0)
- return -1;
- fd = _open_imc(num, &engines->imc_writes, engines->imc_fd);
- if (fd < 0)
- return -1;
-
- engines->imc_reads.present = true;
- engines->imc_writes.present = true;
- }
+ imc_reads_open(&engines->imc_reads, engines);
+ imc_writes_open(&engines->imc_writes, engines);
return 0;
}
@@ -692,13 +607,6 @@ static void pmu_sample(struct engines *engines)
unsigned int i;
engines->ts.prev = engines->ts.cur;
-
- if (engines->imc_fd >= 0) {
- pmu_read_multi(engines->imc_fd, 2, val);
- update_sample(&engines->imc_reads, val);
- update_sample(&engines->imc_writes, val);
- }
-
engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
update_sample(&engines->freq_req, val);
@@ -719,6 +627,12 @@ static void pmu_sample(struct engines *engines)
update_sample(&engines->r_gpu, val);
update_sample(&engines->r_pkg, val);
}
+
+ if (engines->num_imc) {
+ pmu_read_multi(engines->imc_fd, engines->num_imc, val);
+ update_sample(&engines->imc_reads, val);
+ update_sample(&engines->imc_writes, val);
+ }
}
static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
@@ -1172,9 +1086,9 @@ static int
print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
{
struct cnt_item imc_items[] = {
- { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads_scale,
+ { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads.scale,
"reads", "rd" },
- { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes_scale,
+ { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes.scale,
"writes", "wr" },
{ NULL, 0, 0, 0.0, 0.0, 0.0, "unit" },
{ },
@@ -1189,12 +1103,15 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
};
int ret;
+ if (!engines->num_imc)
+ return lines;
+
ret = asprintf((char **)&imc_group.display_name, "IMC %s/s",
- engines->imc_reads_unit);
+ engines->imc_reads.units);
assert(ret >= 0);
ret = asprintf((char **)&imc_items[2].unit, "%s/s",
- engines->imc_reads_unit);
+ engines->imc_reads.units);
assert(ret >= 0);
print_groups(groups);
@@ -1205,11 +1122,11 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
if (output_mode == INTERACTIVE) {
if (lines++ < con_h)
printf(" IMC reads: %s %s/s\n",
- imc_items[0].buf, engines->imc_reads_unit);
+ imc_items[0].buf, engines->imc_reads.units);
if (lines++ < con_h)
printf(" IMC writes: %s %s/s\n",
- imc_items[1].buf, engines->imc_writes_unit);
+ imc_items[1].buf, engines->imc_writes.units);
if (lines++ < con_h)
printf("\n");
@@ -1509,9 +1426,7 @@ int main(int argc, char **argv)
t, lines, con_w, con_h,
&consumed);
- if (engines->imc_fd)
- lines = print_imc(engines, t, lines, con_w,
- con_h);
+ lines = print_imc(engines, t, lines, con_w, con_h);
lines = print_engines_header(engines, t, lines, con_w,
con_h);
--
2.29.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power
2020-11-24 23:27 [Intel-gfx] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power Chris Wilson
2020-11-24 23:27 ` [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter Chris Wilson
@ 2020-11-25 8:56 ` Tvrtko Ursulin
1 sibling, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2020-11-25 8:56 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
On 24/11/2020 23:27, Chris Wilson wrote:
> With integrated graphics the TDP is shared between the gpu and the cpu,
> knowing the total energy consumed by the package is relevant to
> understanding throttling.
>
> v2: Tidy integration by eliminating struct rapl and improve output
> formatting.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> tools/intel_gpu_top.c | 218 +++++++++++++++++++++++++-----------------
> 1 file changed, 130 insertions(+), 88 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 86de09aa9..0a49cfecc 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -50,10 +50,12 @@ struct pmu_pair {
> };
>
> struct pmu_counter {
> - bool present;
> + uint64_t type;
> uint64_t config;
> unsigned int idx;
> struct pmu_pair val;
> + double scale;
> + bool present;
> };
>
> struct engine {
> @@ -79,8 +81,8 @@ struct engines {
> struct pmu_pair ts;
>
> int rapl_fd;
> - double rapl_scale;
> - const char *rapl_unit;
> + struct pmu_counter r_gpu, r_pkg;
> + unsigned int num_rapl;
>
> int imc_fd;
> double imc_reads_scale;
> @@ -92,7 +94,6 @@ struct engines {
> struct pmu_counter freq_act;
> struct pmu_counter irq;
> struct pmu_counter rc6;
> - struct pmu_counter rapl;
> struct pmu_counter imc_reads;
> struct pmu_counter imc_writes;
>
> @@ -108,6 +109,109 @@ struct engines {
>
> };
>
> +__attribute__((format(scanf,3,4)))
> +static int igt_sysfs_scanf(int dir, const char *attr, const char *fmt, ...)
> +{
> + FILE *file;
> + int fd;
> + int ret = -1;
> +
> + fd = openat(dir, attr, O_RDONLY);
> + if (fd < 0)
> + return -1;
> +
> + file = fdopen(fd, "r");
> + if (file) {
> + va_list ap;
> +
> + va_start(ap, fmt);
> + ret = vfscanf(file, fmt, ap);
> + va_end(ap);
> +
> + fclose(file);
> + } else {
> + close(fd);
> + }
> +
> + return ret;
> +}
> +
> +static int rapl_parse(struct pmu_counter *pmu, const char *str)
> +{
> + locale_t locale, oldlocale;
> + bool result = true;
> + char buf[128] = {};
> + int dir;
> +
> + dir = open("/sys/devices/power", O_RDONLY);
> + if (dir < 0)
> + return -errno;
> +
> + /* Replace user environment with plain C to match kernel format */
> + locale = newlocale(LC_ALL, "C", 0);
> + oldlocale = uselocale(locale);
> +
> + result &= igt_sysfs_scanf(dir, "type", "%"PRIu64, &pmu->type) == 1;
> +
> + snprintf(buf, sizeof(buf) - 1, "events/energy-%s", str);
> + result &= igt_sysfs_scanf(dir, buf, "event=%"PRIx64, &pmu->config) == 1;
> +
> + snprintf(buf, sizeof(buf) - 1, "events/energy-%s.scale", str);
> + result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
> +
> + snprintf(buf, sizeof(buf) - 1, "events/energy-%s.unit", str);
> + if (igt_sysfs_scanf(dir, buf, "%127s", buf) == 1 &&
> + strcmp(buf, "Joules"))
> + fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
> + str, buf);
> +
> + uselocale(oldlocale);
> + freelocale(locale);
> +
> + close(dir);
> +
> + if (!result)
> + return -EINVAL;
> +
> + if (isnan(pmu->scale) || !pmu->scale)
> + return -ERANGE;
> +
> + return 0;
> +}
> +
> +static void
> +rapl_open(struct pmu_counter *pmu,
> + const char *domain,
> + struct engines *engines)
> +{
> + int fd;
> +
> + if (rapl_parse(pmu, domain) < 0)
> + return;
> +
> + fd = igt_perf_open_group(pmu->type, pmu->config, engines->rapl_fd);
> + if (fd < 0)
> + return;
> +
> + if (engines->rapl_fd == -1)
> + engines->rapl_fd = fd;
> +
> + pmu->idx = engines->num_rapl++;
> + pmu->present = true;
> +}
> +
> +static void gpu_power_open(struct pmu_counter *pmu,
> + struct engines *engines)
> +{
> + rapl_open(pmu, "gpu", engines);
> +}
> +
> +static void pkg_power_open(struct pmu_counter *pmu,
> + struct engines *engines)
> +{
> + rapl_open(pmu, "pkg", engines);
> +}
> +
> static uint64_t
> get_pmu_config(int dirfd, const char *name, const char *counter)
> {
> @@ -357,38 +461,6 @@ static double filename_to_double(const char *filename)
> return v;
> }
>
> -#define RAPL_ROOT "/sys/devices/power/"
> -#define RAPL_EVENT "/sys/devices/power/events/"
> -
> -static uint64_t rapl_type_id(void)
> -{
> - return filename_to_u64(RAPL_ROOT "type", 10);
> -}
> -
> -static uint64_t rapl_gpu_power(void)
> -{
> - return filename_to_u64(RAPL_EVENT "energy-gpu", 0);
> -}
> -
> -static double rapl_gpu_power_scale(void)
> -{
> - return filename_to_double(RAPL_EVENT "energy-gpu.scale");
> -}
> -
> -static const char *rapl_gpu_power_unit(void)
> -{
> - char buf[32];
> -
> - if (filename_to_buf(RAPL_EVENT "energy-gpu.unit",
> - buf, sizeof(buf)) == 0)
> - if (!strcmp(buf, "Joules"))
> - return strdup("Watts");
> - else
> - return strdup(buf);
> - else
> - return NULL;
> -}
> -
> #define IMC_ROOT "/sys/devices/uncore_imc/"
> #define IMC_EVENT "/sys/devices/uncore_imc/events/"
>
> @@ -517,22 +589,9 @@ static int pmu_init(struct engines *engines)
> }
>
> engines->rapl_fd = -1;
> - if (!engines->discrete && rapl_type_id()) {
> - engines->rapl_scale = rapl_gpu_power_scale();
> - engines->rapl_unit = rapl_gpu_power_unit();
> - if (!engines->rapl_unit)
> - return -1;
> -
> - engines->rapl.config = rapl_gpu_power();
> - if (!engines->rapl.config)
> - return -1;
> -
> - engines->rapl_fd = igt_perf_open(rapl_type_id(),
> - engines->rapl.config);
> - if (engines->rapl_fd < 0)
> - return -1;
> -
> - engines->rapl.present = true;
> + if (!engines->discrete) {
> + gpu_power_open(&engines->r_gpu, engines);
> + pkg_power_open(&engines->r_pkg, engines);
> }
>
> engines->imc_fd = -1;
> @@ -614,25 +673,6 @@ static void fill_str(char *buf, unsigned int bufsz, char c, unsigned int num)
> *buf = 0;
> }
>
> -static uint64_t __pmu_read_single(int fd, uint64_t *ts)
> -{
> - uint64_t data[2] = { };
> - ssize_t len;
> -
> - len = read(fd, data, sizeof(data));
> - assert(len == sizeof(data));
> -
> - if (ts)
> - *ts = data[1];
> -
> - return data[0];
> -}
> -
> -static uint64_t pmu_read_single(int fd)
> -{
> - return __pmu_read_single(fd, NULL);
> -}
> -
> static void __update_sample(struct pmu_counter *counter, uint64_t val)
> {
> counter->val.prev = counter->val.cur;
> @@ -653,10 +693,6 @@ static void pmu_sample(struct engines *engines)
>
> engines->ts.prev = engines->ts.cur;
>
> - if (engines->rapl_fd >= 0)
> - __update_sample(&engines->rapl,
> - pmu_read_single(engines->rapl_fd));
> -
> if (engines->imc_fd >= 0) {
> pmu_read_multi(engines->imc_fd, 2, val);
> update_sample(&engines->imc_reads, val);
> @@ -677,6 +713,12 @@ static void pmu_sample(struct engines *engines)
> update_sample(&engine->sema, val);
> update_sample(&engine->wait, val);
> }
> +
> + if (engines->num_rapl) {
> + pmu_read_multi(engines->rapl_fd, engines->num_rapl, val);
> + update_sample(&engines->r_gpu, val);
> + update_sample(&engines->r_pkg, val);
> + }
> }
>
> static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -1076,14 +1118,14 @@ print_header(const struct igt_device_card *card,
> .items = rc6_items,
> };
> struct cnt_item power_items[] = {
> - { &engines->rapl, 4, 2, 1.0, t, engines->rapl_scale, "value",
> - "W" },
> + { &engines->r_gpu, 4, 2, 1.0, t, engines->r_gpu.scale, "GPU", "gpu" },
> + { &engines->r_pkg, 4, 2, 1.0, t, engines->r_pkg.scale, "Package", "pkg" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "W" },
> { },
> };
> struct cnt_group power_group = {
> .name = "power",
> - .display_name = "Power",
> + .display_name = "Power W",
> .items = power_items,
> };
> struct cnt_group *groups[] = {
> @@ -1108,17 +1150,17 @@ print_header(const struct igt_device_card *card,
>
> if (lines++ < con_h) {
> printf("intel-gpu-top: %s - ", card->card);
> - if (!engines->discrete)
> - printf("%s/%s MHz; %s%% RC6; %s %s; %s irqs/s\n",
> - freq_items[1].buf, freq_items[0].buf,
> - rc6_items[0].buf, power_items[0].buf,
> - engines->rapl_unit,
> - irq_items[0].buf);
> - else
> - printf("%s/%s MHz; %s%% RC6; %s irqs/s\n",
> - freq_items[1].buf, freq_items[0].buf,
> - rc6_items[0].buf, irq_items[0].buf);
> + printf("%s/%s MHz; %s%% RC6; ",
> + freq_items[1].buf, freq_items[0].buf,
> + rc6_items[0].buf);
> + if (engines->r_gpu.present) {
> + printf("%s/%s W; ",
> + power_items[0].buf,
> + power_items[1].buf);
> + }
> + printf("%s irqs/s\n", irq_items[0].buf);
> }
> +
> if (lines++ < con_h)
> printf("\n");
> }
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter
2020-11-24 23:27 ` [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter Chris Wilson
@ 2020-11-25 8:58 ` Tvrtko Ursulin
2020-11-25 10:04 ` [Intel-gfx] [PATCH i-g-t] " Chris Wilson
2020-11-25 10:35 ` [Intel-gfx] " Chris Wilson
2 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2020-11-25 8:58 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
On 24/11/2020 23:27, Chris Wilson wrote:
> Follow the same pattern as rapl for imc, and consolidate everything to
> work on struct pmu_counter.
Looks nicer than before. Could you also do a common helper for imc_parse
and rapl_parse, since only the pmu name string would be different and
path could easily be constructed in the existing buf?
Regards,
Tvrtko
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> tools/intel_gpu_top.c | 249 ++++++++++++++----------------------------
> 1 file changed, 82 insertions(+), 167 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 0a49cfecc..9af1c29b6 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -55,6 +55,7 @@ struct pmu_counter {
> unsigned int idx;
> struct pmu_pair val;
> double scale;
> + const char *units;
> bool present;
> };
>
> @@ -85,17 +86,14 @@ struct engines {
> unsigned int num_rapl;
>
> int imc_fd;
> - double imc_reads_scale;
> - const char *imc_reads_unit;
> - double imc_writes_scale;
> - const char *imc_writes_unit;
> + struct pmu_counter imc_reads;
> + struct pmu_counter imc_writes;
> + unsigned int num_imc;
>
> struct pmu_counter freq_req;
> struct pmu_counter freq_act;
> struct pmu_counter irq;
> struct pmu_counter rc6;
> - struct pmu_counter imc_reads;
> - struct pmu_counter imc_writes;
>
> bool discrete;
> char *device;
> @@ -400,146 +398,93 @@ static struct engines *discover_engines(char *device)
> return engines;
> }
>
> -static int
> -filename_to_buf(const char *filename, char *buf, unsigned int bufsize)
> -{
> - int fd, err;
> - ssize_t ret;
> -
> - fd = open(filename, O_RDONLY);
> - if (fd < 0)
> - return -1;
> -
> - ret = read(fd, buf, bufsize - 1);
> - err = errno;
> - close(fd);
> - if (ret < 1) {
> - errno = ret < 0 ? err : ENOMSG;
> -
> - return -1;
> - }
> +#define _open_pmu(type, cnt, pmu, fd) \
> +({ \
> + int fd__; \
> +\
> + fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
> + if (fd__ >= 0) { \
> + if ((fd) == -1) \
> + (fd) = fd__; \
> + (pmu)->present = true; \
> + (pmu)->idx = (cnt)++; \
> + } \
> +\
> + fd__; \
> +})
>
> - if (ret > 1 && buf[ret - 1] == '\n')
> - buf[ret - 1] = '\0';
> - else
> - buf[ret] = '\0';
> +static int imc_parse(struct pmu_counter *pmu, const char *str)
> +{
> + locale_t locale, oldlocale;
> + bool result = true;
> + char buf[128] = {};
> + int dir;
>
> - return 0;
> -}
> + dir = open("/sys/devices/uncore_imc", O_RDONLY);
> + if (dir < 0)
> + return -errno;
>
> -static uint64_t filename_to_u64(const char *filename, int base)
> -{
> - char buf[64], *b;
> + /* Replace user environment with plain C to match kernel format */
> + locale = newlocale(LC_ALL, "C", 0);
> + oldlocale = uselocale(locale);
>
> - if (filename_to_buf(filename, buf, sizeof(buf)))
> - return 0;
> + result &= igt_sysfs_scanf(dir, "type", "%"PRIu64, &pmu->type) == 1;
>
> - /*
> - * Handle both single integer and key=value formats by skipping
> - * leading non-digits.
> - */
> - b = buf;
> - while (*b && !isdigit(*b))
> - b++;
> + snprintf(buf, sizeof(buf) - 1, "events/%s", str);
> + result &= igt_sysfs_scanf(dir, buf, "event=%"PRIx64, &pmu->config) == 1;
>
> - return strtoull(b, NULL, base);
> -}
> + snprintf(buf, sizeof(buf) - 1, "events/%s.scale", str);
> + result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
>
> -static double filename_to_double(const char *filename)
> -{
> - char *oldlocale;
> - char buf[80];
> - double v;
> + snprintf(buf, sizeof(buf) - 1, "events/%s.unit", str);
> + result &= igt_sysfs_scanf(dir, buf, "%127s", buf) == 1;
> + pmu->units = strdup(buf);
>
> - if (filename_to_buf(filename, buf, sizeof(buf)))
> - return 0;
> + uselocale(oldlocale);
> + freelocale(locale);
>
> - oldlocale = setlocale(LC_ALL, "C");
> - v = strtod(buf, NULL);
> - setlocale(LC_ALL, oldlocale);
> + close(dir);
>
> - return v;
> -}
> + if (!result)
> + return -EINVAL;
>
> -#define IMC_ROOT "/sys/devices/uncore_imc/"
> -#define IMC_EVENT "/sys/devices/uncore_imc/events/"
> + if (isnan(pmu->scale) || !pmu->scale)
> + return -ERANGE;
>
> -static uint64_t imc_type_id(void)
> -{
> - return filename_to_u64(IMC_ROOT "type", 10);
> + return 0;
> }
>
> -static uint64_t imc_data_reads(void)
> +static void
> +imc_open(struct pmu_counter *pmu,
> + const char *domain,
> + struct engines *engines)
> {
> - return filename_to_u64(IMC_EVENT "data_reads", 0);
> -}
> + int fd;
>
> -static double imc_data_reads_scale(void)
> -{
> - return filename_to_double(IMC_EVENT "data_reads.scale");
> -}
> + if (imc_parse(pmu, domain) < 0)
> + return;
>
> -static const char *imc_data_reads_unit(void)
> -{
> - char buf[32];
> + fd = igt_perf_open_group(pmu->type, pmu->config, engines->imc_fd);
> + if (fd < 0)
> + return;
>
> - if (filename_to_buf(IMC_EVENT "data_reads.unit", buf, sizeof(buf)) == 0)
> - return strdup(buf);
> - else
> - return NULL;
> -}
> + if (engines->imc_fd == -1)
> + engines->imc_fd = fd;
>
> -static uint64_t imc_data_writes(void)
> -{
> - return filename_to_u64(IMC_EVENT "data_writes", 0);
> + pmu->idx = engines->num_imc++;
> + pmu->present = true;
> }
>
> -static double imc_data_writes_scale(void)
> +static void imc_writes_open(struct pmu_counter *pmu, struct engines *engines)
> {
> - return filename_to_double(IMC_EVENT "data_writes.scale");
> + imc_open(pmu, "data_writes", engines);
> }
>
> -static const char *imc_data_writes_unit(void)
> +static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
> {
> - char buf[32];
> -
> - if (filename_to_buf(IMC_EVENT "data_writes.unit",
> - buf, sizeof(buf)) == 0)
> - return strdup(buf);
> - else
> - return NULL;
> + imc_open(pmu, "data_reads", engines);
> }
>
> -#define _open_pmu(type, cnt, pmu, fd) \
> -({ \
> - int fd__; \
> -\
> - fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
> - if (fd__ >= 0) { \
> - if ((fd) == -1) \
> - (fd) = fd__; \
> - (pmu)->present = true; \
> - (pmu)->idx = (cnt)++; \
> - } \
> -\
> - fd__; \
> -})
> -
> -#define _open_imc(cnt, pmu, fd) \
> -({ \
> - int fd__; \
> -\
> - fd__ = igt_perf_open_group(imc_type_id(), (pmu)->config, (fd)); \
> - if (fd__ >= 0) { \
> - if ((fd) == -1) \
> - (fd) = fd__; \
> - (pmu)->present = true; \
> - (pmu)->idx = (cnt)++; \
> - } \
> -\
> - fd__; \
> -})
> -
> static int pmu_init(struct engines *engines)
> {
> unsigned int i;
> @@ -595,38 +540,8 @@ static int pmu_init(struct engines *engines)
> }
>
> engines->imc_fd = -1;
> - if (imc_type_id()) {
> - unsigned int num = 0;
> -
> - engines->imc_reads_scale = imc_data_reads_scale();
> - engines->imc_writes_scale = imc_data_writes_scale();
> -
> - engines->imc_reads_unit = imc_data_reads_unit();
> - if (!engines->imc_reads_unit)
> - return -1;
> -
> - engines->imc_writes_unit = imc_data_writes_unit();
> - if (!engines->imc_writes_unit)
> - return -1;
> -
> - engines->imc_reads.config = imc_data_reads();
> - if (!engines->imc_reads.config)
> - return -1;
> -
> - engines->imc_writes.config = imc_data_writes();
> - if (!engines->imc_writes.config)
> - return -1;
> -
> - fd = _open_imc(num, &engines->imc_reads, engines->imc_fd);
> - if (fd < 0)
> - return -1;
> - fd = _open_imc(num, &engines->imc_writes, engines->imc_fd);
> - if (fd < 0)
> - return -1;
> -
> - engines->imc_reads.present = true;
> - engines->imc_writes.present = true;
> - }
> + imc_reads_open(&engines->imc_reads, engines);
> + imc_writes_open(&engines->imc_writes, engines);
>
> return 0;
> }
> @@ -692,13 +607,6 @@ static void pmu_sample(struct engines *engines)
> unsigned int i;
>
> engines->ts.prev = engines->ts.cur;
> -
> - if (engines->imc_fd >= 0) {
> - pmu_read_multi(engines->imc_fd, 2, val);
> - update_sample(&engines->imc_reads, val);
> - update_sample(&engines->imc_writes, val);
> - }
> -
> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>
> update_sample(&engines->freq_req, val);
> @@ -719,6 +627,12 @@ static void pmu_sample(struct engines *engines)
> update_sample(&engines->r_gpu, val);
> update_sample(&engines->r_pkg, val);
> }
> +
> + if (engines->num_imc) {
> + pmu_read_multi(engines->imc_fd, engines->num_imc, val);
> + update_sample(&engines->imc_reads, val);
> + update_sample(&engines->imc_writes, val);
> + }
> }
>
> static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -1172,9 +1086,9 @@ static int
> print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> {
> struct cnt_item imc_items[] = {
> - { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads_scale,
> + { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads.scale,
> "reads", "rd" },
> - { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes_scale,
> + { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes.scale,
> "writes", "wr" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit" },
> { },
> @@ -1189,12 +1103,15 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> };
> int ret;
>
> + if (!engines->num_imc)
> + return lines;
> +
> ret = asprintf((char **)&imc_group.display_name, "IMC %s/s",
> - engines->imc_reads_unit);
> + engines->imc_reads.units);
> assert(ret >= 0);
>
> ret = asprintf((char **)&imc_items[2].unit, "%s/s",
> - engines->imc_reads_unit);
> + engines->imc_reads.units);
> assert(ret >= 0);
>
> print_groups(groups);
> @@ -1205,11 +1122,11 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> if (output_mode == INTERACTIVE) {
> if (lines++ < con_h)
> printf(" IMC reads: %s %s/s\n",
> - imc_items[0].buf, engines->imc_reads_unit);
> + imc_items[0].buf, engines->imc_reads.units);
>
> if (lines++ < con_h)
> printf(" IMC writes: %s %s/s\n",
> - imc_items[1].buf, engines->imc_writes_unit);
> + imc_items[1].buf, engines->imc_writes.units);
>
> if (lines++ < con_h)
> printf("\n");
> @@ -1509,9 +1426,7 @@ int main(int argc, char **argv)
> t, lines, con_w, con_h,
> &consumed);
>
> - if (engines->imc_fd)
> - lines = print_imc(engines, t, lines, con_w,
> - con_h);
> + lines = print_imc(engines, t, lines, con_w, con_h);
>
> lines = print_engines_header(engines, t, lines, con_w,
> con_h);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] [PATCH i-g-t] tools/intel_gpu_top: Consolidate imc to use pmu_counter
2020-11-24 23:27 ` [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter Chris Wilson
2020-11-25 8:58 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
@ 2020-11-25 10:04 ` Chris Wilson
2020-11-25 10:17 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2020-11-25 10:35 ` [Intel-gfx] " Chris Wilson
2 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2020-11-25 10:04 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Chris Wilson
Follow the same pattern as rapl for imc, and consolidate everything to
work on struct pmu_counter.
v2: Combine rapl_parse/imc_parse into pmu_parse
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tools/intel_gpu_top.c | 248 +++++++++++-------------------------------
1 file changed, 65 insertions(+), 183 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 0a49cfecc..ffdac280c 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -55,6 +55,7 @@ struct pmu_counter {
unsigned int idx;
struct pmu_pair val;
double scale;
+ const char *units;
bool present;
};
@@ -85,17 +86,14 @@ struct engines {
unsigned int num_rapl;
int imc_fd;
- double imc_reads_scale;
- const char *imc_reads_unit;
- double imc_writes_scale;
- const char *imc_writes_unit;
+ struct pmu_counter imc_reads;
+ struct pmu_counter imc_writes;
+ unsigned int num_imc;
struct pmu_counter freq_req;
struct pmu_counter freq_act;
struct pmu_counter irq;
struct pmu_counter rc6;
- struct pmu_counter imc_reads;
- struct pmu_counter imc_writes;
bool discrete;
char *device;
@@ -136,14 +134,14 @@ static int igt_sysfs_scanf(int dir, const char *attr, const char *fmt, ...)
return ret;
}
-static int rapl_parse(struct pmu_counter *pmu, const char *str)
+static int pmu_parse(struct pmu_counter *pmu, const char *path, const char *str)
{
locale_t locale, oldlocale;
bool result = true;
char buf[128] = {};
int dir;
- dir = open("/sys/devices/power", O_RDONLY);
+ dir = open(path, O_RDONLY);
if (dir < 0)
return -errno;
@@ -160,10 +158,8 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
snprintf(buf, sizeof(buf) - 1, "events/energy-%s.unit", str);
- if (igt_sysfs_scanf(dir, buf, "%127s", buf) == 1 &&
- strcmp(buf, "Joules"))
- fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
- str, buf);
+ result &= igt_sysfs_scanf(dir, buf, "%127s", buf) == 1;
+ pmu->units = strdup(buf);
uselocale(oldlocale);
freelocale(locale);
@@ -179,6 +175,11 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
return 0;
}
+static int rapl_parse(struct pmu_counter *pmu, const char *str)
+{
+ return pmu_parse(pmu, "/sys/devices/power", str);
+}
+
static void
rapl_open(struct pmu_counter *pmu,
const char *domain,
@@ -400,146 +401,57 @@ static struct engines *discover_engines(char *device)
return engines;
}
-static int
-filename_to_buf(const char *filename, char *buf, unsigned int bufsize)
-{
- int fd, err;
- ssize_t ret;
-
- fd = open(filename, O_RDONLY);
- if (fd < 0)
- return -1;
-
- ret = read(fd, buf, bufsize - 1);
- err = errno;
- close(fd);
- if (ret < 1) {
- errno = ret < 0 ? err : ENOMSG;
-
- return -1;
- }
-
- if (ret > 1 && buf[ret - 1] == '\n')
- buf[ret - 1] = '\0';
- else
- buf[ret] = '\0';
-
- return 0;
-}
-
-static uint64_t filename_to_u64(const char *filename, int base)
-{
- char buf[64], *b;
-
- if (filename_to_buf(filename, buf, sizeof(buf)))
- return 0;
-
- /*
- * Handle both single integer and key=value formats by skipping
- * leading non-digits.
- */
- b = buf;
- while (*b && !isdigit(*b))
- b++;
-
- return strtoull(b, NULL, base);
-}
-
-static double filename_to_double(const char *filename)
-{
- char *oldlocale;
- char buf[80];
- double v;
-
- if (filename_to_buf(filename, buf, sizeof(buf)))
- return 0;
-
- oldlocale = setlocale(LC_ALL, "C");
- v = strtod(buf, NULL);
- setlocale(LC_ALL, oldlocale);
-
- return v;
-}
-
-#define IMC_ROOT "/sys/devices/uncore_imc/"
-#define IMC_EVENT "/sys/devices/uncore_imc/events/"
+#define _open_pmu(type, cnt, pmu, fd) \
+({ \
+ int fd__; \
+\
+ fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
+ if (fd__ >= 0) { \
+ if ((fd) == -1) \
+ (fd) = fd__; \
+ (pmu)->present = true; \
+ (pmu)->idx = (cnt)++; \
+ } \
+\
+ fd__; \
+})
-static uint64_t imc_type_id(void)
+static int imc_parse(struct pmu_counter *pmu, const char *str)
{
- return filename_to_u64(IMC_ROOT "type", 10);
+ return pmu_parse(pmu, "/sys/devices/uncore_imc", str);
}
-static uint64_t imc_data_reads(void)
+static void
+imc_open(struct pmu_counter *pmu,
+ const char *domain,
+ struct engines *engines)
{
- return filename_to_u64(IMC_EVENT "data_reads", 0);
-}
+ int fd;
-static double imc_data_reads_scale(void)
-{
- return filename_to_double(IMC_EVENT "data_reads.scale");
-}
+ if (imc_parse(pmu, domain) < 0)
+ return;
-static const char *imc_data_reads_unit(void)
-{
- char buf[32];
+ fd = igt_perf_open_group(pmu->type, pmu->config, engines->imc_fd);
+ if (fd < 0)
+ return;
- if (filename_to_buf(IMC_EVENT "data_reads.unit", buf, sizeof(buf)) == 0)
- return strdup(buf);
- else
- return NULL;
-}
+ if (engines->imc_fd == -1)
+ engines->imc_fd = fd;
-static uint64_t imc_data_writes(void)
-{
- return filename_to_u64(IMC_EVENT "data_writes", 0);
+ pmu->idx = engines->num_imc++;
+ pmu->present = true;
}
-static double imc_data_writes_scale(void)
+static void imc_writes_open(struct pmu_counter *pmu, struct engines *engines)
{
- return filename_to_double(IMC_EVENT "data_writes.scale");
+ imc_open(pmu, "data_writes", engines);
}
-static const char *imc_data_writes_unit(void)
+static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
{
- char buf[32];
-
- if (filename_to_buf(IMC_EVENT "data_writes.unit",
- buf, sizeof(buf)) == 0)
- return strdup(buf);
- else
- return NULL;
+ imc_open(pmu, "data_reads", engines);
}
-#define _open_pmu(type, cnt, pmu, fd) \
-({ \
- int fd__; \
-\
- fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
- if (fd__ >= 0) { \
- if ((fd) == -1) \
- (fd) = fd__; \
- (pmu)->present = true; \
- (pmu)->idx = (cnt)++; \
- } \
-\
- fd__; \
-})
-
-#define _open_imc(cnt, pmu, fd) \
-({ \
- int fd__; \
-\
- fd__ = igt_perf_open_group(imc_type_id(), (pmu)->config, (fd)); \
- if (fd__ >= 0) { \
- if ((fd) == -1) \
- (fd) = fd__; \
- (pmu)->present = true; \
- (pmu)->idx = (cnt)++; \
- } \
-\
- fd__; \
-})
-
static int pmu_init(struct engines *engines)
{
unsigned int i;
@@ -595,38 +507,8 @@ static int pmu_init(struct engines *engines)
}
engines->imc_fd = -1;
- if (imc_type_id()) {
- unsigned int num = 0;
-
- engines->imc_reads_scale = imc_data_reads_scale();
- engines->imc_writes_scale = imc_data_writes_scale();
-
- engines->imc_reads_unit = imc_data_reads_unit();
- if (!engines->imc_reads_unit)
- return -1;
-
- engines->imc_writes_unit = imc_data_writes_unit();
- if (!engines->imc_writes_unit)
- return -1;
-
- engines->imc_reads.config = imc_data_reads();
- if (!engines->imc_reads.config)
- return -1;
-
- engines->imc_writes.config = imc_data_writes();
- if (!engines->imc_writes.config)
- return -1;
-
- fd = _open_imc(num, &engines->imc_reads, engines->imc_fd);
- if (fd < 0)
- return -1;
- fd = _open_imc(num, &engines->imc_writes, engines->imc_fd);
- if (fd < 0)
- return -1;
-
- engines->imc_reads.present = true;
- engines->imc_writes.present = true;
- }
+ imc_reads_open(&engines->imc_reads, engines);
+ imc_writes_open(&engines->imc_writes, engines);
return 0;
}
@@ -692,13 +574,6 @@ static void pmu_sample(struct engines *engines)
unsigned int i;
engines->ts.prev = engines->ts.cur;
-
- if (engines->imc_fd >= 0) {
- pmu_read_multi(engines->imc_fd, 2, val);
- update_sample(&engines->imc_reads, val);
- update_sample(&engines->imc_writes, val);
- }
-
engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
update_sample(&engines->freq_req, val);
@@ -719,6 +594,12 @@ static void pmu_sample(struct engines *engines)
update_sample(&engines->r_gpu, val);
update_sample(&engines->r_pkg, val);
}
+
+ if (engines->num_imc) {
+ pmu_read_multi(engines->imc_fd, engines->num_imc, val);
+ update_sample(&engines->imc_reads, val);
+ update_sample(&engines->imc_writes, val);
+ }
}
static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
@@ -1172,9 +1053,9 @@ static int
print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
{
struct cnt_item imc_items[] = {
- { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads_scale,
+ { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads.scale,
"reads", "rd" },
- { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes_scale,
+ { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes.scale,
"writes", "wr" },
{ NULL, 0, 0, 0.0, 0.0, 0.0, "unit" },
{ },
@@ -1189,12 +1070,15 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
};
int ret;
+ if (!engines->num_imc)
+ return lines;
+
ret = asprintf((char **)&imc_group.display_name, "IMC %s/s",
- engines->imc_reads_unit);
+ engines->imc_reads.units);
assert(ret >= 0);
ret = asprintf((char **)&imc_items[2].unit, "%s/s",
- engines->imc_reads_unit);
+ engines->imc_reads.units);
assert(ret >= 0);
print_groups(groups);
@@ -1205,11 +1089,11 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
if (output_mode == INTERACTIVE) {
if (lines++ < con_h)
printf(" IMC reads: %s %s/s\n",
- imc_items[0].buf, engines->imc_reads_unit);
+ imc_items[0].buf, engines->imc_reads.units);
if (lines++ < con_h)
printf(" IMC writes: %s %s/s\n",
- imc_items[1].buf, engines->imc_writes_unit);
+ imc_items[1].buf, engines->imc_writes.units);
if (lines++ < con_h)
printf("\n");
@@ -1509,9 +1393,7 @@ int main(int argc, char **argv)
t, lines, con_w, con_h,
&consumed);
- if (engines->imc_fd)
- lines = print_imc(engines, t, lines, con_w,
- con_h);
+ lines = print_imc(engines, t, lines, con_w, con_h);
lines = print_engines_header(engines, t, lines, con_w,
con_h);
--
2.29.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tools/intel_gpu_top: Consolidate imc to use pmu_counter
2020-11-25 10:04 ` [Intel-gfx] [PATCH i-g-t] " Chris Wilson
@ 2020-11-25 10:17 ` Tvrtko Ursulin
0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2020-11-25 10:17 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
On 25/11/2020 10:04, Chris Wilson wrote:
> Follow the same pattern as rapl for imc, and consolidate everything to
> work on struct pmu_counter.
>
> v2: Combine rapl_parse/imc_parse into pmu_parse
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> tools/intel_gpu_top.c | 248 +++++++++++-------------------------------
> 1 file changed, 65 insertions(+), 183 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 0a49cfecc..ffdac280c 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -55,6 +55,7 @@ struct pmu_counter {
> unsigned int idx;
> struct pmu_pair val;
> double scale;
> + const char *units;
> bool present;
> };
>
> @@ -85,17 +86,14 @@ struct engines {
> unsigned int num_rapl;
>
> int imc_fd;
> - double imc_reads_scale;
> - const char *imc_reads_unit;
> - double imc_writes_scale;
> - const char *imc_writes_unit;
> + struct pmu_counter imc_reads;
> + struct pmu_counter imc_writes;
> + unsigned int num_imc;
>
> struct pmu_counter freq_req;
> struct pmu_counter freq_act;
> struct pmu_counter irq;
> struct pmu_counter rc6;
> - struct pmu_counter imc_reads;
> - struct pmu_counter imc_writes;
>
> bool discrete;
> char *device;
> @@ -136,14 +134,14 @@ static int igt_sysfs_scanf(int dir, const char *attr, const char *fmt, ...)
> return ret;
> }
>
> -static int rapl_parse(struct pmu_counter *pmu, const char *str)
> +static int pmu_parse(struct pmu_counter *pmu, const char *path, const char *str)
> {
> locale_t locale, oldlocale;
> bool result = true;
> char buf[128] = {};
> int dir;
>
> - dir = open("/sys/devices/power", O_RDONLY);
> + dir = open(path, O_RDONLY);
> if (dir < 0)
> return -errno;
>
> @@ -160,10 +158,8 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
> result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
>
> snprintf(buf, sizeof(buf) - 1, "events/energy-%s.unit", str);
> - if (igt_sysfs_scanf(dir, buf, "%127s", buf) == 1 &&
> - strcmp(buf, "Joules"))
> - fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
> - str, buf);
Maybe keep the unit check in rapl_parse?
Regards,
Tvrtko
> + result &= igt_sysfs_scanf(dir, buf, "%127s", buf) == 1;
> + pmu->units = strdup(buf);
>
> uselocale(oldlocale);
> freelocale(locale);
> @@ -179,6 +175,11 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
> return 0;
> }
>
> +static int rapl_parse(struct pmu_counter *pmu, const char *str)
> +{
> + return pmu_parse(pmu, "/sys/devices/power", str);
> +}
> +
> static void
> rapl_open(struct pmu_counter *pmu,
> const char *domain,
> @@ -400,146 +401,57 @@ static struct engines *discover_engines(char *device)
> return engines;
> }
>
> -static int
> -filename_to_buf(const char *filename, char *buf, unsigned int bufsize)
> -{
> - int fd, err;
> - ssize_t ret;
> -
> - fd = open(filename, O_RDONLY);
> - if (fd < 0)
> - return -1;
> -
> - ret = read(fd, buf, bufsize - 1);
> - err = errno;
> - close(fd);
> - if (ret < 1) {
> - errno = ret < 0 ? err : ENOMSG;
> -
> - return -1;
> - }
> -
> - if (ret > 1 && buf[ret - 1] == '\n')
> - buf[ret - 1] = '\0';
> - else
> - buf[ret] = '\0';
> -
> - return 0;
> -}
> -
> -static uint64_t filename_to_u64(const char *filename, int base)
> -{
> - char buf[64], *b;
> -
> - if (filename_to_buf(filename, buf, sizeof(buf)))
> - return 0;
> -
> - /*
> - * Handle both single integer and key=value formats by skipping
> - * leading non-digits.
> - */
> - b = buf;
> - while (*b && !isdigit(*b))
> - b++;
> -
> - return strtoull(b, NULL, base);
> -}
> -
> -static double filename_to_double(const char *filename)
> -{
> - char *oldlocale;
> - char buf[80];
> - double v;
> -
> - if (filename_to_buf(filename, buf, sizeof(buf)))
> - return 0;
> -
> - oldlocale = setlocale(LC_ALL, "C");
> - v = strtod(buf, NULL);
> - setlocale(LC_ALL, oldlocale);
> -
> - return v;
> -}
> -
> -#define IMC_ROOT "/sys/devices/uncore_imc/"
> -#define IMC_EVENT "/sys/devices/uncore_imc/events/"
> +#define _open_pmu(type, cnt, pmu, fd) \
> +({ \
> + int fd__; \
> +\
> + fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
> + if (fd__ >= 0) { \
> + if ((fd) == -1) \
> + (fd) = fd__; \
> + (pmu)->present = true; \
> + (pmu)->idx = (cnt)++; \
> + } \
> +\
> + fd__; \
> +})
>
> -static uint64_t imc_type_id(void)
> +static int imc_parse(struct pmu_counter *pmu, const char *str)
> {
> - return filename_to_u64(IMC_ROOT "type", 10);
> + return pmu_parse(pmu, "/sys/devices/uncore_imc", str);
> }
>
> -static uint64_t imc_data_reads(void)
> +static void
> +imc_open(struct pmu_counter *pmu,
> + const char *domain,
> + struct engines *engines)
> {
> - return filename_to_u64(IMC_EVENT "data_reads", 0);
> -}
> + int fd;
>
> -static double imc_data_reads_scale(void)
> -{
> - return filename_to_double(IMC_EVENT "data_reads.scale");
> -}
> + if (imc_parse(pmu, domain) < 0)
> + return;
>
> -static const char *imc_data_reads_unit(void)
> -{
> - char buf[32];
> + fd = igt_perf_open_group(pmu->type, pmu->config, engines->imc_fd);
> + if (fd < 0)
> + return;
>
> - if (filename_to_buf(IMC_EVENT "data_reads.unit", buf, sizeof(buf)) == 0)
> - return strdup(buf);
> - else
> - return NULL;
> -}
> + if (engines->imc_fd == -1)
> + engines->imc_fd = fd;
>
> -static uint64_t imc_data_writes(void)
> -{
> - return filename_to_u64(IMC_EVENT "data_writes", 0);
> + pmu->idx = engines->num_imc++;
> + pmu->present = true;
> }
>
> -static double imc_data_writes_scale(void)
> +static void imc_writes_open(struct pmu_counter *pmu, struct engines *engines)
> {
> - return filename_to_double(IMC_EVENT "data_writes.scale");
> + imc_open(pmu, "data_writes", engines);
> }
>
> -static const char *imc_data_writes_unit(void)
> +static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
> {
> - char buf[32];
> -
> - if (filename_to_buf(IMC_EVENT "data_writes.unit",
> - buf, sizeof(buf)) == 0)
> - return strdup(buf);
> - else
> - return NULL;
> + imc_open(pmu, "data_reads", engines);
> }
>
> -#define _open_pmu(type, cnt, pmu, fd) \
> -({ \
> - int fd__; \
> -\
> - fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
> - if (fd__ >= 0) { \
> - if ((fd) == -1) \
> - (fd) = fd__; \
> - (pmu)->present = true; \
> - (pmu)->idx = (cnt)++; \
> - } \
> -\
> - fd__; \
> -})
> -
> -#define _open_imc(cnt, pmu, fd) \
> -({ \
> - int fd__; \
> -\
> - fd__ = igt_perf_open_group(imc_type_id(), (pmu)->config, (fd)); \
> - if (fd__ >= 0) { \
> - if ((fd) == -1) \
> - (fd) = fd__; \
> - (pmu)->present = true; \
> - (pmu)->idx = (cnt)++; \
> - } \
> -\
> - fd__; \
> -})
> -
> static int pmu_init(struct engines *engines)
> {
> unsigned int i;
> @@ -595,38 +507,8 @@ static int pmu_init(struct engines *engines)
> }
>
> engines->imc_fd = -1;
> - if (imc_type_id()) {
> - unsigned int num = 0;
> -
> - engines->imc_reads_scale = imc_data_reads_scale();
> - engines->imc_writes_scale = imc_data_writes_scale();
> -
> - engines->imc_reads_unit = imc_data_reads_unit();
> - if (!engines->imc_reads_unit)
> - return -1;
> -
> - engines->imc_writes_unit = imc_data_writes_unit();
> - if (!engines->imc_writes_unit)
> - return -1;
> -
> - engines->imc_reads.config = imc_data_reads();
> - if (!engines->imc_reads.config)
> - return -1;
> -
> - engines->imc_writes.config = imc_data_writes();
> - if (!engines->imc_writes.config)
> - return -1;
> -
> - fd = _open_imc(num, &engines->imc_reads, engines->imc_fd);
> - if (fd < 0)
> - return -1;
> - fd = _open_imc(num, &engines->imc_writes, engines->imc_fd);
> - if (fd < 0)
> - return -1;
> -
> - engines->imc_reads.present = true;
> - engines->imc_writes.present = true;
> - }
> + imc_reads_open(&engines->imc_reads, engines);
> + imc_writes_open(&engines->imc_writes, engines);
>
> return 0;
> }
> @@ -692,13 +574,6 @@ static void pmu_sample(struct engines *engines)
> unsigned int i;
>
> engines->ts.prev = engines->ts.cur;
> -
> - if (engines->imc_fd >= 0) {
> - pmu_read_multi(engines->imc_fd, 2, val);
> - update_sample(&engines->imc_reads, val);
> - update_sample(&engines->imc_writes, val);
> - }
> -
> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>
> update_sample(&engines->freq_req, val);
> @@ -719,6 +594,12 @@ static void pmu_sample(struct engines *engines)
> update_sample(&engines->r_gpu, val);
> update_sample(&engines->r_pkg, val);
> }
> +
> + if (engines->num_imc) {
> + pmu_read_multi(engines->imc_fd, engines->num_imc, val);
> + update_sample(&engines->imc_reads, val);
> + update_sample(&engines->imc_writes, val);
> + }
> }
>
> static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -1172,9 +1053,9 @@ static int
> print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> {
> struct cnt_item imc_items[] = {
> - { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads_scale,
> + { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads.scale,
> "reads", "rd" },
> - { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes_scale,
> + { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes.scale,
> "writes", "wr" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit" },
> { },
> @@ -1189,12 +1070,15 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> };
> int ret;
>
> + if (!engines->num_imc)
> + return lines;
> +
> ret = asprintf((char **)&imc_group.display_name, "IMC %s/s",
> - engines->imc_reads_unit);
> + engines->imc_reads.units);
> assert(ret >= 0);
>
> ret = asprintf((char **)&imc_items[2].unit, "%s/s",
> - engines->imc_reads_unit);
> + engines->imc_reads.units);
> assert(ret >= 0);
>
> print_groups(groups);
> @@ -1205,11 +1089,11 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> if (output_mode == INTERACTIVE) {
> if (lines++ < con_h)
> printf(" IMC reads: %s %s/s\n",
> - imc_items[0].buf, engines->imc_reads_unit);
> + imc_items[0].buf, engines->imc_reads.units);
>
> if (lines++ < con_h)
> printf(" IMC writes: %s %s/s\n",
> - imc_items[1].buf, engines->imc_writes_unit);
> + imc_items[1].buf, engines->imc_writes.units);
>
> if (lines++ < con_h)
> printf("\n");
> @@ -1509,9 +1393,7 @@ int main(int argc, char **argv)
> t, lines, con_w, con_h,
> &consumed);
>
> - if (engines->imc_fd)
> - lines = print_imc(engines, t, lines, con_w,
> - con_h);
> + lines = print_imc(engines, t, lines, con_w, con_h);
>
> lines = print_engines_header(engines, t, lines, con_w,
> con_h);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] [PATCH i-g-t] tools/intel_gpu_top: Consolidate imc to use pmu_counter
2020-11-24 23:27 ` [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter Chris Wilson
2020-11-25 8:58 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2020-11-25 10:04 ` [Intel-gfx] [PATCH i-g-t] " Chris Wilson
@ 2020-11-25 10:35 ` Chris Wilson
2020-11-25 10:47 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2020-11-25 10:35 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Chris Wilson
Follow the same pattern as rapl for imc, and consolidate everything to
work on struct pmu_counter.
v2: Combine rapl_parse/imc_parse into pmu_parse
v3: Keep the error message for RAPL not reporting Joules
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tools/intel_gpu_top.c | 261 +++++++++++++-----------------------------
1 file changed, 78 insertions(+), 183 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 0a49cfecc..5d42a2fad 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -55,6 +55,7 @@ struct pmu_counter {
unsigned int idx;
struct pmu_pair val;
double scale;
+ const char *units;
bool present;
};
@@ -85,17 +86,14 @@ struct engines {
unsigned int num_rapl;
int imc_fd;
- double imc_reads_scale;
- const char *imc_reads_unit;
- double imc_writes_scale;
- const char *imc_writes_unit;
+ struct pmu_counter imc_reads;
+ struct pmu_counter imc_writes;
+ unsigned int num_imc;
struct pmu_counter freq_req;
struct pmu_counter freq_act;
struct pmu_counter irq;
struct pmu_counter rc6;
- struct pmu_counter imc_reads;
- struct pmu_counter imc_writes;
bool discrete;
char *device;
@@ -136,14 +134,14 @@ static int igt_sysfs_scanf(int dir, const char *attr, const char *fmt, ...)
return ret;
}
-static int rapl_parse(struct pmu_counter *pmu, const char *str)
+static int pmu_parse(struct pmu_counter *pmu, const char *path, const char *str)
{
locale_t locale, oldlocale;
bool result = true;
char buf[128] = {};
int dir;
- dir = open("/sys/devices/power", O_RDONLY);
+ dir = open(path, O_RDONLY);
if (dir < 0)
return -errno;
@@ -160,10 +158,8 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
snprintf(buf, sizeof(buf) - 1, "events/energy-%s.unit", str);
- if (igt_sysfs_scanf(dir, buf, "%127s", buf) == 1 &&
- strcmp(buf, "Joules"))
- fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
- str, buf);
+ result &= igt_sysfs_scanf(dir, buf, "%127s", buf) == 1;
+ pmu->units = strdup(buf);
uselocale(oldlocale);
freelocale(locale);
@@ -179,6 +175,24 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
return 0;
}
+static int rapl_parse(struct pmu_counter *pmu, const char *str)
+{
+ const char *expected_units = "Joules";
+ int err;
+
+ err = pmu_parse(pmu, "/sys/devices/power", str);
+ if (err < 0)
+ return err;
+
+ if (!pmu->units || strcmp(pmu->units, expected_units)) {
+ fprintf(stderr,
+ "Unexpected units for RAPL %s: found '%s', expected '%s'\n",
+ str, pmu->units, expected_units);
+ }
+
+ return 0;
+}
+
static void
rapl_open(struct pmu_counter *pmu,
const char *domain,
@@ -400,146 +414,57 @@ static struct engines *discover_engines(char *device)
return engines;
}
-static int
-filename_to_buf(const char *filename, char *buf, unsigned int bufsize)
-{
- int fd, err;
- ssize_t ret;
-
- fd = open(filename, O_RDONLY);
- if (fd < 0)
- return -1;
-
- ret = read(fd, buf, bufsize - 1);
- err = errno;
- close(fd);
- if (ret < 1) {
- errno = ret < 0 ? err : ENOMSG;
-
- return -1;
- }
-
- if (ret > 1 && buf[ret - 1] == '\n')
- buf[ret - 1] = '\0';
- else
- buf[ret] = '\0';
-
- return 0;
-}
-
-static uint64_t filename_to_u64(const char *filename, int base)
-{
- char buf[64], *b;
-
- if (filename_to_buf(filename, buf, sizeof(buf)))
- return 0;
-
- /*
- * Handle both single integer and key=value formats by skipping
- * leading non-digits.
- */
- b = buf;
- while (*b && !isdigit(*b))
- b++;
-
- return strtoull(b, NULL, base);
-}
-
-static double filename_to_double(const char *filename)
-{
- char *oldlocale;
- char buf[80];
- double v;
-
- if (filename_to_buf(filename, buf, sizeof(buf)))
- return 0;
-
- oldlocale = setlocale(LC_ALL, "C");
- v = strtod(buf, NULL);
- setlocale(LC_ALL, oldlocale);
-
- return v;
-}
-
-#define IMC_ROOT "/sys/devices/uncore_imc/"
-#define IMC_EVENT "/sys/devices/uncore_imc/events/"
+#define _open_pmu(type, cnt, pmu, fd) \
+({ \
+ int fd__; \
+\
+ fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
+ if (fd__ >= 0) { \
+ if ((fd) == -1) \
+ (fd) = fd__; \
+ (pmu)->present = true; \
+ (pmu)->idx = (cnt)++; \
+ } \
+\
+ fd__; \
+})
-static uint64_t imc_type_id(void)
+static int imc_parse(struct pmu_counter *pmu, const char *str)
{
- return filename_to_u64(IMC_ROOT "type", 10);
+ return pmu_parse(pmu, "/sys/devices/uncore_imc", str);
}
-static uint64_t imc_data_reads(void)
+static void
+imc_open(struct pmu_counter *pmu,
+ const char *domain,
+ struct engines *engines)
{
- return filename_to_u64(IMC_EVENT "data_reads", 0);
-}
+ int fd;
-static double imc_data_reads_scale(void)
-{
- return filename_to_double(IMC_EVENT "data_reads.scale");
-}
+ if (imc_parse(pmu, domain) < 0)
+ return;
-static const char *imc_data_reads_unit(void)
-{
- char buf[32];
+ fd = igt_perf_open_group(pmu->type, pmu->config, engines->imc_fd);
+ if (fd < 0)
+ return;
- if (filename_to_buf(IMC_EVENT "data_reads.unit", buf, sizeof(buf)) == 0)
- return strdup(buf);
- else
- return NULL;
-}
+ if (engines->imc_fd == -1)
+ engines->imc_fd = fd;
-static uint64_t imc_data_writes(void)
-{
- return filename_to_u64(IMC_EVENT "data_writes", 0);
+ pmu->idx = engines->num_imc++;
+ pmu->present = true;
}
-static double imc_data_writes_scale(void)
+static void imc_writes_open(struct pmu_counter *pmu, struct engines *engines)
{
- return filename_to_double(IMC_EVENT "data_writes.scale");
+ imc_open(pmu, "data_writes", engines);
}
-static const char *imc_data_writes_unit(void)
+static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
{
- char buf[32];
-
- if (filename_to_buf(IMC_EVENT "data_writes.unit",
- buf, sizeof(buf)) == 0)
- return strdup(buf);
- else
- return NULL;
+ imc_open(pmu, "data_reads", engines);
}
-#define _open_pmu(type, cnt, pmu, fd) \
-({ \
- int fd__; \
-\
- fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
- if (fd__ >= 0) { \
- if ((fd) == -1) \
- (fd) = fd__; \
- (pmu)->present = true; \
- (pmu)->idx = (cnt)++; \
- } \
-\
- fd__; \
-})
-
-#define _open_imc(cnt, pmu, fd) \
-({ \
- int fd__; \
-\
- fd__ = igt_perf_open_group(imc_type_id(), (pmu)->config, (fd)); \
- if (fd__ >= 0) { \
- if ((fd) == -1) \
- (fd) = fd__; \
- (pmu)->present = true; \
- (pmu)->idx = (cnt)++; \
- } \
-\
- fd__; \
-})
-
static int pmu_init(struct engines *engines)
{
unsigned int i;
@@ -595,38 +520,8 @@ static int pmu_init(struct engines *engines)
}
engines->imc_fd = -1;
- if (imc_type_id()) {
- unsigned int num = 0;
-
- engines->imc_reads_scale = imc_data_reads_scale();
- engines->imc_writes_scale = imc_data_writes_scale();
-
- engines->imc_reads_unit = imc_data_reads_unit();
- if (!engines->imc_reads_unit)
- return -1;
-
- engines->imc_writes_unit = imc_data_writes_unit();
- if (!engines->imc_writes_unit)
- return -1;
-
- engines->imc_reads.config = imc_data_reads();
- if (!engines->imc_reads.config)
- return -1;
-
- engines->imc_writes.config = imc_data_writes();
- if (!engines->imc_writes.config)
- return -1;
-
- fd = _open_imc(num, &engines->imc_reads, engines->imc_fd);
- if (fd < 0)
- return -1;
- fd = _open_imc(num, &engines->imc_writes, engines->imc_fd);
- if (fd < 0)
- return -1;
-
- engines->imc_reads.present = true;
- engines->imc_writes.present = true;
- }
+ imc_reads_open(&engines->imc_reads, engines);
+ imc_writes_open(&engines->imc_writes, engines);
return 0;
}
@@ -692,13 +587,6 @@ static void pmu_sample(struct engines *engines)
unsigned int i;
engines->ts.prev = engines->ts.cur;
-
- if (engines->imc_fd >= 0) {
- pmu_read_multi(engines->imc_fd, 2, val);
- update_sample(&engines->imc_reads, val);
- update_sample(&engines->imc_writes, val);
- }
-
engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
update_sample(&engines->freq_req, val);
@@ -719,6 +607,12 @@ static void pmu_sample(struct engines *engines)
update_sample(&engines->r_gpu, val);
update_sample(&engines->r_pkg, val);
}
+
+ if (engines->num_imc) {
+ pmu_read_multi(engines->imc_fd, engines->num_imc, val);
+ update_sample(&engines->imc_reads, val);
+ update_sample(&engines->imc_writes, val);
+ }
}
static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
@@ -1172,9 +1066,9 @@ static int
print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
{
struct cnt_item imc_items[] = {
- { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads_scale,
+ { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads.scale,
"reads", "rd" },
- { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes_scale,
+ { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes.scale,
"writes", "wr" },
{ NULL, 0, 0, 0.0, 0.0, 0.0, "unit" },
{ },
@@ -1189,12 +1083,15 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
};
int ret;
+ if (!engines->num_imc)
+ return lines;
+
ret = asprintf((char **)&imc_group.display_name, "IMC %s/s",
- engines->imc_reads_unit);
+ engines->imc_reads.units);
assert(ret >= 0);
ret = asprintf((char **)&imc_items[2].unit, "%s/s",
- engines->imc_reads_unit);
+ engines->imc_reads.units);
assert(ret >= 0);
print_groups(groups);
@@ -1205,11 +1102,11 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
if (output_mode == INTERACTIVE) {
if (lines++ < con_h)
printf(" IMC reads: %s %s/s\n",
- imc_items[0].buf, engines->imc_reads_unit);
+ imc_items[0].buf, engines->imc_reads.units);
if (lines++ < con_h)
printf(" IMC writes: %s %s/s\n",
- imc_items[1].buf, engines->imc_writes_unit);
+ imc_items[1].buf, engines->imc_writes.units);
if (lines++ < con_h)
printf("\n");
@@ -1509,9 +1406,7 @@ int main(int argc, char **argv)
t, lines, con_w, con_h,
&consumed);
- if (engines->imc_fd)
- lines = print_imc(engines, t, lines, con_w,
- con_h);
+ lines = print_imc(engines, t, lines, con_w, con_h);
lines = print_engines_header(engines, t, lines, con_w,
con_h);
--
2.29.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tools/intel_gpu_top: Consolidate imc to use pmu_counter
2020-11-25 10:35 ` [Intel-gfx] " Chris Wilson
@ 2020-11-25 10:47 ` Tvrtko Ursulin
0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2020-11-25 10:47 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
On 25/11/2020 10:35, Chris Wilson wrote:
> Follow the same pattern as rapl for imc, and consolidate everything to
> work on struct pmu_counter.
>
> v2: Combine rapl_parse/imc_parse into pmu_parse
> v3: Keep the error message for RAPL not reporting Joules
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> tools/intel_gpu_top.c | 261 +++++++++++++-----------------------------
> 1 file changed, 78 insertions(+), 183 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 0a49cfecc..5d42a2fad 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -55,6 +55,7 @@ struct pmu_counter {
> unsigned int idx;
> struct pmu_pair val;
> double scale;
> + const char *units;
> bool present;
> };
>
> @@ -85,17 +86,14 @@ struct engines {
> unsigned int num_rapl;
>
> int imc_fd;
> - double imc_reads_scale;
> - const char *imc_reads_unit;
> - double imc_writes_scale;
> - const char *imc_writes_unit;
> + struct pmu_counter imc_reads;
> + struct pmu_counter imc_writes;
> + unsigned int num_imc;
>
> struct pmu_counter freq_req;
> struct pmu_counter freq_act;
> struct pmu_counter irq;
> struct pmu_counter rc6;
> - struct pmu_counter imc_reads;
> - struct pmu_counter imc_writes;
>
> bool discrete;
> char *device;
> @@ -136,14 +134,14 @@ static int igt_sysfs_scanf(int dir, const char *attr, const char *fmt, ...)
> return ret;
> }
>
> -static int rapl_parse(struct pmu_counter *pmu, const char *str)
> +static int pmu_parse(struct pmu_counter *pmu, const char *path, const char *str)
> {
> locale_t locale, oldlocale;
> bool result = true;
> char buf[128] = {};
> int dir;
>
> - dir = open("/sys/devices/power", O_RDONLY);
> + dir = open(path, O_RDONLY);
> if (dir < 0)
> return -errno;
>
> @@ -160,10 +158,8 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
> result &= igt_sysfs_scanf(dir, buf, "%lf", &pmu->scale) == 1;
>
> snprintf(buf, sizeof(buf) - 1, "events/energy-%s.unit", str);
> - if (igt_sysfs_scanf(dir, buf, "%127s", buf) == 1 &&
> - strcmp(buf, "Joules"))
> - fprintf(stderr, "Unexpected units for RAPL %s: found %s\n",
> - str, buf);
> + result &= igt_sysfs_scanf(dir, buf, "%127s", buf) == 1;
> + pmu->units = strdup(buf);
>
> uselocale(oldlocale);
> freelocale(locale);
> @@ -179,6 +175,24 @@ static int rapl_parse(struct pmu_counter *pmu, const char *str)
> return 0;
> }
>
> +static int rapl_parse(struct pmu_counter *pmu, const char *str)
> +{
> + const char *expected_units = "Joules";
> + int err;
> +
> + err = pmu_parse(pmu, "/sys/devices/power", str);
> + if (err < 0)
> + return err;
> +
> + if (!pmu->units || strcmp(pmu->units, expected_units)) {
> + fprintf(stderr,
> + "Unexpected units for RAPL %s: found '%s', expected '%s'\n",
> + str, pmu->units, expected_units);
> + }
> +
> + return 0;
> +}
> +
> static void
> rapl_open(struct pmu_counter *pmu,
> const char *domain,
> @@ -400,146 +414,57 @@ static struct engines *discover_engines(char *device)
> return engines;
> }
>
> -static int
> -filename_to_buf(const char *filename, char *buf, unsigned int bufsize)
> -{
> - int fd, err;
> - ssize_t ret;
> -
> - fd = open(filename, O_RDONLY);
> - if (fd < 0)
> - return -1;
> -
> - ret = read(fd, buf, bufsize - 1);
> - err = errno;
> - close(fd);
> - if (ret < 1) {
> - errno = ret < 0 ? err : ENOMSG;
> -
> - return -1;
> - }
> -
> - if (ret > 1 && buf[ret - 1] == '\n')
> - buf[ret - 1] = '\0';
> - else
> - buf[ret] = '\0';
> -
> - return 0;
> -}
> -
> -static uint64_t filename_to_u64(const char *filename, int base)
> -{
> - char buf[64], *b;
> -
> - if (filename_to_buf(filename, buf, sizeof(buf)))
> - return 0;
> -
> - /*
> - * Handle both single integer and key=value formats by skipping
> - * leading non-digits.
> - */
> - b = buf;
> - while (*b && !isdigit(*b))
> - b++;
> -
> - return strtoull(b, NULL, base);
> -}
> -
> -static double filename_to_double(const char *filename)
> -{
> - char *oldlocale;
> - char buf[80];
> - double v;
> -
> - if (filename_to_buf(filename, buf, sizeof(buf)))
> - return 0;
> -
> - oldlocale = setlocale(LC_ALL, "C");
> - v = strtod(buf, NULL);
> - setlocale(LC_ALL, oldlocale);
> -
> - return v;
> -}
> -
> -#define IMC_ROOT "/sys/devices/uncore_imc/"
> -#define IMC_EVENT "/sys/devices/uncore_imc/events/"
> +#define _open_pmu(type, cnt, pmu, fd) \
> +({ \
> + int fd__; \
> +\
> + fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
> + if (fd__ >= 0) { \
> + if ((fd) == -1) \
> + (fd) = fd__; \
> + (pmu)->present = true; \
> + (pmu)->idx = (cnt)++; \
> + } \
> +\
> + fd__; \
> +})
>
> -static uint64_t imc_type_id(void)
> +static int imc_parse(struct pmu_counter *pmu, const char *str)
> {
> - return filename_to_u64(IMC_ROOT "type", 10);
> + return pmu_parse(pmu, "/sys/devices/uncore_imc", str);
> }
>
> -static uint64_t imc_data_reads(void)
> +static void
> +imc_open(struct pmu_counter *pmu,
> + const char *domain,
> + struct engines *engines)
> {
> - return filename_to_u64(IMC_EVENT "data_reads", 0);
> -}
> + int fd;
>
> -static double imc_data_reads_scale(void)
> -{
> - return filename_to_double(IMC_EVENT "data_reads.scale");
> -}
> + if (imc_parse(pmu, domain) < 0)
> + return;
>
> -static const char *imc_data_reads_unit(void)
> -{
> - char buf[32];
> + fd = igt_perf_open_group(pmu->type, pmu->config, engines->imc_fd);
> + if (fd < 0)
> + return;
>
> - if (filename_to_buf(IMC_EVENT "data_reads.unit", buf, sizeof(buf)) == 0)
> - return strdup(buf);
> - else
> - return NULL;
> -}
> + if (engines->imc_fd == -1)
> + engines->imc_fd = fd;
>
> -static uint64_t imc_data_writes(void)
> -{
> - return filename_to_u64(IMC_EVENT "data_writes", 0);
> + pmu->idx = engines->num_imc++;
> + pmu->present = true;
> }
>
> -static double imc_data_writes_scale(void)
> +static void imc_writes_open(struct pmu_counter *pmu, struct engines *engines)
> {
> - return filename_to_double(IMC_EVENT "data_writes.scale");
> + imc_open(pmu, "data_writes", engines);
> }
>
> -static const char *imc_data_writes_unit(void)
> +static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
> {
> - char buf[32];
> -
> - if (filename_to_buf(IMC_EVENT "data_writes.unit",
> - buf, sizeof(buf)) == 0)
> - return strdup(buf);
> - else
> - return NULL;
> + imc_open(pmu, "data_reads", engines);
> }
>
> -#define _open_pmu(type, cnt, pmu, fd) \
> -({ \
> - int fd__; \
> -\
> - fd__ = igt_perf_open_group((type), (pmu)->config, (fd)); \
> - if (fd__ >= 0) { \
> - if ((fd) == -1) \
> - (fd) = fd__; \
> - (pmu)->present = true; \
> - (pmu)->idx = (cnt)++; \
> - } \
> -\
> - fd__; \
> -})
> -
> -#define _open_imc(cnt, pmu, fd) \
> -({ \
> - int fd__; \
> -\
> - fd__ = igt_perf_open_group(imc_type_id(), (pmu)->config, (fd)); \
> - if (fd__ >= 0) { \
> - if ((fd) == -1) \
> - (fd) = fd__; \
> - (pmu)->present = true; \
> - (pmu)->idx = (cnt)++; \
> - } \
> -\
> - fd__; \
> -})
> -
> static int pmu_init(struct engines *engines)
> {
> unsigned int i;
> @@ -595,38 +520,8 @@ static int pmu_init(struct engines *engines)
> }
>
> engines->imc_fd = -1;
> - if (imc_type_id()) {
> - unsigned int num = 0;
> -
> - engines->imc_reads_scale = imc_data_reads_scale();
> - engines->imc_writes_scale = imc_data_writes_scale();
> -
> - engines->imc_reads_unit = imc_data_reads_unit();
> - if (!engines->imc_reads_unit)
> - return -1;
> -
> - engines->imc_writes_unit = imc_data_writes_unit();
> - if (!engines->imc_writes_unit)
> - return -1;
> -
> - engines->imc_reads.config = imc_data_reads();
> - if (!engines->imc_reads.config)
> - return -1;
> -
> - engines->imc_writes.config = imc_data_writes();
> - if (!engines->imc_writes.config)
> - return -1;
> -
> - fd = _open_imc(num, &engines->imc_reads, engines->imc_fd);
> - if (fd < 0)
> - return -1;
> - fd = _open_imc(num, &engines->imc_writes, engines->imc_fd);
> - if (fd < 0)
> - return -1;
> -
> - engines->imc_reads.present = true;
> - engines->imc_writes.present = true;
> - }
> + imc_reads_open(&engines->imc_reads, engines);
> + imc_writes_open(&engines->imc_writes, engines);
>
> return 0;
> }
> @@ -692,13 +587,6 @@ static void pmu_sample(struct engines *engines)
> unsigned int i;
>
> engines->ts.prev = engines->ts.cur;
> -
> - if (engines->imc_fd >= 0) {
> - pmu_read_multi(engines->imc_fd, 2, val);
> - update_sample(&engines->imc_reads, val);
> - update_sample(&engines->imc_writes, val);
> - }
> -
> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>
> update_sample(&engines->freq_req, val);
> @@ -719,6 +607,12 @@ static void pmu_sample(struct engines *engines)
> update_sample(&engines->r_gpu, val);
> update_sample(&engines->r_pkg, val);
> }
> +
> + if (engines->num_imc) {
> + pmu_read_multi(engines->imc_fd, engines->num_imc, val);
> + update_sample(&engines->imc_reads, val);
> + update_sample(&engines->imc_writes, val);
> + }
> }
>
> static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -1172,9 +1066,9 @@ static int
> print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> {
> struct cnt_item imc_items[] = {
> - { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads_scale,
> + { &engines->imc_reads, 6, 0, 1.0, t, engines->imc_reads.scale,
> "reads", "rd" },
> - { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes_scale,
> + { &engines->imc_writes, 6, 0, 1.0, t, engines->imc_writes.scale,
> "writes", "wr" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit" },
> { },
> @@ -1189,12 +1083,15 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> };
> int ret;
>
> + if (!engines->num_imc)
> + return lines;
> +
> ret = asprintf((char **)&imc_group.display_name, "IMC %s/s",
> - engines->imc_reads_unit);
> + engines->imc_reads.units);
> assert(ret >= 0);
>
> ret = asprintf((char **)&imc_items[2].unit, "%s/s",
> - engines->imc_reads_unit);
> + engines->imc_reads.units);
> assert(ret >= 0);
>
> print_groups(groups);
> @@ -1205,11 +1102,11 @@ print_imc(struct engines *engines, double t, int lines, int con_w, int con_h)
> if (output_mode == INTERACTIVE) {
> if (lines++ < con_h)
> printf(" IMC reads: %s %s/s\n",
> - imc_items[0].buf, engines->imc_reads_unit);
> + imc_items[0].buf, engines->imc_reads.units);
>
> if (lines++ < con_h)
> printf(" IMC writes: %s %s/s\n",
> - imc_items[1].buf, engines->imc_writes_unit);
> + imc_items[1].buf, engines->imc_writes.units);
>
> if (lines++ < con_h)
> printf("\n");
> @@ -1509,9 +1406,7 @@ int main(int argc, char **argv)
> t, lines, con_w, con_h,
> &consumed);
>
> - if (engines->imc_fd)
> - lines = print_imc(engines, t, lines, con_w,
> - con_h);
> + lines = print_imc(engines, t, lines, con_w, con_h);
>
> lines = print_engines_header(engines, t, lines, con_w,
> con_h);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-25 10:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-24 23:27 [Intel-gfx] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power Chris Wilson
2020-11-24 23:27 ` [Intel-gfx] [PATCH i-g-t 2/2] tools/intel_gpu_top: Consolidate imc to use pmu_counter Chris Wilson
2020-11-25 8:58 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2020-11-25 10:04 ` [Intel-gfx] [PATCH i-g-t] " Chris Wilson
2020-11-25 10:17 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2020-11-25 10:35 ` [Intel-gfx] " Chris Wilson
2020-11-25 10:47 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2020-11-25 8:56 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tools/intel_gpu_top: Include total package power Tvrtko Ursulin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox