From 59c32c01529a647bd5fa840dff03ea74fcb9a9a9 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 7 Jun 2024 16:29:09 +0200 Subject: [PATCH 1/7] ipc4: a failure to find a driver might not be fatal When ipc4_get_drv() fails to find a driver, it might mean, that the driver needs to be linked dynamically. Printing an error in such a case wrongly fails CI testing. Signed-off-by: Guennadi Liakhovetski --- src/ipc/ipc4/helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index f680feb47156..d427643ef3ea 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -946,11 +946,11 @@ const struct comp_driver *ipc4_get_drv(const uint8_t *uuid) } } - tr_err(&comp_tr, "get_drv(): the provided UUID (%08x %08x %08x %08x) can't be found!", - *(uint32_t *)(&uuid[0]), - *(uint32_t *)(&uuid[4]), - *(uint32_t *)(&uuid[8]), - *(uint32_t *)(&uuid[12])); + tr_warn(&comp_tr, "get_drv(): the provided UUID (%08x %08x %08x %08x) can't be found!", + *(uint32_t *)(&uuid[0]), + *(uint32_t *)(&uuid[4]), + *(uint32_t *)(&uuid[8]), + *(uint32_t *)(&uuid[12])); out: irq_local_enable(flags); From 851c8bea48b3f9be036e826f6340afa3bf2b64ba Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 13 Jun 2024 14:30:37 +0200 Subject: [PATCH 2/7] samples: (cosmetic) clean up Kconfig spacing Use consistent TABs and spaces in src/samples/audio/Kconfig Signed-off-by: Guennadi Liakhovetski --- src/samples/audio/Kconfig | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/samples/audio/Kconfig b/src/samples/audio/Kconfig index e853b0668f1a..0338ced15875 100644 --- a/src/samples/audio/Kconfig +++ b/src/samples/audio/Kconfig @@ -2,27 +2,27 @@ menu "Audio component samples" - config SAMPLE_SMART_AMP - tristate "Smart amplifier test component" - default y - help - Select for test smart amplifier component + config SAMPLE_SMART_AMP + tristate "Smart amplifier test component" + default y + help + Select for test smart amplifier component - config SAMPLE_KEYPHRASE + config SAMPLE_KEYPHRASE depends on CAVS || IMX || ACE - bool "Keyphrase test component" - default y - help - Select for Keyphrase test component. - Provides basic functionality for use in testing of keyphrase detection pipelines. + bool "Keyphrase test component" + default y + help + Select for Keyphrase test component. + Provides basic functionality for use in testing of keyphrase detection pipelines. config KWD_NN_SAMPLE_KEYPHRASE - depends on IMX - bool "KWD NN Keyphrase test component" - default n - help - Select for KWD NN Keyphrase test component based on neural network. - Provides ML functionality for use in testing of keyphrase detection pipelines. - Use KWD based on NN as alternative to the default KWD component. - Provides neural network as a library. + depends on IMX + bool "KWD NN Keyphrase test component" + default n + help + Select for KWD NN Keyphrase test component based on neural network. + Provides ML functionality for use in testing of keyphrase detection pipelines. + Use KWD based on NN as alternative to the default KWD component. + Provides neural network as a library. endmenu From 0727103fed76682302904cede6153e5b1bfbfe30 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 14 Jun 2024 15:44:34 +0200 Subject: [PATCH 3/7] llext: add maximum instance count support Maximum instance count cannot be zero, they have to be supplied by respective modules. Signed-off-by: Guennadi Liakhovetski --- src/audio/eq_iir/eq_iir.c | 2 +- src/audio/mixin_mixout/mixin_mixout.c | 4 ++-- src/include/module/module/llext.h | 3 ++- src/samples/audio/smart_amp_test_ipc4.c | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/audio/eq_iir/eq_iir.c b/src/audio/eq_iir/eq_iir.c index f81a943a7928..44a665343c08 100644 --- a/src/audio/eq_iir/eq_iir.c +++ b/src/audio/eq_iir/eq_iir.c @@ -269,7 +269,7 @@ SOF_MODULE_INIT(eq_iir, sys_comp_module_eq_iir_interface_init); SOF_LLEXT_MOD_ENTRY(eq_iir, &eq_iir_interface); static const struct sof_man_module_manifest mod_manifest __section(".module") __used = - SOF_LLEXT_MODULE_MANIFEST("EQIIR", eq_iir_llext_entry, 1, UUID_EQIIR); + SOF_LLEXT_MODULE_MANIFEST("EQIIR", eq_iir_llext_entry, 1, UUID_EQIIR, 40); SOF_LLEXT_BUILDINFO; diff --git a/src/audio/mixin_mixout/mixin_mixout.c b/src/audio/mixin_mixout/mixin_mixout.c index 105ef9c8e131..8834327ab22b 100644 --- a/src/audio/mixin_mixout/mixin_mixout.c +++ b/src/audio/mixin_mixout/mixin_mixout.c @@ -980,8 +980,8 @@ SOF_LLEXT_MOD_ENTRY(mixout, &mixout_interface); static const struct sof_man_module_manifest mod_manifest[] __section(".module") __used = { - SOF_LLEXT_MODULE_MANIFEST("MIXIN", mixin_llext_entry, 1, UUID_MIXIN), - SOF_LLEXT_MODULE_MANIFEST("MIXOUT", mixout_llext_entry, 1, UUID_MIXOUT), + SOF_LLEXT_MODULE_MANIFEST("MIXIN", mixin_llext_entry, 1, UUID_MIXIN, 30), + SOF_LLEXT_MODULE_MANIFEST("MIXOUT", mixout_llext_entry, 1, UUID_MIXOUT, 30), }; SOF_LLEXT_BUILDINFO; diff --git a/src/include/module/module/llext.h b/src/include/module/module/llext.h index 61532e3fb70f..05b8f0ffb5f5 100644 --- a/src/include/module/module/llext.h +++ b/src/include/module/module/llext.h @@ -6,12 +6,13 @@ #ifndef MODULE_LLEXT_H #define MODULE_LLEXT_H -#define SOF_LLEXT_MODULE_MANIFEST(manifest_name, entry, affinity, mod_uuid) \ +#define SOF_LLEXT_MODULE_MANIFEST(manifest_name, entry, affinity, mod_uuid, instances) \ { \ .module = { \ .name = manifest_name, \ .uuid = {mod_uuid}, \ .entry_point = (uint32_t)(entry), \ + .instance_max_count = instances, \ .type = { \ .load_type = SOF_MAN_MOD_TYPE_LLEXT, \ .domain_ll = 1, \ diff --git a/src/samples/audio/smart_amp_test_ipc4.c b/src/samples/audio/smart_amp_test_ipc4.c index 9017408cf235..bec6634a7f25 100644 --- a/src/samples/audio/smart_amp_test_ipc4.c +++ b/src/samples/audio/smart_amp_test_ipc4.c @@ -415,6 +415,7 @@ static const struct sof_man_module_manifest main_manifest __section(".module") _ .uuid = {0x1E, 0x96, 0x7A, 0x16, 0xE4, 0x8A, 0xEA, 0x11, 0x89, 0xF1, 0x00, 0x0C, 0x29, 0xCE, 0x16, 0x35}, .entry_point = (uint32_t)smart_amp_test_llext_entry, + .instance_max_count = 1, .type = { #ifdef __SOF_MODULE_SERVICE_BUILD__ .load_type = SOF_MAN_MOD_TYPE_MODULE, From 8a34c8362e52ee7eead7bb8cade6de569a100f18 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 14 Jun 2024 15:47:08 +0200 Subject: [PATCH 4/7] rimage: don't overwrite maximum module instance counts Use maximum instance count from TOML when building a manifest. Signed-off-by: Guennadi Liakhovetski --- tools/rimage/src/manifest.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/rimage/src/manifest.c b/tools/rimage/src/manifest.c index 5fb766c87173..52b328e85ca7 100644 --- a/tools/rimage/src/manifest.c +++ b/tools/rimage/src/manifest.c @@ -464,9 +464,6 @@ static int man_module_create_reloc(struct image *image, struct manifest_module * /* stack size ??? convert sizes to PAGES */ man_module->instance_bss_size = 1; - /* max number of instances of this module ?? */ - man_module->instance_max_count = 1; - module_print_zones(&module->file); /* main module */ From 55429c674961ae92d9ef863cd240849323578d1e Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 17 Jun 2024 17:24:35 +0200 Subject: [PATCH 5/7] llext: remove logging during module freeing When pipelines are destroyed, component drivers' .reset() and .free() are called. If those drivers were loaded dynamically their memory is then unmapped. But logging takes place in a low priority task, so it is important that no logging is done from those methods. Signed-off-by: Guennadi Liakhovetski --- src/audio/eq_iir/eq_iir.c | 4 ---- src/audio/mixin_mixout/mixin_mixout.c | 8 -------- src/samples/audio/smart_amp_test_ipc4.c | 4 ---- 3 files changed, 16 deletions(-) diff --git a/src/audio/eq_iir/eq_iir.c b/src/audio/eq_iir/eq_iir.c index 44a665343c08..a5b04a418691 100644 --- a/src/audio/eq_iir/eq_iir.c +++ b/src/audio/eq_iir/eq_iir.c @@ -98,8 +98,6 @@ static int eq_iir_free(struct processing_module *mod) { struct comp_data *cd = module_get_private_data(mod); - comp_info(mod->dev, "eq_iir_free()"); - eq_iir_free_delaylines(cd); comp_data_blob_handler_free(cd->model_handler); @@ -232,8 +230,6 @@ static int eq_iir_reset(struct processing_module *mod) struct comp_data *cd = module_get_private_data(mod); int i; - comp_info(mod->dev, "eq_iir_reset()"); - eq_iir_free_delaylines(cd); cd->eq_iir_func = NULL; diff --git a/src/audio/mixin_mixout/mixin_mixout.c b/src/audio/mixin_mixout/mixin_mixout.c index 8834327ab22b..24f30ff31677 100644 --- a/src/audio/mixin_mixout/mixin_mixout.c +++ b/src/audio/mixin_mixout/mixin_mixout.c @@ -173,9 +173,7 @@ static int mixout_init(struct processing_module *mod) static int mixin_free(struct processing_module *mod) { struct mixin_data *md = module_get_private_data(mod); - struct comp_dev *dev = mod->dev; - comp_dbg(dev, "mixin_free()"); rfree(md); return 0; @@ -183,7 +181,6 @@ static int mixin_free(struct processing_module *mod) static int mixout_free(struct processing_module *mod) { - comp_dbg(mod->dev, "mixout_free()"); rfree(module_get_private_data(mod)); return 0; @@ -554,9 +551,6 @@ static int mixout_process(struct processing_module *mod, static int mixin_reset(struct processing_module *mod) { struct mixin_data *mixin_data = module_get_private_data(mod); - struct comp_dev *dev = mod->dev; - - comp_dbg(dev, "mixin_reset()"); mixin_data->mix = NULL; mixin_data->gain_mix = NULL; @@ -568,8 +562,6 @@ static int mixout_reset(struct processing_module *mod) { struct comp_dev *dev = mod->dev; - comp_dbg(dev, "mixout_reset()"); - /* FIXME: move this to module_adapter_reset() */ if (dev->pipeline->source_comp->direction == SOF_IPC_STREAM_PLAYBACK) { int i; diff --git a/src/samples/audio/smart_amp_test_ipc4.c b/src/samples/audio/smart_amp_test_ipc4.c index bec6634a7f25..e2e06db9ea6b 100644 --- a/src/samples/audio/smart_amp_test_ipc4.c +++ b/src/samples/audio/smart_amp_test_ipc4.c @@ -157,8 +157,6 @@ static inline int smart_amp_get_config(struct processing_module *mod, static int smart_amp_free(struct processing_module *mod) { - LOG_DBG("smart_amp_free()"); - #ifndef __SOF_MODULE_SERVICE_BUILD__ struct smart_amp_data *sad = module_get_private_data(mod); @@ -320,8 +318,6 @@ static int smart_amp_process(struct processing_module *mod, static int smart_amp_reset(struct processing_module *mod) { - LOG_DBG("smart_amp_reset()"); - return 0; } From 1ca65774916464ccaf14639ac8e3227e4d43c265 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 2 Jul 2024 10:53:07 +0200 Subject: [PATCH 6/7] llext: disable when testing reproducible builds So far we cannot build identical LLEXT modules under Linux and Windows, build a monolithic firmware for this test. Signed-off-by: Guennadi Liakhovetski --- app/overlays/repro-build.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/app/overlays/repro-build.conf b/app/overlays/repro-build.conf index d7b434c39660..25e49e619e6c 100644 --- a/app/overlays/repro-build.conf +++ b/app/overlays/repro-build.conf @@ -1,2 +1,3 @@ CONFIG_OUTPUT_DISASSEMBLY=y CONFIG_OUTPUT_DISASSEMBLY_WITH_SOURCE=n +CONFIG_MODULES=n From e85e99269b60af76e02182a8e6fcdd536f435421 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 7 Jun 2024 09:11:41 +0200 Subject: [PATCH 7/7] llext: fix Windows builds Under windows the Python interpreter has to be called explicitly. Without it an attempt to execute a Python script fails silently. Signed-off-by: Guennadi Liakhovetski --- scripts/xtensa-build-zephyr.py | 6 +++++- zephyr/CMakeLists.txt | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/xtensa-build-zephyr.py b/scripts/xtensa-build-zephyr.py index 4914120170c3..531cb58655a0 100755 --- a/scripts/xtensa-build-zephyr.py +++ b/scripts/xtensa-build-zephyr.py @@ -945,7 +945,11 @@ def install_lib(sof_lib_dir, abs_build_dir, platform_wconfig): llext_input = entry_path / (llext_base + '.llext') llext_output = entry_path / (llext_file + '.ri') - sign_cmd = [platform_wconfig.get("rimage.path"), "-o", str(llext_output), + # See why the shlex() parsing step is required at + # https://docs.zephyrproject.org/latest/develop/west/sign.html#rimage + # and in Zephyr commit 030b740bd1ec + rimage_cmd = shlex.split(platform_wconfig.get('rimage.path'))[0] + sign_cmd = [rimage_cmd, "-o", str(llext_output), "-e", "-c", str(rimage_cfg), "-k", str(signing_key), "-l", "-r", str(llext_input)] diff --git a/zephyr/CMakeLists.txt b/zephyr/CMakeLists.txt index c9de5f9b5cbb..ce04fee7a366 100644 --- a/zephyr/CMakeLists.txt +++ b/zephyr/CMakeLists.txt @@ -90,7 +90,7 @@ function(sof_llext_build module) get_target_property(proc_out_file ${module} pkg_input) add_llext_command(TARGET ${module} POST_BUILD - COMMAND ${SOF_BASE}scripts/llext_link_helper.py + COMMAND ${PYTHON_EXECUTABLE} ${SOF_BASE}scripts/llext_link_helper.py --text-addr="${SOF_LLEXT_TEXT_ADDR}" -f ${proc_in_file} ${CMAKE_C_COMPILER} -- -o ${proc_out_file} ${EXTRA_LINKER_PARAMS} $