platform: simplify setting CORE_COUNT and MAX_CORE_COUNT #10334
Conversation
All normal SOF builds use Zephyr build system, so common settings like system core count can be simply set based on Zephyr build options. There are a few speciality builds left to build SOF without Zephyr. Testbench, cmocka and the SOF ALSA plugin are a few such examples. To keep these builds functional, add a simple definition of key Zephyr definitions (like in this case MP_MAX_NUM_CPUS) to allow these builds to use common build rules even when Zephyr definitions are not available. The old arch/host/Kconfig is removed. It is no longer needed and caused confusion as it added a unconditional default to CORE_COUNT that was used (more or less unintentionally) by some of the speciality build targets. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
requested review from
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the configuration of core counts in the SOF build system by leveraging Zephyr's MP_MAX_NUM_CPUS definition and removing redundant configuration. The changes allow both Zephyr-based builds and specialty non-Zephyr builds (testbench, cmocka, SOF ALSA plugin) to use common build rules.
- Removed platform-specific defaults from
MAX_CORE_COUNTand made it derive fromMP_MAX_NUM_CPUS - Simplified
CORE_COUNTto always default toMP_MAX_NUM_CPUSwith a range constraint - Removed the confusing
arch/host/Kconfigfile that provided an unconditional default
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/platform/Kconfig | Added MP_MAX_NUM_CPUS definition for non-Zephyr builds; simplified MAX_CORE_COUNT and CORE_COUNT to derive from MP_MAX_NUM_CPUS |
| src/arch/host/Kconfig | Removed file entirely to eliminate redundant and confusing CORE_COUNT default |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # For non-Zephyr builds like testbench, cmocka and SOF ALSA plugin, | ||
| # set core count separately. | ||
| # | ||
| if !ZEPHYR_SOF_MODULE |
There was a problem hiding this comment.
where is this defined? isnt this in Zephyr and when SOF is a module? It will go away AFAIK, so we need a better way to detect a zephyr build I guess.
There was a problem hiding this comment.
@nashif Indeed, the two-way dependencies are even more complicated. We do use this ZEPHYR_SOF_MODULE already elsewhere, so we need a replacement anyways.
So in short, when we check out the work/sof/west.yml manifest, and build with
west build --board intel_adsp/ace30/ptl /work/sof/app
We should have some Kconfig enabled to identify this is a west build that includes Zephyr.
There was a problem hiding this comment.
@nashif Ok, CONFIG_ZEPHYR_SOF_MODULES is still ok, it is set even with your zephyrproject-rtos/zephyr#97946 . IOW it is set by Zephyr build system based on the manifest content, and in normal SOF build, it looks like:
$ cat /home/kvehmane/work/build-ptl/zephyr_modules.txt
"sof":"/home/kvehmane/work/sof":"/home/kvehmane/work/sof/zephyr"
"cmsis_6":"/home/kvehmane/work/modules/hal/cmsis_6":"${ZEPHYR_CMSIS_6_CMAKE_DIR}"
"hal_nxp":"/home/kvehmane/work/modules/hal/nxp":"${ZEPHYR_HAL_NXP_CMAKE_DIR}"
"xtensa":"/home/kvehmane/work/modules/hal/xtensa":"/home/kvehmane/pr/work/modules/hal/xtensa"
"mbedtls":"/home/kvehmane/work/modules/crypto/mbedtls":"${ZEPHYR_MBEDTLS_CMAKE_DIR}"
"mipi-sys-t":"/home/kvehmane/work/modules/debug/mipi-sys-t":"/home/kvehmane/work/modules/debug/mipi-sys-t"
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Thinking long term it probably make sense to namespace our Kconfigs to avoid collisions or confusions with Zephyr, e.g. we had a CORE_COUNT collision many year ago.
|
@lrudyX bad DUT ? |
|
V2 pushed:
|
|
The imx95_evk build fails for some reason. I can run this OK locally, but in github job it keeps failing: west build --build-dir build-imx95 --board imx95_evk/mimx9596/m7/ddr /__w/sof/sof/sof/app -- -DEXTRA_CFLAGS=-Werror -DEXTRA_CXXFLAGS=-Werror '-DEXTRA_AFLAGS=-Werror -Wa,--fatal-warnings' --warn-uninitialized -DEXTRA_CONF_FILE=/__w/sof/sof/sof/app/configs/repro-build.conf ... not sure what goes wrong and how it is related to this PR, but seems to be systematic. FYI @thesofproject/nxp |
I believe it's because of zephyrproject-rtos/zephyr#97988. #10319 should do the trick but we'll most likely need a manifest update? LE: can't test right now but I think it's actually because of CONFIG_SOF, which is dropped in this PR? Before zephyrproject-rtos/zephyr#97988 we would set the rimage target for imx95 based on CONFIG_SOF (if set to y then add target, otherwise don't) @kv2019i would it be too inconvenient if we had #10319 go in before this one? LE (2): I think we can make do without the manifest update from #10319 and just have the rimage-related bit go into this PR? This way, we can avoid having to look at the potential CI failures caused by the manifest update itself. If you're ok with this option, I'll double check tomorrow to make sure everything works well. |
|
Thanks a lot @LaurentiuM1234 for quick response and analysis. That is indeed the problem. I was doing this to pave the way for zephyrproject-rtos/zephyr#97946 (do all SOF side changes we can do beforehand). But there's no rush, we can for sure let #10319 go in first. Let me actually split #10334 and #10335 again, so then we can go in order:
We seem to have DUT issues in Intel CI @abonislawski @lrudyX , one DUT is failing all the time (also for this PR). |
kv2019i
force-pushed
the
202510-corecount-refactor
branch
from
c0fc72b to
7ceab0b
Compare
|
V3:
|
Yes, DUT problem has been solved. I see green for this PR. |
8 participants
All normal SOF builds use Zephyr build system, so common settings like system core count can be simply set based on Zephyr build options.
There are a few speciality builds left to build SOF without Zephyr. Testbench, cmocka and the SOF ALSA plugin are a few such examples. To keep these builds functional, add a simple definition of key Zephyr definitions (like in this case MP_MAX_NUM_CPUS) to allow these builds to use common build rules even when Zephyr definitions are not available.
The old arch/host/Kconfig is removed. It is no longer needed and caused confusion as it added a unconditional default to CORE_COUNT that was used (more or less unintentionally) by some of the speciality build targets.