feat: Add Experimental CPU / GPU power control to AYN, Retroid Pocket and Samsung Devices - #1698
feat: Add Experimental CPU / GPU power control to AYN, Retroid Pocket and Samsung Devices#1698joshuatam wants to merge 39 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a pluggable CPU/GPU power-control system with Samsung and PServer drivers, lifecycle integration, power profiles, QuickMenu controls, SDK wiring, documentation, and localized UI strings. ChangesPower Control
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PluviaApp
participant XServerScreen
participant QuickMenu
participant PowerManager
participant PerformanceDriver
PluviaApp->>PowerManager: initialize(context)
XServerScreen->>PowerManager: start()
QuickMenu->>PowerManager: load and apply power settings
PowerManager->>PerformanceDriver: read or write CPU/GPU values
PluviaApp->>PowerManager: stop()
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt (2)
347-373: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCommented-out dead code references undeclared variables.
The commented "Available Frequencies" section references
selectedMinFreq,selectedMaxFreq, and aFrequencyChipcomposable that don't exist in this file. If uncommented, this would fail to compile. Either remove it or add a TODO with a tracking issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt` around lines 347 - 373, Remove the commented-out “Available Frequencies” block near the power-control menu content, since it references undeclared selectedMinFreq, selectedMaxFreq, and FrequencyChip symbols; do not add replacement UI or implementation.
134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer safe scoping over
!!forcpuInfo.The
!!is technically safe due to theelse if (cpuInfo != null)guard, but a safeletscope is more idiomatic and robust against future refactoring:♻️ Proposed refactor
- } else if (cpuInfo != null) { - val info = cpuInfo!! - - if (!hasPServer) { + } else { + cpuInfo?.let { info -> + if (!hasPServer) {This requires adjusting the closing braces for the
letblock.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt` around lines 134 - 135, Update the cpuInfo branch in PowerControlQuickMenuTab to use a safe let-scoped value instead of the forced unwrap in the `else if (cpuInfo != null)` block, and adjust the closing braces accordingly while preserving the existing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/PluviaApp.kt`:
- Around line 241-243: Update the shutdownEnvironment restoration block around
CpuFreqManager.restoreOriginalSettings() to dispatch the synchronous
CPU-frequency restoration work to a background thread instead of running it on
the main thread. Preserve the existing runCatching failure logging within that
background task and keep shutdownEnvironment non-blocking.
In `@app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt`:
- Line 225: Secure writeSysfsFile by validating the value before executing the
root shell command and constructing the echo redirection with safe single-quoted
arguments, preventing shell metacharacters from being interpreted. In
setGovernor, validate the requested governor against getAvailableGovernors() and
only invoke writeSysfsFile for an allowed value.
In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt`:
- Line 333: Update QuickMenu’s isPowerControlAvailable initialization so
CpuFreqManager.isPServerAvailable() is not called inside remember during
composition. Load the flag from a LaunchedEffect running on Dispatchers.IO,
preserving the resulting state for the UI and its existing availability
behavior.
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`:
- Around line 219-227: Update the profile-selection onClick handler in
PowerControlQuickMenuTab so CpuFreqManager.setGovernor, setMinFrequency,
setMaxFrequency, and getCpuInfo run within a coroutine on Dispatchers.IO. Keep
selectedProfileName and isProfileDropdownExpanded state updates on the UI
thread, applying the refreshed cpuInfo through the appropriate UI-safe state
update after the background work completes.
- Around line 54-55: Change isInitialized in the quick-menu state declaration to
use remember rather than rememberSaveable, while leaving selectedProfileName
saveable. Ensure initialization can run again after process recreation so
CpuFreqManager.saveOriginalSettings() captures fresh in-memory CPU state.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`:
- Around line 347-373: Remove the commented-out “Available Frequencies” block
near the power-control menu content, since it references undeclared
selectedMinFreq, selectedMaxFreq, and FrequencyChip symbols; do not add
replacement UI or implementation.
- Around line 134-135: Update the cpuInfo branch in PowerControlQuickMenuTab to
use a safe let-scoped value instead of the forced unwrap in the `else if
(cpuInfo != null)` block, and adjust the closing braces accordingly while
preserving the existing logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60e80b4f-3c29-49e6-a644-a89377cee860
📒 Files selected for processing (22)
app/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/powercontrol/CpuFreqManager.ktapp/src/main/java/app/gamenative/powercontrol/CpuGovernor.ktapp/src/main/java/app/gamenative/powercontrol/PServerExecutor.ktapp/src/main/java/app/gamenative/powercontrol/PowerProfile.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:81">
P3: A device with a matching governor but non-profile frequency bounds is labeled Power Save/Balanced/Performance instead of Custom. Match governor and both current frequency bounds before selecting a named profile.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:88">
P3: CPU read failures and fallback labels bypass Android resources, so localized builds show English text. Move these messages and `N/A` to string resources.
(Based on your team's feedback about hardcoded UI strings.) [FEEDBACK_USED].</violation>
<violation number="3" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:183">
P2: Devices with no PServer still expose enabled profile and governor controls despite the warning that edits cannot be made. Gate both click handlers with `hasPServer` so unavailable controls cannot be opened or selected.</violation>
<violation number="4" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:220">
P2: The profile label can report a new selection after one or more writes fail, hiding an unchanged or partially changed CPU configuration. Update selection only after all writes succeed and show a failure state otherwise.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:29">
P2: `saveOriginalSettings()` uses a non-atomic check-then-act pattern on the `originalGovernor == null` guard. Since `CpuFreqManager` is a singleton object whose state is accessed from both the UI thread (first-use save) and the environment shutdown path (restore), two threads passing the null check before either writes would result in the second overwriting the first, or a write from one thread being invisible to the other due to the lack of a `@Volatile` or synchronization. This would cause `restoreOriginalSettings()` to restore a partial or incorrect set of original values.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:48">
P1: Restoring a higher original minimum after a low-power cap can fail because the current maximum is still lower. Apply limits in bound-safe order—raise max before raising min, and lower min before lowering max—and retain/rollback state on failures.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:60">
P1: After the first successful teardown, later sessions keep restoring the stale first-session baseline instead of their own original CPU state. Clear the saved values after a successful restore so the next `saveOriginalSettings()` records a new baseline.</violation>
<violation number="4" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:78">
P1: On multi-cluster devices, profile application and rollback can partially fail or overwrite clusters with policy-0 values. Track governor and limits per cpufreq policy (using each policy's related CPUs) and apply only values supported by that policy.</violation>
<violation number="5" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:80">
P2: Missing or unreadable cpufreq data is reported as a valid zero-valued CPU instead of an unsupported device. Require readable, valid governor/min/max values and return `null` otherwise, preventing controls and rollback from using empty/zero settings.</violation>
<violation number="6" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:113">
P2: Devices without the optional discrete-frequency attribute expose no power profiles even when current min/max limits are available. Fall back to readable scaling min/max limits (or another supported frequency source) before declaring the frequency list empty.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/component/QuickMenu.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/QuickMenu.kt:333">
P2: `CpuFreqManager.isPServerAvailable()` triggers lazy initialization of `PServerExecutor`, whose constructor performs a synchronous `ServiceManager.getService()` binder lookup via reflection. Running this inside `remember {}` blocks the composition thread on first access. Load this flag from a `LaunchedEffect` on `Dispatchers.IO` instead, defaulting to `false` until the check completes.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:225">
P2: Rapidly choosing profiles/governors can leave CPU settings from an older selection while the UI shows the newer one. Serialize or cancel an in-flight apply operation, and disable selectors until its governor/frequency writes and refresh complete.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
04bb113 to
dbd5efc
Compare
joshuatam
changed the title
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/powercontrol/PowerManager.kt (1)
88-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
isPServerAvailable()— it returns true for Samsung devices too.The method delegates to
getDriver().isDriverSupported(), which returnstruefor any supported driver (Samsung, PServer), not just PServer. The name misleads callers into thinking this is a PServer-specific check.♻️ Proposed rename
- fun isPServerAvailable(): Boolean { + fun isPowerControlAvailable(): Boolean { return getDriver().isDriverSupported() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/powercontrol/PowerManager.kt` around lines 88 - 90, Rename isPServerAvailable() to a driver-agnostic name that reflects getDriver().isDriverSupported() returning true for any supported driver, and update all callers and references to use the new name consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/PluviaApp.kt`:
- Around line 243-244: Wrap the PowerManager.stop() call in the teardown
sequence with runCatching, matching the per-step failure isolation described by
the nearby comment. Keep the subsequent cleanup steps—nulling xEnvironment,
clearing ActiveGameRegistry, and resetting SteamService state—executing even if
stopping the performance driver throws.
In `@app/src/main/java/app/gamenative/powercontrol/drivers/PerformanceDriver.kt`:
- Around line 93-101: Replace the separate setMinCpuValue and setMaxCpuValue
operations with a single atomic CPU-range setter in PerformanceDriver,
implementing safe write ordering and rollback when either bound update fails.
Update the profile apply path in PowerControlQuickMenuTab to call this setter
once and handle its success result, preserving consistent min/max bounds on
failure.
In `@app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt`:
- Around line 35-36: Update PServerDriver’s modifiedSysfsFiles tracking and the
related stop()/PowerManager shutdown flow to snapshot each policy’s original
governor, min/max frequencies, modes, and permissions before its first write.
Serialize mutations with cleanup, and await lifecycle shutdown completion before
restoring every captured value exactly; do not select an arbitrary governor or
retain paths alone.
- Around line 497-505: Update the Binder transaction handling in the command
execution flow used by writeGpuPowerLevel() and writeSysfsFile() so the boolean
result of binder.transact() is validated. When transact() returns false, return
Result.failure with an appropriate exception instead of decoding the reply or
reporting success; preserve the existing success path for accepted transactions
and exception handling for thrown errors.
- Around line 372-394: Update setGpuValue() to write a writable devfreq control
endpoint instead of "$GPU_DEVFREQ_PATH/cur_freq", using the supported
userspace/set_freq or min/max frequency path and preserving the existing unit
conversion and result handling. Update isGpuSupported() so it returns true only
when the selected writable endpoint exists, keeping unsupported devices on the
existing false path.
In `@app/src/main/java/app/gamenative/powercontrol/PowerManager.kt`:
- Line 16: Add `@Volatile` to the driver field and update getDriver() so creating
a NoOpPerformanceDriver does not assign it back to driver. Preserve initialize()
as the sole path that stores the real PerformanceDriver and ensure getDriver()
returns the fallback only when no initialized driver is visible.
- Around line 36-38: Update the PowerManager driver-selection logic to avoid
constructing PServerDriver twice: move the PServerDriver support check into the
fallback else branch, instantiate it once there, and reuse that instance for
both isDriverSupported() and the returned driver. Preserve the existing Samsung
branch behavior and logging.
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`:
- Around line 365-366: Update the frequency slider rendering around the
availableFrequencies and corresponding GPU frequency check to render the
localized “N/A” fallback when the frequency list is empty, instead of omitting
the field. Preserve the existing slider rendering for non-empty lists and use
the project’s established localization resource for “N/A”.
- Around line 54-57: Update PowerControlQuickMenuTab to apply the optional
focusRequester to the first interactive control in the tab, using the
appropriate focusRequester modifier while preserving existing behavior when it
is null. Ensure the supplied powerItemFocusRequester from QuickMenu.kt routes
controller/keyboard focus into that control.
- Around line 288-300: Serialize all power-control mutations in
PowerControlQuickMenuTab, including the profile-selection handler and the
additional mutation sites at the referenced ranges, through one shared queue or
manager operation. Ensure each selection’s governor, min/max CPU values, GPU
settings, and subsequent cpuInfo/gpuInfo refresh execute in order so older jobs
cannot overwrite newer selections or displayed state.
- Around line 155-160: Update the error handling in the CPU frequency and
related power-control paths around the visible catch blocks to log the caught
exception for diagnostics, while assigning localized, user-safe generic messages
to errorMessage. Replace both hardcoded failure strings and the interpolated
exception message with existing string resources or the project’s established
localization mechanism, without exposing e.message to users.
- Around line 136-146: Update the profile matching logic in the initialization
block of PowerControlQuickMenuTab so it compares each profile’s governor and
minimum/maximum frequency values against info.currentGovernor,
info.currentMinValue, and info.currentMaxValue. Only assign the matching profile
name when all relevant bounds match; otherwise retain PerformancePreset.CUSTOM.
- Around line 573-577: Remove the extra unmatched closing brace at the end of
the composable/function block, preserving the braces required by the surrounding
scopes so the file parses successfully.
- Around line 366-396: Handle the single-frequency case before rendering the min
and other frequency sliders in this block. Ensure Slider receives a non-negative
steps value, such as clamping the calculated steps to zero, while preserving the
existing range and selection behavior for lists with multiple frequencies.
- Around line 294-299: Update PowerManager to provide a setCpuRange(min, max)
operation that reads the current CPU bounds and applies
setMinCpuValue/setMaxCpuValue in an order that avoids invalid intermediate
ranges. Replace the separate calls in the coroutine within
PowerControlQuickMenuTab with setCpuRange(profile.minFreq, profile.maxFreq),
preserving the subsequent CPU info refresh.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/powercontrol/PowerManager.kt`:
- Around line 88-90: Rename isPServerAvailable() to a driver-agnostic name that
reflects getDriver().isDriverSupported() returning true for any supported
driver, and update all callers and references to use the new name consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8877a87-7f46-46c7-8601-fe9bbcbb2f92
⛔ Files ignored due to path filters (1)
app/src/main/lib/perfsdk-v1.0.0.jaris excluded by!**/*.jar
📒 Files selected for processing (29)
app/build.gradle.ktsapp/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/powercontrol/PowerManager.ktapp/src/main/java/app/gamenative/powercontrol/PowerProfile.ktapp/src/main/java/app/gamenative/powercontrol/README.mdapp/src/main/java/app/gamenative/powercontrol/drivers/NoOpPerformanceDriver.ktapp/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.ktapp/src/main/java/app/gamenative/powercontrol/drivers/PerformanceDriver.ktapp/src/main/java/app/gamenative/powercontrol/drivers/SamsungPerformanceDriver.ktapp/src/main/java/app/gamenative/powercontrol/profiles/CpuGovernor.ktapp/src/main/java/app/gamenative/powercontrol/profiles/PerformancePreset.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (10)
- app/src/main/res/values-pl/strings.xml
- app/src/main/res/values-fr/strings.xml
- app/src/main/res/values-da/strings.xml
- app/src/main/res/values-uk/strings.xml
- app/src/main/res/values-es/strings.xml
- app/src/main/java/app/gamenative/powercontrol/PowerProfile.kt
- app/src/main/res/values-ja/strings.xml
- app/src/main/res/values-ro/strings.xml
- app/src/main/res/values-de/strings.xml
- app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
| // Stop performance driver | ||
| PowerManager.stop() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Wrap PowerManager.stop() in runCatching for consistency with the teardown design.
The comment at line 233 states "per-step catch so one failing teardown doesn't prevent the rest from running," yet PowerManager.stop() is the only cleanup step not wrapped in runCatching. If it throws, the remaining cleanup (nulling xEnvironment, clearing ActiveGameRegistry, resetting SteamService state) is skipped. Current driver implementations catch their own exceptions, but a future driver might not.
🛡️ Proposed fix
- // Stop performance driver
- PowerManager.stop()
+ runCatching { PowerManager.stop() }
+ .onFailure { Timber.e(it, "shutdownEnvironment: PowerManager.stop") }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Stop performance driver | |
| PowerManager.stop() | |
| // Stop performance driver | |
| runCatching { PowerManager.stop() } | |
| .onFailure { Timber.e(it, "shutdownEnvironment: PowerManager.stop") } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/app/gamenative/PluviaApp.kt` around lines 243 - 244, Wrap
the PowerManager.stop() call in the teardown sequence with runCatching, matching
the per-step failure isolation described by the nearby comment. Keep the
subsequent cleanup steps—nulling xEnvironment, clearing ActiveGameRegistry, and
resetting SteamService state—executing even if stopping the performance driver
throws.
3e92b19 to
c9f7610
Compare
|
@joshuatam would you be willing to investigate a sort of "automatic" profile that tries to keep FPS at the target you set? |
Yeah, I am referencing the concept in the Pulse implementation, will add support for fan control and AutoTDP profile next. |
I would strongly recommend doing a clean-slate implementation. AutoTDP has a lot of issues and is a very naive (and vibecoded) implementation. Really looking forward to seeing this merged, awesome PR. |
|
Really excited about this feature, thanks for putting it together. The driver setup and the README made it easy to follow. One small thing I noticed while reading, and I might be missing context so please correct me. In /** Set GPU minimum power level (0 = fastest, higher = slower) */
abstract fun setMinGpuPowerLevel(level: Int): BooleanBut the README and the PServer implementation seem to treat it the other way, where 0 is the lowest and higher means better performance (the Same kind of thing in the README's "Display Units" section. It lists Could be I'm missing something here, so please let me know. Glad to help with a small fix if it helps. |
|
Been thinking about this one a bit more and wanted to ask, again I might be missing something. While a game is running, the CPU files end up read-only because each write finishes with a val command = "chmod 644 '$path'; echo '$value' > '$path'; chmod 444 '$path'"They get set back to A related small thing in the same area: on shutdown the governor is restored to the first one in the list rather than whatever the device was using before: val defaultGovernor = availableGovernors.first()
setGovernor(defaultGovernor)I was wondering if snapshotting the original governor and min/max when the driver starts, and restoring those, might cover both cases more cleanly. Could well be there''s a reason it''s done this way that I''m not seeing though, so I''d love to hear your thinking. Happy to help out with it either way. |
There was a problem hiding this comment.
6 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:73">
P1: A failed profile application is saved as if it succeeded, so the next session can restore settings never applied to this session. Update `currentProfile` only after a successful `PowerManager.update` (or restore the prior profile on failure).</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuContent.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuContent.kt:171">
P3: Section headers remain English in localized Quick Menus because these labels bypass string resources. Add localized resources for Profile, CPU, and GPU and resolve them with `stringResource`.
(Based on your team's feedback about not hardcoding UI strings.) [FEEDBACK_USED]</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuContent.kt:310">
P2: A single reported frequency crashes composition because this expression evaluates to `-1` for `Slider.steps`. Clamp step counts to zero; apply the same change to both CPU sliders and the GPU frequency slider.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt:123">
P2: A failed batched profile restore can leave GPU limits changed while CPU changes were never executed. Queue GPU power-level writes under `isBatchMode` too, so `commit()` applies the profile consistently.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt:340">
P2: An aborted launch can erase the user's saved profile because `stop()` saves an empty string when no profile was initialized. Leave the existing preference untouched when `currentProfile` is null.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt:355">
P2: First launch with no persisted profile never applies the Balanced defaults: this branch only assigns `currentProfile`, then returns. Apply the default profile through `update` before returning so UI state and CPU/GPU state agree.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| Timber.d("Applying profile: $profile") | ||
|
|
||
| // Update PowerManager's current profile reference immediately | ||
| PowerManager.setCurrentProfile(profile) |
There was a problem hiding this comment.
P1: A failed profile application is saved as if it succeeded, so the next session can restore settings never applied to this session. Update currentProfile only after a successful PowerManager.update (or restore the prior profile on failure).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt, line 73:
<comment>A failed profile application is saved as if it succeeded, so the next session can restore settings never applied to this session. Update `currentProfile` only after a successful `PowerManager.update` (or restore the prior profile on failure).</comment>
<file context>
@@ -1,176 +1,134 @@
+ Timber.d("Applying profile: $profile")
+
+ // Update PowerManager's current profile reference immediately
+ PowerManager.setCurrentProfile(profile)
+
+ val success = PowerManager.update {
</file context>
| onMinFreqChanged(selectedMinFreqIndex) | ||
| }, | ||
| valueRange = 0f..(state.cpuInfo.availableFrequencies.size - 1).toFloat(), | ||
| steps = state.cpuInfo.availableFrequencies.size - 2, |
There was a problem hiding this comment.
P2: A single reported frequency crashes composition because this expression evaluates to -1 for Slider.steps. Clamp step counts to zero; apply the same change to both CPU sliders and the GPU frequency slider.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuContent.kt, line 310:
<comment>A single reported frequency crashes composition because this expression evaluates to `-1` for `Slider.steps`. Clamp step counts to zero; apply the same change to both CPU sliders and the GPU frequency slider.</comment>
<file context>
@@ -0,0 +1,458 @@
+ onMinFreqChanged(selectedMinFreqIndex)
+ },
+ valueRange = 0f..(state.cpuInfo.availableFrequencies.size - 1).toFloat(),
+ steps = state.cpuInfo.availableFrequencies.size - 2,
+ modifier = Modifier.weight(1f)
+ )
</file context>
| * Begin a batch update session. | ||
| * Collects commands to execute in a single root call for better performance. | ||
| */ | ||
| override fun beginUpdate() { |
There was a problem hiding this comment.
P2: A failed batched profile restore can leave GPU limits changed while CPU changes were never executed. Queue GPU power-level writes under isBatchMode too, so commit() applies the profile consistently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt, line 123:
<comment>A failed batched profile restore can leave GPU limits changed while CPU changes were never executed. Queue GPU power-level writes under `isBatchMode` too, so `commit()` applies the profile consistently.</comment>
<file context>
@@ -107,6 +116,102 @@ class PServerDriver : PerformanceDriver() {
+ * Begin a batch update session.
+ * Collects commands to execute in a single root call for better performance.
+ */
+ override fun beginUpdate() {
+ batchCommands.clear()
+ batchFilePaths.clear()
</file context>
| try { | ||
| val json = if (currentProfile != null) { | ||
| Json.encodeToString(currentProfile) | ||
| } else "" |
There was a problem hiding this comment.
P2: An aborted launch can erase the user's saved profile because stop() saves an empty string when no profile was initialized. Leave the existing preference untouched when currentProfile is null.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/PowerManager.kt, line 340:
<comment>An aborted launch can erase the user's saved profile because `stop()` saves an empty string when no profile was initialized. Leave the existing preference untouched when `currentProfile` is null.</comment>
<file context>
@@ -191,13 +308,75 @@ object PowerManager {
+ try {
+ val json = if (currentProfile != null) {
+ Json.encodeToString(currentProfile)
+ } else ""
+ PrefManager.powerControlProfile = json
+ Timber.tag("PowerManager").d("Saved power profile: $json")
</file context>
| } else "" | |
| } else return |
| try { | ||
| val json = PrefManager.powerControlProfile | ||
| if (json.isEmpty()) { | ||
| currentProfile = driver?.getDefaultProfile() |
There was a problem hiding this comment.
P2: First launch with no persisted profile never applies the Balanced defaults: this branch only assigns currentProfile, then returns. Apply the default profile through update before returning so UI state and CPU/GPU state agree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/PowerManager.kt, line 355:
<comment>First launch with no persisted profile never applies the Balanced defaults: this branch only assigns `currentProfile`, then returns. Apply the default profile through `update` before returning so UI state and CPU/GPU state agree.</comment>
<file context>
@@ -191,13 +308,75 @@ object PowerManager {
+ try {
+ val json = PrefManager.powerControlProfile
+ if (json.isEmpty()) {
+ currentProfile = driver?.getDefaultProfile()
+ Timber.tag("PowerManager").d("No saved profile to restore")
+ return
</file context>
| } | ||
| } | ||
|
|
||
| SectionHeader(title = "Profile") |
There was a problem hiding this comment.
P3: Section headers remain English in localized Quick Menus because these labels bypass string resources. Add localized resources for Profile, CPU, and GPU and resolve them with stringResource.
(Based on your team's feedback about not hardcoding UI strings.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuContent.kt, line 171:
<comment>Section headers remain English in localized Quick Menus because these labels bypass string resources. Add localized resources for Profile, CPU, and GPU and resolve them with `stringResource`.
(Based on your team's feedback about not hardcoding UI strings.) </comment>
<file context>
@@ -0,0 +1,458 @@
+ }
+ }
+
+ SectionHeader(title = "Profile")
+
+ Text(
</file context>
2026-07-14.18-41-24.mp4adding in the recording for Samsung devices |
Thank you for your comments, there are some more changes about apply / restore during driver start / stop, please take a look. The README and code comments was generated with the aid of AI during implementation, could be outdated as i didn't always check / regenerate. |
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/PowerManager.kt:372">
P3: Out-of-range RAM levels are persisted as requested even though Samsung clamps and applies a different level. Keep `currentProfile` in sync with the driver's effective level (for both min and max), so saved profiles and subsequent restores reflect hardware state.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| val result = getDriver().setMinBusLevel(level) | ||
|
|
||
| if (result) { | ||
| currentProfile?.minBusLevel = level |
There was a problem hiding this comment.
P3: Out-of-range RAM levels are persisted as requested even though Samsung clamps and applies a different level. Keep currentProfile in sync with the driver's effective level (for both min and max), so saved profiles and subsequent restores reflect hardware state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/PowerManager.kt, line 372:
<comment>Out-of-range RAM levels are persisted as requested even though Samsung clamps and applies a different level. Keep `currentProfile` in sync with the driver's effective level (for both min and max), so saved profiles and subsequent restores reflect hardware state.</comment>
<file context>
@@ -326,6 +342,49 @@ object PowerManager {
+ val result = getDriver().setMinBusLevel(level)
+
+ if (result) {
+ currentProfile?.minBusLevel = level
+ }
+
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 14 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt:333">
P2: A rapid game restart can lose the new session's CPU pinning and policy cache when the prior asynchronous `stop()` reaches this reset afterward. Serialize start/stop cleanup or scope cleanup to the session that initiated it.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| modifiedSysfsFiles.clear() | ||
|
|
||
| // Reset app process CPU affinity to all cores | ||
| resetAppCpuAffinity() |
There was a problem hiding this comment.
P2: A rapid game restart can lose the new session's CPU pinning and policy cache when the prior asynchronous stop() reaches this reset afterward. Serialize start/stop cleanup or scope cleanup to the session that initiated it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/drivers/PServerDriver.kt, line 333:
<comment>A rapid game restart can lose the new session's CPU pinning and policy cache when the prior asynchronous `stop()` reaches this reset afterward. Serialize start/stop cleanup or scope cleanup to the session that initiated it.</comment>
<file context>
@@ -284,6 +328,13 @@ class PServerDriver(private val context: Context? = null) : PerformanceDriver()
modifiedSysfsFiles.clear()
+
+ // Reset app process CPU affinity to all cores
+ resetAppCpuAffinity()
+
+ // Clear CPU policies and clusters to force re-discovery on next start()
</file context>
de345a2 to
82fd699
Compare
… and Samsung Devices
Introduces a new "Power Control" feature, enabling users to fine-tune CPU governors,
frequencies, and GPU performance levels directly from the Quick Menu.
This system is built on an extensible driver architecture, automatically
detecting and integrating with device-specific mechanisms. It currently
supports:
* **PServer-enabled devices**: Such as AYN Odin and Retroid Pocket, by
interfacing with sysfs paths for CPU and Adreno GPU control.
* **Samsung Galaxy devices**: Utilizing the Samsung Performance SDK for CPU and
GPU performance level adjustments.
Performance controls are initialized at app startup, activated when a game
environment starts, and gracefully restored to default settings upon game
shutdown. A new "Power Control" tab provides a dedicated UI for managing
settings and applying power profiles.
following the SDK and the other methods already implemented, calls using TYPE_BUS_MIN and TYPE_BUS_MAX also added a few minor corrections to default values and comments on samsung driver fixed GPU power slider not showing for samsung
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/powercontrol/autotuning/PerformanceAutoTuner.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/autotuning/PerformanceAutoTuner.kt:295">
P3: Samsung sessions submit two identical performance-lock updates every tuning cycle once the discretized bus level is stable. Cache the last applied bus level and invoke this callback only when that level changes.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| val busLevel = targetLevel.coerceIn(0, numBusLevels - 1) | ||
|
|
||
| // Apply bus level change | ||
| onBusLevelChange(busLevel) |
There was a problem hiding this comment.
P3: Samsung sessions submit two identical performance-lock updates every tuning cycle once the discretized bus level is stable. Cache the last applied bus level and invoke this callback only when that level changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/autotuning/PerformanceAutoTuner.kt, line 295:
<comment>Samsung sessions submit two identical performance-lock updates every tuning cycle once the discretized bus level is stable. Cache the last applied bus level and invoke this callback only when that level changes.</comment>
<file context>
@@ -239,6 +263,46 @@ class PerformanceAutoTuner(
+ val busLevel = targetLevel.coerceIn(0, numBusLevels - 1)
+
+ // Apply bus level change
+ onBusLevelChange(busLevel)
+
+ if (enableLogging) {
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…ntal-power-control
…atibility - Enhance process identification by switching to `ps` command output parsing. - Dynamically detect the correct `taskset` mask format (e.g., "0xf8" vs "f8") to improve system compatibility. - Expand the scope of pinned processes to include `libsteamstrap.so` and rename `pinWineInfrastructure` to `pinBackgroundProcesses`.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 21 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…Retroid Pocket, Samsung) Resolved strings.xml tail conflict (kept both language strings and power-control strings).
…ntal-power-control
…ntal-power-control
…ntal-power-control
4 participants
Description
Introduces a comprehensive Power Control system that enables fine-grained performance tuning for Android gaming devices through an extensible driver architecture. The system provides CPU governor control, frequency scaling, GPU performance management, process affinity optimization, and intelligent automatic performance tuning.
Supported Devices
The driver architecture automatically detects and integrates with device-specific mechanisms:
Key Features:
CPU Pinning / Process Affinity Control:
Automatic app process pinning to efficiency cores (frees performance cores for games)(Removed due to possible ANR happening)GameMode-Inspired Optimizations:
Automatic Performance Tuning:
Performance controls are initialized at app startup, activated when a game environment starts, and gracefully restored to default settings upon game shutdown. A new "Power Control" tab provides a dedicated UI for managing settings and applying power profiles.
All works are based on the following open source projects, thank you!
https://github.com/AurelioB/ClusterTune
https://github.com/keiretrogaming/pulse
https://github.com/FeralInteractive/gamemode
Recording
screen-20260720-073625.2.mp4
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds an experimental Power Control tab with CPU/GPU and RAM/bus tuning for AYN/Retroid and Samsung devices. Adds an auto‑tuner to hold target FPS, profile save/restore, and cluster‑aware CPU affinity for more reliable game/audio/background pinning.
New Features
perfsdk-v1.0.0.jar; initialize at app start; start after environment launch; stop on shutdown.Bug Fixes
ps; auto‑detecttasksetmask format; adapt to CPU cluster count; improved game process detection with longer retry.Written for commit cead5cc. Summary will update on new commits.
Summary by CodeRabbit