[PATCH] refactor w_display_cpu_Xstates()

[Sergey Senozhatsky]

Currently we have w_display_cpu_pstates() and w_display_cpu_cstates(),
which are 99% similar (big) functions with the only difference in
abstract_cpu calls: pstate vs. state.

This patch introduces helper functions:
- has_state_level() -- call has_pstate_level() or has_cstate_level()
- fill_state_name() -- call fill_pstate_name() or fill_cstate_name()
- fill_state_line() -- call fill_pstate_line() or fill_pstate_line()

So implementation of w_display_cpu_pstates() and w_display_cpu_cstates()
could be merged into one function, since those functions doesn't know anymore
the difference between pstate and state, which is helper function's
duty.

We still have w_display_cpu_pstates() and w_display_cpu_cstates(), but
the only thing they do is calling
impl_w_display_cpu_states(PSTATE) or impl_w_display_cpu_states(CSTATE)
accordingly.

The same could and should be done with gigantic report_display_cpu_cstates()
and report_display_cpu_pstates(). We already have needed helper functions,
the only problem could be with some differences in output, e.g. freq_class()
and package_class() calls, which we could overcome with PSTATE/CSTATE flags
in order to remove massive code duplicate.
(perhaps I'll try to do it after some sleep).


Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky-***@public.gmane.org>

---

cpu/cpu.cpp | 172 +++++++++++++++++++++--------------------------------------
cpu/cpu.h | 3 +
2 files changed, 64 insertions(+), 111 deletions(-)

diff --git a/cpu/cpu.cpp b/cpu/cpu.cpp
index dc4fd03..6357b78 100644
--- a/cpu/cpu.cpp
+++ b/cpu/cpu.cpp
@@ -336,6 +336,46 @@ static const char *freq_class(int line)
return "cpu_even_req";
}

+static int has_state_level(class abstract_cpu *acpu, int state, int line)
+{
+ switch (state) {
+ case PSTATE:
+ return acpu->has_pstate_level(line);
+ break;
+ case CSTATE:
+ return acpu->has_cstate_level(line);
+ break;
+ }
+ return 0;
+}
+
+static const char * fill_state_name(class abstract_cpu *acpu, int state, int line, char *buf)
+{
+ switch (state) {
+ case PSTATE:
+ return acpu->fill_pstate_name(line, buf);
+ break;
+ case CSTATE:
+ return acpu->fill_cstate_name(line, buf);
+ break;
+ }
+ return "-EINVAL";
+}
+
+static const char * fill_state_line(class abstract_cpu *acpu, int state, int line,
+ char *buf, const char *sep = "")
+{
+ switch (state) {
+ case PSTATE:
+ return acpu->fill_pstate_line(line, buf);
+ break;
+ case CSTATE:
+ return acpu->fill_cstate_line(line, buf, sep);
+ break;
+ }
+ return "-EINVAL";
+}
+
void report_display_cpu_cstates(void)
{
char buffer[512], buffer2[512];
@@ -535,100 +575,6 @@ void report_display_cpu_cstates(void)
}
}

-void w_display_cpu_cstates(void)
-{
-#ifndef DISABLE_NCURSES
- WINDOW *win;
- char buffer[128];
- char linebuf[1024];
- unsigned int package, core, cpu;
- int line;
- class abstract_cpu *_package, * _core, * _cpu;
- int ctr = 0;
-
- win = get_ncurses_win("Idle stats");
- if (!win)
- return;
-
- wclear(win);
- wmove(win, 2,0);
-
- for (package = 0; package < system_level.children.size(); package++) {
- int first_pkg = 0;
- _package = system_level.children[package];
- if (!_package)
- continue;
-
-
- for (core = 0; core < _package->children.size(); core++) {
- _core = _package->children[core];
- if (!_core)
- continue;
-
- for (line = LEVEL_HEADER; line < 20; line++) {
- int first = 1;
- ctr = 0;
- linebuf[0] = 0;
- if (!_package->has_cstate_level(line))
- continue;
-
-
- buffer[0] = 0;
- if (first_pkg == 0) {
- strcat(linebuf, _package->fill_cstate_name(line, buffer));
- expand_string(linebuf, ctr+10);
- strcat(linebuf, _package->fill_cstate_line(line, buffer));
- }
- ctr += 20;
- expand_string(linebuf, ctr);
-
- strcat(linebuf, "| ");
- ctr += strlen("| ");
-
- if (!_core->can_collapse()) {
- buffer[0] = 0;
- strcat(linebuf, _core->fill_cstate_name(line, buffer));
- expand_string(linebuf, ctr + 10);
- strcat(linebuf, _core->fill_cstate_line(line, buffer));
- ctr += 20;
- expand_string(linebuf, ctr);
-
- strcat(linebuf, "| ");
- ctr += strlen("| ");
- }
-
- for (cpu = 0; cpu < _core->children.size(); cpu++) {
- _cpu = _core->children[cpu];
- if (!_cpu)
- continue;
-
- if (first == 1) {
- strcat(linebuf, _cpu->fill_cstate_name(line, buffer));
- expand_string(linebuf, ctr + 10);
- first = 0;
- ctr += 12;
- }
- buffer[0] = 0;
- strcat(linebuf, _cpu->fill_cstate_line(line, buffer));
- ctr += 18;
- expand_string(linebuf, ctr);
-
- }
- strcat(linebuf, "\n");
- wprintw(win, "%s", linebuf);
- }
-
- wprintw(win, "\n");
- first_pkg++;
- }
-
-
- }
-#endif // DISABLE_NCURSES
-}
-
-
-
void report_display_cpu_pstates(void)
{
char buffer[512], buffer2[512];
@@ -823,9 +769,7 @@ void report_display_cpu_pstates(void)
fprintf(reportout.csv_report, "\n");
}

-
-
-void w_display_cpu_pstates(void)
+void impl_w_display_cpu_states(int state)
{
#ifndef DISABLE_NCURSES
WINDOW *win;
@@ -836,21 +780,23 @@ void w_display_cpu_pstates(void)
class abstract_cpu *_package, * _core, * _cpu;
int ctr = 0;

- win = get_ncurses_win("Frequency stats");
+ if (state == PSTATE)
+ win = get_ncurses_win("Frequency stats");
+ else
+ win = get_ncurses_win("Idle stats");
+
if (!win)
return;

wclear(win);
wmove(win, 2,0);

-
for (package = 0; package < system_level.children.size(); package++) {
int first_pkg = 0;
_package = system_level.children[package];
if (!_package)
continue;

-
for (core = 0; core < _package->children.size(); core++) {
_core = _package->children[core];
if (!_core)
@@ -861,15 +807,14 @@ void w_display_cpu_pstates(void)
ctr = 0;
linebuf[0] = 0;

- if (!_package->has_pstate_level(line))
+ if (!has_state_level(_package, state, line))
continue;

-
buffer[0] = 0;
if (first_pkg == 0) {
- strcat(linebuf, _package->fill_pstate_name(line, buffer));
+ strcat(linebuf, fill_state_name(_package, state, line, buffer));
expand_string(linebuf, ctr + 10);
- strcat(linebuf, _package->fill_pstate_line(line, buffer));
+ strcat(linebuf, fill_state_line(_package, state, line, buffer));
}
ctr += 20;
expand_string(linebuf, ctr);
@@ -879,9 +824,9 @@ void w_display_cpu_pstates(void)

if (!_core->can_collapse()) {
buffer[0] = 0;
- strcat(linebuf, _core->fill_pstate_name(line, buffer));
+ strcat(linebuf, fill_state_name(_core, state, line, buffer));
expand_string(linebuf, ctr + 10);
- strcat(linebuf, _core->fill_pstate_line(line, buffer));
+ strcat(linebuf, fill_state_line(_core, state, line, buffer));
ctr += 20;
expand_string(linebuf, ctr);

@@ -895,13 +840,13 @@ void w_display_cpu_pstates(void)
continue;

if (first == 1) {
- strcat(linebuf, _cpu->fill_pstate_name(line, buffer));
+ strcat(linebuf, fill_state_name(_cpu, state, line, buffer));
expand_string(linebuf, ctr + 10);
first = 0;
ctr += 12;
}
buffer[0] = 0;
- strcat(linebuf, _cpu->fill_pstate_line(line, buffer));
+ strcat(linebuf, fill_state_line(_cpu, state, line, buffer));
ctr += 10;
expand_string(linebuf, ctr);

@@ -909,17 +854,22 @@ void w_display_cpu_pstates(void)
strcat(linebuf, "\n");
wprintw(win, "%s", linebuf);
}
-
wprintw(win, "\n");
first_pkg++;
}
-
-
}
#endif // DISABLE_NCURSES
}

+void w_display_cpu_pstates(void)
+{
+ impl_w_display_cpu_states(PSTATE);
+}

+void w_display_cpu_cstates(void)
+{
+ impl_w_display_cpu_states(CSTATE);
+}

struct power_entry {
#ifndef __i386__
diff --git a/cpu/cpu.h b/cpu/cpu.h
index d59dbb3..ebb5e3d 100644
--- a/cpu/cpu.h
+++ b/cpu/cpu.h
@@ -39,6 +39,9 @@ class abstract_cpu;
#define LEVEL_C0 -1
#define LEVEL_HEADER -2

+#define PSTATE 1
+#define CSTATE 2
+
struct idle_state {
char linux_name[16]; /* state0 etc.. cpuidle name */
char human_name[32];