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

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

[Niels Penneman]

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

Updated patch below.


From: Niels Penneman <niels.penneman(a)elis.ugent.be>
Date: Mon, 8 Jul 2013 14:53:23 +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..4d61a71 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(int sysfs_num)
 {
     class abstract_cpu *ret = NULL;

     ret = new class i965_core;
     ret->childcount = 0;
     ret->set_type("GPU");
+    static_cast<class i965_core *>(ret)->sysfs_num = sysfs_num;

     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(int gpu)
 {
     unsigned int core_number = 0;
     class abstract_cpu *package;
@@ -274,11 +277,55 @@ 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 int probe_i965_gpu()
+{
+    DIR *dir;
+    struct dirent *entry;
+    int gpu = -1;
+
+    dir = opendir("/sys/class/drm");
+    if (!dir)
+        return false;
+    while ((entry = readdir(dir))) {
+        ifstream file;
+        char filename[PATH_MAX];
+        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) {
+            gpu = atoi(&entry->d_name[4]);
+            break;
+        }
+    }
+    closedir(dir);
+    return gpu;
+}
+

 void enumerate_cpus(void)
 {
@@ -290,6 +337,8 @@ void enumerate_cpus(void)
     int family = 0;
     int model = 0;

+    int gpu;
+
     file.open("/proc/cpuinfo",  ios::in);

     if (!file)
@@ -350,8 +399,9 @@ void enumerate_cpus(void)

     file.close();

-    if (access("/sys/class/drm/card0/power/rc6_residency_ms", R_OK) == 0)
-        handle_i965_gpu();
+    gpu = probe_i965_gpu();
+    if (gpu >= 0)
+        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..af70bde 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -137,6 +137,8 @@ private:
     struct timeval    after;

 public:
+    int sysfs_num;
+
     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..1e4b228 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[PATH_MAX];

     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, "/sys/class/drm/card%d/power/rc6_residency_ms",
sysfs_num);
+    rc6_before = read_sysfs(filename, NULL);
+    sprintf(filename, "/sys/class/drm/card%d/power/rc6p_residency_ms",
sysfs_num);
+    rc6p_before = read_sysfs(filename, NULL);
+    sprintf(filename, "/sys/class/drm/card%d/power/rc6pp_residency_ms",
sysfs_num);
+    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, "/sys/class/drm/card%d/power/rc6_residency_ms",
sysfs_num);
+    rc6_after = read_sysfs(filename, NULL);
+    sprintf(filename, "/sys/class/drm/card%d/power/rc6p_residency_ms",
sysfs_num);
+    rc6p_after = read_sysfs(filename, NULL);
+    sprintf(filename, "/sys/class/drm/card%d/power/rc6pp_residency_ms",
sysfs_num);
+    rc6pp_after = read_sysfs(filename, NULL);
 }

 char * i965_core::fill_pstate_line(int line_nr, char *buffer)
-- 
1.8.1.5




On Mon, Jul 8, 2013 at 12:39 PM, Sergey Senozhatsky <
sergey.senozhatsky(a)gmail.com> wrote:

> On (07/08/13 14:30), Niels Penneman wrote:
> [..]
> > >> +
> > > do we really need to store 512 bytes instead of 4? as a public member?
> > >
> > > btw, why 512?
> >
> > Indeed, we can store GPU number instead. I looked at how paths were
> > stored throughout powertop and found buffer sizes of 256 for filenames,
> > so I doubled it to 512 when adding '/sys/class/drm' etc. GPU number is
> > better though, I'll edit the patch.
> >
>
> thanks.
>
>
> [..]
> > >> +    char filename[512];
> > >>
> > > 4K
> > Did you mean buffer size should be 4k ? It would be helpful if powertop
> > sources actually had constants for this.
> >
>
> better to use standard ones.
>
>
> linux/limits.h
> [..]
> #define NAME_MAX         255    /* # chars in a file name */
> #define PATH_MAX        4096    /* # chars in a path name including nul
> [..]
>
>         -ss
>



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

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 8602 bytes --]

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

[Niels Penneman]

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

I've got some questions -- they are also inline.

On 07/08/2013 12:09 PM, Sergey Senozhatsky wrote:
> On (07/04/13 20:33), Niels Penneman wrote:
>> 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.
>>
> thanks for your patch. I took a quick look, please see inlined.
>
>> 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];
>> +
> do we really need to store 512 bytes instead of 4? as a public member?
>
> btw, why 512?

Indeed, we can store GPU number instead. I looked at how paths were
stored throughout powertop and found buffer sizes of 256 for filenames,
so I doubled it to 512 when adding '/sys/class/drm' etc. GPU number is
better though, I'll edit the patch.

>
>
>>      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];
>>  
> 4K
Did you mean buffer size should be 4k ? It would be helpful if powertop
sources actually had constants for this.
>
>>      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);
> or we can sprintf gpu number?
>
> 	/sys/class/drm/card%d/power/rc6_residency_ms
>
>
>>      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);
>>  }
>>
> same as above.
>
>
> 	-ss
>
>>  
>>  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
>>
>
>
>> _______________________________________________
>> PowerTop mailing list
>> PowerTop(a)lists.01.org
>> https://lists.01.org/mailman/listinfo/powertop

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

[Sergey Senozhatsky]

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

On (07/08/13 14:30), Niels Penneman wrote:
[..]
> >> +
> > do we really need to store 512 bytes instead of 4? as a public member?
> >
> > btw, why 512?
> 
> Indeed, we can store GPU number instead. I looked at how paths were
> stored throughout powertop and found buffer sizes of 256 for filenames,
> so I doubled it to 512 when adding '/sys/class/drm' etc. GPU number is
> better though, I'll edit the patch.
>

thanks.


[..]
> >> +    char filename[512];
> >>  
> > 4K
> Did you mean buffer size should be 4k ? It would be helpful if powertop
> sources actually had constants for this.
>

better to use standard ones.


linux/limits.h
[..]
#define NAME_MAX         255    /* # chars in a file name */
#define PATH_MAX        4096    /* # chars in a path name including nul
[..]

	-ss

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

[Sergey Senozhatsky]

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

On (07/04/13 20:33), Niels Penneman wrote:
> 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.
> 

thanks for your patch. I took a quick look, please see inlined.

> 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];
> +

do we really need to store 512 bytes instead of 4? as a public member?

btw, why 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];
>  

4K


>      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);

or we can sprintf gpu number?

	/sys/class/drm/card%d/power/rc6_residency_ms


>      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);
>  }
>

same as above.


	-ss

>  
>  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
> 



> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop

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

[Sergey Senozhatsky]

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

On (07/04/13 20:33), Niels Penneman wrote:
> 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.
>

Hello,
Thanks, I'll take a look in a day or two.

	-ss


> 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
> 



> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop

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

[Niels Penneman]

[-- 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 --]

[Powertop] Intel GPU statistics on multi-GPU systems

[Niels Penneman]

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

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 --]