-
Notifications
You must be signed in to change notification settings - Fork 127
Refactor thermald to support non-Intel platforms #523
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
base: master
Are you sure you want to change the base?
Conversation
|
@idlethread please review this PR. |
src/thd_engine.cpp
Outdated
| return cthd_platform_intel::check_cpu_id_intel(proc_list_matched); | ||
| } else if (cthd_platform::is_arm_platform()) { | ||
| thd_log_info("Calling ARM platform CPU ID check\n"); | ||
| return cthd_platform_arm::check_cpu_id_arm(proc_list_matched); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any value in doing CPU ID checks for ARM platforms. Perhaps limit it to just intel platforms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that can be done. Just to be clear, are you suggesting that proc_list_matched be set to true directly at line 758, instead of setting it in check_cpu_id_arm, and then removing check_cpu_id_arm altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the new suggestion from spandruvada , this part of the code is now deprecated.
Please let me know if you have any feedback on the latest patch set.
|
Good, you have Qualcomm sign off in the commits. Otherwise it would have been problematic. |
|
Hi Amit, I have no way to verify changes (done by me or others in the community) on ARM devices. |
We'd be happy to help run the test suite locally where applicable on our platforms. We'll need to understand your release frequency and process though. |
spandruvada
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may have more ARM SoC with different detection or may have other goodies. Thermald uses virtual classes instead of adding checks. Can you check if you can refactor like this? Better to avoid static.
class cthd_platform {
public:
virtual void detect_platform() {
}
// Any other you may need
};
// intel_platform.cpp
class intel_platform : public cthd_platform {
public:
void detect_platform() override {
}
};
// arm_platform.cpp
class arm_platform : public cthd_platform {
public:
void detect_platform() override {
}
};
|
Thank you for the feedback. The virtual class approach makes sense for better extensibility. I'll refactor the platform detection accordingly and submit the updated version soon. |
Thermald currently only supports x86 platforms but is a very useful tool for controlling user-space thermal policy on a wide range of systems. As more SoCs and architectures adopt user-space driven thermal management models, the limitation to x86-only backends prevents thermald from being used on non-x86 platforms. This series adds support for non-x86 platforms by first refactoring all x86-specific logic into a dedicated backend. This separation enables a clean and modular structure where additional platform backends can be introduced without impacting the existing Intel implementation. Once the backend abstraction is in place, subsequent patches add support for ARM backends and integrate them into the new platform detection logic. Suggested-by: Amit Kucheria <[email protected]> Signed-off-by: Priyansh Jain <[email protected]>
The initialization flow currently invokes platform_match without calling parser_init first. As a result, the platform matching logic runs with no parsed configuration data available. This can cause incorrect platform detection, or failure to load the expected thermal configurations. This patch adds the missing parser_init() call before platform_match(), ensuring that configuration files are properly parsed and populated before any platform-specific matching occurs. This makes initialization reliable and aligns with the intended dependency order. Suggested-by: Amit Kucheria <[email protected]> Signed-off-by: Priyansh Jain <[email protected]>
thermald historically supported only Intel platforms. As the codebase is being refactored to allow multi-platform support, ARM platforms require their own backend implementation to handle platform-specific thermal controls, capabilities, and configuration rules. This patch adds the initial ARM-specific backend files and integrates ARM selection into the newly introduced platform detection mechanism. With this change, thermald can correctly identify ARM systems and route thermal management operations to the appropriate backend. This establishes the foundation needed for future ARM thermal features and expands thermald's usefulness beyond Intel-based platforms. Suggested-by: Amit Kucheria <[email protected]> Signed-off-by: Priyansh Jain <[email protected]>
7086e11 to
93cee8c
Compare
|
Always run |
|
Applied changes to testing_next branch. Please check. |
|
We have tested the testing_next branch , it is working as expected. |
This series introduces a platform‑agnostic backend structure in thermald, refactoring Intel‑specific logic and fixing initialization order issues. It also adds initial ARM backend support and platform detection, enabling thermald to operate correctly on non‑Intel systems.