Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support setting the platform profile #517

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MithicSpirit
Copy link

@MithicSpirit MithicSpirit commented Jan 29, 2025

See commit messages for more details.

This adds support for setting the platform profile via /sys/firmware/acpi/platform_profile. It behaves very similarly to the governor, but with just one file.

Note that setting the platform profile may change other options (like the governor) and prevent them from being altered further. Thus, this also necessitates a small refactor where the initial values are stored before any changes.

The tests for platform profile treat it as required (set the status to -1 if they fail). However, I'm not sure whether this should be the case, as I don't know how common this feature is yet.

My nitpicking also got the best of me, so I extracted split lock mitigation stuff into common-splitlock.c. For now, this really only matters for the path, but, in theory, procsysctl could be altered to also allow getting the split lock mitigation state, rather than just setting it. Please let me know if this change is undesired, as I can just remove the last commit if so.

Closes #516

@MithicSpirit MithicSpirit force-pushed the platform-profile branch 2 times, most recently from a169ed5 to 525dc55 Compare January 29, 2025 18:43
The platform profile lives in /sys/firmware/acpi/platform_profile. The
desiredprof and defaultprof options correspond to the values for the
platform profile set when gamescope begins and ends, respectively.

HACK: The platform profile may restrict what values the governor can
take, so we choose to set the governor before the platform profile in
order to store the correct default governor, and restore the platform
profile before the governor. This is done to maximize correctness after
restoration, but it can cause issues if the previous platform profile
restricts the governor.

TODO: Save all the state we care about before any of it is changed. In
thsi case, we should set (and restore) the platform profile before the
governor.
Note that the platform profile feature is considered required. Maybe
this shouldn't be the case, as I'm not sure how standard this feature is
yet.
The platform profile may restrict what values the governor can take, so
we need to save all state first to ensure the restored state is correct.
Then, the platform profile is the first thing we set (and restore) to
ensure all other changes are made to the best of our ability.
This reduces duplication of the split lock mitigation path.
Additionally, it allows extending the functionality of procsysctl into
also getting the split lock mitigation state.
@afayaz-feral
Copy link
Contributor

I was in the process of looking through this, and just saw that you made some further changes.
Do you want to make this PR a draft for now?

@MithicSpirit
Copy link
Author

I was in the process of looking through this, and just saw that you made some further changes. Do you want to make this PR a draft for now?

I just forgot about gamemoded -t and had to add checks for that. I've been running this on my system for a few days with no problems, so unless there's another oversight like that, I don't think it needs to be a draft. Sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for platform profile
2 participants