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 <[email protected]>

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

src/audio/pipeline/pipeline-graph.c

@@ -12,6 +12,7 @@
 #include <rtos/interrupt.h>
 #include <sof/lib/mm_heap.h>
 #include <sof/lib/uuid.h>
+#include <sof/llext_manager.h>
 #include <sof/compiler_attributes.h>
 #include <sof/list.h>
 #include <rtos/spinlock.h>
@@ -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

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,

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 {

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");

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