From 87db3f3897a2eeae3cda69318cb4646025effca8 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 19 Jul 2024 17:13:32 +0200 Subject: [PATCH] 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 --- app/boards/intel_adsp_ace15_mtpm.conf | 1 + src/audio/pipeline/pipeline-graph.c | 6 ++ src/audio/pipeline/pipeline-stream.c | 120 +++++++++----------------- src/include/sof/audio/pipeline.h | 1 + src/ipc/ipc4/helper.c | 12 +++ src/library_manager/Kconfig | 11 +++ 6 files changed, 70 insertions(+), 81 deletions(-) diff --git a/app/boards/intel_adsp_ace15_mtpm.conf b/app/boards/intel_adsp_ace15_mtpm.conf index e97d8965fd0b..930735222e86 100644 --- a/app/boards/intel_adsp_ace15_mtpm.conf +++ b/app/boards/intel_adsp_ace15_mtpm.conf @@ -41,6 +41,7 @@ CONFIG_DMA_DW_LLI_POOL_SIZE=50 CONFIG_INTEL_MODULES=y CONFIG_LIBRARY_MANAGER=y CONFIG_LIBRARY_AUTH_SUPPORT=y +CONFIG_LIBRARY_PIPELINE_FORCE_MIN_LIFETIME=100 CONFIG_INTEL_ADSP_TIMER=y CONFIG_MM_DRV_INTEL_ADSP_TLB_REMAP_UNUSED_RAM=y CONFIG_AMS=y diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 42fbdc7bae8f..0f5874a09b7d 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -260,6 +261,11 @@ static int pipeline_comp_complete(struct comp_dev *current, /* complete component init */ current->pipeline = ppl_data->p; + + if (comp_is_llext(current)) + /* pipelines are allocated using rzalloc(), so initially .init_time = 0 */ + current->pipeline->init_time = sof_cycle_get_64(); + /* LL module has its period always eq period of the pipeline * DP period is set to 0 as sink format may not yet been set * It will be calculated during module prepare operation diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index a233355d489a..3e82523db5ed 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,49 @@ 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; + + 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; + + /* Don't add components on the pipeline, that we're shutting down */ + if (comp->state >= COMP_STATE_PREPARE && + (comp->pipeline != p || starting)) { + 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_info(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 +403,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 +414,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 +424,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, diff --git a/src/include/sof/audio/pipeline.h b/src/include/sof/audio/pipeline.h index 2410f774f00d..4291e4d1d637 100644 --- a/src/include/sof/audio/pipeline.h +++ b/src/include/sof/audio/pipeline.h @@ -94,6 +94,7 @@ struct pipeline { bool aborted; /* STOP or PAUSE failed, stay active */ bool pending; /* trigger scheduled but not executed yet */ } trigger; + uint64_t init_time; }; struct pipeline_walk_context { diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index d427643ef3ea..5a2aefa1adcc 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -331,6 +331,7 @@ static int ipc_pipeline_module_free(uint32_t pipeline_id) int ipc_pipeline_free(struct ipc *ipc, uint32_t comp_id) { struct ipc_comp_dev *ipc_pipe; + uint64_t life_time; int ret; /* check whether pipeline exists */ @@ -342,6 +343,17 @@ int ipc_pipeline_free(struct ipc *ipc, uint32_t comp_id) if (!cpu_is_me(ipc_pipe->core)) return ipc4_process_on_core(ipc_pipe->core, false); + if (ipc_pipe->pipeline->init_time) { + /* + * This pipeline must not be destroyed before a minimum time + * since its creation has passed + */ + life_time = k_cyc_to_ms_near64(sof_cycle_get_64() - ipc_pipe->pipeline->init_time); + pipe_dbg(ipc_pipe->pipeline, "Extend pipeline life beyond %llu", life_time); + if (life_time < CONFIG_LIBRARY_PIPELINE_FORCE_MIN_LIFETIME) + k_msleep(CONFIG_LIBRARY_PIPELINE_FORCE_MIN_LIFETIME - life_time); + } + ret = ipc_pipeline_module_free(ipc_pipe->pipeline->pipeline_id); if (ret != IPC4_SUCCESS) { tr_err(&ipc_tr, "ipc_pipeline_free(): module free () failed"); diff --git a/src/library_manager/Kconfig b/src/library_manager/Kconfig index e167f956a37a..627553790beb 100644 --- a/src/library_manager/Kconfig +++ b/src/library_manager/Kconfig @@ -33,4 +33,15 @@ config LIBRARY_AUTH_SUPPORT could be used if enabled. If unsure say N. +config LIBRARY_PIPELINE_FORCE_MIN_LIFETIME + int "Minimum pipeline life-time in ms" + default 0 + range 0 500 + help + Typically pipelines are created for streaming, which lasts for + considerable time, at least seconds. But in some test-cases pipelines + are created and quickly destroyed again with no streaming at all. In + some configurations this leads to failures. This option allows setting + a minimum pipeline life-time, which fixes those scenarios. + endmenu