Skip to content

Start decentralizing zephyr/CMakeLists.txt #8422

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.

Sign up for GitHub

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

Merged
lgirdwood merged 2 commits into from
Nov 8, 2023
Merged

Start decentralizing zephyr/CMakeLists.txt #8422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6028c5
cmake/zephyr: decentralize src/init/
marc-hb Oct 31, 2023
3e1ca7e
cmake/zephyr: decentralize src/ipc/
marc-hb Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions scripts/cmake/misc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ function(get_optimization_flag OUT_VAR)
endif()
endfunction()

# Zephyr duplicate in sof/zephyr/CMakeLists.txt; keep in sync
macro(is_zephyr ret)
if(CONFIG_ZEPHYR_SOF_MODULE)
set(${ret} TRUE)
else()
set(${ret} FALSE)
endif()
endmacro()

# Adds sources to target like target_sources, but assumes that
# paths are relative to subdirectory.
# Works like:
Expand Down
16 changes: 16 additions & 0 deletions src/init/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
# SPDX-License-Identifier: BSD-3-Clause

is_zephyr(it_is)
if(it_is) ### Zephyr ###


zephyr_library_sources(

@andyross andyross Oct 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a spot where Zephyr's promiscuous cmake layer hurts pretty badly, because you can't add files to the project in the same way in both modes. Again, maybe work around this by adding yet another layer of indirection and have a "sof_sources()".

Also, FWIW: the "_library_sources" variant really doesn't do anything useful beyond make the build slower by taking the time to create a static library. There is (AFAIK) exactly one spot in Zephyr where we have to do this for obscure linkage reasons[1], but for general C code it's almost always faster and cleaner to link all the object files at once.

I've tried to make that point upstream and basically failed: "zephyr_library_sources()" is a pervasive paradigm. But really it's needless except in specific situations.

(For reference: the reason to use it is that a static library only presents internal files to the link if one of their symbols is accessed, so you can build it and include static stuff like e.g. SYS_INIT() or interrupt declarations, and avoid having those end up in the final output unless something is using the file. But DTS and kconfig are the right mechanisms for that kind of dependency tracking, doing it with the linker is dangerous and surprising.)

@marc-hb marc-hb Oct 31, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, maybe work around this by adding yet another layer of indirection and have a "sof_sources()".

I thought about this and decided against it. There are already layers of indirection-obfuscation in both XTOS and Zephyr, don't want to stack yet another one on top. A dead simple set(common_files ...) works great and avoids enough duplication.

@marc-hb marc-hb Oct 31, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to make that point upstream and basically failed: "zephyr_library_sources()" is a pervasive paradigm. But really it's needless except in specific situations.

Would you care to change this in sof/zephyr/CMakeLists.txt right now? Don't mind conflicting with this, I'll happily rebase[*] It's a good time now because:

  • we just started the SOF release cycle (and the Zephyr one for that matter)
  • They are all still in one file for now! Quick :-)
  • There is no rush to "de-centralize" yet.

[*] As much as one can be happy resolving git conflicts :-D

@andyross andyross Oct 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I've spent my bikeshed quota for the month already.

The way this goes is that someone says "libraries make linking faster!" and the response is "only after the ranlib stage builds the table, and in zephyr most symbols are actually needed by the final link", then someone else demands benchmarks, etc...

Really I was mostly just whining about a personal peeve. I've made my peace with it: per-directory/per-app fine grained static libraries are the way zephyr links itself.

init.c
ext_manifest.c
)


else() ### Not Zephyr ###


add_local_sources(sof init.c)

add_library(ext_manifest STATIC "")
Expand All @@ -21,3 +34,6 @@ sof_append_relative_path_definitions(ext_manifest)

target_link_libraries(ext_manifest sof_options)
target_link_libraries(sof_static_libraries INTERFACE ext_manifest)


endif() # Zephyr
27 changes: 26 additions & 1 deletion src/ipc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,33 @@ elseif (CONFIG_IPC_MAJOR_4)
add_subdirectory(ipc4)
endif()

add_local_sources(sof
set(base_files
ipc-common.c
ipc-helper.c
)

is_zephyr(it_is)
if(it_is) ### Zephyr ###

zephyr_library_sources(
${base_files}
dma-copy.c
)

if (CONFIG_SOC_SERIES_INTEL_CAVS_V25 OR CONFIG_SOC_SERIES_INTEL_ACE)

@kv2019i kv2019i Nov 6, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks strange, but it matches the existing logic, so great to have this become more visible in the cmake code. The long per-product sections in one-giant-zephyr-cmakefile easily hide these things (when we have per-platform compilation of stuff that really shouldn't be per-platform).

zephyr_library_sources(
ipc-zephyr.c
)
endif()


else() ### Not Zephyr ###


add_local_sources(sof
${base_files}
)

if (CONFIG_TRACE)
add_local_sources(sof
dma-copy.c)
Expand All @@ -19,3 +41,6 @@ endif()
if (CONFIG_LIBRARY)
return()
endif()


endif() # Zephyr
20 changes: 18 additions & 2 deletions src/ipc/ipc3/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
# SPDX-License-Identifier: BSD-3-Clause

add_local_sources(sof
set(common_files
handler.c
helper.c
dai.c
)

is_zephyr(it_is)
if(it_is) ### Zephyr ###

zephyr_library_sources(
${common_files}
host-page-table.c
)

else() ### Not Zephyr ###

add_local_sources(sof
${common_files}
)

if (CONFIG_HOST_PTABLE)
add_local_sources(sof
host-page-table.c)
endif()
endif()

endif() # Zephyr
26 changes: 25 additions & 1 deletion src/ipc/ipc4/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,36 @@
# SPDX-License-Identifier: BSD-3-Clause

add_local_sources(sof
set(common_files
dai.c
handler.c
helper.c
logging.c
notification.c
)


is_zephyr(it_is)
if(it_is) ### Zephyr ###


zephyr_library_sources(
${common_files}
)

zephyr_library_sources_ifdef(CONFIG_AMS
ams_helpers.c
)


else() ### Not Zephyr ####


add_local_sources(sof
${common_files}
ams_helpers.c
)

target_include_directories(sof_options INTERFACE ${RIMAGE_TOP}/src/include)


endif() # Zephyr
55 changes: 23 additions & 32 deletions zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# This is still WIP - Not fully validated on any platform.

# When west is installed, Zephyr's CMake invokes west to list and try to
# compile every Zephyr module that can be found.
# compile every Zephyr module that can be found. See
# sof/zephyr/module.yml and
# https://docs.zephyrproject.org/latest/develop/modules.html
if(CONFIG_SOF)

if(CONFIG_LIBRARY)
Expand Down Expand Up @@ -114,14 +114,30 @@ zephyr_include_directories(
# SOC level sources
# Files that are commented may not be needed.


# New, "de-centralized Zephyr" way. Requires "is_zephyr()" conditionals in
# the decentralized CMakeLists.txt files shared with XTOS.

# XTOS duplicate in sof/scripts/misc.cmake; keep in sync
macro(is_zephyr ret)
if(CONFIG_ZEPHYR_SOF_MODULE)
set(${ret} TRUE)
else()
set(${ret} FALSE)
endif()
endmacro()

add_subdirectory(../src/init/ init_unused_install/)
add_subdirectory(../src/ipc/ ipc_unused_install/)



# Old way below: all .c files added by this giant CMake file.

# Intel TGL and CAVS 2.5 platforms
if (CONFIG_SOC_SERIES_INTEL_CAVS_V25)

# Driver sources
zephyr_library_sources(
${SOF_IPC_PATH}/ipc-zephyr.c
)

zephyr_library_sources_ifdef(CONFIG_INTEL_HDA
${SOF_DRIVERS_PATH}/intel/hda/hda-dma.c
)
Expand All @@ -147,11 +163,6 @@ endif()
# Intel ACE 1.5 and newer platforms
if (CONFIG_SOC_SERIES_INTEL_ACE)

# Driver sources
zephyr_library_sources(
${SOF_IPC_PATH}/ipc-zephyr.c
)

# Platform sources
zephyr_library_sources(
${SOF_PLATFORM_PATH}/intel/ace/platform.c
Expand Down Expand Up @@ -309,9 +320,6 @@ zephyr_include_directories(${SOF_PLATFORM_PATH}/${PLATFORM}/include)
# Mandatory Files used on all platforms.
# Commented files will be added/removed as integration dictates.
zephyr_library_sources(
${SOF_IPC_PATH}/dma-copy.c
${SOF_IPC_PATH}/ipc-common.c
${SOF_IPC_PATH}/ipc-helper.c

# SOF math utilities
${SOF_MATH_PATH}/decibels.c
Expand Down Expand Up @@ -344,8 +352,6 @@ zephyr_library_sources(
${SOF_AUDIO_PATH}/pipeline/pipeline-xrun.c

# SOF core infrastructure - runs on top of Zephyr
${SOF_SRC_PATH}/init/init.c
${SOF_SRC_PATH}/init/ext_manifest.c
${SOF_SRC_PATH}/arch/xtensa/drivers/cache_attr.c
${SOF_SRC_PATH}/schedule/zephyr_domain.c
${SOF_SRC_PATH}/schedule/schedule.c
Expand Down Expand Up @@ -392,20 +398,6 @@ else()
)
endif()

zephyr_library_sources_ifdef(CONFIG_IPC_MAJOR_3
${SOF_IPC_PATH}/ipc3/handler.c
${SOF_IPC_PATH}/ipc3/helper.c
${SOF_IPC_PATH}/ipc3/dai.c
${SOF_IPC_PATH}/ipc3/host-page-table.c
)

zephyr_library_sources_ifdef(CONFIG_IPC_MAJOR_4
${SOF_IPC_PATH}/ipc4/handler.c
${SOF_IPC_PATH}/ipc4/helper.c
${SOF_IPC_PATH}/ipc4/dai.c
${SOF_IPC_PATH}/ipc4/logging.c
${SOF_IPC_PATH}/ipc4/notification.c
)

zephyr_library_sources_ifdef(CONFIG_TRACE
${SOF_SRC_PATH}/trace/dma-trace.c
Expand Down Expand Up @@ -760,7 +752,6 @@ zephyr_library_sources_ifdef(CONFIG_HAVE_AGENT

zephyr_library_sources_ifdef(CONFIG_AMS
${SOF_LIB_PATH}/ams.c
${SOF_IPC_PATH}/ipc4/ams_helpers.c
)

zephyr_library_sources_ifdef(CONFIG_GDB_DEBUG
Expand Down