Hi,
This patch set includes a few build system improvements and a few UI fixes.
The last patch is ugly, and personally, I don't care if it's included. But it's
an attempt to kill a warning. Better suggestions are welcome.
Thanks,
Brian
Brian Norris (8):
tools/thermal: tmon: add --target-temp parameter
tools/thermal: tmon: add min/max macros
tools/thermal: tmon: tui: don't hard-code dialog window size
assumptions
tools/thermal: tmon: fixup tui windowing calculations
tools/thermal: tmon: add .gitignore
tools/thermal: tmon: support cross-compiling
tools/thermal: tmon: use pkg-config to determine library dependencies
tools/thermal: tmon: silence 'set but not used' warnings
tools/thermal/tmon/.gitignore | 1 +
tools/thermal/tmon/Makefile | 15 ++++++++++++---
tools/thermal/tmon/tmon.8 | 2 ++
tools/thermal/tmon/tmon.c | 14 ++++++++++++--
tools/thermal/tmon/tui.c | 45 ++++++++++++++++++++++++++++++++++---------
5 files changed, 63 insertions(+), 14 deletions(-)
create mode 100644 tools/thermal/tmon/.gitignore
--
1.9.1
If we launch in daemon mode (--daemon), we don't have the ncurses UI,
but we might want to set the target temperature still. For example,
someone might stick the following in their boot script:
tmon --control intel_powerclamp --target-temp 90 --log --daemon
This would turn on CPU idle injection when we're around 90 degrees
celsius, and would log temperature and throttling info to
/var/tmp/tmon.log.
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/tmon.8 | 2 ++
tools/thermal/tmon/tmon.c | 14 ++++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/thermal/tmon/tmon.8 b/tools/thermal/tmon/tmon.8
index 0be727cb9892..02d5179803aa 100644
--- a/tools/thermal/tmon/tmon.8
+++ b/tools/thermal/tmon/tmon.8
@@ -55,6 +55,8 @@ The \fB-l --log\fP option write data to /var/tmp/tmon.log
.PP
The \fB-t --time-interval\fP option sets the polling interval in seconds
.PP
+The \fB-T --target-temp\fP option sets the initial target temperature
+.PP
The \fB-v --version\fP option shows the version of \fBtmon \fP
.PP
The \fB-z --zone\fP option sets the target therma zone instance to be controlled
diff --git a/tools/thermal/tmon/tmon.c b/tools/thermal/tmon/tmon.c
index 09b7c3218334..9aa19652e8e8 100644
--- a/tools/thermal/tmon/tmon.c
+++ b/tools/thermal/tmon/tmon.c
@@ -64,6 +64,7 @@ void usage()
printf(" -h, --help show this help message\n");
printf(" -l, --log log data to /var/tmp/tmon.log\n");
printf(" -t, --time-interval sampling time interval, > 1 sec.\n");
+ printf(" -T, --target-temp initial target temperature\n");
printf(" -v, --version show version\n");
printf(" -z, --zone target thermal zone id\n");
@@ -219,6 +220,7 @@ static struct option opts[] = {
{ "control", 1, NULL, 'c' },
{ "daemon", 0, NULL, 'd' },
{ "time-interval", 1, NULL, 't' },
+ { "target-temp", 1, NULL, 'T' },
{ "log", 0, NULL, 'l' },
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'v' },
@@ -231,7 +233,7 @@ int main(int argc, char **argv)
{
int err = 0;
int id2 = 0, c;
- double yk = 0.0; /* controller output */
+ double yk = 0.0, temp; /* controller output */
int target_tz_index;
if (geteuid() != 0) {
@@ -239,7 +241,7 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
- while ((c = getopt_long(argc, argv, "c:dlht:vgz:", opts, &id2)) != -1) {
+ while ((c = getopt_long(argc, argv, "c:dlht:T:vgz:", opts, &id2)) != -1) {
switch (c) {
case 'c':
no_control = 0;
@@ -254,6 +256,14 @@ int main(int argc, char **argv)
if (ticktime < 1)
ticktime = 1;
break;
+ case 'T':
+ temp = strtod(optarg, NULL);
+ if (temp < 0) {
+ fprintf(stderr, "error: temperature must be positive\n");
+ return 1;
+ }
+ target_temp_user = temp;
+ break;
case 'l':
printf("Logging data to /var/tmp/tmon.log\n");
logging = 1;
--
1.9.1
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/tui.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/thermal/tmon/tui.c b/tools/thermal/tmon/tui.c
index 89f8ef0e15c8..43c5aecf71da 100644
--- a/tools/thermal/tmon/tui.c
+++ b/tools/thermal/tmon/tui.c
@@ -30,6 +30,18 @@
#include "tmon.h"
+#define min(x, y) ({ \
+ typeof(x) _min1 = (x); \
+ typeof(y) _min2 = (y); \
+ (void) (&_min1 == &_min2); \
+ _min1 < _min2 ? _min1 : _min2; })
+
+#define max(x, y) ({ \
+ typeof(x) _max1 = (x); \
+ typeof(y) _max2 = (y); \
+ (void) (&_max1 == &_max2); \
+ _max1 > _max2 ? _max1 : _max2; })
+
static PANEL *data_panel;
static PANEL *dialogue_panel;
static PANEL *top;
--
1.9.1
We can use the ncurses API to get the number of rows.
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/tui.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/thermal/tmon/tui.c b/tools/thermal/tmon/tui.c
index 43c5aecf71da..2779573a53cb 100644
--- a/tools/thermal/tmon/tui.c
+++ b/tools/thermal/tmon/tui.c
@@ -274,11 +274,14 @@ const char DIAG_TITLE[] = "[ TUNABLES ]";
void show_dialogue(void)
{
int j, x = 0, y = 0;
+ int rows, cols;
WINDOW *w = dialogue_window;
if (tui_disabled || !w)
return;
+ getmaxyx(w, rows, cols);
+
werase(w);
box(w, 0, 0);
mvwprintw(w, 0, maxx/4, DIAG_TITLE);
@@ -297,10 +300,8 @@ void show_dialogue(void)
wattron(w, A_BOLD);
mvwprintw(w, DIAG_DEV_ROWS+1, 1, "Enter Choice [A-Z]?");
wattroff(w, A_BOLD);
- /* y size of dialogue win is nr cdev + 5, so print legend
- * at the bottom line
- */
- mvwprintw(w, ptdata.nr_cooling_dev+3, 1,
+ /* print legend at the bottom line */
+ mvwprintw(w, rows - 2, 1,
"Legend: A=Active, P=Passive, C=Critical");
wrefresh(dialogue_window);
--
1.9.1
The number of rows in the dialog vary according to the number of cooling
devices. However, some of the windowing computations were assuming a
fixed number of rows. This computation is OK when we have between 4 and
9 cooling devices (and they wrap to the next column), but with fewer
devices, we end up printing off the end of the window.
This unifies the row computation into a single function and uses that
throughout the TUI code. This also accounts for increasing the number of
rows when there are more than 9 total cooling devices.
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/tui.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/thermal/tmon/tui.c b/tools/thermal/tmon/tui.c
index 2779573a53cb..36e1f86c8452 100644
--- a/tools/thermal/tmon/tui.c
+++ b/tools/thermal/tmon/tui.c
@@ -110,6 +110,18 @@ void write_status_bar(int x, char *line)
wrefresh(status_bar_window);
}
+/* wrap at 5 */
+#define DIAG_DEV_ROWS 5
+/*
+ * list cooling devices + "set temp" entry; wraps after 5 rows, if they fit
+ */
+static int diag_dev_rows(void)
+{
+ int entries = ptdata.nr_cooling_dev + 1;
+ int rows = max(DIAG_DEV_ROWS, (entries + 1) / 2);
+ return min(rows, entries);
+}
+
void setup_windows(void)
{
int y_begin = 1;
@@ -134,7 +146,7 @@ void setup_windows(void)
* dialogue window is a pop-up, when needed it lays on top of cdev win
*/
- dialogue_window = subwin(stdscr, ptdata.nr_cooling_dev+5, maxx-50,
+ dialogue_window = subwin(stdscr, diag_dev_rows() + 5, maxx-50,
DIAG_Y, DIAG_X);
thermal_data_window = subwin(stdscr, ptdata.nr_tz_sensor *
@@ -270,7 +282,6 @@ void show_cooling_device(void)
}
const char DIAG_TITLE[] = "[ TUNABLES ]";
-#define DIAG_DEV_ROWS 5
void show_dialogue(void)
{
int j, x = 0, y = 0;
@@ -287,7 +298,7 @@ void show_dialogue(void)
mvwprintw(w, 0, maxx/4, DIAG_TITLE);
/* list all the available tunables */
for (j = 0; j <= ptdata.nr_cooling_dev; j++) {
- y = j % DIAG_DEV_ROWS;
+ y = j % diag_dev_rows();
if (y == 0 && j != 0)
x += 20;
if (j == ptdata.nr_cooling_dev)
@@ -298,7 +309,7 @@ void show_dialogue(void)
ptdata.cdi[j].type, ptdata.cdi[j].instance);
}
wattron(w, A_BOLD);
- mvwprintw(w, DIAG_DEV_ROWS+1, 1, "Enter Choice [A-Z]?");
+ mvwprintw(w, diag_dev_rows()+1, 1, "Enter Choice [A-Z]?");
wattroff(w, A_BOLD);
/* print legend at the bottom line */
mvwprintw(w, rows - 2, 1,
@@ -450,7 +461,7 @@ static void handle_input_choice(int ch)
snprintf(buf, sizeof(buf), "New Value for %.10s-%2d: ",
ptdata.cdi[cdev_id].type,
ptdata.cdi[cdev_id].instance);
- write_dialogue_win(buf, DIAG_DEV_ROWS+2, 2);
+ write_dialogue_win(buf, diag_dev_rows() + 2, 2);
handle_input_val(cdev_id);
} else {
snprintf(buf, sizeof(buf), "Invalid selection %d", ch);
--
1.9.1
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/.gitignore | 1 +
1 file changed, 1 insertion(+)
create mode 100644 tools/thermal/tmon/.gitignore
diff --git a/tools/thermal/tmon/.gitignore b/tools/thermal/tmon/.gitignore
new file mode 100644
index 000000000000..06e96be65276
--- /dev/null
+++ b/tools/thermal/tmon/.gitignore
@@ -0,0 +1 @@
+/tmon
--
1.9.1
We might want to prepare CFLAGS outside of this Makefile, so don't
overwrite its initial value.
Then, support $(CROSS_COMPILE), so we can use a cross-compile toolchain.
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index e775adcbd29f..13d1876c7889 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -2,8 +2,8 @@ VERSION = 1.0
BINDIR=usr/bin
WARNFLAGS=-Wall -Wshadow -W -Wformat -Wimplicit-function-declaration -Wimplicit-int
-CFLAGS= -O1 ${WARNFLAGS} -fstack-protector
-CC=gcc
+CFLAGS+= -O1 ${WARNFLAGS} -fstack-protector
+CC=$(CROSS_COMPILE)gcc
CFLAGS+=-D VERSION=\"$(VERSION)\"
LDFLAGS+=
--
1.9.1
Some distros (e.g., Arch Linux) don't package the tinfo library
separately from ncurses, so don't unconditionally include it. Instead,
use pkg-config.
The $(STATIC) ugliness is to handle the reported build case from commit
6b533269fb25 ("tools/thermal: tmon: fix compilation errors when building
statically"), where a developer wants to be able to build with:
make LDFLAGS=-static
which requires an additional pkg-config flag.
Finally, support a lowest common denominator fallback (-lpanel
-lncurses) for build systems that don't have pkg-config entries for
ncurses.
Signed-off-by: Brian Norris <[email protected]>
---
tools/thermal/tmon/Makefile | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index 13d1876c7889..0788621c8d76 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -16,12 +16,21 @@ INSTALL_CONFIGFILE=install -m 644 -p
CONFIG_FILE=
CONFIG_PATH=
+# Static builds might require -ltinfo, for instance
+ifneq ($(findstring -static, $(LDFLAGS)),)
+STATIC := --static
+endif
+
+TMON_LIBS=-lm -lpthread
+TMON_LIBS += $(shell pkg-config --libs $(STATIC) panelw ncursesw 2> /dev/null || \
+ pkg-config --libs $(STATIC) panel ncurses 2> /dev/null || \
+ echo -lpanel -lncurses)
OBJS = tmon.o tui.o sysfs.o pid.o
OBJS +=
tmon: $(OBJS) Makefile tmon.h
- $(CC) ${CFLAGS} $(LDFLAGS) $(OBJS) -o $(TARGET) -lm -lpanel -lncursesw -ltinfo -lpthread
+ $(CC) $(CFLAGS) $(LDFLAGS) $(OBJS) -o $(TARGET) $(TMON_LIBS)
valgrind: tmon
sudo valgrind -v --track-origins=yes --tool=memcheck --leak-check=yes --show-reachable=yes --num-callers=20 --track-fds=yes ./$(TARGET) 1> /dev/null
--
1.9.1
gcc complains about the 'cols' variable being unused. This is
unavoidable, given the ncurses getmaxyx() macro-based API, which wants
to assign to a variable directly, even when we're not going to use it.
Warning:
gcc -O1 -Wall -Wshadow -W -Wformat -Wimplicit-function-declaration -Wimplicit-int -fstack-protector -D VERSION=\"1.0\" -c -o tui.o tui.c
tui.c: In function ‘show_dialogue’:
tui.c:288:12: warning: variable ‘cols’ set but not used [-Wunused-but-set-variable]
int rows, cols;
^
So, add a hack to get rid of that warning.
Signed-off-by: Brian Norris <[email protected]>
---
This patch is ugly and of little value IMO, but it does squash the warning.
Take it or leave it.
tools/thermal/tmon/tui.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/thermal/tmon/tui.c b/tools/thermal/tmon/tui.c
index 36e1f86c8452..b5d1c6b22dd3 100644
--- a/tools/thermal/tmon/tui.c
+++ b/tools/thermal/tmon/tui.c
@@ -293,6 +293,9 @@ void show_dialogue(void)
getmaxyx(w, rows, cols);
+ /* Silence compiler 'unused' warnings */
+ (void)cols;
+
werase(w);
box(w, 0, 0);
mvwprintw(w, 0, maxx/4, DIAG_TITLE);
--
1.9.1
On Tue, 17 Feb 2015 18:18:28 -0800
Brian Norris <[email protected]> wrote:
> Hi,
>
> This patch set includes a few build system improvements and a few UI
> fixes.
>
> The last patch is ugly, and personally, I don't care if it's
> included. But it's an attempt to kill a warning. Better suggestions
> are welcome.
>
Good improvements.
Acked-by: Jacob Pan <[email protected]>
> Thanks,
> Brian
>
> Brian Norris (8):
> tools/thermal: tmon: add --target-temp parameter
> tools/thermal: tmon: add min/max macros
> tools/thermal: tmon: tui: don't hard-code dialog window size
> assumptions
> tools/thermal: tmon: fixup tui windowing calculations
> tools/thermal: tmon: add .gitignore
> tools/thermal: tmon: support cross-compiling
> tools/thermal: tmon: use pkg-config to determine library
> dependencies tools/thermal: tmon: silence 'set but not used' warnings
>
> tools/thermal/tmon/.gitignore | 1 +
> tools/thermal/tmon/Makefile | 15 ++++++++++++---
> tools/thermal/tmon/tmon.8 | 2 ++
> tools/thermal/tmon/tmon.c | 14 ++++++++++++--
> tools/thermal/tmon/tui.c | 45
> ++++++++++++++++++++++++++++++++++--------- 5 files changed, 63
> insertions(+), 14 deletions(-) create mode 100644
> tools/thermal/tmon/.gitignore
>
> --
> 1.9.1
>
--
Jacob Pan
On 17/02/15 18:18, Brian Norris wrote:
> Hi,
>
> This patch set includes a few build system improvements and a few UI fixes.
>
> The last patch is ugly, and personally, I don't care if it's included. But it's
> an attempt to kill a warning. Better suggestions are welcome.
FWIW:
Reviewed-by: Florian Fainelli <[email protected]>
>
> Thanks,
> Brian
>
> Brian Norris (8):
> tools/thermal: tmon: add --target-temp parameter
> tools/thermal: tmon: add min/max macros
> tools/thermal: tmon: tui: don't hard-code dialog window size
> assumptions
> tools/thermal: tmon: fixup tui windowing calculations
> tools/thermal: tmon: add .gitignore
> tools/thermal: tmon: support cross-compiling
> tools/thermal: tmon: use pkg-config to determine library dependencies
> tools/thermal: tmon: silence 'set but not used' warnings
>
> tools/thermal/tmon/.gitignore | 1 +
> tools/thermal/tmon/Makefile | 15 ++++++++++++---
> tools/thermal/tmon/tmon.8 | 2 ++
> tools/thermal/tmon/tmon.c | 14 ++++++++++++--
> tools/thermal/tmon/tui.c | 45 ++++++++++++++++++++++++++++++++++---------
> 5 files changed, 63 insertions(+), 14 deletions(-)
> create mode 100644 tools/thermal/tmon/.gitignore
>
--
Florian