ondemand cpufreq handler

[Sebastian Kuzminsky]

"Kurt H Maier" <karmaflux at gmail.com> wrote:
> Is there a reason why powertop always recommends ondemand?
> Conservative is a much more sensible
> The patch against 1.2 replaces the ondemand check with a check for conservative.

Both are acceptable, as is the powersave governor.  powertop should only
complain if none of these three is in use.

The "suggests" facility is great for users, I'm very happy powertop
has it.  But the implementation is a bit strained.  It'd be nice to
refactor it and extend it a bit.  For example, it's not enough to have
a good powersaving cpufreq governor available, you have to actually use
it too.  ;-)


But for now, here's a patch that does the following:

    Adds a pair of functions: kconfig_get_value() and
    kconfig_is_enabled().

    Makes suggest_kernel_config() use kconfig_is_enabled() (without
    changing its semantics).

    And finally, changes the ondemand check in main so it accepts any
    of the three power-saving governors.


Index: powertop.c
===================================================================
--- powertop.c	(revision 35)
+++ powertop.c	(working copy)
@@ -521,9 +521,20 @@
 				      "not very efficient and costs a significant amount of battery life.");
 		suggest_kernel_config("CONFIG_USB_SUSPEND", 1,
 				      "Suggestion: Enable the CONFIG_USB_SUSPEND kernel configuration option.\nThis option will automatically disable UHCI USB when not in use, and may\nsave approximately 1 Watt of power.");
-		suggest_kernel_config("CONFIG_CPU_FREQ_GOV_ONDEMAND", 1,
-				      "Suggestion: Enable the CONFIG_CPU_FREQ_GOV_ONDEMAND kernel configuration option.\n"
-				      "The 'ondemand' CPU speed governer will minimize the CPU power usage while\n" "giving you performance when it is needed.");
+
+                if (
+                    ! kconfig_is_enabled("CONFIG_CPU_FREQ_GOV_ONDEMAND") &&
+                    ! kconfig_is_enabled("CONFIG_CPU_FREQ_GOV_CONSERVATIVE") &&
+                    ! kconfig_is_enabled("CONFIG_CPU_FREQ_GOV_POWERSAVE")
+                ) {
+                    suggest_kernel_config("CONFIG_CPU_FREQ_GOV_ONDEMAND", 1,
+                                          "Suggestion: Enable at least one of these kernel configuration options:\n"
+                                          "    CONFIG_CPU_FREQ_GOV_ONDEMAND\n"
+                                          "    CONFIG_CPU_FREQ_GOV_CONSERVATIVE\n"
+                                          "    CONFIG_CPU_FREQ_GOV_POWERSAVE\n"
+                                          "These CPU frequency governors lower CPU power usage.\n");
+                }
+
 		suggest_kernel_config("CONFIG_NO_HZ", 1, "Suggestion: Enable the CONFIG_NO_HZ kernel configuration option.\nThis option is required to get any kind of longer sleep times in the CPU.");
 		suggest_kernel_config("CONFIG_HPET", 1,
 				      "Suggestion: Enable the CONFIG_HPET kernel configuration option.\n"
Index: powertop.h
===================================================================
--- powertop.h	(revision 35)
+++ powertop.h	(working copy)
@@ -31,6 +31,30 @@
 	int	count;
 };
 
+
+// 
+// kconfig_get_value() looks in the (cached) kernel config info for the
+// specified variable.  If found, it returns its value (0 for yes, 1 for
+// module, or the atoi of the value).  If not found, it returns -1.
+//
+// FIXME: doesnt handle string values
+//
+
+#define KCONFIG_NO      -1
+#define KCONFIG_YES     0
+#define KCONFIG_MODULE  1
+
+int kconfig_get_value(char *kconfig_variable);
+
+
+// 
+// kconfig_is_enabled() returns true (1) if the specified variable is set
+// to 'y' or 'm', and returns false (0) if not.
+//
+
+int kconfig_is_enabled(char *kconfig_variable);
+
+
 void suggest_process_death(char *process, struct line *lines, int linecount, char *comment);
 void suggest_kernel_config(char *string, int onoff, char *comment);
 
Index: config.c
===================================================================
--- config.c	(revision 35)
+++ config.c	(working copy)
@@ -77,34 +77,64 @@
 	fclose(file);
 }
 
+
+int kconfig_get_value(char *kconfig_variable) {
+    int i;
+
+    read_kernel_config();
+
+    for (i = 0; i < configcount; i++) {
+        char *valptr;
+
+        if (strncmp(configlines[i], kconfig_variable, strlen(kconfig_variable)) != 0) {
+            continue;
+        }
+
+        if (configlines[i][strlen(kconfig_variable)] != '=') {
+            continue;
+        }
+
+        valptr = &configlines[i][strlen(kconfig_variable) + 1];
+
+        if (strcmp(valptr, "y") == 0) return KCONFIG_YES;
+        if (strcmp(valptr, "m") == 0) return KCONFIG_MODULE;
+
+        return atoi(valptr);
+    }
+
+    // kconfig variable not found
+    return KCONFIG_NO;
+}
+
+int kconfig_is_enabled(char *kconfig_variable) {
+    int val;
+
+    val = kconfig_get_value(kconfig_variable);
+    if ((val == KCONFIG_YES) || (val == KCONFIG_MODULE)) {
+        return 1;
+    }
+
+    return 0;
+}
+
 /*
  * Suggest the user to turn on/off a kernel config option.
  * "comment" gets displayed if it's not already set to the right value 
  */
 void suggest_kernel_config(char *string, int onoff, char *comment)
 {
-	int i;
-	char searchon[100];
-	char searchoff[100];
-	int found = 0;
+        int enabled;
 
 	if (suggestioncount > 0)
 		return;
-	read_kernel_config();
 
-	sprintf(searchon, "%s=", string);
-	sprintf(searchoff, "# %s is not set", string);
+        enabled = kconfig_is_enabled(string);
 
-	for (i = 0; i < configcount; i++) {
-		if (onoff && strstr(configlines[i], searchon))
-			return;
-		if (onoff==0 && strstr(configlines[i], searchoff))
-			return;
-		if (onoff==0 && strstr(configlines[i], searchon))
-			found = 1;
-	}
-	if (onoff || found)
-		printf("%s\n", comment);
+        if (onoff && enabled) return;
+        if (!onoff && !enabled) return;
+
+        printf("%s\n", comment);
 	fflush(stdout);
+
 	suggestioncount++;
 }