Re: [PATCH 2/2] Remove hardcoded c and p states

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.arora@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