From ede0bc8af998f94b727c840bd2ce5e3647fc9afb Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 27 Sep 2022 07:24:12 +0000 Subject: [PATCH 1/6] topology1: prefix custom targets with their subdirectory name This makes it possible to find stuff in the output of `ninja help`. Zero effect on the build artefacts. Signed-off-by: Marc Herbert --- tools/topology/topology1/development/CMakeLists.txt | 4 ++-- tools/topology/topology1/dsp_enhancement/CMakeLists.txt | 4 ++-- .../topology/topology1/kernel_dependent/v5.19/CMakeLists.txt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/topology/topology1/development/CMakeLists.txt b/tools/topology/topology1/development/CMakeLists.txt index 973703177b7c..f6760d9edb3c 100644 --- a/tools/topology/topology1/development/CMakeLists.txt +++ b/tools/topology/topology1/development/CMakeLists.txt @@ -96,8 +96,8 @@ foreach(tplg ${TPLGS}) add_alsatplg_command(${output}.conf ${output}.tplg) - add_custom_target(topology_${output} DEPENDS ${output}.tplg) - add_dependencies(dev_topologies1 topology_${output}) + add_custom_target(dev_topology_${output} DEPENDS ${output}.tplg) + add_dependencies(dev_topologies1 dev_topology_${output}) endforeach() # Duplicate of above to handle topologies in parent directory diff --git a/tools/topology/topology1/dsp_enhancement/CMakeLists.txt b/tools/topology/topology1/dsp_enhancement/CMakeLists.txt index 9399b48e5c21..dc0ef468c88b 100644 --- a/tools/topology/topology1/dsp_enhancement/CMakeLists.txt +++ b/tools/topology/topology1/dsp_enhancement/CMakeLists.txt @@ -41,6 +41,6 @@ foreach(tplg ${TPLGS_UP}) add_alsatplg_command(${output}.conf ${output}.tplg) - add_custom_target(topology_${output} DEPENDS ${output}.tplg) - add_dependencies(dsp_topologies1 topology_${output}) + add_custom_target(dsp_topology_${output} DEPENDS ${output}.tplg) + add_dependencies(dsp_topologies1 dsp_topology_${output}) endforeach() diff --git a/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt b/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt index 89154fb9c16f..cf823066e60a 100644 --- a/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt +++ b/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt @@ -79,6 +79,6 @@ foreach(tplg ${TPLGS_UP}) ) add_alsatplg_command(${output}.conf ${output}.tplg) - add_custom_target(topology_${output} DEPENDS ${output}.tplg) - add_dependencies(kernel_dependent_v5_19_topologies1 topology_${output}) + add_custom_target(k519_topology_${output} DEPENDS ${output}.tplg) + add_dependencies(kernel_dependent_v5_19_topologies1 k519_topology_${output}) endforeach() From 55c219a1a3b80ab564d7ae44ff34c20ae5bda8f1 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Thu, 11 Mar 2021 01:42:47 +0000 Subject: [PATCH 2/6] scripts/build-tools.sh: switch to Ninja because make is too verbose Make prints a line for every topology that is already up to date and not rebuilt. It's impossible to see what was done. Signed-off-by: Marc Herbert --- .github/workflows/pull-request.yml | 2 +- scripts/build-tools.sh | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 5e8db8263cdc..a8517951201a 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -108,7 +108,7 @@ jobs: - name: apt get run: sudo apt-get update && - sudo apt-get -y install valgrind alsa-utils libasound2-dev + sudo apt-get -y install valgrind alsa-utils libasound2-dev ninja-build octave octave-io octave-signal # testbench needs some topologies. diff --git a/scripts/build-tools.sh b/scripts/build-tools.sh index 496ca4c2d7cc..29182d6ae0e4 100755 --- a/scripts/build-tools.sh +++ b/scripts/build-tools.sh @@ -37,12 +37,12 @@ reconfigure_build() mkdir -p "$BUILD_TOOLS_DIR" ( cd "$BUILD_TOOLS_DIR" - cmake -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools" + cmake -GNinja -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools" ) mkdir "$BUILD_TOOLS_DIR/fuzzer" ( cd "$BUILD_TOOLS_DIR/fuzzer" - cmake -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools/fuzzer" + cmake -GNinja -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools/fuzzer" ) } @@ -70,17 +70,17 @@ print_build_info() cat < Date: Tue, 27 Sep 2022 08:27:07 +0000 Subject: [PATCH 3/6] topology1: fix incremental build dependency on abi.h Modifying abi.h now rebuilds topologies v1 Signed-off-by: Marc Herbert --- tools/topology/topology1/CMakeLists.txt | 15 +++++++++++++-- .../topology/topology1/development/CMakeLists.txt | 4 ++-- .../topology1/dsp_enhancement/CMakeLists.txt | 2 +- .../kernel_dependent/v5.19/CMakeLists.txt | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/topology/topology1/CMakeLists.txt b/tools/topology/topology1/CMakeLists.txt index 3de145bd9078..661bc22ac864 100644 --- a/tools/topology/topology1/CMakeLists.txt +++ b/tools/topology/topology1/CMakeLists.txt @@ -9,8 +9,12 @@ file(GLOB TPLG_DEPS ) add_custom_target(abi_v1 + DEPENDS abi.h +) +add_custom_command(OUTPUT abi.h COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/get_abi.sh ${SOF_ROOT_SOURCE_DIRECTORY} - DEPENDS ${TPLG_DEPS} + DEPENDS ${TPLG_DEPS} ${SOF_ROOT_SOURCE_DIRECTORY}/src/include/kernel/abi.h + # get_abi.h generates (a smaller) abi.h in the current directory WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} VERBATIM USES_TERMINAL @@ -316,6 +320,8 @@ foreach(tplg ${TPLGS}) endforeach() endif() + # m4 + # TODO: reduce duplication with subdirectories add_custom_command( OUTPUT production/${output}.conf COMMAND m4 --fatal-warnings @@ -329,11 +335,16 @@ foreach(tplg ${TPLGS}) ${CMAKE_CURRENT_SOURCE_DIR}/${input}.m4 > production/${output}.conf MAIN_DEPENDENCY ${CMAKE_CURRENT_SOURCE_DIR}/${input}.m4 - DEPENDS abi_v1 + # The 'abi.h' file is the real dependency that triggers + # a rebuild. 'abi_v1' is the target that avoids a race + # to rebuild abi.h. See the CMake FAQ or (better) Sam + # Thursfield's blog about custom targets. + DEPENDS abi_v1 ${CMAKE_CURRENT_BINARY_DIR}/abi.h VERBATIM USES_TERMINAL ) + # alsatplg add_alsatplg_command(production/${output}.conf production/${output}.tplg) add_custom_target(topology1_${output} DEPENDS production/${output}.tplg) diff --git a/tools/topology/topology1/development/CMakeLists.txt b/tools/topology/topology1/development/CMakeLists.txt index f6760d9edb3c..be0faa223a91 100644 --- a/tools/topology/topology1/development/CMakeLists.txt +++ b/tools/topology/topology1/development/CMakeLists.txt @@ -89,7 +89,7 @@ foreach(tplg ${TPLGS}) ${CMAKE_CURRENT_SOURCE_DIR}/../common/abi.m4 ${CMAKE_CURRENT_SOURCE_DIR}/${input}.m4 > ${output}.conf - DEPENDS abi_v1 + DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h VERBATIM USES_TERMINAL ) @@ -125,7 +125,7 @@ foreach(tplg ${TPLGS_UP}) ${CMAKE_CURRENT_SOURCE_DIR}/../common/abi.m4 ${CMAKE_CURRENT_SOURCE_DIR}/../${input}.m4 > ${output}.conf - DEPENDS abi_v1 + DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h VERBATIM USES_TERMINAL ) diff --git a/tools/topology/topology1/dsp_enhancement/CMakeLists.txt b/tools/topology/topology1/dsp_enhancement/CMakeLists.txt index dc0ef468c88b..bdb655dd7800 100644 --- a/tools/topology/topology1/dsp_enhancement/CMakeLists.txt +++ b/tools/topology/topology1/dsp_enhancement/CMakeLists.txt @@ -34,7 +34,7 @@ foreach(tplg ${TPLGS_UP}) ${CMAKE_CURRENT_SOURCE_DIR}/../common/abi.m4 ${CMAKE_CURRENT_SOURCE_DIR}/../${input}.m4 > ${output}.conf - DEPENDS abi_v1 + DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h VERBATIM USES_TERMINAL ) diff --git a/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt b/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt index cf823066e60a..c141eeceb88f 100644 --- a/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt +++ b/tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt @@ -73,7 +73,7 @@ foreach(tplg ${TPLGS_UP}) ${CMAKE_CURRENT_SOURCE_DIR}/../../common/abi.m4 ${CMAKE_CURRENT_SOURCE_DIR}/../../${input}.m4 > ${output}.conf - DEPENDS abi_v1 + DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h VERBATIM USES_TERMINAL ) From a2daabedbb9501e9ef0977c294e3cfb87c782514 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 27 Sep 2022 00:00:52 +0000 Subject: [PATCH 4/6] topology2: concat abi.conf at build time to fix incremental build It's now possible to rebuild just only one modified topology without rebuilding all of them from scratch. Building topologies v2 has two steps: 1. concatenation of abi.conf with topo.conf 2. run alsatplg on the concatenation Concatenation step 1. was wrongly implemented at configuration time, not at build time. So when the topology source of step 1 (topo.conf) was modified, the dependency of step 2 (= the concatenation) was not affected and the topology was not rebuilt. Fix this by moving the concatenation to build time. Fixes commit 308a24a92b44 ("topology2: Add build support") Signed-off-by: Marc Herbert --- tools/topology/CMakeLists.txt | 15 +++++++++------ tools/topology/topology2/cavs/CMakeLists.txt | 12 +++--------- tools/topology/topology2/get_abi.sh | 2 +- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/tools/topology/CMakeLists.txt b/tools/topology/CMakeLists.txt index 4b7b36d1a2a1..a251354021fe 100644 --- a/tools/topology/CMakeLists.txt +++ b/tools/topology/CMakeLists.txt @@ -28,21 +28,24 @@ macro(add_alsatplg_command) ) endmacro() -macro(add_alsatplg2_command input_file output_file include_path) +macro(add_alsatplg2_command conf_header input_name output_name include_path) # command line definitions are optional # Set defines if there is an optional argument - if (${ARGC} GREATER 3) - set (defines ${ARGV3}) + if (${ARGC} GREATER 4) + set (defines ${ARGV4}) else() set (defines "") endif() add_custom_command( - MAIN_DEPENDENCY ${input_file} - OUTPUT ${output_file} + MAIN_DEPENDENCY ${input_name}.conf + OUTPUT ${output_name}.tplg + + COMMAND cat ${conf_header} ${input_name}.conf > ${output_name}.conf + # -p to pre-process Topology2.0 conf file COMMAND ALSA_CONFIG_DIR=${CMAKE_SOURCE_DIR}/topology/topology2 alsatplg \$\${VERBOSE:+-v 1} - -I ${include_path} -D "'${defines}'" -p -c ${input_file} -o ${output_file} + -I ${include_path} -D "'${defines}'" -p -c ${output_name}.conf -o ${output_name}.tplg USES_TERMINAL ) endmacro() diff --git a/tools/topology/topology2/cavs/CMakeLists.txt b/tools/topology/topology2/cavs/CMakeLists.txt index ee60915e5cf9..a9d89fba4122 100644 --- a/tools/topology/topology2/cavs/CMakeLists.txt +++ b/tools/topology/topology2/cavs/CMakeLists.txt @@ -68,15 +68,9 @@ foreach(tplg ${TPLGS}) list(GET tplg ${last_index} defines) endif() - # copy ABI and input conf file contents - # - # BUG: move this from CMake configuration time to build time to - # fix incremental builds - configure_file(${CMAKE_CURRENT_BINARY_DIR}/abi.conf ${CMAKE_CURRENT_BINARY_DIR}/${output}.conf) - file(READ ${CMAKE_CURRENT_SOURCE_DIR}/${input}.conf CONTENTS) - file(APPEND ${CMAKE_CURRENT_BINARY_DIR}/${output}.conf "${CONTENTS}") - - add_alsatplg2_command("${output}.conf" "${output}.tplg" "${CMAKE_CURRENT_SOURCE_DIR}" "${defines}") + add_alsatplg2_command("${CMAKE_CURRENT_BINARY_DIR}/abi.conf" + "${CMAKE_CURRENT_SOURCE_DIR}/${input}" "${output}" + "${CMAKE_CURRENT_SOURCE_DIR}" "${defines}") add_custom_target(topology2_${output} DEPENDS ${output}.tplg) add_dependencies(topology2_cavs topology2_${output}) endforeach() diff --git a/tools/topology/topology2/get_abi.sh b/tools/topology/topology2/get_abi.sh index 73a0bac5c47e..1ddb8549b11e 100755 --- a/tools/topology/topology2/get_abi.sh +++ b/tools/topology/topology2/get_abi.sh @@ -36,4 +36,4 @@ case "$2" in ipc4) print_2bytes "$ABI_PATCH" ;; esac -printf '\"\n\t}\n}' +printf '\"\n\t}\n}\n' From 3b4900678be3b3ab60d1e7972e9b178a296873d0 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 27 Sep 2022 09:00:21 +0000 Subject: [PATCH 5/6] topology2: generate abi.conf at build time, fix incremental on abi.h Move abi.conf generation to build time. Changing abi.h now rebuilds all v2 topologies. Signed-off-by: Marc Herbert --- tools/topology/CMakeLists.txt | 12 +++++++++--- tools/topology/topology2/cavs/CMakeLists.txt | 11 +++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tools/topology/CMakeLists.txt b/tools/topology/CMakeLists.txt index a251354021fe..2ef3feb784be 100644 --- a/tools/topology/CMakeLists.txt +++ b/tools/topology/CMakeLists.txt @@ -28,17 +28,23 @@ macro(add_alsatplg_command) ) endmacro() -macro(add_alsatplg2_command conf_header input_name output_name include_path) +macro(add_alsatplg2_command conf_header conf_target input_name output_name include_path) # command line definitions are optional # Set defines if there is an optional argument - if (${ARGC} GREATER 4) - set (defines ${ARGV4}) + if (${ARGC} GREATER 5) + set (defines ${ARGV5}) else() set (defines "") endif() add_custom_command( MAIN_DEPENDENCY ${input_name}.conf + # The conf_header file is the real dependency that + # triggers a rebuild. conf_target is the target that + # avoids a race to rebuild conf_header. See the CMake FAQ + # or (better) Sam Thursfield's blog about custom + # targets. + DEPENDS ${conf_header} ${conf_target} OUTPUT ${output_name}.tplg COMMAND cat ${conf_header} ${input_name}.conf > ${output_name}.conf diff --git a/tools/topology/topology2/cavs/CMakeLists.txt b/tools/topology/topology2/cavs/CMakeLists.txt index a9d89fba4122..61ae8379bc92 100644 --- a/tools/topology/topology2/cavs/CMakeLists.txt +++ b/tools/topology/topology2/cavs/CMakeLists.txt @@ -46,9 +46,12 @@ PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-ace-mtl-nocodec.bin" ) # generate ABI for IPC4 -execute_process( - COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/../get_abi.sh ${SOF_ROOT_SOURCE_DIRECTORY} "ipc4" - OUTPUT_FILE ${CMAKE_CURRENT_BINARY_DIR}/abi.conf +add_custom_command(OUTPUT abi.conf + COMMAND > abi.conf ${CMAKE_CURRENT_SOURCE_DIR}/../get_abi.sh ${SOF_ROOT_SOURCE_DIRECTORY} "ipc4" + DEPENDS ${SOF_ROOT_SOURCE_DIRECTORY}/src/include/kernel/abi.h +) +add_custom_target(cavs_abi_target + DEPENDS abi.conf ) add_custom_target(topology2_cavs) @@ -68,7 +71,7 @@ foreach(tplg ${TPLGS}) list(GET tplg ${last_index} defines) endif() - add_alsatplg2_command("${CMAKE_CURRENT_BINARY_DIR}/abi.conf" + add_alsatplg2_command("${CMAKE_CURRENT_BINARY_DIR}/abi.conf" cavs_abi_target "${CMAKE_CURRENT_SOURCE_DIR}/${input}" "${output}" "${CMAKE_CURRENT_SOURCE_DIR}" "${defines}") add_custom_target(topology2_${output} DEPENDS ${output}.tplg) From 6eb06d20433473ff54902924a56c8ebc376e31ee Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 27 Sep 2022 05:46:21 +0000 Subject: [PATCH 6/6] scripts/build-tools.sh: switch to incremental build by default Faster build and focused and readable logs at last. Add a warning not to surprise anyone. Signed-off-by: Marc Herbert --- scripts/build-tools.sh | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/scripts/build-tools.sh b/scripts/build-tools.sh index 29182d6ae0e4..3118cd55f40a 100755 --- a/scripts/build-tools.sh +++ b/scripts/build-tools.sh @@ -10,10 +10,10 @@ SOF_TOP=$(cd "$(dirname "$0")/.." && pwd) print_usage() { cat <