From 5d481090f30af72cac7b64daedd4cac1f183ce45 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 9 Jul 2021 16:11:53 +0200 Subject: [PATCH 1/3] pipeline: (cosmetic) simplify pipeline_task_init() pipeline_task_init() is always called with pipeline_task as its last parameter. Remove the parameter and use the function explicitly. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-schedule.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index df2b4412e300..ceb146ae0b66 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -71,8 +71,7 @@ static enum task_state pipeline_task(void *arg) return SOF_TASK_STATE_RESCHEDULE; } -static struct task *pipeline_task_init(struct pipeline *p, uint32_t type, - enum task_state (*func)(void *data)) +static struct task *pipeline_task_init(struct pipeline *p, uint32_t type) { struct pipeline_task *task = NULL; @@ -82,7 +81,7 @@ static struct task *pipeline_task_init(struct pipeline *p, uint32_t type, return NULL; if (schedule_task_init_ll(&task->task, SOF_UUID(pipe_task_uuid), type, - p->priority, func, + p->priority, pipeline_task, p, p->core, 0) < 0) { rfree(task); return NULL; @@ -159,7 +158,7 @@ int pipeline_comp_task_init(struct pipeline *p) type = pipeline_is_timer_driven(p) ? SOF_SCHEDULE_LL_TIMER : SOF_SCHEDULE_LL_DMA; - p->pipe_task = pipeline_task_init(p, type, pipeline_task); + p->pipe_task = pipeline_task_init(p, type); if (!p->pipe_task) { pipe_err(p, "pipeline_comp_task_init(): task init failed"); return -ENOMEM; From 47b12d33ad6e23558dc30f1fe4dc1cdc7038536c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 9 Jul 2021 16:39:26 +0200 Subject: [PATCH 2/3] pipeline: (cosmetic) use bool type, adjust line lengths Use bool type for boolean flags, split some too long lines, merge some needlessly split lines. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-stream.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index 578447df7030..0277bf751ea3 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -64,23 +64,24 @@ static int pipeline_comp_trigger(struct comp_dev *current, struct pipeline_walk_context *ctx, int dir) { struct pipeline_data *ppl_data = ctx->comp_data; - int is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); - int is_same_sched = + bool is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); + bool is_same_sched = pipeline_is_same_sched_comp(current->pipeline, ppl_data->start->pipeline); int err; - pipe_dbg(current->pipeline, "pipeline_comp_trigger(), current->comp.id = %u, dir = %u", + pipe_dbg(current->pipeline, + "pipeline_comp_trigger(), current->comp.id = %u, dir = %u", dev_comp_id(current), dir); /* 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"); + pipe_dbg(current->pipeline, + "pipeline_comp_trigger(), current is from another pipeline"); - if (pipeline_should_report_enodata_on_trigger(current, ctx, - dir)) + if (pipeline_should_report_enodata_on_trigger(current, ctx, dir)) return -ENODATA; return 0; @@ -101,14 +102,15 @@ static int pipeline_comp_copy(struct comp_dev *current, struct pipeline_walk_context *ctx, int dir) { struct pipeline_data *ppl_data = ctx->comp_data; - int is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); + bool is_single_ppl = comp_is_single_pipeline(current, ppl_data->start); int err; pipe_dbg(current->pipeline, "pipeline_comp_copy(), current->comp.id = %u, dir = %u", dev_comp_id(current), dir); if (!is_single_ppl) { - pipe_dbg(current->pipeline, "pipeline_comp_copy(), current is from another pipeline and can't be scheduled together"); + pipe_dbg(current->pipeline, + "pipeline_comp_copy(), current is from another pipeline and can't be scheduled together"); return 0; } From b6776ec3a5b04945c5e02833e64e85820b797c31 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 9 Jul 2021 16:58:44 +0200 Subject: [PATCH 3/3] pipeline: trigger START from the task context When the firmware receives a START or RELEASE IPC message, it immediately triggers all involved components, which starts DMA. Then it schedules the pipeline task, but since the scheduler can be already running at that time, the task might be scheduled when DMA data isn't available yet or has already overflowed. To fix this change the control flow to also trigger all components from the task during its first run. Actual data processing then begins with the next period. Note, that this is currently only possible with pipelines, using timer-based scheduling. Pipelines, using DMA interrupts for scheduling are unaffected. Signed-off-by: Guennadi Liakhovetski --- src/audio/pipeline/pipeline-graph.c | 1 + src/audio/pipeline/pipeline-schedule.c | 74 ++++++++++---- src/audio/pipeline/pipeline-stream.c | 134 ++++++++++++++++++++++--- src/audio/pipeline/pipeline-xrun.c | 2 +- src/drivers/intel/cavs/ipc.c | 7 +- src/include/sof/audio/pipeline.h | 20 +++- src/include/sof/ipc/common.h | 7 ++ src/ipc/ipc-common.c | 9 ++ src/ipc/ipc3/handler.c | 15 ++- 9 files changed, 228 insertions(+), 41 deletions(-) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index f4e907c271cc..d013fd2a59fd 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -127,6 +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; 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 ceb146ae0b66..ce536e366ae1 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -55,12 +55,44 @@ static enum task_state pipeline_task(void *arg) return SOF_TASK_STATE_COMPLETED; } + if (p->trigger.cmd >= 0) { + /* First pipeline task run for either START or RELEASE */ + struct sof_ipc_reply reply = { + .hdr.cmd = SOF_IPC_GLB_REPLY, + .hdr.size = sizeof(reply), + }; + + err = pipeline_trigger_run(p, p->trigger.host, p->trigger.cmd); + p->trigger.cmd = -EINVAL; + + 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 { + p->status = COMP_STATE_ACTIVE; + err = SOF_TASK_STATE_RESCHEDULE; + } + + ipc_msg_reply(&reply); + + return err; + } + + /* + * 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 */ err = pipeline_xrun_recover(p); if (err < 0) { - pipe_err(p, "pipeline_task(): xrun recover failed! pipeline will be stopped!"); + pipe_err(p, "pipeline_task(): xrun recovery failed! pipeline is stopped."); /* failed - host will stop this pipeline */ return SOF_TASK_STATE_COMPLETED; } @@ -107,9 +139,11 @@ int pipeline_schedule_config(struct pipeline *p, uint32_t sched_id, return 0; } +/* trigger connected pipelines: either immediately or schedule them */ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx, int cmd) { + struct pipeline_data *ppl_data = ctx->comp_data; struct list_item *tlist; struct pipeline *p; uint32_t flags; @@ -121,25 +155,31 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx, */ irq_local_disable(flags); - list_for_item(tlist, &ctx->pipelines) { - p = container_of(tlist, struct pipeline, list); - - switch (cmd) { - case COMP_TRIGGER_PAUSE: - case COMP_TRIGGER_STOP: + switch (cmd) { + case COMP_TRIGGER_PAUSE: + 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; - break; - case COMP_TRIGGER_RELEASE: - case COMP_TRIGGER_START: + } + break; + case COMP_TRIGGER_RELEASE: + case COMP_TRIGGER_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 (cmd >= 0) { + p->trigger.cmd = cmd; + p->trigger.host = ppl_data->start; + cmd = -EINVAL; + } + } else { + p->xrun_bytes = 0; + p->status = COMP_STATE_ACTIVE; + } pipeline_schedule_copy(p, 0); - p->xrun_bytes = 0; - p->status = COMP_STATE_ACTIVE; - break; - case COMP_TRIGGER_SUSPEND: - case COMP_TRIGGER_RESUME: - default: - break; } } diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index 0277bf751ea3..8350036012dc 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -59,21 +59,40 @@ 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 = - pipeline_is_same_sched_comp(current->pipeline, - ppl_data->start->pipeline); + bool is_same_sched, async; 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_PAUSE: + case COMP_TRIGGER_STOP: + /* + * PAUSE and STOP are triggered in IPC context, not from the + * pipeline task + */ + async = true; + break; + case COMP_TRIGGER_RELEASE: + case COMP_TRIGGER_START: + async = !pipeline_is_timer_driven(current->pipeline); + break; + default: + return -EINVAL; + } + + 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 */ @@ -92,7 +111,12 @@ static int pipeline_comp_trigger(struct comp_dev *current, if (err < 0 || err == PPL_STATUS_PATH_STOP) return err; - pipeline_comp_trigger_sched_comp(current->pipeline, current, ctx); + /* + * Add scheduling components to the list. This is only needed for the + * stopping flow. + */ + if (async) + pipeline_comp_trigger_sched_comp(current->pipeline, current, ctx); return pipeline_for_each_comp(current, ctx, dir); } @@ -172,10 +196,87 @@ int pipeline_copy(struct pipeline *p) return ret; } -/* trigger pipeline */ +/* only collect scheduling components */ +static int pipeline_comp_list(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 = pipeline_is_same_sched_comp(current->pipeline, + ppl_data->start->pipeline); + + if (!is_single_ppl && !is_same_sched) { + pipe_dbg(current->pipeline, + "pipeline_comp_list(), current is from another pipeline"); + return 0; + } + + /* Add scheduling components to the list */ + pipeline_comp_trigger_sched_comp(current->pipeline, current, ctx); + + return pipeline_for_each_comp(current, ctx, dir); +} + +/* build a list of connected pipelines' scheduling components and trigger them */ +static int pipeline_trigger_list(struct pipeline *p, struct comp_dev *host, int cmd) +{ + struct pipeline_data data = { + .start = host, + .cmd = cmd, + }; + struct pipeline_walk_context walk_ctx = { + .comp_func = pipeline_comp_list, + .comp_data = &data, + .skip_incomplete = true, + }; + int ret; + + list_init(&walk_ctx.pipelines); + + ret = walk_ctx.comp_func(host, NULL, &walk_ctx, host->direction); + if (ret < 0) + pipe_err(p, "pipeline_trigger_list(): ret = %d, host->comp.id = %u, cmd = %d", + ret, dev_comp_id(host), cmd); + else + pipeline_schedule_triggered(&walk_ctx, cmd); + + return ret; +} + +/* trigger pipeline in IPC context */ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) { - struct pipeline_data data; + int ret; + + pipe_info(p, "pipe trigger cmd %d", cmd); + + 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; + case COMP_TRIGGER_RELEASE: + case COMP_TRIGGER_START: + /* Add all connected pipelines to the list and schedule them all */ + ret = pipeline_trigger_list(p, host, cmd); + if (ret < 0) + return ret; + /* IPC response will be sent from the task */ + return 1; + } + + return 0; +} + +/* 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) +{ + struct pipeline_data data = { + .start = host, + .cmd = cmd, + }; struct pipeline_walk_context walk_ctx = { .comp_func = pipeline_comp_trigger, .comp_data = &data, @@ -183,7 +284,7 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) }; int ret; - pipe_info(p, "pipe trigger cmd %d", cmd); + pipe_dbg(p, "execute trigger cmd %d on pipe %u", cmd, p->pipeline_id); list_init(&walk_ctx.pipelines); @@ -193,20 +294,23 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) if (ret < 0) { pipe_err(p, "xrun handle: ret = %d", ret); return ret; - } else if (ret == PPL_STATUS_PATH_STOP) + } + + if (ret == PPL_STATUS_PATH_STOP) /* no further action needed*/ - return 0; + return pipeline_is_timer_driven(p); } - data.start = host; - data.cmd = 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", + if (ret < 0) + pipe_err(p, "pipeline_trigger_run(): ret = %d, host->comp.id = %u, cmd = %d", ret, dev_comp_id(host), cmd); - } + /* + * When called from the pipeline task, pipeline_comp_trigger() will not + * add pipelines to the list, so pipeline_schedule_triggered() will have + * no effect. + */ pipeline_schedule_triggered(&walk_ctx, cmd); return ret; diff --git a/src/audio/pipeline/pipeline-xrun.c b/src/audio/pipeline/pipeline-xrun.c index 1f22fbb93b51..04b85f3abfab 100644 --- a/src/audio/pipeline/pipeline-xrun.c +++ b/src/audio/pipeline/pipeline-xrun.c @@ -153,7 +153,7 @@ void pipeline_xrun(struct pipeline *p, struct comp_dev *dev, return; /* notify all pipeline comps we are in XRUN, and stop copying */ - ret = pipeline_trigger(p, p->source_comp, COMP_TRIGGER_XRUN); + ret = pipeline_trigger_run(p, p->source_comp, COMP_TRIGGER_XRUN); if (ret < 0) pipe_err(p, "pipeline_xrun(): Pipelines notification about XRUN failed, ret = %d", ret); diff --git a/src/drivers/intel/cavs/ipc.c b/src/drivers/intel/cavs/ipc.c index 67528d6748b4..a272ebb8e46b 100644 --- a/src/drivers/intel/cavs/ipc.c +++ b/src/drivers/intel/cavs/ipc.c @@ -201,10 +201,13 @@ enum task_state ipc_platform_do_cmd(void *data) void ipc_platform_complete_cmd(void *data) { struct ipc *ipc = data; + uint32_t flags; - if (!cpu_is_me(ipc->core)) + if (!cpu_is_me(ipc->core) || ipc->delayed_response) return; + spin_lock_irq(&ipc->lock, flags); + /* write 1 to clear busy, and trigger interrupt to host*/ #if CAVS_VERSION == CAVS_VERSION_1_5 ipc_write(IPC_DIPCT, ipc_read(IPC_DIPCT) | IPC_DIPCT_BUSY); @@ -229,6 +232,8 @@ void ipc_platform_complete_cmd(void *data) } #endif + + spin_unlock_irq(&ipc->lock, flags); } int ipc_platform_send_msg(struct ipc_msg *msg) diff --git a/src/include/sof/audio/pipeline.h b/src/include/sof/audio/pipeline.h index bc3f5fc0e411..a23b9f7bf61d 100644 --- a/src/include/sof/audio/pipeline.h +++ b/src/include/sof/audio/pipeline.h @@ -73,6 +73,10 @@ struct pipeline { /* position update */ uint32_t posn_offset; /* position update array offset*/ struct ipc_msg *msg; + struct { + int cmd; + struct comp_dev *host; + } trigger; }; struct pipeline_walk_context { @@ -218,14 +222,22 @@ int pipeline_prepare(struct pipeline *p, struct comp_dev *cd); */ /** - * \brief Trigger pipeline - atomic + * \brief Trigger pipeline - IPC context * \param[in] p pipeline. - * \param[in] host_cd Host DMA component. + * \param[in] host Host DMA component. * \param[in] cmd Pipeline trigger command. * \return 0 on success. */ -/* */ -int pipeline_trigger(struct pipeline *p, struct comp_dev *host_cd, int cmd); +int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd); + +/** + * \brief Trigger pipeline - either IPC or pipeline task context + * \param[in] p pipeline. + * \param[in] host Host DMA component. + * \param[in] cmd Pipeline trigger command. + * \return 0 on success. + */ +int pipeline_trigger_run(struct pipeline *p, struct comp_dev *host, int cmd); /** * \brief Copy data along a pipeline. diff --git a/src/include/sof/ipc/common.h b/src/include/sof/ipc/common.h index a53010a02f69..79cf8de4c8e5 100644 --- a/src/include/sof/ipc/common.h +++ b/src/include/sof/ipc/common.h @@ -68,6 +68,7 @@ struct ipc { struct list_item msg_list; /* queue of messages to be sent */ bool is_notification_pending; /* notification is being sent to host */ + bool delayed_response; /* response will be sent from a different context */ unsigned int core; /* core, processing the IPC */ struct list_item comp_list; /* list of component devices */ @@ -196,4 +197,10 @@ void ipc_cmd(ipc_cmd_hdr *hdr); */ int ipc_process_on_core(uint32_t core, bool blocking); +/** + * \brief reply to an IPC message. + * @param[in] reply pointer to the reply structure. + */ +void ipc_msg_reply(struct sof_ipc_reply *reply); + #endif /* __SOF_DRIVERS_IPC_H__ */ diff --git a/src/ipc/ipc-common.c b/src/ipc/ipc-common.c index f88412558a37..9c341636de90 100644 --- a/src/ipc/ipc-common.c +++ b/src/ipc/ipc-common.c @@ -220,6 +220,15 @@ void ipc_msg_send(struct ipc_msg *msg, void *data, bool high_priority) spin_unlock_irq(&ipc->lock, flags); } +void ipc_msg_reply(struct sof_ipc_reply *reply) +{ + struct ipc *ipc = ipc_get(); + + ipc->delayed_response = false; + mailbox_hostbox_write(0, reply, reply->hdr.size); + ipc_platform_complete_cmd(ipc); +} + void ipc_schedule_process(struct ipc *ipc) { schedule_task(&ipc->ipc_task, 0, IPC_PERIOD_USEC); diff --git a/src/ipc/ipc3/handler.c b/src/ipc/ipc3/handler.c index eb00efe70033..cf01a1e9a6a9 100644 --- a/src/ipc/ipc3/handler.c +++ b/src/ipc/ipc3/handler.c @@ -438,11 +438,18 @@ static int ipc_stream_trigger(uint32_t header) } /* trigger the component */ - ret = pipeline_trigger(pcm_dev->cd->pipeline, pcm_dev->cd, cmd); - if (ret < 0) { + if (pipeline_is_timer_driven(pcm_dev->cd->pipeline)) { + ret = pipeline_trigger(pcm_dev->cd->pipeline, pcm_dev->cd, cmd); + if (ret > 0) + ipc->delayed_response = true; + } else { + ret = pipeline_trigger_run(pcm_dev->cd->pipeline, pcm_dev->cd, cmd); + } + + if (ret < 0) tr_err(&ipc_tr, "ipc: comp %d trigger 0x%x failed %d", stream.comp_id, ipc_command, ret); - } + return ret; } @@ -1510,6 +1517,8 @@ void ipc_cmd(ipc_cmd_hdr *_hdr) /* A new IPC from the host, delivered to the primary core */ ipc->core = PLATFORM_PRIMARY_CORE_ID; + ipc->delayed_response = false; + type = iGS(hdr->cmd); switch (type) {