Kobo, Mk.5: Tweak CPU frequency scaling (#6905)

* Run grep against /proc/modules directly instead of forking to lsmod

* Revamp the CPUFreq governor choice logic

I finally figured out why the scaling on the H2O was so wonky:
Because it relies on an obscure i.MX hardware feature called DVFS,
and it gets flipped for mysterious workaround-y reasons depending on
Wi-Fi state...

* Start playing with conservative, because just staying pegged at max
clock is entirely stupid.

And DVFS is extremely conservative, it needs somewhat sustained load to
clock up...

* Take care of the DVFS switcheroo in the Wi-Fi scripts

.ci/helper_shellchecks.sh

@@ -8,19 +8,24 @@ source "${CI_DIR}/common.sh"
 mapfile -t shellscript_locations < <({ git grep -lE '^#!(/usr)?/bin/(env )?(bash|sh)' && git submodule --quiet foreach '[ "$path" = "base" -o "$path" = "platform/android/luajit-launcher" ] || git grep -lE "^#!(/usr)?/bin/(env )?(bash|sh)" | sed "s|^|$path/|"' && git ls-files ./*.sh; } | sort | uniq)
 
 SHELLSCRIPT_ERROR=0
+SHFMT_OPTIONS="-i 4 -ci"
 
 for shellscript in "${shellscript_locations[@]}"; do
     echo -e "${ANSI_GREEN}Running shellcheck on ${shellscript}"
     shellcheck "${shellscript}" || SHELLSCRIPT_ERROR=1
     echo -e "${ANSI_GREEN}Running shfmt on ${shellscript}"
-    if ! shfmt -i 4 -ci "${shellscript}" >/dev/null 2>&1; then
+    # shellcheck disable=2086
+    if ! shfmt ${SHFMT_OPTIONS} -kp "${shellscript}" >/dev/null 2>&1; then
         echo -e "${ANSI_RED}Warning: ${shellscript} contains the following problem:"
-        shfmt -i 4 -ci "${shellscript}" || SHELLSCRIPT_ERROR=1
+        # shellcheck disable=2086
+        shfmt ${SHFMT_OPTIONS} -kp "${shellscript}" || SHELLSCRIPT_ERROR=1
         continue
     fi
-    if [ "$(cat "${shellscript}")" != "$(shfmt -i 4 -ci "${shellscript}")" ]; then
+    # shellcheck disable=2086
+    if [ "$(cat "${shellscript}")" != "$(shfmt ${SHFMT_OPTIONS} "${shellscript}")" ]; then
         echo -e "${ANSI_RED}Warning: ${shellscript} does not abide by coding style, diff for expected style:"
-        shfmt -i 4 -ci "${shellscript}" | diff "${shellscript}" - || SHELLSCRIPT_ERROR=1
+        # shellcheck disable=2086
+        shfmt ${SHFMT_OPTIONS} -d "${shellscript}" || SHELLSCRIPT_ERROR=1
     fi
 done
 

.ci/install.sh

@@ -60,8 +60,8 @@ else
 fi
 
 # install shfmt
-SHFMT_URL="https://github.com/mvdan/sh/releases/download/v3.0.1/shfmt_v3.0.1_linux_amd64"
+SHFMT_URL="https://github.com/mvdan/sh/releases/download/v3.2.0/shfmt_v3.2.0_linux_amd64"
-if [ "$(shfmt --version)" != "v3.0.1" ]; then
+if [ "$(shfmt --version)" != "v3.2.0" ]; then
     curl -sSL "${SHFMT_URL}" -o "${HOME}/bin/shfmt"
     chmod +x "${HOME}/bin/shfmt"
 else

platform/kobo/disable-wifi.sh

@@ -40,11 +40,21 @@ ifconfig "${INTERFACE}" down
 
 # Some sleep in between may avoid system getting hung
 # (we test if a module is actually loaded to avoid unneeded sleeps)
-if lsmod | grep -q "${WIFI_MODULE}"; then
+if grep -q "${WIFI_MODULE}" "/proc/modules"; then
     usleep 250000
     rmmod "${WIFI_MODULE}"
 fi
-if lsmod | grep -q sdio_wifi_pwr; then
+if grep -q "sdio_wifi_pwr" "/proc/modules"; then
+    # Handle the shitty DVFS switcheroo...
+    if [ -n "${CPUFREQ_DVFS}" ]; then
+        echo "0" >"/sys/devices/platform/mxc_dvfs_core.0/enable"
+        if [ -n "${CPUFREQ_CONSERVATIVE}" ]; then
+            echo "conservative" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+        else
+            echo "userspace" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+            cat "/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed"
+        fi
+    fi
     usleep 250000
     rmmod sdio_wifi_pwr
 fi

platform/kobo/enable-wifi.sh

@@ -14,11 +14,21 @@ for fd in /proc/"$$"/fd/*; do
 done
 
 # Load wifi modules and enable wifi.
-lsmod | grep -q sdio_wifi_pwr || insmod "/drivers/${PLATFORM}/wifi/sdio_wifi_pwr.ko"
+if ! grep -q "sdio_wifi_pwr" "/proc/modules"; then
+    if [ -e "/drivers/${PLATFORM}/wifi/sdio_wifi_pwr.ko" ]; then
+        # Handle the shitty DVFS switcheroo...
+        if [ -n "${CPUFREQ_DVFS}" ]; then
+            echo "userspace" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+            echo "1" >"/sys/devices/platform/mxc_dvfs_core.0/enable"
+        fi
+
+        insmod "/drivers/${PLATFORM}/wifi/sdio_wifi_pwr.ko"
+    fi
+fi
 # Moar sleep!
 usleep 250000
 # NOTE: Used to be exported in WIFI_MODULE_PATH before FW 4.23
-lsmod | grep -q "${WIFI_MODULE}" || insmod "/drivers/${PLATFORM}/wifi/${WIFI_MODULE}.ko"
+grep -q "${WIFI_MODULE}" "/proc/modules" || insmod "/drivers/${PLATFORM}/wifi/${WIFI_MODULE}.ko"
 # Race-y as hell, don't try to optimize this!
 sleep 1
 

platform/kobo/koreader.sh

@@ -18,19 +18,68 @@ fi
 
 # Attempt to switch to a sensible CPUFreq governor when that's not already the case...
 IFS= read -r current_cpufreq_gov <"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
-# NOTE: We're being fairly conservative here, because what's used and what's available varies depending on HW...
+# NOTE: What's available depends on the HW, so, we'll have to take it step by step...
-if [ "${current_cpufreq_gov}" != "ondemand" ] && [ "${current_cpufreq_gov}" != "interactive" ]; then
+#       Roughly follow Nickel's behavior (which prefers interactive), and prefer interactive, then ondemand, and finally conservative/dvfs.
-    # NOTE: Go with ondemand, because it's likely to be the lowest common denominator.
+if [ "${current_cpufreq_gov}" != "interactive" ]; then
-    #       Plus, interactive is hard to tune right, and only really interesting when it's a recent version,
+    if grep -q "interactive" "/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors"; then
-    #       which I somehow doubt is the case anywhere here...
+        ORIG_CPUFREQ_GOV="${current_cpufreq_gov}"
-    if grep -q ondemand /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors; then
+        echo "interactive" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+    elif [ "${current_cpufreq_gov}" != "ondemand" ]; then
+        if grep -q "ondemand" "/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors"; then
+            # NOTE: This should never really happen: every kernel that supports ondemand already supports interactive ;).
+            #       They were both introduced on Mk. 6
             ORIG_CPUFREQ_GOV="${current_cpufreq_gov}"
             echo "ondemand" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+        elif [ -e "/sys/devices/platform/mxc_dvfs_core.0/enable" ]; then
+            # The rest of this block assumes userspace is available...
+            if grep -q "userspace" "/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors"; then
+                ORIG_CPUFREQ_GOV="${current_cpufreq_gov}"
+                export CPUFREQ_DVFS="true"
+
+                # If we can use conservative, do so, but we'll tweak it a bit to make it somewhat useful given our load patterns...
+                # We unfortunately don't have any better choices on those kernels,
+                # the only other governors available are powersave & performance (c.f., #4114)...
+                if grep -q "conservative" "/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors"; then
+                    export CPUFREQ_CONSERVATIVE="true"
+                    echo "conservative" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+                    # NOTE: The knobs survive a governor switch, which is why we do this now ;).
+                    echo "2" >"/sys/devices/system/cpu/cpufreq/conservative/sampling_down_factor"
+                    echo "50" >"/sys/devices/system/cpu/cpufreq/conservative/freq_step"
+                    echo "11" >"/sys/devices/system/cpu/cpufreq/conservative/down_threshold"
+                    echo "12" >"/sys/devices/system/cpu/cpufreq/conservative/up_threshold"
+                    # NOTE: The default sampling_rate is a bit high for my tastes,
+                    #       but it unfortunately defaults to its lowest possible setting...
+                fi
+
+                # NOTE: Now, here comes the freaky stuff... On a H2O, DVFS is only enabled when Wi-Fi is *on*.
+                #       When it's off, DVFS is off, which pegs the CPU @ max clock given that DVFS means the userspace governor.
+                #       The flip may originally have been switched by the sdio_wifi_pwr module itself,
+                #       via ntx_wifi_power_ctrl @ arch/arm/mach-mx5/mx50_ntx_io.c (which is also the CM_WIFI_CTRL (208) ntx_io ioctl),
+                #       but the code in the published H2O kernel sources actually does the reverse, and is commented out ;).
+                #       It is now entirely handled by Nickel, right *before* loading/unloading that module.
+                #       (There's also a bug(?) where that behavior is inverted for the *first* Wi-Fi session after a cold boot...)
+                if grep -q "sdio_wifi_pwr" "/proc/modules"; then
+                    # Wi-Fi is enabled, make sure DVFS is on
+                    echo "userspace" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+                    echo "1" >"/sys/devices/platform/mxc_dvfs_core.0/enable"
+                else
+                    # Wi-Fi is disabled, make sure DVFS is off
+                    echo "0" >"/sys/devices/platform/mxc_dvfs_core.0/enable"
+
+                    # Switch to conservative to avoid being stuck at max clock if we can...
+                    if [ -n "${CPUFREQ_CONSERVATIVE}" ]; then
+                        echo "conservative" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+                    else
+                        # Otherwise, we'll be pegged at max clock...
+                        echo "userspace" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+                        # The kernel should already be taking care of that...
+                        cat "/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed"
+                    fi
+                fi
+            fi
+        fi
     fi
 fi
-# NOTE: That doesn't actually help us poor userspace plebs, but, short of switching to performance,
-#       I don't really have a golden bullet here... (conservative's rubberbanding is terrible, so that's a hard pass).
-#       All I can say is that userspace is a terrible idea and behaves *very* strangely (c.f., #4114).
 
 # update to new version from OTA directory
 ko_update_check() {
@@ -462,6 +511,8 @@ fi
 # Restore original CPUFreq governor if need be...
 if [ -n "${ORIG_CPUFREQ_GOV}" ]; then
     echo "${ORIG_CPUFREQ_GOV}" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+
+    # NOTE: Leave DVFS alone, it'll be handled by Nickel if necessary.
 fi
 
 # If we requested a reboot/shutdown, no need to bother with this...

platform/kobo/nickel.sh

@@ -23,7 +23,7 @@ unset KOREADER_DIR KO_NO_CBB KO_DONT_GRAB_INPUT
 
 # Make sure we kill the Wi-Fi first, because nickel apparently doesn't like it if it's up... (cf. #1520)
 # NOTE: That check is possibly wrong on PLATFORM == freescale (because I don't know if the sdio_wifi_pwr module exists there), but we don't terribly care about that.
-if lsmod | grep -q sdio_wifi_pwr; then
+if grep -q "sdio_wifi_pwr" "/proc/modules"; then
     killall -q -TERM restore-wifi-async.sh enable-wifi.sh obtain-ip.sh
     cp -a "/etc/resolv.conf" "/tmp/resolv.ko"
     old_hash="$(md5sum "/etc/resolv.conf" | cut -f1 -d' ')"
@@ -62,10 +62,19 @@ if lsmod | grep -q sdio_wifi_pwr; then
     # c.f., #2394?
     usleep 250000
     rmmod "${WIFI_MODULE}"
+
+    if [ -n "${CPUFREQ_DVFS}" ]; then
+        echo "0" >"/sys/devices/platform/mxc_dvfs_core.0/enable"
+        # Leave Nickel in its usual state, don't try to use conservative
+        echo "userspace" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"
+        cat "/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq" >"/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed"
+    fi
     usleep 250000
     rmmod sdio_wifi_pwr
 fi
 
+unset CPUFREQ_DVFS CPUFREQ_CONSERVATIVE
+
 # Recreate Nickel's FIFO ourselves, like rcS does, because udev *will* write to it!
 # Plus, we actually *do* want the stuff udev writes in there to be processed by Nickel, anyway.
 rm -f "/tmp/nickel-hardware-status"
@@ -84,7 +93,7 @@ fi
 
 # And finally, simply restart nickel.
 # We don't care about horribly legacy stuff, because if people switch between nickel and KOReader in the first place, I assume they're using a decently recent enough FW version.
-# Last tested on an H2O & a Forma running FW 4.7.x - 4.24.x
+# Last tested on an H2O & a Forma running FW 4.7.x - 4.25.x
 /usr/local/Kobo/hindenburg &
 LIBC_FATAL_STDERR_=1 /usr/local/Kobo/nickel -platform kobo -skipFontLoad &
 [ "${PLATFORM}" != "freescale" ] && udevadm trigger &