Re: [Powertop] Intel GPU statistics on multi-GPU systems

[Niels Penneman <niels at penneman.org>]

[-- Attachment #1: Type: text/plain, Size: 8010 bytes --]

Hi again,

I've noticed most of the code in powertop deals with sysfs directly, so
I guess 'scanning' sysfs looking for an Intel GPU is the way to go. I've
checked the latest version of xf86-video-intel (2.21.11) and it looks
like Intel GPUs all have the same PCI vendor ID (0x8086). Below is a
patch that looks for all items /sys/class/drm/card[0-9]+ and checks
whether the vendor is Intel. It then checks whether it can find the
power/rc6_residency_ms file like it did earlier and uses the first GPU
that passes all these checks.

What it does not fix:
- Situations where there may be multiple Intel GPUs. I don't know
whether such systems exist, and in that case each GPU would need to be
tied to a specific processor (socket).
- Hardcoded paths used to retrieve backlight information.

Side effects:
- In the case that any video driver other than Intel's exposed a
'power/rc6_residency_ms' file, the patch below will not consider the
device because it checks vendor ID. If that is not desired, the vendor
ID check should be omitted.


>From 71b0cea0b9b5cbe9ad7f673375004928ae842d9f Mon Sep 17 00:00:00 2001
From: Niels Penneman <niels.penneman(a)elis.ugent.be>
Date: Thu, 4 Jul 2013 20:20:01 +0200
Subject: [PATCH] Probe for Intel GPU

---
 src/cpu/cpu.cpp       | 60
++++++++++++++++++++++++++++++++++++++++++++++-----
 src/cpu/intel_cpus.h  |  2 ++
 src/cpu/intel_gpu.cpp | 21 ++++++++++++------
 3 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index 5d0db28..7fe3f84 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -22,11 +22,13 @@
  * Authors:
  *    Arjan van de Ven <arjan(a)linux.intel.com>
  */
+#include <cctype>
 #include <iostream>
 #include <fstream>
 #include <vector>
 #include <string.h>
 #include <stdlib.h>
+#include <dirent.h>
 #include <ncurses.h>
 #include <unistd.h>
 #include "cpu.h"
@@ -151,13 +153,14 @@ static class abstract_cpu * new_core(int core, int
cpu, char * vendor, int famil
     return ret;
 }
 
-static class abstract_cpu * new_i965_gpu(void)
+static class abstract_cpu * new_i965_gpu(const char *gpu)
 {
     class abstract_cpu *ret = NULL;
 
     ret = new class i965_core;
     ret->childcount = 0;
     ret->set_type("GPU");
+    sprintf(static_cast<class i965_core *>(ret)->sysfs_dir,
"/sys/class/drm/%s/power/", gpu);
 
     return ret;
 }
@@ -260,7 +263,7 @@ static void handle_one_cpu(unsigned int number, char
*vendor, int family, int mo
     all_cpus[number] = cpu;
 }
 
-static void handle_i965_gpu(void)
+static void handle_i965_gpu(const char *gpu)
 {
     unsigned int core_number = 0;
     class abstract_cpu *package;
@@ -274,11 +277,56 @@ static void handle_i965_gpu(void)
         package->children.resize(core_number + 1, NULL);
 
     if (!package->children[core_number]) {
-        package->children[core_number] = new_i965_gpu();
+        package->children[core_number] = new_i965_gpu(gpu);
         package->childcount++;
     }
 }
 
+static bool probe_i965_gpu(char *gpu)
+{
+    DIR *dir;
+    struct dirent *entry;
+    bool found = false;
+
+    dir = opendir("/sys/class/drm");
+    if (!dir)
+        return false;
+    while ((entry = readdir(dir))) {
+        ifstream file;
+        char filename[4096];
+        char *p;
+        uint16_t vendor;
+
+        /* Match only cardN entries (N=0,1,...) */
+        if (strncmp(entry->d_name, "card", 4) != 0)
+            continue;
+        p = &entry->d_name[4];
+        while (*p && std::isdigit(*p++));
+        if (*p)
+            continue;
+
+        /* Check for Intel vendor ID */
+        sprintf(filename, "/sys/class/drm/%s/device/vendor",
entry->d_name);
+        file.open(filename);
+        if (file) {
+            file >> hex >> vendor;
+            file.close();
+        }
+        if (vendor != 0x8086)
+            continue;
+
+        /* Check for one of the interesting files */
+        sprintf(filename, "/sys/class/drm/%s/power/rc6_residency_ms",
entry->d_name);
+        if (access(filename, R_OK) == 0) {
+            strcpy(gpu, entry->d_name);
+            found = true;
+            break;
+        }
+    }
+    closedir(dir);
+    return found;
+}
+
 
 void enumerate_cpus(void)
 {
@@ -290,6 +338,8 @@ void enumerate_cpus(void)
     int family = 0;
     int model = 0;
 
+    char gpu[256];
+
     file.open("/proc/cpuinfo",  ios::in);
 
     if (!file)
@@ -350,8 +400,8 @@ void enumerate_cpus(void)
 
     file.close();
 
-    if (access("/sys/class/drm/card0/power/rc6_residency_ms", R_OK) == 0)
-        handle_i965_gpu();
+    if (probe_i965_gpu(gpu))
+        handle_i965_gpu(gpu);
 
     perf_events = new perf_power_bundle();
 
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index 64d74f2..32679c1 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -137,6 +137,8 @@ private:
     struct timeval    after;
 
 public:
+    char sysfs_dir[512];
+
     virtual void    measurement_start(void);
     virtual void    measurement_end(void);
     virtual int     can_collapse(void) { return 0;};
diff --git a/src/cpu/intel_gpu.cpp b/src/cpu/intel_gpu.cpp
index e0f4ac2..a579e28 100644
--- a/src/cpu/intel_gpu.cpp
+++ b/src/cpu/intel_gpu.cpp
@@ -43,11 +43,15 @@
 void i965_core::measurement_start(void)
 {
     ifstream file;
+    char filename[512];
 
     gettimeofday(&before, NULL);
-    rc6_before =
read_sysfs("/sys/class/drm/card0/power/rc6_residency_ms", NULL);
-    rc6p_before =
read_sysfs("/sys/class/drm/card0/power/rc6p_residency_ms", NULL);
-    rc6pp_before =
read_sysfs("/sys/class/drm/card0/power/rc6pp_residency_ms", NULL);
+    sprintf(filename, "%src6_residency_ms", sysfs_dir);
+    rc6_before = read_sysfs(filename, NULL);
+    sprintf(filename, "%src6p_residency_ms", sysfs_dir);
+    rc6p_before = read_sysfs(filename, NULL);
+    sprintf(filename, "%src6pp_residency_ms", sysfs_dir);
+    rc6pp_before = read_sysfs(filename, NULL);
 
     update_cstate("gpu c0", "Powered On", 0, 0, 1, 0);
     update_cstate("gpu rc6", "RC6", 0, rc6_before, 1, 1);
@@ -101,11 +105,16 @@ char * i965_core::fill_cstate_line(int line_nr,
char *buffer, const char *separa
 
 void i965_core::measurement_end(void)
 {
+    char filename[512];
+
     gettimeofday(&after, NULL);
 
-    rc6_after =
read_sysfs("/sys/class/drm/card0/power/rc6_residency_ms", NULL);
-    rc6p_after =
read_sysfs("/sys/class/drm/card0/power/rc6p_residency_ms", NULL);
-    rc6pp_after =
read_sysfs("/sys/class/drm/card0/power/rc6pp_residency_ms", NULL);
+    sprintf(filename, "%src6_residency_ms", sysfs_dir);
+    rc6_after = read_sysfs(filename, NULL);
+    sprintf(filename, "%src6p_residency_ms", sysfs_dir);
+    rc6p_after = read_sysfs(filename, NULL);
+    sprintf(filename, "%src6pp_residency_ms", sysfs_dir);
+    rc6pp_after = read_sysfs(filename, NULL);
 }
 
 char * i965_core::fill_pstate_line(int line_nr, char *buffer)
-- 
1.8.1.5




Regards,


On 07/04/2013 09:47 AM, Niels Penneman wrote:
> Hi all,
>
> Currently powertop tries to find Intel GPU statistics in sysfs with a
> hardcoded path; in src/cpu/cpu.cpp:353-354:
>
> if (access("/sys/class/drm/card0/power/rc6_residency_ms", R_OK) == 0)
>  handle_i965_gpu();
>
> In systems with multiple GPUs it is not guaranteed that the Intel GPU is
> labeled 'card0' in sysfs. In my system with both integrated Intel and
> discrete AMD Radeon the Intel GPU is 'card1', and powertop will not
> display GPU statistics.
>
> One possible solution would be to scan all /sys/class/drm/card[0-9]+
> folders and check the vendor & device IDs in 'device/vendor' and
> 'device/device'. Perhaps there is a better solution, e.g. by examining
> the PCI device tree.
>
>
> Regards,
>

--
Niels Penneman
Computer Systems Lab
Electronics and Information Systems Department
Ghent University


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 901 bytes --]