*** DNM*** PoC for no wait in multi-pipeline-set-state

posix/include/rtos/task.h

@@ -58,7 +58,7 @@ struct task {
 	uint64_t start;		/**< start time in [ms] since now (LL only) */
 	const struct sof_uuid_entry *uid; /**< Uuid */
 	uint16_t type;		/**< type of the task (LL or EDF) */
-	uint16_t priority;	/**< priority of the task (used by LL) */
+	int16_t priority;	/**< priority of the task (used by LL); lower runs first */
 	uint16_t core;		/**< execution core */
 	uint16_t flags;		/**< custom flags */
 	struct schedule_data *sch;	/**< scheduler bound to task */

src/audio/pipeline/pipeline-graph.c

@@ -266,6 +266,12 @@ int pipeline_free(struct pipeline *p)
 #endif
 		sof_heap_free(p->heap, p->pipe_task);
 	}
+	if (p->trigger_task) {
+#if !CONFIG_LIBRARY || UNIT_TEST
+		schedule_task_free(p->trigger_task);
+#endif
+		sof_heap_free(p->heap, p->trigger_task);
+	}
 
 	ipc_msg_free(p->msg);
 

src/audio/pipeline/pipeline-schedule.c

@@ -20,6 +20,7 @@
 #include <rtos/task.h>
 #include <rtos/spinlock.h>
 #include <rtos/string.h>
+#include <rtos/userspace_helper.h>
 #include <ipc/header.h>
 #include <ipc/stream.h>
 #include <ipc/topology.h>
@@ -33,6 +34,12 @@
 LOG_MODULE_DECLARE(pipe, CONFIG_SOF_LOG_LEVEL);
 
 SOF_DEFINE_REG_UUID(pipe_task);
+SOF_DEFINE_REG_UUID(pipe_trigger_task);
+
+#define PIPELINE_TRIGGER_TASK_PRIORITY	(SOF_TASK_PRI_HIGH - 1)
+
+/** Pipeline whose delayed trigger sequence must complete first on each core. */
+static APP_SYSUSER_BSS struct pipeline *delayed_trigger_owner[CONFIG_CORE_COUNT];
 
 #if CONFIG_ZEPHYR_DP_SCHEDULER
 
@@ -56,8 +63,8 @@ static void pipeline_schedule_cancel(struct pipeline *p)
 	sa_set_panic_on_delay(true);
 }
 
-static enum task_state pipeline_task_cmd(struct pipeline *p,
-					 struct sof_ipc_reply *reply)
+static enum task_state pipeline_trigger_task_cmd(struct pipeline *p,
+						 struct sof_ipc_reply *reply)
 {
 	struct comp_dev *host = p->trigger.host;
 	int err, cmd = p->trigger.cmd;
@@ -162,54 +169,23 @@ static enum task_state pipeline_task_cmd(struct pipeline *p,
 
 static enum task_state pipeline_task(void *arg)
 {
-	struct sof_ipc_reply reply = {
-		.hdr.cmd = SOF_IPC_GLB_REPLY,
-		.hdr.size = sizeof(reply),
-	};
 	struct pipeline *p = arg;
 	int err;
 
 	pipe_dbg(p, "entry");
 
 	/* are we in xrun ? */
 	if (p->xrun_bytes) {
-		/*
-		 * This happens when one of the connected pipelines runs into an xrun even before
-		 * this pipeline task gets a chance to run. But the host is still waiting for a
-		 * trigger IPC response. So, send an error response to prevent it from getting
-		 * timed out. No point triggering the pipeline in this case. It will be stopped
-		 * anyway by the host.
-		 */
-		if (p->trigger.cmd != COMP_TRIGGER_NO_ACTION) {
-			struct sof_ipc_reply reply = {
-				.hdr.cmd = SOF_IPC_GLB_REPLY,
-				.hdr.size = sizeof(reply),
-				.error = -EPIPE,
-			};
-
-			p->trigger.cmd = COMP_TRIGGER_NO_ACTION;
-
-			ipc_msg_reply(&reply);
-		}
-
 		/* try to recover */
 		err = pipeline_xrun_recover(p);
 		if (err < 0)
 			/* skip copy if still in xrun */
 			return SOF_TASK_STATE_COMPLETED;
 	}
 
-	if (p->trigger.delay) {
-		p->trigger.delay--;
+	/* Do not copy until the pipeline trigger sequence has completed. */
+	if (task_is_active(p->trigger_task))
 		return SOF_TASK_STATE_RESCHEDULE;
-	}
-
-	if (p->trigger.cmd != COMP_TRIGGER_NO_ACTION) {
-		/* Process an offloaded command */
-		err = pipeline_task_cmd(p, &reply);
-		if (err != SOF_TASK_STATE_RUNNING)
-			return err;
-	}
 
 	if (p->status == COMP_STATE_PAUSED)
 		/*
@@ -218,11 +194,6 @@ static enum task_state pipeline_task(void *arg)
 		 */
 		return SOF_TASK_STATE_COMPLETED;
 
-	/*
-	 * The first execution of the pipeline task above has triggered all
-	 * pipeline components. Subsequent iterations actually perform data
-	 * copying below.
-	 */
 	err = pipeline_copy(p);
 	if (err < 0) {
 		/* try to recover */
@@ -239,6 +210,83 @@ static enum task_state pipeline_task(void *arg)
 	return SOF_TASK_STATE_RESCHEDULE;
 }
 
+/** Run a pipeline trigger task. */
+static enum task_state pipeline_trigger_task(void *arg)
+{
+	struct sof_ipc_reply reply = {
+		.hdr.cmd = SOF_IPC_GLB_REPLY,
+		.hdr.size = sizeof(reply),
+	};
+	struct pipeline *p = arg;
+	enum task_state state;
+	struct pipeline *owner = delayed_trigger_owner[p->core];
+
+	if (owner && owner != p)
+		return SOF_TASK_STATE_RESCHEDULE;
+
+	/*
+	 * A connected pipeline can enter xrun before its trigger task runs.
+	 * Complete the delayed IPC with an error instead of triggering it.
+	 */
+	if (p->xrun_bytes) {
+		if (owner == p)
+			delayed_trigger_owner[p->core] = NULL;
+
+		if (p->trigger.cmd != COMP_TRIGGER_NO_ACTION) {
+			p->trigger.cmd = COMP_TRIGGER_NO_ACTION;
+			reply.error = -EPIPE;
+			ipc_msg_reply(&reply);
+		}
+
+		return SOF_TASK_STATE_COMPLETED;
+	}
+
+	if (p->trigger.delay) {
+		p->trigger.delay--;
+		return SOF_TASK_STATE_RESCHEDULE;
+	}
+
+	if (p->trigger.cmd == COMP_TRIGGER_NO_ACTION) {
+		if (owner == p)
+			delayed_trigger_owner[p->core] = NULL;
+
+		return SOF_TASK_STATE_COMPLETED;
+	}
+
+	state = pipeline_trigger_task_cmd(p, &reply);
+	if (state == SOF_TASK_STATE_RESCHEDULE && p->trigger.delay) {
+		delayed_trigger_owner[p->core] = p;
+		return state;
+	}
+
+	if (delayed_trigger_owner[p->core] == p)
+		delayed_trigger_owner[p->core] = NULL;
+
+	/* RUNNING means that the independent copy task should keep running. */
+	return state == SOF_TASK_STATE_RUNNING ? SOF_TASK_STATE_COMPLETED : state;
+}
+
+/** Allocate and initialize a pipeline trigger task. */
+static struct task *pipeline_trigger_task_init(struct pipeline *p, uint32_t type)
+{
+	struct task *task;
+
+	task = sof_heap_alloc(p->heap, SOF_MEM_FLAG_USER, sizeof(*task), 0);
+	if (!task)
+		return NULL;
+
+	memset(task, 0, sizeof(*task));
+
+	if (schedule_task_init_ll(task, SOF_UUID(pipe_trigger_task_uuid), type,
+				  PIPELINE_TRIGGER_TASK_PRIORITY, pipeline_trigger_task,
+				  p, p->core, 0) < 0) {
+		sof_heap_free(p->heap, task);
+		return NULL;
+	}
+
+	return task;
+}
+
 static struct task *pipeline_task_init(struct pipeline *p, uint32_t type)
 {
 	struct pipeline_task *task = NULL;
@@ -325,6 +373,8 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx,
 				p->trigger.pending = true;
 				p->trigger.host = ppl_data->start;
 				ppl_data->start = NULL;
+				if (schedule_task(p->trigger_task, 0, 0) < 0)
+					pipe_err(p, "failed to schedule trigger task");
 			} else {
 				pipeline_schedule_cancel(p);
 				p->status = COMP_STATE_PAUSED;
@@ -346,6 +396,8 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx,
 				p->trigger.pending = true;
 				p->trigger.host = ppl_data->start;
 				ppl_data->start = NULL;
+				if (schedule_task(p->trigger_task, 0, 0) < 0)
+					pipe_err(p, "failed to schedule trigger task");
 			} else {
 				p->status = COMP_STATE_ACTIVE;
 			}
@@ -374,17 +426,28 @@ int pipeline_comp_ll_task_init(struct pipeline *p)
 {
 	uint32_t type;
 
-	/* initialize task if necessary */
-	if (!p->pipe_task) {
+	/* initialize tasks if necessary */
+	if (!p->trigger_task || !p->pipe_task) {
 		/* right now we always consider pipeline as a low latency
 		 * component, but it may change in the future
 		 */
 		type = pipeline_is_timer_driven(p) ? SOF_SCHEDULE_LL_TIMER :
 			SOF_SCHEDULE_LL_DMA;
 
+		if (!p->trigger_task) {
+			p->trigger_task = pipeline_trigger_task_init(p, type);
+			if (!p->trigger_task) {
+				pipe_err(p, "trigger task init failed");
+				return -ENOMEM;
+			}
+		}
+
+		if (p->pipe_task)
+			return 0;
+
 		p->pipe_task = pipeline_task_init(p, type);
 		if (!p->pipe_task) {
-			pipe_err(p, "task init failed");
+			pipe_err(p, "copy task init failed");
 			return -ENOMEM;
 		}
 	}

src/include/sof/audio/pipeline.h

@@ -74,6 +74,7 @@ struct pipeline {
 
 	/* scheduling */
 	struct task *pipe_task;		/* pipeline processing task */
+	struct task *trigger_task;	/* pipeline trigger task */
 	struct pipeline *sched_next;	/* pipeline scheduled after this */
 	struct pipeline *sched_prev;	/* pipeline scheduled before this */
 

src/include/sof/schedule/ll_schedule.h

@@ -40,14 +40,14 @@ int scheduler_init_ll(struct ll_schedule_domain *domain);
 
 int schedule_task_init_ll(struct task *task,
 			  const struct sof_uuid_entry *uid, uint16_t type,
-			  uint16_t priority, enum task_state (*run)(void *data),
+			  int16_t priority, enum task_state (*run)(void *data),
 			  void *data, uint16_t core, uint32_t flags);
 #else
 int zephyr_ll_scheduler_init(struct ll_schedule_domain *domain);
 
 int zephyr_ll_task_init(struct task *task,
 			const struct sof_uuid_entry *uid, uint16_t type,
-			uint16_t priority, enum task_state (*run)(void *data),
+			int16_t priority, enum task_state (*run)(void *data),
 			void *data, uint16_t core, uint32_t flags);
 
 #define scheduler_init_ll zephyr_ll_scheduler_init

src/include/sof/schedule/schedule.h

@@ -359,7 +359,7 @@ static inline struct k_thread *scheduler_init_context(struct task *task)
  */
 int schedule_task_init(struct task *task,
 		       const struct sof_uuid_entry *uid, uint16_t type,
-		       uint16_t priority, enum task_state (*run)(void *data),
+		       int16_t priority, enum task_state (*run)(void *data),
 		       void *data, uint16_t core, uint32_t flags);
 
 /**

src/ipc/ipc4/handler-user.c

@@ -556,17 +556,6 @@ int ipc4_set_pipeline_state(struct ipc4_message_request *ipc4)
 			ipc_compound_pre_start(state.primary.r.type);
 			ret = ipc4_pipeline_trigger(ppl_icd, cmd, &delayed);
 			ipc_compound_post_start(state.primary.r.type, ret, delayed);
-			if (delayed) {
-				/* To maintain pipeline order for triggers, we must
-				 * do a blocking wait until trigger is processed.
-				 * This will add a max delay of 'ppl_count' LL ticks
-				 * to process the full trigger list.
-				 */
-				if (ipc_wait_for_compound_msg() != 0) {
-					ipc_cmd_err(&ipc_tr, "ipc4: fail with delayed trigger");
-					return IPC4_FAILURE;
-				}
-			}
 		}
 
 		if (ret != 0)

src/platform/library/include/platform/lib/ll_schedule.h

@@ -19,7 +19,7 @@ int scheduler_init_ll(struct ll_schedule_domain *domain);
 
 int schedule_task_init_ll(struct task *task,
 			  const struct sof_uuid_entry *uid, uint16_t type,
-			  uint16_t priority, enum task_state (*run)(void *data),
+			  int16_t priority, enum task_state (*run)(void *data),
 			  void *data, uint16_t core, uint32_t flags);
 
 #endif /* __LIBRARY_INCLUDE_LIB_SCHEDULE_H__ */

src/platform/library/schedule/ll_schedule.c

@@ -105,7 +105,7 @@ static struct scheduler_ops schedule_ll_ops = {
 
 int schedule_task_init_ll(struct task *task,
 			  const struct sof_uuid_entry *uid, uint16_t type,
-			  uint16_t priority, enum task_state (*run)(void *data),
+			  int16_t priority, enum task_state (*run)(void *data),
 			  void *data, uint16_t core, uint32_t flags)
 {
 	return schedule_task_init(task, uid, SOF_SCHEDULE_LL_TIMER, 0, run,

src/platform/library/schedule/schedule.c

@@ -22,7 +22,7 @@ struct schedulers **arch_schedulers_get(void)
 
 int schedule_task_init(struct task *task,
 		       const struct sof_uuid_entry *uid, uint16_t type,
-		       uint16_t priority, enum task_state (*run)(void *data),
+		       int16_t priority, enum task_state (*run)(void *data),
 		       void *data, uint16_t core, uint32_t flags)
 {
 	struct schedulers *schedulers = *arch_schedulers_get();

src/schedule/ll_schedule_xtos.c

@@ -607,7 +607,7 @@ static int schedule_ll_task_after(void *data, struct task *task, uint64_t start,
 
 int schedule_task_init_ll(struct task *task,
 			  const struct sof_uuid_entry *uid, uint16_t type,
-			  uint16_t priority, enum task_state (*run)(void *data),
+			  int16_t priority, enum task_state (*run)(void *data),
 			  void *data, uint16_t core, uint32_t flags)
 {
 	struct ll_task_pdata *ll_pdata;

src/schedule/schedule.c

@@ -35,7 +35,7 @@ static inline bool scheduler_is_user(int type)
 
 int schedule_task_init(struct task *task,
 		       const struct sof_uuid_entry *uid, uint16_t type,
-		       uint16_t priority, enum task_state (*run)(void *data),
+		       int16_t priority, enum task_state (*run)(void *data),
 		       void *data, uint16_t core, uint32_t flags)
 {
 	struct schedulers *schedulers;

src/schedule/zephyr_ll.c

@@ -160,8 +160,8 @@ static void zephyr_ll_task_insert_unlocked(struct zephyr_ll *sch, struct task *t
 
 	/*
 	 * Tasks are added into the list in priority order. List order
-	 * defines schedule order. Priority 0 indicates highest
-	 * priority and is run first. Tasks with the same priority are
+	 * defines schedule order. Lower values indicate higher priority
+	 * and run first. Tasks with the same priority are
 	 * served on a first-come-first-served basis.
 	 */
 	list_for_item(list, &sch->tasks) {
@@ -662,7 +662,7 @@ void user_ll_unlock_sched(int core)
 
 int zephyr_ll_task_init(struct task *task,
 			const struct sof_uuid_entry *uid, uint16_t type,
-			uint16_t priority, enum task_state (*run)(void *data),
+			int16_t priority, enum task_state (*run)(void *data),
 			void *data, uint16_t core, uint32_t flags)
 {
 	struct zephyr_ll_pdata *pdata;

test/cmocka/src/common_mocks.c

@@ -312,7 +312,7 @@ struct schedulers ** WEAK arch_schedulers_get(void)
 
 int WEAK schedule_task_init(struct task *task,
 			    const struct sof_uuid_entry *uid, uint16_t type,
-			    uint16_t priority, enum task_state (*run)(void *data),
+			    int16_t priority, enum task_state (*run)(void *data),
 			    void *data, uint16_t core, uint32_t flags)
 {
 	(void)task;
@@ -329,7 +329,7 @@ int WEAK schedule_task_init(struct task *task,
 
 int WEAK schedule_task_init_ll(struct task *task,
 			       const struct sof_uuid_entry *uid, uint16_t type,
-			       uint16_t priority, enum task_state (*run)(void *data),
+			       int16_t priority, enum task_state (*run)(void *data),
 			       void *data, uint16_t core, uint32_t flags)
 {
 	return 0;

uuid-registry.txt

@@ -136,6 +136,7 @@ f36BF24B-9AAF-83f4-8677E072E8AEADB7 notification_pool
 09fbcb7a-a9c5-4a57-84344440e598ab24 phase_vocoder
 4e934adb-b0ec-4d33-a086c1022f921321 pipe
 f11818eb-e92e-4082-82a3dc54c604ebb3 pipe_task
+069b6fab-ae56-432f-8f4285ce63ea98bf pipe_trigger_task
 d7f6712d-131c-45a7-82ed6aa9dc2291ea pm_runtime
 76cc9773-440c-4df9-95a872defe7796fc power
 9d1fb66e-4ffb-497f-994b17719686596e probe