Skip to content

Commit e7686bc

Browse files
pvaibhavpvaibhav
committed
fix: address kernel-cpu-governor review feedback
* Simplify the CPU Speed description in MinArch * Switch the governor scripts to the shared policy0 path * Remove the unrelated diff about rom_path Also stop running the CPU monitor when it is not needed by gating it behind the Debug HUD setting and removing the NextUI-side thread launch. Previously, nextui.c was launching this thread in teh back- ground and detaching from it. Now, it's launched conditionally in minarch.c only when the Debug HUD switch is turned on. The platform monitor loop now checks the shared enabled state so the sampling work shuts down when the HUD is off. Hope this squeezes a little more performance out of the cpu.
1 parent 482794b commit e7686bc

12 files changed

Lines changed: 139 additions & 75 deletions

File tree

‎skeleton/SYSTEM/tg5040/bin/auto_governor.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#!/bin/sh
22
# auto_governor.sh - ondemand governor, min freq to one step below max
3-
for CPU_PATH in /sys/devices/system/cpu/cpu*/cpufreq; do
4-
[ -f "$CPU_PATH/scaling_available_frequencies" ] || continue
5-
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
6-
MIN_FREQ=$(echo "$FREQS" | head -1)
7-
SECOND_MAX=$(echo "$FREQS" | tail -2 | head -1)
8-
echo ondemand > "$CPU_PATH/scaling_governor" 2>/dev/null || true
9-
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
10-
echo "$SECOND_MAX" > "$CPU_PATH/scaling_max_freq"
11-
done
3+
# All cpu*/cpufreq entries point at this same shared policy0 path.
4+
CPU_PATH=/sys/devices/system/cpu/cpufreq/policy0
5+
[ -f "$CPU_PATH/scaling_available_frequencies" ] || exit 0
6+
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
7+
MIN_FREQ=$(echo "$FREQS" | head -1)
8+
SECOND_MAX=$(echo "$FREQS" | tail -2 | head -1)
9+
echo ondemand > "$CPU_PATH/scaling_governor" 2>/dev/null || true
10+
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
11+
echo "$SECOND_MAX" > "$CPU_PATH/scaling_max_freq"

‎skeleton/SYSTEM/tg5040/bin/performance_governor.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#!/bin/sh
22
# performance_governor.sh - schedutil governor, min freq to max freq
3-
for CPU_PATH in /sys/devices/system/cpu/cpu*/cpufreq; do
4-
[ -f "$CPU_PATH/scaling_available_frequencies" ] || continue
5-
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
6-
MIN_FREQ=$(echo "$FREQS" | head -1)
7-
MAX_FREQ=$(echo "$FREQS" | tail -1)
8-
echo schedutil > "$CPU_PATH/scaling_governor" 2>/dev/null || true
9-
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
10-
echo "$MAX_FREQ" > "$CPU_PATH/scaling_max_freq"
11-
done
3+
# All cpu*/cpufreq entries point at this same shared policy0 path.
4+
CPU_PATH=/sys/devices/system/cpu/cpufreq/policy0
5+
[ -f "$CPU_PATH/scaling_available_frequencies" ] || exit 0
6+
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
7+
MIN_FREQ=$(echo "$FREQS" | head -1)
8+
MAX_FREQ=$(echo "$FREQS" | tail -1)
9+
echo schedutil > "$CPU_PATH/scaling_governor" 2>/dev/null || true
10+
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
11+
echo "$MAX_FREQ" > "$CPU_PATH/scaling_max_freq"

‎skeleton/SYSTEM/tg5040/bin/powersave_governor.sh‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
#!/bin/sh
22
# powersave_governor.sh - conservative governor, min freq to midpoint max
3-
for CPU_PATH in /sys/devices/system/cpu/cpu*/cpufreq; do
4-
[ -f "$CPU_PATH/scaling_available_frequencies" ] || continue
5-
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
6-
COUNT=$(echo "$FREQS" | wc -l)
7-
MID=$(( (COUNT + 1) / 2 ))
8-
MIN_FREQ=$(echo "$FREQS" | head -1)
9-
MID_FREQ=$(echo "$FREQS" | sed -n "${MID}p")
10-
echo conservative > "$CPU_PATH/scaling_governor" 2>/dev/null || true
11-
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
12-
echo "$MID_FREQ" > "$CPU_PATH/scaling_max_freq"
13-
done
3+
# All cpu*/cpufreq entries point at this same shared policy0 path.
4+
CPU_PATH=/sys/devices/system/cpu/cpufreq/policy0
5+
[ -f "$CPU_PATH/scaling_available_frequencies" ] || exit 0
6+
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
7+
COUNT=$(echo "$FREQS" | wc -l)
8+
MID=$(( (COUNT + 1) / 2 ))
9+
MIN_FREQ=$(echo "$FREQS" | head -1)
10+
MID_FREQ=$(echo "$FREQS" | sed -n "${MID}p")
11+
echo conservative > "$CPU_PATH/scaling_governor" 2>/dev/null || true
12+
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
13+
echo "$MID_FREQ" > "$CPU_PATH/scaling_max_freq"

‎skeleton/SYSTEM/tg5050/bin/auto_governor.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#!/bin/sh
22
# auto_governor.sh - ondemand governor, min freq to one step below max
3-
for CPU_PATH in /sys/devices/system/cpu/cpu*/cpufreq; do
4-
[ -f "$CPU_PATH/scaling_available_frequencies" ] || continue
5-
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
6-
MIN_FREQ=$(echo "$FREQS" | head -1)
7-
SECOND_MAX=$(echo "$FREQS" | tail -2 | head -1)
8-
echo ondemand > "$CPU_PATH/scaling_governor" 2>/dev/null || true
9-
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
10-
echo "$SECOND_MAX" > "$CPU_PATH/scaling_max_freq"
11-
done
3+
# All cpu*/cpufreq entries point at this same shared policy0 path.
4+
CPU_PATH=/sys/devices/system/cpu/cpufreq/policy0
5+
[ -f "$CPU_PATH/scaling_available_frequencies" ] || exit 0
6+
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
7+
MIN_FREQ=$(echo "$FREQS" | head -1)
8+
SECOND_MAX=$(echo "$FREQS" | tail -2 | head -1)
9+
echo ondemand > "$CPU_PATH/scaling_governor" 2>/dev/null || true
10+
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
11+
echo "$SECOND_MAX" > "$CPU_PATH/scaling_max_freq"

‎skeleton/SYSTEM/tg5050/bin/performance_governor.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#!/bin/sh
22
# performance_governor.sh - schedutil governor, min freq to max freq
3-
for CPU_PATH in /sys/devices/system/cpu/cpu*/cpufreq; do
4-
[ -f "$CPU_PATH/scaling_available_frequencies" ] || continue
5-
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
6-
MIN_FREQ=$(echo "$FREQS" | head -1)
7-
MAX_FREQ=$(echo "$FREQS" | tail -1)
8-
echo schedutil > "$CPU_PATH/scaling_governor" 2>/dev/null || true
9-
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
10-
echo "$MAX_FREQ" > "$CPU_PATH/scaling_max_freq"
11-
done
3+
# All cpu*/cpufreq entries point at this same shared policy0 path.
4+
CPU_PATH=/sys/devices/system/cpu/cpufreq/policy0
5+
[ -f "$CPU_PATH/scaling_available_frequencies" ] || exit 0
6+
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
7+
MIN_FREQ=$(echo "$FREQS" | head -1)
8+
MAX_FREQ=$(echo "$FREQS" | tail -1)
9+
echo schedutil > "$CPU_PATH/scaling_governor" 2>/dev/null || true
10+
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
11+
echo "$MAX_FREQ" > "$CPU_PATH/scaling_max_freq"

‎skeleton/SYSTEM/tg5050/bin/powersave_governor.sh‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
#!/bin/sh
22
# powersave_governor.sh - conservative governor, min freq to midpoint max
3-
for CPU_PATH in /sys/devices/system/cpu/cpu*/cpufreq; do
4-
[ -f "$CPU_PATH/scaling_available_frequencies" ] || continue
5-
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
6-
COUNT=$(echo "$FREQS" | wc -l)
7-
MID=$(( (COUNT + 1) / 2 ))
8-
MIN_FREQ=$(echo "$FREQS" | head -1)
9-
MID_FREQ=$(echo "$FREQS" | sed -n "${MID}p")
10-
echo conservative > "$CPU_PATH/scaling_governor" 2>/dev/null || true
11-
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
12-
echo "$MID_FREQ" > "$CPU_PATH/scaling_max_freq"
13-
done
3+
# All cpu*/cpufreq entries point at this same shared policy0 path.
4+
CPU_PATH=/sys/devices/system/cpu/cpufreq/policy0
5+
[ -f "$CPU_PATH/scaling_available_frequencies" ] || exit 0
6+
FREQS=$(cat "$CPU_PATH/scaling_available_frequencies" | tr ' ' '\n' | grep -v '^$' | sort -n)
7+
COUNT=$(echo "$FREQS" | wc -l)
8+
MID=$(( (COUNT + 1) / 2 ))
9+
MIN_FREQ=$(echo "$FREQS" | head -1)
10+
MID_FREQ=$(echo "$FREQS" | sed -n "${MID}p")
11+
echo conservative > "$CPU_PATH/scaling_governor" 2>/dev/null || true
12+
echo "$MIN_FREQ" > "$CPU_PATH/scaling_min_freq"
13+
echo "$MID_FREQ" > "$CPU_PATH/scaling_max_freq"

‎workspace/all/common/api.c‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,49 @@ int currentshadertexh = 0;
215215

216216
int should_rotate = 0;
217217

218+
static pthread_mutex_t perf_cpu_monitor_mutex = PTHREAD_MUTEX_INITIALIZER;
219+
static int perf_cpu_monitor_enabled = 0;
220+
static int perf_cpu_monitor_running = 0;
221+
222+
void Perf_setCPUMonitorEnabled(int enabled)
223+
{
224+
pthread_mutex_lock(&perf_cpu_monitor_mutex);
225+
perf_cpu_monitor_enabled = enabled;
226+
pthread_mutex_unlock(&perf_cpu_monitor_mutex);
227+
}
228+
229+
int Perf_isCPUMonitorEnabled(void)
230+
{
231+
int enabled;
232+
233+
pthread_mutex_lock(&perf_cpu_monitor_mutex);
234+
enabled = perf_cpu_monitor_enabled;
235+
pthread_mutex_unlock(&perf_cpu_monitor_mutex);
236+
237+
return enabled;
238+
}
239+
240+
int Perf_tryBeginCPUMonitor(void)
241+
{
242+
int should_run = 0;
243+
244+
pthread_mutex_lock(&perf_cpu_monitor_mutex);
245+
if (perf_cpu_monitor_enabled && !perf_cpu_monitor_running) {
246+
perf_cpu_monitor_running = 1;
247+
should_run = 1;
248+
}
249+
pthread_mutex_unlock(&perf_cpu_monitor_mutex);
250+
251+
return should_run;
252+
}
253+
254+
void Perf_endCPUMonitor(void)
255+
{
256+
pthread_mutex_lock(&perf_cpu_monitor_mutex);
257+
perf_cpu_monitor_running = 0;
258+
pthread_mutex_unlock(&perf_cpu_monitor_mutex);
259+
}
260+
218261
FALLBACK_IMPLEMENTATION void PLAT_pinToCores(int core_type)
219262
{
220263
// no-op
@@ -4393,4 +4436,4 @@ FALLBACK_IMPLEMENTATION void PLAT_bluetoothStreamBegin(int buffersize) {}
43934436
FALLBACK_IMPLEMENTATION void PLAT_bluetoothStreamEnd() {}
43944437
FALLBACK_IMPLEMENTATION void PLAT_bluetoothStreamQuit() {}
43954438
FALLBACK_IMPLEMENTATION int PLAT_bluetoothVolume() { return 100; }
4396-
FALLBACK_IMPLEMENTATION void PLAT_bluetoothSetVolume(int vol) {}
4439+
FALLBACK_IMPLEMENTATION void PLAT_bluetoothSetVolume(int vol) {}

‎workspace/all/common/api.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,11 @@ int PLAT_supportsDeepSleep(void);
677677
int PLAT_deepSleep(void);
678678
void PLAT_powerOff(int reboot);
679679

680+
void Perf_setCPUMonitorEnabled(int enabled);
681+
int Perf_isCPUMonitorEnabled(void);
682+
int Perf_tryBeginCPUMonitor(void);
683+
void Perf_endCPUMonitor(void);
684+
680685
void *PLAT_cpu_monitor(void *arg);
681686
void PLAT_setCPUSpeed(int speed); // enum
682687
void PLAT_setCustomCPUSpeed(int speed);

‎workspace/all/minarch/minarch.c‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2639,7 +2639,7 @@ static struct Config {
26392639
[FE_OPT_OVERCLOCK] = {
26402640
.key = "minarch_cpu_speed",
26412641
.name = "CPU Speed",
2642-
.desc = "Select the CPU governor profile.\nAuto uses ondemand scaling, Performance\nallows max frequency, Powersave limits\nto a conservative midpoint.",
2642+
.desc = "Choose how the CPU scales.\nAuto is recommended for most users.",
26432643
.default_value = 0,
26442644
.value = 0,
26452645
.count = 3,
@@ -3005,6 +3005,21 @@ static void run_governor_script(const char* script_name) {
30053005
if (ret != 0) LOG_info("WARNING: governor script '%s' exited with status %d\n", script_name, ret);
30063006
}
30073007

3008+
static void updateCPUMonitor(void) {
3009+
Perf_setCPUMonitorEnabled(show_debug);
3010+
if (!show_debug) return;
3011+
3012+
pthread_t cpucheckthread;
3013+
pthread_attr_t attr;
3014+
pthread_attr_init(&attr);
3015+
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
3016+
if (pthread_create(&cpucheckthread, &attr, PLAT_cpu_monitor, NULL) != 0) {
3017+
LOG_info("WARNING: failed to start CPU monitor thread\n");
3018+
Perf_setCPUMonitorEnabled(0);
3019+
}
3020+
pthread_attr_destroy(&attr);
3021+
}
3022+
30083023
static void setOverclock(int i) {
30093024
overclock = i;
30103025
switch (i) {
@@ -3078,7 +3093,9 @@ static void Config_syncFrontend(char* key, int value) {
30783093
i = FE_OPT_OVERCLOCK;
30793094
}
30803095
else if (exactMatch(key,config.frontend.options[FE_OPT_DEBUG].key)) {
3081-
show_debug = value;
3096+
int prev_show_debug = show_debug;
3097+
show_debug = value;
3098+
if (prev_show_debug != show_debug) updateCPUMonitor();
30823099
i = FE_OPT_DEBUG;
30833100
}
30843101
else if (exactMatch(key,config.frontend.options[FE_OPT_MAXFF].key)) {
@@ -8966,13 +8983,6 @@ int main(int argc , char* argv[]) {
89668983
//else
89678984
// LOG_info("asoundrc does not exist at %s\n", asoundpath);
89688985

8969-
pthread_t cpucheckthread;
8970-
pthread_attr_t attr;
8971-
pthread_attr_init(&attr);
8972-
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
8973-
pthread_create(&cpucheckthread, &attr, PLAT_cpu_monitor, NULL);
8974-
pthread_attr_destroy(&attr);
8975-
89768986
if(argc < 2)
89778987
return EXIT_FAILURE;
89788988

@@ -9186,6 +9196,7 @@ int main(int argc , char* argv[]) {
91869196

91879197
finish:
91889198

9199+
Perf_setCPUMonitorEnabled(0);
91899200
run_governor_script("auto_governor.sh"); // restore auto governor on return to menu
91909201

91919202
// Unload game and shutdown RetroAchievements before Core_quit

‎workspace/all/nextui/nextui.c‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2269,11 +2269,6 @@ int main (int argc, char *argv[]) {
22692269
int had_bt = PLAT_btIsConnected();
22702270
int had_sink = GetAudioSink();
22712271

2272-
pthread_t cpucheckthread = 0;
2273-
if (pthread_create(&cpucheckthread, NULL, PLAT_cpu_monitor, NULL) == 0) {
2274-
pthread_detach(cpucheckthread);
2275-
}
2276-
22772272
int selected_row = top->selected - top->start;
22782273
float targetY;
22792274
float previousY;

0 commit comments

Comments
 (0)