On 10 Jul 29, Amit Arora wrote:
> This patch gets the available C-states from cpuidle framework (sysfs) and 
> gets the P-states information from the cpufreq (again, from sysfs). Thus it 
> removes the hardcoded values for c and p states, and makes it generic enough 
> to run on all the systems which plugin to cpuidle and cpufreq kernel 
> framework.
> 
> Signed-off-by: Amit Arora <amit.ar...@linaro.org>
> ---
>  Makefile          |    2 +-
>  cpufreqstats.c    |    3 +-
>  display.c         |   15 ++++++---
>  generic_cstates.c |   63 ++++++++++++++++++++++++++++++++++++++++
>  omapcstates.c     |   64 -----------------------------------------
>  powertop.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  powertop.h        |   23 +++-----------
>  7 files changed, 154 insertions(+), 98 deletions(-)
>  create mode 100644 generic_cstates.c
>  delete mode 100644 omapcstates.c
> 
> diff --git a/Makefile b/Makefile
> index 0b41dc4..d9e4883 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -17,7 +17,7 @@ CC?=gcc
>  
>  OBJS = powertop.o config.o process.o misctips.o bluetooth.o display.o 
> suggestions.o wireless.o cpufreq.o \
>       sata.o xrandr.o ethernet.o cpufreqstats.o usb.o urbnum.o intelcstates.o 
> wifi-new.o perf.o \
> -     alsa-power.o ahci-alpm.o dmesg.o devicepm.o omapcstates.o
> +     alsa-power.o ahci-alpm.o dmesg.o devicepm.o generic_cstates.o
>       
>  
>  powertop: $(OBJS) Makefile powertop.h
> diff --git a/cpufreqstats.c b/cpufreqstats.c
> index b250407..6cd36ee 100644
> --- a/cpufreqstats.c
> +++ b/cpufreqstats.c
> @@ -42,7 +42,7 @@ struct cpufreqdata oldfreqs[16];
>  
>  struct cpufreqdata delta[16];
>  
> -char cpufreqstrings[MAX_NUM_PSTATES+1][80];
> +char **cpufreqstrings;
>  int topfreq = -1;
>  
>  static void zap(void)
> @@ -111,7 +111,6 @@ void  do_cpufreq_stats(void)
>       uint64_t total_time = 0;
>  
>       memcpy(&oldfreqs, &freqs, sizeof(freqs));
> -     memset(&cpufreqstrings, 0, sizeof(cpufreqstrings));
>       sprintf(cpufreqstrings[0], _("P-states (frequencies)\n"));
>  
>       for (ret = 0; ret<16; ret++)
> diff --git a/display.c b/display.c
> index cc81efe..de9146b 100644
> --- a/display.c
> +++ b/display.c
> @@ -91,7 +91,12 @@ int maxwidth = 200;
>  
>  void setup_windows(void)
>  {
> -     int yline = MAX_NUM_CSTATES;
> +     int yline;
> +
> +     if (max_num_cstates > max_num_pstates)
> +             yline = max_num_cstates;
> +     else
> +             yline = max_num_pstates;
>  
>       getmaxyx(stdscr, maxy, maxx);
>  
> @@ -152,7 +157,7 @@ void show_title_bar(void)
>       werase(status_bar_window);
>  
>       x = 0;
> -     for (i=0; i < MAX_CSTATE_LINES; i++) {
> +     for (i=0; i < max_cstate_lines; i++) {
>               if (strlen(status_bar_slots[i])==0)
>                       continue;
>               wattron(status_bar_window, A_REVERSE);
> @@ -168,18 +173,18 @@ void show_cstates(void)
>       int i, count = 0;
>       werase(cstate_window);
>  
> -     for (i=0; i < MAX_CSTATE_LINES; i++) {
> +     for (i=0; i < max_cstate_lines; i++) {
>               if (i == topcstate+1)
>                       wattron(cstate_window, A_BOLD);
>               else
>                       wattroff(cstate_window, A_BOLD);
> -             if (strlen(cstate_lines[i]) && count <= MAX_CSTATE_LINES) {
> +             if (strlen(cstate_lines[i]) && count <= max_cstate_lines) {
>                       print(cstate_window, count, 0, "%s", cstate_lines[i]);
>                       count++;
>               }
>       }
>  
> -     for (i=0; i <= MAX_NUM_PSTATES; i++) {
> +     for (i=0; i <= max_num_pstates; i++) {
>               if (i == topfreq+1)
>                       wattron(cstate_window, A_BOLD);
>               else
> diff --git a/generic_cstates.c b/generic_cstates.c
> new file mode 100644
> index 0000000..dbee3c1
> --- /dev/null
> +++ b/generic_cstates.c
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright 2008, Texas Instruments Incorporated.
> + * Copyright 2010, IBM Corporation
> + *
> + * This file prints the C states supported by any processor.
> + * (Based on intelcstates.c)
> + *
> + * This program file is free software; you can redistribute it and/or modify 
> it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> + * for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program in a file named COPYING; if not, write to the
> + * Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor,
> + * Boston, MA 02110-1301 USA
> + */
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +#include <ctype.h>
> +
> +#include "powertop.h"
> +
> +
> +/**
> + * print_generic_cstates() - Prints the list of supported C-states.
> + *
> + * This functions uses standard sysfs interface of the cpuidle framework
> + * to extract the information of the C-states supported by the Linux
> + * kernel. 
> + **/
> +void print_generic_cstates(void)
> +{
> +     DIR *dir;
> +     struct dirent *entry;
> +
> +     dir = opendir("/sys/devices/system/cpu/cpu0/cpuidle");

Is this safe for multi-core cases? Is it safe to assume that the c-states on
all cores will be the same?

Also, I see that powertop shows "Avg. residency" in each C-state. Is that an
average of the the residency of each core in a particular C-state?

> +
> +     if (dir) {
> +             printf(_("Supported C-states : "));
> +
> +             while ((entry = readdir(dir))) {
> +                     if (strlen(entry->d_name) < 3)
> +                             continue;
> +
> +                     printf("C%s ", entry->d_name);
> +             }
> +             printf("\n");
> +
> +             closedir(dir);
> +     }
> +}
> diff --git a/omapcstates.c b/omapcstates.c
> deleted file mode 100644
> index 9d1da4f..0000000
> --- a/omapcstates.c
> +++ /dev/null
> @@ -1,64 +0,0 @@
> -/*
> - * Copyright 2008, Texas Instruments Incorporated.
> - *
> - * This file prints the C states supported by the OMAP processor.
> - * (Based on intelcstates.c)
> - *
> - * This program file is free software; you can redistribute it and/or modify 
> it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; version 2 of the License.
> - *
> - * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> - * for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program in a file named COPYING; if not, write to the
> - * Free Software Foundation, Inc.,
> - * 51 Franklin Street, Fifth Floor,
> - * Boston, MA 02110-1301 USA
> - */
> -
> -#include <unistd.h>
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <stdint.h>
> -#include <sys/types.h>
> -#include <dirent.h>
> -#include <ctype.h>
> -
> -#include "powertop.h"
> -
> -
> -#if defined(OMAP3)
> -/**
> - * print_omap3_cstates() - Prints the list of supported C-states.
> - *
> - * This functions uses standard sysfs interface of the cpuidle framework
> - * to extract the information of the C-states supported by the Linux
> - * kernel. 
> - **/
> -void print_omap3_cstates(void)
> -{
> -     DIR *dir;
> -     struct dirent *entry;
> -
> -     dir = opendir("/sys/devices/system/cpu/cpu0/cpuidle");
> -
> -     if (dir) {
> -             printf(_("Supported C-states : "));
> -
> -             while ((entry = readdir(dir))) {
> -                     if (strlen(entry->d_name) < 3)
> -                             continue;
> -
> -                     printf("C%s ", entry->d_name);
> -             }
> -             printf("\n");
> -
> -             closedir(dir);
> -     }
> -}
> -#endif
> diff --git a/powertop.c b/powertop.c
> index 7a32623..6fd3efe 100644
> --- a/powertop.c
> +++ b/powertop.c
> @@ -40,6 +40,7 @@
>  
>  #include "powertop.h"
>  
> +extern void print_generic_cstates(void);
>  
>  uint64_t start_usage[8], start_duration[8];
>  uint64_t last_usage[8], last_duration[8];
> @@ -85,6 +86,70 @@ time_t prev_bat_time = 0;
>  
>  double displaytime = 0.0;
>  
> +char **cstate_lines;
> +
> +void init_numstates(void)
> +{
> +     DIR *dir;
> +     FILE *fp;
> +     struct dirent *entry;
> +     int i, count = 0;
> +     char line[1024];
> +
> +     dir = opendir("/sys/devices/system/cpu/cpu0/cpuidle");

Ditto.

> +     if (dir) {
> +             while ((entry = readdir(dir))) {
> +                     if (strlen(entry->d_name) < 3)
> +                             continue;
> +                     count++;
> +             }
> +             closedir(dir);
> +     }
> +     max_num_cstates = count;
> +     count = 0;
> +
> +     fp = fopen("/sys/devices/system/cpu/cpu0/cpufreq/stats/time_in_state", 
> +                     "r");
> +     if (fp) {
> +             while (fgets(line, sizeof(line), fp)!=NULL)
> +                     count++;
> +             fclose(fp);
> +     }
> +     max_num_pstates = count;
> +
> +     if (max_num_cstates > max_num_pstates)
> +             max_cstate_lines = max_num_cstates + 3;
> +     else
> +             max_cstate_lines = max_num_pstates + 3;
> +
> +     /* Now, allocate memory */
> +
> +     cpufreqstrings = (char **)calloc((max_num_pstates+1), sizeof(char *));
> +     assert(cpufreqstrings!=0);
> +     for (i=0; i<max_num_pstates+1; i++) {
> +             cpufreqstrings[i] = (char *)calloc(80, sizeof(char));
> +             assert(cpufreqstrings[i]!=0);
> +     }
> +
> +     cstate_lines = (char **)calloc(max_cstate_lines, sizeof(char *));
> +     assert(cstate_lines!=0);
> +     for (i=0; i<max_cstate_lines; i++) {
> +             cstate_lines[i] = (char *)calloc(200, sizeof(char));
> +             assert(cstate_lines[i]!=0);
> +     }
> +
> +}
> +
> +void free_numstates(void)
> +{
> +     int i;
> +
> +     for (i=0; i<max_num_pstates+1; i++)
> +             free(cpufreqstrings[i]);
> +     free(cpufreqstrings);
> +     cpufreqstrings = NULL;
> +}
> +
>  void push_line(char *string, int count)
>  {
>       int i;
> @@ -824,8 +889,6 @@ void print_battery_sysfs(void)
>       show_acpi_power_line(rate, cap, prev_bat_cap - cap, time(NULL) - 
> prev_bat_time);
>  }
>  
> -char cstate_lines[MAX_CSTATE_LINES][200];
> -
>  void usage()
>  {
>       printf(_("Usage: powertop [OPTION...]\n"));
> @@ -902,6 +965,8 @@ int main(int argc, char **argv)
>       memcpy(last_usage, start_usage, sizeof(last_usage));
>       memcpy(last_duration, start_duration, sizeof(last_duration));
>  
> +     init_numstates();
> +
>       do_proc_irq();
>       do_proc_irq();
>       do_cpufreq_stats();
> @@ -924,8 +989,8 @@ int main(int argc, char **argv)
>       printf("\n\n");
>  #if defined (__I386__)
>       print_intel_cstates();

If we now have print_generic_cstates (below), then can't we get rid of the
#ifdef'ery for I386 vs. others?

> -#elif defined (OMAP3)
> -     print_omap3_cstates();
> +#else 
> +     print_generic_cstates();
>  #endif
>       stop_timerstats();
>  
> @@ -963,7 +1028,7 @@ int main(int argc, char **argv)
>  
>               totalticks = 0;
>               totalevents = 0;
> -             for (i = 0; i < MAX_NUM_CSTATES; i++)
> +             for (i = 0; i < max_num_cstates; i++)
>                       if (cur_usage[i]) {
>                               totalticks += cur_duration[i] - 
> last_duration[i];
>                               totalevents += cur_usage[i] - last_usage[i];
> @@ -978,8 +1043,7 @@ int main(int argc, char **argv)
>                       show_title_bar();
>               }
>  
> -             memset(&cstate_lines, 0, sizeof(cstate_lines));
> -             topcstate = -(MAX_NUM_CSTATES);
> +             topcstate = -(max_num_cstates);
>               if (totalevents == 0 && maxcstate <= 1) {
>                       sprintf(cstate_lines[5],_("< Detailed C-state 
> information is not available.>\n"));
>               } else {
> @@ -993,7 +1057,7 @@ int main(int argc, char **argv)
>                       sprintf(cstate_lines[1], _("C0 (cpu running)        
> (%4.1f%%)\n"), percentage);
>                       if (percentage > 50)
>                               topcstate = 0;
> -                     for (i = 0; i < MAX_NUM_CSTATES; i++)
> +                     for (i = 0; i < max_num_cstates; i++)
>                               if (cur_usage[i]) {
>                                       sleept = (cur_duration[i] - 
> last_duration[i]) / (cur_usage[i] - last_usage[i]
>                                                                               
>         + 0.1) / FREQ;
> @@ -1300,5 +1364,7 @@ int main(int argc, char **argv)
>       }
>  
>       end_data_dirty_capture();
> +
> +     free_numstates();
>       return 0;
>  }
> diff --git a/powertop.h b/powertop.h
> index 21f5538..4017cfe 100644
> --- a/powertop.h
> +++ b/powertop.h
> @@ -30,19 +30,9 @@
>  
>  #define VERSION "1.12"
>  
> -#if defined(__i386__)
> -#define MAX_NUM_CSTATES 4
> -#define MAX_NUM_PSTATES 5
> -
> -#elif defined(OMAP3)
> -#define MAX_NUM_CSTATES 7
> -#define MAX_NUM_PSTATES 5
> -
> -#else
> -#error "No valid architecture is defined."
> -#endif
> -
> -#define MAX_CSTATE_LINES (MAX_NUM_CSTATES + 3)
> +int max_num_cstates;
> +int max_num_pstates;
> +int max_cstate_lines;
>  
>  struct line {
>       char    *string;
> @@ -83,11 +73,8 @@ void usb_activity_hint(void);
>  void devicepm_activity_hint(void);
>  
>  
> -
> -
> -
> -extern char cstate_lines[MAX_CSTATE_LINES][200];
> -extern char cpufreqstrings[MAX_NUM_PSTATES+1][80];
> +extern char **cpufreqstrings;
> +extern char **cstate_lines;
>  
>  extern int topcstate;
>  extern int topfreq;
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to