Background: Currently, PowerTOP has hard coded values for the number of C and P states supported. Hence it doesn't work well on systems which may have more states than these hard coded ones. This is specially true for some of the ARM SoCs which may have as many as 8 to 9 C-states.
Suggested fix: This patch defines MAX values of C and P states for allocating enough memory for data structures statically. But, uses actual values of the MAX C and P states for display (using, maxcstate and maxpstate variables). Testing done: This patch has been tested on a few x86 desktops, x86_64 servers, a laptop and a couple of ARM boards (OMAP3 and imx53). Signed-off-by: Amit Arora <[email protected]> --- cpufreqstats.c | 15 ++++++++------- display.c | 24 +++++++++++++++++------- powertop.c | 56 +++++++++++++++++++++++++++++++++++--------------------- powertop.h | 16 ++++++++++++++-- 4 files changed, 74 insertions(+), 37 deletions(-) diff --git a/cpufreqstats.c b/cpufreqstats.c index d10b047..6e948cd 100644 --- a/cpufreqstats.c +++ b/cpufreqstats.c @@ -37,12 +37,12 @@ struct cpufreqdata { uint64_t count; }; -struct cpufreqdata freqs[16]; -struct cpufreqdata oldfreqs[16]; +struct cpufreqdata freqs[MAX_NUM_PSTATES]; +struct cpufreqdata oldfreqs[MAX_NUM_PSTATES]; -struct cpufreqdata delta[16]; +struct cpufreqdata delta[MAX_NUM_PSTATES]; -char cpufreqstrings[6][80]; +char cpufreqstrings[MAX_NUM_PSTATES+1][80]; int topfreq = -1; static void zap(void) @@ -114,7 +114,7 @@ void do_cpufreq_stats(void) memset(&cpufreqstrings, 0, sizeof(cpufreqstrings)); sprintf(cpufreqstrings[0], _("P-states (frequencies)\n")); - for (ret = 0; ret<16; ret++) + for (ret = 0; ret < MAX_NUM_PSTATES; ret++) freqs[ret].count = 0; dir = opendir("/sys/devices/system/cpu"); @@ -153,7 +153,7 @@ void do_cpufreq_stats(void) if (f && maxfreq < i) maxfreq = i; i++; - if (i>15) + if (i >= MAX_NUM_PSTATES) break; } fclose(file); @@ -161,7 +161,7 @@ void do_cpufreq_stats(void) closedir(dir); - for (ret = 0; ret < 16; ret++) { + for (ret = 0; ret < MAX_NUM_PSTATES; ret++) { delta[ret].count = freqs[ret].count - oldfreqs[ret].count; total_time += delta[ret].count; delta[ret].frequency = freqs[ret].frequency; @@ -176,6 +176,7 @@ void do_cpufreq_stats(void) qsort(&delta, maxfreq+1, sizeof(struct cpufreqdata), sort_by_count); if (maxfreq>4) maxfreq=4; + maxpstate = maxfreq; qsort(&delta, maxfreq+1, sizeof(struct cpufreqdata), sort_by_freq); if (delta[0].frequency == delta[1].frequency + 1000) diff --git a/display.c b/display.c index 79732ab..df23622 100644 --- a/display.c +++ b/display.c @@ -91,15 +91,20 @@ int maxwidth = 200; void setup_windows(void) { + int yline = (maxcstate >= maxpstate) ? maxcstate+1 : maxpstate; + + /* number of states is one more than the MAX state! */ + yline++; + getmaxyx(stdscr, maxy, maxx); zap_windows(); title_bar_window = subwin(stdscr, 1, maxx, 0, 0); - cstate_window = subwin(stdscr, 7, maxx, 2, 0); - wakeup_window = subwin(stdscr, 1, maxx, 9, 0); - battery_power_window = subwin(stdscr, 2, maxx, 10, 0); - timerstat_window = subwin(stdscr, maxy-16, maxx, 12, 0); + cstate_window = subwin(stdscr, (yline + 3), maxx, 2, 0); + wakeup_window = subwin(stdscr, 1, maxx, (yline + 4), 0); + battery_power_window = subwin(stdscr, 2, maxx, (yline + 5), 0); + timerstat_window = subwin(stdscr, maxy-16, maxx, (yline + 7), 0); maxtimerstats = maxy-16 -2; maxwidth = maxx - 18; suggestion_window = subwin(stdscr, 3, maxx, maxy-4, 0); @@ -164,20 +169,25 @@ void show_title_bar(void) void show_cstates(void) { int i, count = 0; + int maxcstatelines = (maxcstate >= maxpstate) ? maxcstate+1 : maxpstate; + + /* Number of states is one more than the MAX state! */ + maxcstatelines++; + werase(cstate_window); - for (i=0; i < 10; i++) { + for (i = 0; i <= maxcstatelines; i++) { if (i == topcstate+1) wattron(cstate_window, A_BOLD); else wattroff(cstate_window, A_BOLD); - if (strlen(cstate_lines[i]) && count <= 6) { + if (strlen(cstate_lines[i]) && count <= maxcstatelines) { print(cstate_window, count, 0, "%s", cstate_lines[i]); count++; } } - for (i=0; i<6; i++) { + for (i = 0; i <= (maxpstate+1); i++) { if (i == topfreq+1) wattron(cstate_window, A_BOLD); else diff --git a/powertop.c b/powertop.c index 74eb328..16e8ab0 100644 --- a/powertop.c +++ b/powertop.c @@ -41,9 +41,9 @@ #include "powertop.h" -uint64_t start_usage[8], start_duration[8]; -uint64_t last_usage[8], last_duration[8]; -char cnames[8][16]; +uint64_t start_usage[MAX_NUM_CSTATES], start_duration[MAX_NUM_CSTATES]; +uint64_t last_usage[MAX_NUM_CSTATES], last_duration[MAX_NUM_CSTATES]; +char cnames[MAX_NUM_CSTATES][16]; double ticktime = 15.0; @@ -51,7 +51,8 @@ int interrupt_0, total_interrupt; int showpids = 0; -static int maxcstate = 0; +int maxcstate; +int maxpstate; int topcstate = 0; int dump = 0; @@ -450,6 +451,10 @@ static void read_data_cpuidle(uint64_t * usage, uint64_t * duration) sprintf(cnames[clevel], "%s\t", "C1 halt"); } } + + if (clevel > maxcstate) + maxcstate = clevel; + sprintf(filename + len, "/%s/usage", entry->d_name); file = fopen(filename, "r"); if (!file) @@ -477,9 +482,6 @@ static void read_data_cpuidle(uint64_t * usage, uint64_t * duration) duration[clevel] += 1+strtoull(line, NULL, 10); clevel++; - if (clevel > maxcstate) - maxcstate = clevel; - } closedir(dir); @@ -824,7 +826,7 @@ void print_battery_sysfs(void) show_acpi_power_line(rate, cap, prev_bat_cap - cap, time(NULL) - prev_bat_time); } -char cstate_lines[12][200]; +char cstate_lines[MAX_CSTATE_LINES][200]; void usage() { @@ -848,7 +850,7 @@ int main(int argc, char **argv) char line[1024]; int ncursesinited=0; FILE *file = NULL; - uint64_t cur_usage[8], cur_duration[8]; + uint64_t cur_usage[MAX_NUM_CSTATES], cur_duration[MAX_NUM_CSTATES]; double wakeups_per_second = 0; setlocale (LC_ALL, ""); @@ -957,25 +959,22 @@ int main(int argc, char **argv) do_proc_irq(); read_data(&cur_usage[0], &cur_duration[0]); + if (maxcstate >= MAX_NUM_CSTATES) { + printf("Actual CSTATES (%d) > MAX_NUM_CSTATES (%d)!\n", + maxcstate+1, MAX_NUM_CSTATES); + exit(1); + } + totalticks = 0; totalevents = 0; - for (i = 0; i < 8; i++) + for (i = 0; i <= maxcstate; i++) if (cur_usage[i]) { totalticks += cur_duration[i] - last_duration[i]; totalevents += cur_usage[i] - last_usage[i]; } - if (!dump) { - if (!ncursesinited) { - initialize_curses(); - ncursesinited++; - } - setup_windows(); - show_title_bar(); - } - memset(&cstate_lines, 0, sizeof(cstate_lines)); - topcstate = -4; + topcstate = -(maxcstate); if (totalevents == 0 && maxcstate <= 1) { sprintf(cstate_lines[5],_("< Detailed C-state information is not available.>\n")); } else { @@ -989,7 +988,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 < 8; i++) + for (i = 0; i <= maxcstate; i++) if (cur_usage[i]) { sleept = (cur_duration[i] - last_duration[i]) / (cur_usage[i] - last_usage[i] + 0.1) / FREQ; @@ -1010,6 +1009,21 @@ int main(int argc, char **argv) } } do_cpufreq_stats(); + + if (maxpstate > MAX_NUM_PSTATES) { + printf("Actual PSTATES (%d) > MAX_NUM_PSTATES (%d)!\n", + maxpstate, MAX_NUM_PSTATES); + exit(1); + } + + if (!dump) { + if (!ncursesinited) { + initialize_curses(); + ncursesinited++; + } + setup_windows(); + show_title_bar(); + } show_cstates(); /* now the timer_stats info */ memset(line, 0, sizeof(line)); diff --git a/powertop.h b/powertop.h index d8f8182..22814d5 100644 --- a/powertop.h +++ b/powertop.h @@ -28,6 +28,16 @@ #include <libintl.h> +/* + * Define the MAX values for [CP]STATES "big enough". + * Defining it too less will result in missing some C/P state information. + * Defining the values too high will result in memory wastage. + */ +#define MAX_NUM_CSTATES 10 +#define MAX_NUM_PSTATES 16 + +#define MAX_CSTATE_LINES (MAX_NUM_CSTATES + 3) + struct line { char *string; int count; @@ -41,6 +51,8 @@ extern struct line *lines; extern int linehead; extern int linesize; extern int linectotal; +extern int maxcstate; +extern int maxpstate; extern double displaytime; @@ -70,8 +82,8 @@ void devicepm_activity_hint(void); -extern char cstate_lines[12][200]; -extern char cpufreqstrings[6][80]; +extern char cstate_lines[MAX_CSTATE_LINES][200]; +extern char cpufreqstrings[MAX_NUM_PSTATES+1][80]; extern int topcstate; extern int topfreq; -- 1.7.0.4 _______________________________________________ Power mailing list [email protected] http://www.bughost.org/mailman/listinfo/power
