From 0bdfdb98d515c904c053ef9ee372e9d1138452a4 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 19 Jul 2024 16:05:29 +0200 Subject: [PATCH 1/8] pipeline: (cosmetic) extract code fragment into a function Extract code to obtain a pointer to the module base configuration, depending on its type, into a separate function. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-stream.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index add2dbd787b6..a233355d489a 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -315,6 +315,17 @@ static void pipeline_trigger_xrun(struct pipeline *p, struct comp_dev **host) } #if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL +static struct ipc4_base_module_cfg *ipc4_get_base_cfg(struct comp_dev *comp) +{ + if (comp->drv->type != SOF_COMP_MODULE_ADAPTER) + return comp_get_drvdata(comp); + + struct processing_module *mod = comp_mod(comp); + struct module_data *md = &mod->priv; + + return &md->cfg.base_cfg; +} + static int pipeline_calc_cps_consumption(struct comp_dev *current, struct comp_buffer *calling_buf, struct pipeline_walk_context *ctx, int dir) @@ -333,14 +344,7 @@ static int pipeline_calc_cps_consumption(struct comp_dev *current, comp_core = current->ipc_config.core; /* modules created through module adapter have different priv_data */ - if (current->drv->type != SOF_COMP_MODULE_ADAPTER) { - cd = comp_get_drvdata(current); - } else { - struct processing_module *mod = comp_mod(current); - struct module_data *md = &mod->priv; - - cd = &md->cfg.base_cfg; - } + cd = ipc4_get_base_cfg(current); if (cd->cpc == 0) { /* Use maximum clock budget, assume 1ms chunk size */ From c75fd3f961e056ddd8e507d5f69c3f7708a5c935 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 19 Jul 2024 16:26:42 +0200 Subject: [PATCH 2/8] lib-manager: add a function to identify component type Extract the module_is_llext() macro into a header and add a function to identify a loadable module type of a component. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/llext_manager.h | 15 ++++++++++++++- src/library_manager/lib_manager.c | 5 ----- src/library_manager/llext_manager.c | 10 ++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/include/sof/llext_manager.h b/src/include/sof/llext_manager.h index 55ca48d31755..c963ef4adfd6 100644 --- a/src/include/sof/llext_manager.h +++ b/src/include/sof/llext_manager.h @@ -6,21 +6,34 @@ #ifndef __SOF_LLEXT_MANAGER_H__ #define __SOF_LLEXT_MANAGER_H__ +#include #include +#if CONFIG_LLEXT +#include + +struct comp_dev; struct comp_driver; struct comp_ipc_config; -#if CONFIG_LLEXT +static inline bool module_is_llext(const struct sof_man_module *mod) +{ + return mod->type.load_type == SOF_MAN_MOD_TYPE_LLEXT; +} + uintptr_t llext_manager_allocate_module(struct processing_module *proc, const struct comp_ipc_config *ipc_config, const void *ipc_specific_config); int llext_manager_free_module(const uint32_t component_id); + +bool comp_is_llext(struct comp_dev *comp); #else +#define module_is_llext(mod) false #define llext_manager_allocate_module(proc, ipc_config, ipc_specific_config) 0 #define llext_manager_free_module(component_id) 0 #define llext_unload(ext) 0 +#define comp_is_llext(comp) false #endif #endif diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index 82e9625a6a38..40dce7169daa 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -339,11 +339,6 @@ static int lib_manager_free_module_instance(uint32_t module_id, uint32_t instanc return sys_mm_drv_unmap_region((__sparse_force void *)va_base, bss_size); } -static inline bool module_is_llext(const struct sof_man_module *mod) -{ - return mod->type.load_type == SOF_MAN_MOD_TYPE_LLEXT; -} - uintptr_t lib_manager_allocate_module(struct processing_module *proc, const struct comp_ipc_config *ipc_config, const void *ipc_specific_config) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index e4341678ba91..196d87c343df 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -363,3 +363,13 @@ int llext_manager_free_module(const uint32_t component_id) return llext_manager_unload_module(base_module_id, mod); } + +bool comp_is_llext(struct comp_dev *comp) +{ + const uint32_t module_id = IPC4_MOD_ID(comp->ipc_config.id); + const unsigned int base_module_id = LIB_MANAGER_GET_LIB_ID(module_id) << + LIB_MANAGER_LIB_ID_SHIFT; + const struct sof_man_module *mod = lib_manager_get_module_manifest(base_module_id); + + return mod && module_is_llext(mod); +} From 97f8b03dce880e86c52f850ee8d501de5f7cbef9 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 19 Jul 2024 17:13:32 +0200 Subject: [PATCH 3/8] pipeline: rebalance KCPS instead of adding or subtracting Adding or subtracting module CPC when starting and stopping pipelines is brittle. Particularly it's prone to mistakes with modules, not specifying their CPC explicitly. Instead recalculate CPC every time a pipeline is started or stopped. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-stream.c | 129 ++++++++++----------------- 1 file changed, 48 insertions(+), 81 deletions(-) diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index a233355d489a..d4afe98b8f49 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -192,7 +192,9 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) return 0; } -#else + +#else /* CONFIG_LIBRARY */ + /* only collect scheduling components */ static int pipeline_comp_list(struct comp_dev *current, struct comp_buffer *calling_buf, @@ -326,70 +328,58 @@ static struct ipc4_base_module_cfg *ipc4_get_base_cfg(struct comp_dev *comp) return &md->cfg.base_cfg; } -static int pipeline_calc_cps_consumption(struct comp_dev *current, - struct comp_buffer *calling_buf, - struct pipeline_walk_context *ctx, int dir) +static void pipeline_cps_rebalance(struct pipeline *p, bool starting) { - struct pipeline_data *ppl_data = ctx->comp_data; - struct ipc4_base_module_cfg *cd; - int comp_core, kcps; - - pipe_dbg(ppl_data->p, "pipeline_calc_cps_consumption(), current->comp.id = %u, dir = %u", - dev_comp_id(current), dir); - - if (!comp_is_single_pipeline(current, ppl_data->start)) { - pipe_dbg(ppl_data->p, "pipeline_calc_cps_consumption(), current is from another pipeline"); - return 0; - } - comp_core = current->ipc_config.core; - - /* modules created through module adapter have different priv_data */ - cd = ipc4_get_base_cfg(current); - - if (cd->cpc == 0) { - /* Use maximum clock budget, assume 1ms chunk size */ - uint32_t core_kcps = core_kcps_get(comp_core); - - if (!current->kcps_inc[comp_core]) { - current->kcps_inc[comp_core] = core_kcps; - ppl_data->kcps[comp_core] = CLK_MAX_CPU_HZ / 1000 - core_kcps; - } else { - ppl_data->kcps[comp_core] = core_kcps - current->kcps_inc[comp_core]; - current->kcps_inc[comp_core] = 0; + unsigned int core_kcps[CONFIG_CORE_COUNT]; + struct ipc *ipc = ipc_get(); + struct ipc_comp_dev *icd; + struct list_item *clist; + const unsigned int clk_max_khz = CLK_MAX_CPU_HZ / 1000; + + for (unsigned int i = 0; i < CONFIG_CORE_COUNT; i++) + core_kcps[i] = i == PLATFORM_PRIMARY_CORE_ID ? PRIMARY_CORE_BASE_CPS_USAGE : + SECONDARY_CORE_BASE_CPS_USAGE; + + list_for_item(clist, &ipc->comp_list) { + icd = container_of(clist, struct ipc_comp_dev, list); + if (icd->type != COMP_TYPE_COMPONENT) + continue; + + struct comp_dev *comp = icd->cd; + + /* + * When a pipeline is started, its components have state PREPARE, when + * a pipeline is terminated, its components still have state ACTIVE + */ + if ((comp->state == COMP_STATE_ACTIVE && + (starting || comp->pipeline != p)) || + ((comp->state == COMP_STATE_PREPARE || comp->state == COMP_STATE_PAUSED) && + starting && comp->pipeline == p)) { + struct ipc4_base_module_cfg *cd = ipc4_get_base_cfg(comp); + + if (cd->cpc && core_kcps[icd->core] < clk_max_khz) + core_kcps[icd->core] += cd->cpc; + else + core_kcps[icd->core] = clk_max_khz; } - tr_warn(pipe, - "0 CPS requested for module: %#x, core: %d using safe max KCPS: %u", - current->ipc_config.id, comp_core, ppl_data->kcps[comp_core]); + } - return PPL_STATUS_PATH_STOP; - } else { - kcps = cd->cpc * 1000 / current->period; - tr_dbg(pipe, "Module: %#x KCPS consumption: %d, core: %d", - current->ipc_config.id, kcps, comp_core); + for (int i = 0; i < arch_num_cpus(); i++) { + int delta_kcps = core_kcps[i] - core_kcps_get(i); - ppl_data->kcps[comp_core] += kcps; + tr_dbg(pipe, "Proposed KCPS consumption: %d, core: %d, delta: %d", + core_kcps[i], i, delta_kcps); + if (delta_kcps) + core_kcps_adjust(i, delta_kcps); } - - return pipeline_for_each_comp(current, ctx, dir); } -#endif +#endif /* CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL */ /* trigger pipeline in IPC context */ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) { int ret; #if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL -/* FIXME: this must be a platform-specific parameter or a Kconfig option */ -#define DSP_MIN_KCPS 50000 - - struct pipeline_data data = { - .start = p->source_comp, - .p = p, - }; - struct pipeline_walk_context walk_ctx = { - .comp_func = pipeline_calc_cps_consumption, - .comp_data = &data, - }; bool trigger_first = false; uint32_t flags = 0; #endif @@ -422,16 +412,8 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) #if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL flags = irq_lock(); /* setup walking ctx for removing consumption */ - if (!trigger_first) { - ret = walk_ctx.comp_func(p->source_comp, NULL, &walk_ctx, PPL_DIR_DOWNSTREAM); - - for (int i = 0; i < arch_num_cpus(); i++) { - if (data.kcps[i] > 0) { - core_kcps_adjust(i, data.kcps[i]); - tr_info(pipe, "Sum of KCPS consumption: %d, core: %d", core_kcps_get(i), i); - } - } - } + if (!trigger_first) + pipeline_cps_rebalance(p, true); #endif ret = pipeline_trigger_list(p, host, cmd); if (ret < 0) { @@ -441,23 +423,8 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) return ret; } #if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL - if (trigger_first) { - ret = walk_ctx.comp_func(p->source_comp, NULL, &walk_ctx, PPL_DIR_DOWNSTREAM); - - for (int i = 0; i < arch_num_cpus(); i++) { - if (data.kcps[i] > 0) { - uint32_t core_kcps = core_kcps_get(i); - - /* Tests showed, that we cannot go below 40000kcps on MTL */ - if (data.kcps[i] > core_kcps - DSP_MIN_KCPS) - data.kcps[i] = core_kcps - DSP_MIN_KCPS; - - core_kcps_adjust(i, -data.kcps[i]); - tr_info(pipe, "Sum of KCPS consumption: %d, core: %d", - core_kcps, i); - } - } - } + if (trigger_first) + pipeline_cps_rebalance(p, false); irq_unlock(flags); #endif /* IPC response will be sent from the task, unless it was paused */ @@ -466,7 +433,7 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) return 0; } -#endif +#endif /* CONFIG_LIBRARY */ /* Runs in IPC or in pipeline task context */ static int pipeline_comp_trigger(struct comp_dev *current, From f87a907d2890c6dc3154db8a6cde159b2e325ce1 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 22 Jul 2024 09:50:28 +0200 Subject: [PATCH 4/8] lib_manager: disable on TGL The library manager isn't supported on TGL, disable it there. Signed-off-by: Guennadi Liakhovetski --- app/boards/intel_adsp_cavs25.conf | 2 +- app/boards/intel_adsp_cavs25_tgph.conf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/boards/intel_adsp_cavs25.conf b/app/boards/intel_adsp_cavs25.conf index ddf70ba9aa85..7cd405b492be 100644 --- a/app/boards/intel_adsp_cavs25.conf +++ b/app/boards/intel_adsp_cavs25.conf @@ -5,7 +5,7 @@ CONFIG_LP_MEMORY_BANKS=1 CONFIG_HP_MEMORY_BANKS=30 CONFIG_MM_DRV=y CONFIG_INTEL_MODULES=y -CONFIG_LIBRARY_MANAGER=y +CONFIG_LIBRARY_MANAGER=n CONFIG_RIMAGE_SIGNING_SCHEMA="tgl" CONFIG_DEBUG_COREDUMP=y diff --git a/app/boards/intel_adsp_cavs25_tgph.conf b/app/boards/intel_adsp_cavs25_tgph.conf index 58855d28183a..4379d6b50549 100644 --- a/app/boards/intel_adsp_cavs25_tgph.conf +++ b/app/boards/intel_adsp_cavs25_tgph.conf @@ -4,7 +4,7 @@ CONFIG_LP_MEMORY_BANKS=1 CONFIG_HP_MEMORY_BANKS=30 CONFIG_MM_DRV=y CONFIG_INTEL_MODULES=y -CONFIG_LIBRARY_MANAGER=y +CONFIG_LIBRARY_MANAGER=n CONFIG_RIMAGE_SIGNING_SCHEMA="tgl-h" CONFIG_DEBUG_COREDUMP=y From e3a5aa456f4b2a38f8f31c8e3117170633c9c442 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 16 Aug 2024 09:09:53 +0200 Subject: [PATCH 5/8] clock: only update clock frequency if actually changing it No need to set clock frequency if the new one is equal to the current one. Signed-off-by: Guennadi Liakhovetski --- src/lib/clk.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/clk.c b/src/lib/clk.c index 1c533fb91fa2..779d8b76e03d 100644 --- a/src/lib/clk.c +++ b/src/lib/clk.c @@ -62,13 +62,15 @@ void clock_set_freq(int clock, uint32_t hz) idx = clock_get_nearest_freq_idx(clk_info->freqs, clk_info->freqs_num, hz); - tr_info(&clock_tr, "clock %d set freq %dHz freq_idx %d", - clock, hz, idx); + if (clk_info->current_freq_idx != idx && + (!clk_info->set_freq || + clk_info->set_freq(clock, idx) == 0)) { + tr_info(&clock_tr, "clock %d set freq %dHz freq_idx %d old %d", + clock, hz, idx, clk_info->current_freq_idx); - if (!clk_info->set_freq || - clk_info->set_freq(clock, idx) == 0) /* update clock frequency */ clk_info->current_freq_idx = idx; + } clock_unlock(key); } From 5ff09edb0374a30534156cfaeafdf12b5dd0dcd4 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 21 Aug 2024 09:32:31 +0200 Subject: [PATCH 6/8] uuid: add a missing header uuid.h uses uint32_t, uint16_t and uint8_t types, it must include stdint.h or inttypes.h. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/lib/uuid.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/include/sof/lib/uuid.h b/src/include/sof/lib/uuid.h index 2f58d89f537b..7c7691bfff38 100644 --- a/src/include/sof/lib/uuid.h +++ b/src/include/sof/lib/uuid.h @@ -15,6 +15,8 @@ #include #endif +#include + /** \addtogroup uuid_api UUID API * UUID API specification. * @{ From 2cd2524997e9742fd00f45ab8d2cf7fa1994d210 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Aug 2024 17:15:57 +0300 Subject: [PATCH 7/8] drc: move logging context to the base image Logging context can be accessed from the deferred logging thread. Therefore it must be always available to avoid access to unmapped memory when an LLEXT module is unloaded. Signed-off-by: Guennadi Liakhovetski --- src/audio/drc/CMakeLists.txt | 15 +++++++++------ src/audio/drc/drc.c | 7 +++---- src/audio/drc/drc_log.c | 15 +++++++++++++++ zephyr/CMakeLists.txt | 6 ++++++ 4 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 src/audio/drc/drc_log.c diff --git a/src/audio/drc/CMakeLists.txt b/src/audio/drc/CMakeLists.txt index 650b09b40612..4e2751035ee5 100644 --- a/src/audio/drc/CMakeLists.txt +++ b/src/audio/drc/CMakeLists.txt @@ -1,6 +1,9 @@ -add_local_sources(sof drc.c) -add_local_sources(sof drc_generic.c) -add_local_sources(sof drc_hifi3.c) -add_local_sources(sof drc_hifi4.c) -add_local_sources(sof drc_math_generic.c) -add_local_sources(sof drc_math_hifi3.c) +add_local_sources(sof + drc.c + drc_generic.c + drc_hifi3.c + drc_hifi4.c + drc_math_generic.c + drc_math_hifi3.c + drc_log.c +) diff --git a/src/audio/drc/drc.c b/src/audio/drc/drc.c index 16c6d89524ac..13b33415953b 100644 --- a/src/audio/drc/drc.c +++ b/src/audio/drc/drc.c @@ -36,11 +36,10 @@ #include "drc.h" #include "drc_algorithm.h" -LOG_MODULE_REGISTER(drc, CONFIG_SOF_LOG_LEVEL); +LOG_MODULE_DECLARE(drc, CONFIG_SOF_LOG_LEVEL); -SOF_DEFINE_REG_UUID(drc); - -DECLARE_TR_CTX(drc_tr, SOF_UUID(drc_uuid), LOG_LEVEL_INFO); +extern const struct sof_uuid drc_uuid; +extern struct tr_ctx drc_tr; void drc_reset_state(struct drc_state *state) { diff --git a/src/audio/drc/drc_log.c b/src/audio/drc/drc_log.c new file mode 100644 index 000000000000..63c88e65101a --- /dev/null +++ b/src/audio/drc/drc_log.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: BSD-3-Clause +// +// Copyright(c) 2024 Intel Corporation. + +#include +#include +#include +#include + +SOF_DEFINE_REG_UUID(drc); +LOG_MODULE_REGISTER(drc, CONFIG_SOF_LOG_LEVEL); +DECLARE_TR_CTX(drc_tr, SOF_UUID(drc_uuid), LOG_LEVEL_INFO); +EXPORT_SYMBOL(drc_tr); +EXPORT_SYMBOL(drc_uuid); +EXPORT_SYMBOL(log_const_drc); diff --git a/zephyr/CMakeLists.txt b/zephyr/CMakeLists.txt index 43003dec8529..e383ec93ff0f 100644 --- a/zephyr/CMakeLists.txt +++ b/zephyr/CMakeLists.txt @@ -755,6 +755,12 @@ elseif(CONFIG_COMP_DRC) ) endif() +if(NOT CONFIG_COMP_DRC STREQUAL "n") + zephyr_library_sources( + ${SOF_AUDIO_PATH}/drc/drc_log.c + ) +endif() + zephyr_library_sources_ifdef(CONFIG_COMP_MULTIBAND_DRC ${SOF_AUDIO_PATH}/multiband_drc/multiband_drc.c ${SOF_AUDIO_PATH}/multiband_drc/multiband_drc_generic.c From 0e71d589035fa0082e8c065afe7d99a1be5216ce Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Aug 2024 16:41:51 +0200 Subject: [PATCH 8/8] Zephyr: update to include LLEXT logging protection Zephyr PR 77289 fixed deferred logging with LLEXT objects, update to a Zephyr version, containing it. Signed-off-by: Guennadi Liakhovetski --- west.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/west.yml b/west.yml index 495da5b6b1a7..4ae5c9285efc 100644 --- a/west.yml +++ b/west.yml @@ -43,7 +43,7 @@ manifest: - name: zephyr repo-path: zephyr - revision: 795ac34f291c2f9895e7a3a03eafc053ad1d36f2 + revision: 494f88070f6bb909389630264e4f89c71072ae53 remote: zephyrproject # Import some projects listed in zephyr/west.yml@revision