From 61e0976c35326c7d177b0d8bfcf14cb789edea05 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 11 Nov 2021 14:13:13 +0100 Subject: [PATCH 1/7] pipeline: (cosmetic) use a dedicated no-action trigger value Currently -EINVAL is used to indicate an invalid trigger code, requiring no handling. Use an explicit new COMP_TRIGGER_NO_ACTION value for that for better clarity. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-graph.c | 2 +- src/audio/pipeline/pipeline-schedule.c | 8 ++++---- src/include/sof/audio/component.h | 1 + 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 52bcabcdb066..19ea6dbbd24d 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -127,7 +127,7 @@ struct pipeline *pipeline_new(uint32_t pipeline_id, uint32_t priority, uint32_t p->priority = priority; p->pipeline_id = pipeline_id; p->status = COMP_STATE_INIT; - p->trigger.cmd = -EINVAL; + p->trigger.cmd = COMP_TRIGGER_NO_ACTION; ret = memcpy_s(&p->tctx, sizeof(struct tr_ctx), &pipe_tr, sizeof(struct tr_ctx)); assert(!ret); diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index 16df23e7edb6..3485a37656a3 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -55,14 +55,14 @@ static enum task_state pipeline_task(void *arg) * timed out. No point triggering the pipeline in this case. It will be stopped * anyway by the host. */ - if (p->trigger.cmd >= 0) { + 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 = -EINVAL; + p->trigger.cmd = COMP_TRIGGER_NO_ACTION; ipc_msg_reply(&reply); } @@ -79,7 +79,7 @@ static enum task_state pipeline_task(void *arg) return SOF_TASK_STATE_RESCHEDULE; } - if (p->trigger.cmd >= 0) { + if (p->trigger.cmd != COMP_TRIGGER_NO_ACTION) { /* First pipeline task run for either START or RELEASE: PRE stage */ struct sof_ipc_reply reply = { .hdr.cmd = SOF_IPC_GLB_REPLY, @@ -105,7 +105,7 @@ static enum task_state pipeline_task(void *arg) err = SOF_TASK_STATE_RESCHEDULE; } - p->trigger.cmd = -EINVAL; + p->trigger.cmd = COMP_TRIGGER_NO_ACTION; ipc_msg_reply(&reply); diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 11d6fa8221a7..d09c6bb0ae58 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -91,6 +91,7 @@ struct timestamp_data; #define COMP_TRIGGER_PRE_RELEASE 10 /**< Prepare to release paused component stream */ #define COMP_TRIGGER_POST_STOP 11 /**< Finalize stop component stream */ #define COMP_TRIGGER_POST_PAUSE 12 /**< Finalize pause component stream */ +#define COMP_TRIGGER_NO_ACTION 13 /**< No action required */ /** @}*/ /** \name Standard Component Control Commands From ee2c502c73e62511b6a26c64a3762da3bf1afd9a Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 3 Nov 2021 15:42:53 +0100 Subject: [PATCH 2/7] ll_schedule: remove an unused structure field struct ll_schedule_domain::registered[] is only used inside ll_schedule_domain.h and only to update itself, it's exclusively self-serving. Remove it. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/schedule/ll_schedule_domain.h | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/include/sof/schedule/ll_schedule_domain.h b/src/include/sof/schedule/ll_schedule_domain.h index 4dd9ecfac041..745ccf0d1e73 100644 --- a/src/include/sof/schedule/ll_schedule_domain.h +++ b/src/include/sof/schedule/ll_schedule_domain.h @@ -55,7 +55,6 @@ struct ll_schedule_domain { bool synchronous; /**< are tasks should be synchronous */ bool full_sync; /**< tasks should be full synchronous, no time dependent */ void *priv_data; /**< pointer to private data */ - bool registered[CONFIG_CORE_COUNT]; /**< registered cores */ bool enabled[CONFIG_CORE_COUNT]; /**< enabled cores */ const struct ll_schedule_domain_ops *ops; /**< domain ops */ }; @@ -121,30 +120,22 @@ static inline int domain_register(struct ll_schedule_domain *domain, struct task *task, void (*handler)(void *arg), void *arg) { - int core = cpu_get_id(); int ret; assert(domain->ops->domain_register); ret = domain->ops->domain_register(domain, task, handler, arg); - if (!ret) { + if (!ret) /* registered one more task, increase the count */ atomic_add(&domain->total_num_tasks, 1); - if (!domain->registered[core]) - /* first task of the core, new client registered */ - domain->registered[core] = true; - } - return ret; } static inline void domain_unregister(struct ll_schedule_domain *domain, struct task *task, uint32_t num_tasks) { - int core = cpu_get_id(); - bool registered = domain->registered[core]; int ret; assert(domain->ops->domain_unregister); @@ -152,22 +143,14 @@ static inline void domain_unregister(struct ll_schedule_domain *domain, /* unregistering a task, decrement the count */ atomic_sub(&domain->total_num_tasks, 1); - /* the last task of the core, unregister the client/core */ - if (!num_tasks && registered) - domain->registered[core] = false; - /* * In some cases .domain_unregister() might not return, terminating the * current thread, that's why we had to update state before calling it. */ ret = domain->ops->domain_unregister(domain, task, num_tasks); - if (ret < 0) { + if (ret < 0) /* Failed to unregister the domain, restore state */ atomic_add(&domain->total_num_tasks, 1); - - if (!num_tasks && domain->registered[core]) - domain->registered[core] = registered; - } } static inline void domain_enable(struct ll_schedule_domain *domain, int core) From 71b7fe6031a4d108438c7b58e0db48d23201dd3b Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 4 Nov 2021 12:21:47 +0100 Subject: [PATCH 3/7] ll_schedule: don't rely on schedule_task_cancel() Currently there are two ways to stop scheduling an LL task: by cancelling it from outside and by returning SOF_TASK_STATE_COMPLETED from the task .run() function. In both cases the task will stop being rescheduled, but in the latter case it will stay on the list of the tasks, waiting to be cancelled. This means that even for those tasks schedule_task_cancel() still has to be called. This is however inconvenient because in some cases there is no suitable context to cancel that task from. E.g. this will be the case with pipeline tasks when we switch them over to handling STOP and PAUSE commands from the task itself. This patch restores the original behaviour when tasks are removed from the list after completion, without waiting for cancellation. For this we extend the calling convention of domain_unregister() to be used with the task parameter equal to NULL when freeing the scheduler. DMA domains preserve their current behaviour: they ignore calls with task == NULL. The timer domain however now uses this call to release the timer interrupt. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/schedule/ll_schedule_domain.h | 5 +++-- src/schedule/dma_multi_chan_domain.c | 2 +- src/schedule/dma_single_chan_domain.c | 2 +- src/schedule/ll_schedule.c | 12 ++++++++---- src/schedule/timer_domain.c | 3 +++ 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/include/sof/schedule/ll_schedule_domain.h b/src/include/sof/schedule/ll_schedule_domain.h index 745ccf0d1e73..6f91061cd681 100644 --- a/src/include/sof/schedule/ll_schedule_domain.h +++ b/src/include/sof/schedule/ll_schedule_domain.h @@ -141,14 +141,15 @@ static inline void domain_unregister(struct ll_schedule_domain *domain, assert(domain->ops->domain_unregister); /* unregistering a task, decrement the count */ - atomic_sub(&domain->total_num_tasks, 1); + if (task) + atomic_sub(&domain->total_num_tasks, 1); /* * In some cases .domain_unregister() might not return, terminating the * current thread, that's why we had to update state before calling it. */ ret = domain->ops->domain_unregister(domain, task, num_tasks); - if (ret < 0) + if (ret < 0 && task) /* Failed to unregister the domain, restore state */ atomic_add(&domain->total_num_tasks, 1); } diff --git a/src/schedule/dma_multi_chan_domain.c b/src/schedule/dma_multi_chan_domain.c index 33f9cb463e2d..4883945aedbb 100644 --- a/src/schedule/dma_multi_chan_domain.c +++ b/src/schedule/dma_multi_chan_domain.c @@ -216,7 +216,7 @@ static int dma_multi_chan_domain_unregister(struct ll_schedule_domain *domain, tr_info(&ll_tr, "dma_multi_chan_domain_unregister()"); /* check if task should be unregistered */ - if (!pipe_task->registrable) + if (!task || !pipe_task->registrable) return 0; for (i = 0; i < dma_domain->num_dma; ++i) { diff --git a/src/schedule/dma_single_chan_domain.c b/src/schedule/dma_single_chan_domain.c index 1d07867ce676..8e3c33b30561 100644 --- a/src/schedule/dma_single_chan_domain.c +++ b/src/schedule/dma_single_chan_domain.c @@ -370,7 +370,7 @@ static int dma_single_chan_domain_unregister(struct ll_schedule_domain *domain, tr_info(&ll_tr, "dma_single_chan_domain_unregister()"); /* check if task should be unregistered */ - if (!pipe_task->registrable) + if (!task || !pipe_task->registrable) return 0; /* channel not registered */ diff --git a/src/schedule/ll_schedule.c b/src/schedule/ll_schedule.c index 558314908140..ec56ce9100f9 100644 --- a/src/schedule/ll_schedule.c +++ b/src/schedule/ll_schedule.c @@ -107,6 +107,12 @@ static void schedule_ll_task_update_start(struct ll_schedule_data *sch, static void schedule_ll_task_done(struct ll_schedule_data *sch, struct task *task) { + /* Remove from the task list, schedule_task_cancel() won't handle it again */ + list_item_del(&task->list); + + /* unregister the task */ + domain_unregister(sch->domain, task, atomic_sub(&sch->num_tasks, 1) - 1); + tr_info(&ll_tr, "task complete %p %pU", task, task->uid); tr_info(&ll_tr, "num_tasks %d total_num_tasks %d", atomic_read(&sch->num_tasks), @@ -137,10 +143,6 @@ static void schedule_ll_tasks_execute(struct ll_schedule_data *sch) /* do we need to reschedule this task */ if (task->state == SOF_TASK_STATE_COMPLETED) { - /* - * keep the task in the list and the subsequent call to - * schedule_ll_task_cancel() will clean it up. - */ schedule_ll_task_done(sch, task); } else { /* update task's start time */ @@ -580,6 +582,8 @@ static void scheduler_free_ll(void *data, uint32_t flags) irq_local_disable(irq_flags); + domain_unregister(sch->domain, NULL, 0); + notifier_unregister(sch, NULL, NOTIFIER_CLK_CHANGE_ID(sch->domain->clk)); diff --git a/src/schedule/timer_domain.c b/src/schedule/timer_domain.c index 6d5cc125547b..fe1cd5cd7d91 100644 --- a/src/schedule/timer_domain.c +++ b/src/schedule/timer_domain.c @@ -70,6 +70,9 @@ static int timer_domain_unregister(struct ll_schedule_domain *domain, struct timer_domain *timer_domain = ll_sch_domain_get_pdata(domain); int core = cpu_get_id(); + if (task) + return 0; + tr_dbg(&ll_tr, "timer_domain_unregister()"); /* tasks still registered on this core */ From 988a976839587a511abbf32b01b8b013bdaa252f Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 27 Jul 2021 22:10:07 +0200 Subject: [PATCH 4/7] pipeline: move STOP / PAUSE trigger to the pipeline task A recent commit moved START and RESUME trigger processing to the pipeline task for timer domain pipelines. This patch does the same for STOP and PAUSE. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-schedule.c | 102 +++++++++++++++++-------- src/audio/pipeline/pipeline-stream.c | 59 ++++++++------ src/include/sof/audio/pipeline.h | 1 + src/schedule/ll_schedule.c | 22 +++++- 4 files changed, 127 insertions(+), 57 deletions(-) diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index 3485a37656a3..db021ceed774 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -35,14 +35,17 @@ static void pipeline_schedule_cancel(struct pipeline *p) /* enable system agent panic, when there are no longer * DMA driven pipelines */ - if (!pipeline_is_timer_driven(p)) - sa_set_panic_on_delay(true); + sa_set_panic_on_delay(true); } 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; + int err, cmd = p->trigger.cmd; pipe_dbg(p, "pipeline_task()"); @@ -79,30 +82,48 @@ static enum task_state pipeline_task(void *arg) return SOF_TASK_STATE_RESCHEDULE; } - if (p->trigger.cmd != COMP_TRIGGER_NO_ACTION) { - /* First pipeline task run for either START or RELEASE: PRE stage */ - struct sof_ipc_reply reply = { - .hdr.cmd = SOF_IPC_GLB_REPLY, - .hdr.size = sizeof(reply), - }; - int cmd = p->trigger.cmd; + if (cmd != COMP_TRIGGER_NO_ACTION) { + if (!p->trigger.host) { + p->trigger.cmd = COMP_TRIGGER_NO_ACTION; + return p->status == COMP_STATE_PAUSED ? SOF_TASK_STATE_COMPLETED : + SOF_TASK_STATE_RESCHEDULE; + } + /* First pipeline task run for either START or RELEASE: PRE stage */ err = pipeline_trigger_run(p, p->trigger.host, cmd); if (err < 0) { pipe_err(p, "pipeline_task(): failed to trigger components: %d", err); reply.error = err; err = SOF_TASK_STATE_COMPLETED; - } else if (err == PPL_STATUS_PATH_STOP) { - pipe_warn(p, "pipeline_task(): stopping for xrun"); - err = SOF_TASK_STATE_COMPLETED; - } else if (p->trigger.cmd != cmd) { - /* PRE stage completed */ - if (p->trigger.delay) - return SOF_TASK_STATE_RESCHEDULE; - /* No delay: the final stage has already run too */ } else { - p->status = COMP_STATE_ACTIVE; - err = SOF_TASK_STATE_RESCHEDULE; + switch (cmd) { + case COMP_TRIGGER_START: + case COMP_TRIGGER_RELEASE: + p->status = COMP_STATE_ACTIVE; + break; + case COMP_TRIGGER_PRE_START: + case COMP_TRIGGER_PRE_RELEASE: + p->status = COMP_STATE_PRE_ACTIVE; + break; + case COMP_TRIGGER_STOP: + case COMP_TRIGGER_PAUSE: + p->status = COMP_STATE_PAUSED; + } + + if (err == PPL_STATUS_PATH_STOP) { + pipe_warn(p, "pipeline_task(): stopping for xrun"); + err = SOF_TASK_STATE_COMPLETED; + } else if (p->trigger.cmd != cmd) { + /* PRE stage completed */ + if (p->trigger.delay) + return SOF_TASK_STATE_RESCHEDULE; + /* No delay: the final stage has already run too */ + } else if (p->status == COMP_STATE_PAUSED && !p->trigger.aborted) { + err = SOF_TASK_STATE_COMPLETED; + } else { + p->status = COMP_STATE_ACTIVE; + err = SOF_TASK_STATE_RESCHEDULE; + } } p->trigger.cmd = COMP_TRIGGER_NO_ACTION; @@ -112,6 +133,13 @@ static enum task_state pipeline_task(void *arg) return err; } + if (p->status == COMP_STATE_PAUSED) + /* + * One of pipelines, being stopped, but not the one, that + * triggers all components + */ + return SOF_TASK_STATE_COMPLETED; + /* * The first execution of the pipeline task above has triggered all * pipeline components. Subsequent iterations actually perform data @@ -190,23 +218,37 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx, case COMP_TRIGGER_STOP: list_for_item(tlist, &ctx->pipelines) { p = container_of(tlist, struct pipeline, list); - pipeline_schedule_cancel(p); - p->status = COMP_STATE_PAUSED; + if (pipeline_is_timer_driven(p) && + p->status != COMP_STATE_PAUSED) { + /* + * Paused pipelines have their tasks stopped + * already, use a running pipeline to trigger + * components. + */ + p->trigger.cmd = cmd; + p->trigger.host = ppl_data->start; + ppl_data->start = NULL; + } else { + pipeline_schedule_cancel(p); + p->status = COMP_STATE_PAUSED; + } } break; case COMP_TRIGGER_PRE_RELEASE: case COMP_TRIGGER_PRE_START: list_for_item(tlist, &ctx->pipelines) { p = container_of(tlist, struct pipeline, list); - if (pipeline_is_timer_driven(p)) { - /* Use the first of connected pipelines to trigger */ - if (first_pipe) { - p->trigger.cmd = cmd; - p->trigger.host = ppl_data->start; - first_pipe = false; - } + p->xrun_bytes = 0; + if (pipeline_is_timer_driven(p) && first_pipe) { + /* + * Use the first of connected pipelines to + * trigger, mark all other connected pipelines + * active immediately. + */ + p->trigger.cmd = cmd; + p->trigger.host = ppl_data->start; + first_pipe = false; } else { - p->xrun_bytes = 0; p->status = COMP_STATE_ACTIVE; } pipeline_schedule_copy(p, 0); diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index 1f4ba3966c07..5033eb66cd18 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -68,7 +68,7 @@ static int pipeline_comp_trigger(struct comp_dev *current, { struct pipeline_data *ppl_data = ctx->comp_data; bool is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); - bool is_same_sched, async; + bool is_same_sched; int err; pipe_dbg(current->pipeline, @@ -76,14 +76,6 @@ static int pipeline_comp_trigger(struct comp_dev *current, dev_comp_id(current), dir); switch (ppl_data->cmd) { - case COMP_TRIGGER_PAUSE: - case COMP_TRIGGER_STOP: - /* - * PAUSE and STOP are triggered in IPC context, not from the - * pipeline task - */ - async = true; - break; case COMP_TRIGGER_PRE_RELEASE: case COMP_TRIGGER_PRE_START: if (comp_get_endpoint_type(current) == COMP_ENDPOINT_DAI) { @@ -91,14 +83,14 @@ static int pipeline_comp_trigger(struct comp_dev *current, ppl_data->delay_ms = dai_get_init_delay_ms(dd->dai); } - - COMPILER_FALLTHROUGH; - case COMP_TRIGGER_RELEASE: - case COMP_TRIGGER_START: - async = !pipeline_is_timer_driven(current->pipeline); break; default: return -EINVAL; + case COMP_TRIGGER_PAUSE: + case COMP_TRIGGER_STOP: + case COMP_TRIGGER_RELEASE: + case COMP_TRIGGER_START: + break; } is_same_sched = pipeline_is_same_sched_comp(current->pipeline, @@ -119,14 +111,26 @@ static int pipeline_comp_trigger(struct comp_dev *current, /* send command to the component and update pipeline state */ err = comp_trigger(current, ppl_data->cmd); - if (err < 0 || err == PPL_STATUS_PATH_STOP) + if (err < 0) return err; + if (err == PPL_STATUS_PATH_STOP) { + current->pipeline->trigger.aborted = true; + return err; + } + + switch (ppl_data->cmd) { + case COMP_TRIGGER_PAUSE: + case COMP_TRIGGER_STOP: + if (pipeline_is_timer_driven(current->pipeline)) + current->pipeline->status = COMP_STATE_PAUSED; + } + /* - * Add scheduling components to the list. This is only needed for the - * stopping flow. + * Add scheduling components to the list. This is only needed for + * asynchronous flows. */ - if (async) + if (!pipeline_is_timer_driven(current->pipeline)) pipeline_comp_trigger_sched_comp(current->pipeline, current, ctx); return pipeline_for_each_comp(current, ctx, dir); @@ -309,14 +313,22 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) pipe_info(p, "pipe trigger cmd %d", cmd); + p->trigger.aborted = false; + switch (cmd) { case COMP_TRIGGER_PAUSE: case COMP_TRIGGER_STOP: - /* Execute immediately */ - ret = pipeline_trigger_run(p, host, cmd); - return ret == PPL_STATUS_PATH_STOP ? 0 : ret; + if (p->status == COMP_STATE_PAUSED || p->xrun_bytes) { + /* The task isn't running, trigger inline */ + ret = pipeline_trigger_run(p, host, cmd); + return ret < 0 ? ret : 0; + } + + COMPILER_FALLTHROUGH; case COMP_TRIGGER_XRUN: - pipeline_trigger_xrun(p, &host); + if (cmd == COMP_TRIGGER_XRUN) + pipeline_trigger_xrun(p, &host); + COMPILER_FALLTHROUGH; case COMP_TRIGGER_PRE_RELEASE: case COMP_TRIGGER_PRE_START: @@ -324,7 +336,7 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) ret = pipeline_trigger_list(p, host, cmd); if (ret < 0) return ret; - /* IPC response will be sent from the task */ + /* IPC response will be sent from the task, unless it was paused */ return 1; } @@ -348,6 +360,7 @@ int pipeline_trigger_run(struct pipeline *p, struct comp_dev *host, int cmd) pipe_dbg(p, "execute trigger cmd %d on pipe %u", cmd, p->pipeline_id); list_init(&walk_ctx.pipelines); + p->trigger.aborted = false; /* handle pipeline global checks before going into each components */ if (p->xrun_bytes) { diff --git a/src/include/sof/audio/pipeline.h b/src/include/sof/audio/pipeline.h index a11519ba1645..3bb9cdf86f1f 100644 --- a/src/include/sof/audio/pipeline.h +++ b/src/include/sof/audio/pipeline.h @@ -77,6 +77,7 @@ struct pipeline { int cmd; struct comp_dev *host; unsigned int delay; /* period count */ + bool aborted; /* STOP or PAUSE failed, stay active */ } trigger; }; diff --git a/src/schedule/ll_schedule.c b/src/schedule/ll_schedule.c index ec56ce9100f9..f9564442f6b8 100644 --- a/src/schedule/ll_schedule.c +++ b/src/schedule/ll_schedule.c @@ -123,22 +123,36 @@ static void schedule_ll_tasks_execute(struct ll_schedule_data *sch) { struct ll_schedule_domain *domain = sch->domain; struct list_item *wlist; - struct list_item *tlist; struct task *task; /* check each task in the list for pending */ - list_for_item_safe(wlist, tlist, &sch->tasks) { - task = container_of(wlist, struct task, list); + wlist = sch->tasks.next; + + /* + * Cannot use list_for_item(_safe)() because the task can cancel some + * other tasks, removing them from the list. This happens, e.g. when + * a pipeline task terminates a DMIC task. + */ + while (wlist != &sch->tasks) { + task = list_item(wlist, struct task, list); - if (task->state != SOF_TASK_STATE_PENDING) + if (task->state != SOF_TASK_STATE_PENDING) { + wlist = task->list.next; continue; + } tr_dbg(&ll_tr, "task %p %pU being started...", task, task->uid); task->state = SOF_TASK_STATE_RUNNING; + /* + * The running task might cancel other tasks, which then get + * removed from the list + */ task->state = task_run(task); + wlist = task->list.next; + spin_lock(&domain->lock); /* do we need to reschedule this task */ From 2809b56c85a1d6561c2ab23328c06a6c334be2d9 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 11 Nov 2021 14:40:41 +0100 Subject: [PATCH 5/7] pipeline: (cosmetic) move a block of code to a function Improve readability by moving a block of code into a function. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-schedule.c | 109 +++++++++++++------------ 1 file changed, 58 insertions(+), 51 deletions(-) diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index db021ceed774..717f0707db2b 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -38,6 +38,60 @@ 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) +{ + int err, cmd = p->trigger.cmd; + + if (!p->trigger.host) { + p->trigger.cmd = COMP_TRIGGER_NO_ACTION; + return p->status == COMP_STATE_PAUSED ? SOF_TASK_STATE_COMPLETED : + SOF_TASK_STATE_RESCHEDULE; + } + + err = pipeline_trigger_run(p, p->trigger.host, cmd); + if (err < 0) { + pipe_err(p, "pipeline_task(): failed to trigger components: %d", err); + reply->error = err; + err = SOF_TASK_STATE_COMPLETED; + } else { + switch (cmd) { + case COMP_TRIGGER_START: + case COMP_TRIGGER_RELEASE: + p->status = COMP_STATE_ACTIVE; + break; + case COMP_TRIGGER_PRE_START: + case COMP_TRIGGER_PRE_RELEASE: + p->status = COMP_STATE_PRE_ACTIVE; + break; + case COMP_TRIGGER_STOP: + case COMP_TRIGGER_PAUSE: + p->status = COMP_STATE_PAUSED; + } + + if (err == PPL_STATUS_PATH_STOP) { + pipe_warn(p, "pipeline_task(): stopping for xrun"); + err = SOF_TASK_STATE_COMPLETED; + } else if (p->trigger.cmd != cmd) { + /* PRE stage completed */ + if (p->trigger.delay) + return SOF_TASK_STATE_RESCHEDULE; + /* No delay: the final stage has already run too */ + } else if (p->status == COMP_STATE_PAUSED && !p->trigger.aborted) { + err = SOF_TASK_STATE_COMPLETED; + } else { + p->status = COMP_STATE_ACTIVE; + err = SOF_TASK_STATE_RESCHEDULE; + } + } + + p->trigger.cmd = COMP_TRIGGER_NO_ACTION; + + ipc_msg_reply(reply); + + return err; +} + static enum task_state pipeline_task(void *arg) { struct sof_ipc_reply reply = { @@ -45,7 +99,7 @@ static enum task_state pipeline_task(void *arg) .hdr.size = sizeof(reply), }; struct pipeline *p = arg; - int err, cmd = p->trigger.cmd; + int err; pipe_dbg(p, "pipeline_task()"); @@ -82,56 +136,9 @@ static enum task_state pipeline_task(void *arg) return SOF_TASK_STATE_RESCHEDULE; } - if (cmd != COMP_TRIGGER_NO_ACTION) { - if (!p->trigger.host) { - p->trigger.cmd = COMP_TRIGGER_NO_ACTION; - return p->status == COMP_STATE_PAUSED ? SOF_TASK_STATE_COMPLETED : - SOF_TASK_STATE_RESCHEDULE; - } - - /* First pipeline task run for either START or RELEASE: PRE stage */ - err = pipeline_trigger_run(p, p->trigger.host, cmd); - if (err < 0) { - pipe_err(p, "pipeline_task(): failed to trigger components: %d", err); - reply.error = err; - err = SOF_TASK_STATE_COMPLETED; - } else { - switch (cmd) { - case COMP_TRIGGER_START: - case COMP_TRIGGER_RELEASE: - p->status = COMP_STATE_ACTIVE; - break; - case COMP_TRIGGER_PRE_START: - case COMP_TRIGGER_PRE_RELEASE: - p->status = COMP_STATE_PRE_ACTIVE; - break; - case COMP_TRIGGER_STOP: - case COMP_TRIGGER_PAUSE: - p->status = COMP_STATE_PAUSED; - } - - if (err == PPL_STATUS_PATH_STOP) { - pipe_warn(p, "pipeline_task(): stopping for xrun"); - err = SOF_TASK_STATE_COMPLETED; - } else if (p->trigger.cmd != cmd) { - /* PRE stage completed */ - if (p->trigger.delay) - return SOF_TASK_STATE_RESCHEDULE; - /* No delay: the final stage has already run too */ - } else if (p->status == COMP_STATE_PAUSED && !p->trigger.aborted) { - err = SOF_TASK_STATE_COMPLETED; - } else { - p->status = COMP_STATE_ACTIVE; - err = SOF_TASK_STATE_RESCHEDULE; - } - } - - p->trigger.cmd = COMP_TRIGGER_NO_ACTION; - - ipc_msg_reply(&reply); - - return err; - } + if (p->trigger.cmd != COMP_TRIGGER_NO_ACTION) + /* Process an offloaded command */ + return pipeline_task_cmd(p, &reply); if (p->status == COMP_STATE_PAUSED) /* From 68bccc84d7c4126f241d439bc8d4e6bfd658f7ad Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 7 Sep 2021 15:29:16 +0200 Subject: [PATCH 6/7] pipeline: (cosmetic) fix function names in trace text Fix trace messages after moving them into different functions. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-schedule.c | 4 ++-- src/audio/pipeline/pipeline-stream.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index 717f0707db2b..baba00083b89 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -51,7 +51,7 @@ static enum task_state pipeline_task_cmd(struct pipeline *p, err = pipeline_trigger_run(p, p->trigger.host, cmd); if (err < 0) { - pipe_err(p, "pipeline_task(): failed to trigger components: %d", err); + pipe_err(p, "pipeline_task_cmd(): failed to trigger components: %d", err); reply->error = err; err = SOF_TASK_STATE_COMPLETED; } else { @@ -70,7 +70,7 @@ static enum task_state pipeline_task_cmd(struct pipeline *p, } if (err == PPL_STATUS_PATH_STOP) { - pipe_warn(p, "pipeline_task(): stopping for xrun"); + pipe_warn(p, "pipeline_task_cmd(): stopping for xrun"); err = SOF_TASK_STATE_COMPLETED; } else if (p->trigger.cmd != cmd) { /* PRE stage completed */ diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index 5033eb66cd18..fc84cecd9bbd 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -407,7 +407,7 @@ int pipeline_trigger_run(struct pipeline *p, struct comp_dev *host, int cmd) ret = walk_ctx.comp_func(host, NULL, &walk_ctx, host->direction); if (ret < 0) - pipe_err(p, "pipeline_trigger(): ret = %d, host->comp.id = %u, cmd = %d", + pipe_err(p, "pipeline_trigger_run(): ret = %d, host->comp.id = %u, cmd = %d", ret, dev_comp_id(host), cmd); else if (ret == PPL_STATUS_PATH_STOP) ret = 0; From 4a20c74dff9260c31c9bdebbe745c6516b99bb35 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 11 Nov 2021 15:12:48 +0100 Subject: [PATCH 7/7] pipeline: (cosmetic) move a function Move a static function closer to where it is used. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-stream.c | 150 +++++++++++++-------------- 1 file changed, 75 insertions(+), 75 deletions(-) diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index fc84cecd9bbd..ddd199c22d1e 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -61,81 +61,6 @@ pipeline_should_report_enodata_on_trigger(struct comp_dev *rsrc, return false; } -/* Runs in IPC or in pipeline task context */ -static int pipeline_comp_trigger(struct comp_dev *current, - struct comp_buffer *calling_buf, - struct pipeline_walk_context *ctx, int dir) -{ - struct pipeline_data *ppl_data = ctx->comp_data; - bool is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); - bool is_same_sched; - int err; - - pipe_dbg(current->pipeline, - "pipeline_comp_trigger(), current->comp.id = %u, dir = %u", - dev_comp_id(current), dir); - - switch (ppl_data->cmd) { - case COMP_TRIGGER_PRE_RELEASE: - case COMP_TRIGGER_PRE_START: - if (comp_get_endpoint_type(current) == COMP_ENDPOINT_DAI) { - struct dai_data *dd = comp_get_drvdata(current); - - ppl_data->delay_ms = dai_get_init_delay_ms(dd->dai); - } - break; - default: - return -EINVAL; - case COMP_TRIGGER_PAUSE: - case COMP_TRIGGER_STOP: - case COMP_TRIGGER_RELEASE: - case COMP_TRIGGER_START: - break; - } - - is_same_sched = pipeline_is_same_sched_comp(current->pipeline, - ppl_data->start->pipeline); - - /* trigger should propagate to the connected pipelines, - * which need to be scheduled together - */ - if (!is_single_ppl && !is_same_sched) { - pipe_dbg(current->pipeline, - "pipeline_comp_trigger(), current is from another pipeline"); - - if (pipeline_should_report_enodata_on_trigger(current, ctx, dir)) - return -ENODATA; - - return 0; - } - - /* send command to the component and update pipeline state */ - err = comp_trigger(current, ppl_data->cmd); - if (err < 0) - return err; - - if (err == PPL_STATUS_PATH_STOP) { - current->pipeline->trigger.aborted = true; - return err; - } - - switch (ppl_data->cmd) { - case COMP_TRIGGER_PAUSE: - case COMP_TRIGGER_STOP: - if (pipeline_is_timer_driven(current->pipeline)) - current->pipeline->status = COMP_STATE_PAUSED; - } - - /* - * Add scheduling components to the list. This is only needed for - * asynchronous flows. - */ - if (!pipeline_is_timer_driven(current->pipeline)) - pipeline_comp_trigger_sched_comp(current->pipeline, current, ctx); - - return pipeline_for_each_comp(current, ctx, dir); -} - static int pipeline_comp_copy(struct comp_dev *current, struct comp_buffer *calling_buf, struct pipeline_walk_context *ctx, int dir) @@ -343,6 +268,81 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) return 0; } +/* Runs in IPC or in pipeline task context */ +static int pipeline_comp_trigger(struct comp_dev *current, + struct comp_buffer *calling_buf, + struct pipeline_walk_context *ctx, int dir) +{ + struct pipeline_data *ppl_data = ctx->comp_data; + bool is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); + bool is_same_sched; + int err; + + pipe_dbg(current->pipeline, + "pipeline_comp_trigger(), current->comp.id = %u, dir = %u", + dev_comp_id(current), dir); + + switch (ppl_data->cmd) { + case COMP_TRIGGER_PRE_RELEASE: + case COMP_TRIGGER_PRE_START: + if (comp_get_endpoint_type(current) == COMP_ENDPOINT_DAI) { + struct dai_data *dd = comp_get_drvdata(current); + + ppl_data->delay_ms = dai_get_init_delay_ms(dd->dai); + } + break; + default: + return -EINVAL; + case COMP_TRIGGER_PAUSE: + case COMP_TRIGGER_STOP: + case COMP_TRIGGER_RELEASE: + case COMP_TRIGGER_START: + break; + } + + is_same_sched = pipeline_is_same_sched_comp(current->pipeline, + ppl_data->start->pipeline); + + /* trigger should propagate to the connected pipelines, + * which need to be scheduled together + */ + if (!is_single_ppl && !is_same_sched) { + pipe_dbg(current->pipeline, + "pipeline_comp_trigger(), current is from another pipeline"); + + if (pipeline_should_report_enodata_on_trigger(current, ctx, dir)) + return -ENODATA; + + return 0; + } + + /* send command to the component and update pipeline state */ + err = comp_trigger(current, ppl_data->cmd); + if (err < 0) + return err; + + if (err == PPL_STATUS_PATH_STOP) { + current->pipeline->trigger.aborted = true; + return err; + } + + switch (ppl_data->cmd) { + case COMP_TRIGGER_PAUSE: + case COMP_TRIGGER_STOP: + if (pipeline_is_timer_driven(current->pipeline)) + current->pipeline->status = COMP_STATE_PAUSED; + } + + /* + * Add scheduling components to the list. This is only needed for + * asynchronous flows. + */ + if (!pipeline_is_timer_driven(current->pipeline)) + pipeline_comp_trigger_sched_comp(current->pipeline, current, ctx); + + return pipeline_for_each_comp(current, ctx, dir); +} + /* actually execute pipeline trigger, including components: either in IPC or in task context */ int pipeline_trigger_run(struct pipeline *p, struct comp_dev *host, int cmd) {