[Sound-open-firmware] [PATCH] scheduler: Allow tasks to be scheduled when DSP enters idle state
Add an API to allow tasks to be added to the scheduler task list but not run until the DSP enters an idle state. i.e. we schedule the task as normal but dont immediately call schedule() to schedule that task for execution and instead wait until schedule() is next called (at the end of some other work).
This patch also calls schedule() before entering the idle state in the main processing loop.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/include/reef/schedule.h | 2 ++ src/lib/schedule.c | 49 ++++++++++++++++++++++++++++++++++----------- src/tasks/audio.c | 3 +++ 3 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/src/include/reef/schedule.h b/src/include/reef/schedule.h index 6d94788..68aa0ad 100644 --- a/src/include/reef/schedule.h +++ b/src/include/reef/schedule.h @@ -77,6 +77,8 @@ void schedule(void);
void schedule_task(struct task *task, uint64_t start, uint64_t deadline);
+void schedule_task_idle(struct task *task, uint64_t deadline); + void schedule_task_complete(struct task *task);
static inline void schedule_task_init(struct task *task, void (*func)(void *), diff --git a/src/lib/schedule.c b/src/lib/schedule.c index 942067c..1310e83 100644 --- a/src/lib/schedule.c +++ b/src/lib/schedule.c @@ -233,15 +233,8 @@ out: } #endif
-/* - * Add a new task to the scheduler to be run and define a scheduling - * window in time for the task to be ran. i.e. task will run between start and - * deadline times. - * - * start is in microseconds relative to last task start time. - * deadline is in microseconds relative to start. - */ -void schedule_task(struct task *task, uint64_t start, uint64_t deadline) + +static int _schedule_task(struct task *task, uint64_t start, uint64_t deadline) { uint32_t flags; uint64_t current; @@ -254,7 +247,7 @@ void schedule_task(struct task *task, uint64_t start, uint64_t deadline) if (task->state == TASK_STATE_RUNNING) { trace_pipe("tsk"); spin_unlock_irq(&sch->lock, flags); - return; + return 0; }
/* get the current time */ @@ -275,8 +268,40 @@ void schedule_task(struct task *task, uint64_t start, uint64_t deadline) task->state = TASK_STATE_QUEUED; spin_unlock_irq(&sch->lock, flags);
- /* rerun scheduler */ - schedule(); + return 1; +} + +/* + * Add a new task to the scheduler to be run and define a scheduling + * deadline in time for the task to be ran. Do not invoke the scheduler + * immediately to run task, but wait intil schedule is next called. + * + * deadline is in microseconds relative to start. + */ +void schedule_task_idle(struct task *task, uint64_t deadline) +{ + _schedule_task(task, 0, deadline); +} + +/* + * Add a new task to the scheduler to be run and define a scheduling + * window in time for the task to be ran. i.e. task will run between start and + * deadline times. + * + * start is in microseconds relative to last task start time. + * deadline is in microseconds relative to start. + */ +void schedule_task(struct task *task, uint64_t start, uint64_t deadline) +{ + int need_sched; + + need_sched = _schedule_task(task, start, deadline); + + /* need to run scheduler if task not already running */ + if (need_sched) { + /* rerun scheduler */ + schedule(); + } }
/* Remove a task from the scheduler when complete */ diff --git a/src/tasks/audio.c b/src/tasks/audio.c index 68f7702..af241d7 100644 --- a/src/tasks/audio.c +++ b/src/tasks/audio.c @@ -85,6 +85,9 @@ int do_task(struct reef *reef)
/* now process any IPC messages from host */ ipc_process_msg_queue(); + + /* schedule any idle taks */ + schedule(); }
/* something bad happened */
Minimise any context switches in the scheduler by first checking for any queued tasks in the task list prior to changing context. If there are no queued tasks then there is no need to context switch.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/lib/schedule.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/src/lib/schedule.c b/src/lib/schedule.c index 1310e83..bfbb1b9 100644 --- a/src/lib/schedule.c +++ b/src/lib/schedule.c @@ -333,8 +333,31 @@ static void scheduler_run(void *unused) /* run the scheduler */ void schedule(void) { + struct list_item *tlist; + struct task *task; + uint32_t flags; + tracev_pipe("sch");
+ spin_lock_irq(&sch->lock, flags); + + /* make sure we have a queued task in the list first before we + start scheduling as contexts switches are not free. */ + list_for_item(tlist, &sch->list) { + task = container_of(tlist, struct task, list); + + /* schedule if we find any queued tasks */ + if (task->state == TASK_STATE_QUEUED) { + spin_unlock_irq(&sch->lock, flags); + goto schedule; + } + } + + /* no task to schedule */ + spin_unlock_irq(&sch->lock, flags); + return; + +schedule: /* TODO: detect current IRQ context and call scheduler_run if both * current context matches scheduler context. saves a DSP context * switch.
Use the new idle schedule API to schedule a pipeline copy for preload after prepare() and before trigger(). This pipeline preload copy will run within the same context as the regular pipeline copy.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/pipeline.c | 11 +++++++++++ src/include/reef/audio/pipeline.h | 1 + 2 files changed, 12 insertions(+)
diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 61d80b4..40ccdc5 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -200,6 +200,9 @@ static void pipeline_cmd_update(struct pipeline *p, struct comp_dev *comp, case COMP_CMD_RELEASE: p->xrun_bytes = 0;
+ /* schedule initial pipeline fill when next idle */ + pipeline_schedule_copy_idle(p); + /* schedule pipeline */ if (p->ipc_pipe.timer) pipeline_schedule_copy(p, 0); @@ -983,6 +986,14 @@ void pipeline_schedule_copy(struct pipeline *p, uint64_t start) schedule_task(&p->pipe_task, start, p->ipc_pipe.deadline); }
+/* notify pipeline that this component requires buffers emptied/filled + * when DSP is next idle. This is intended to be used to preload pipeline + * buffers prior to trigger start. */ +void pipeline_schedule_copy_idle(struct pipeline *p) +{ + schedule_task_idle(&p->pipe_task, p->ipc_pipe.deadline); +} + void pipeline_schedule_cancel(struct pipeline *p) { schedule_task_complete(&p->pipe_task); diff --git a/src/include/reef/audio/pipeline.h b/src/include/reef/audio/pipeline.h index 41cfffc..c5ba49f 100644 --- a/src/include/reef/audio/pipeline.h +++ b/src/include/reef/audio/pipeline.h @@ -114,6 +114,7 @@ int init_pipeline(void);
/* schedule a copy operation for this pipeline */ void pipeline_schedule_copy(struct pipeline *p, uint64_t start); +void pipeline_schedule_copy_idle(struct pipeline *p); void pipeline_schedule_cancel(struct pipeline *p);
/* get time pipeline timestamps from host to dai */
Currently the preloader is bespoke and runs in the same context as the IPC. This means some components may delay IPC and block other components.
Reuse the existing pipeline copy() infrastructure so that the preload is done after prepare() but before trigger() using the same pipeline copy code on the same DSP core and context. i.e. we preload a host buffer (for playback) and then schedule the pipeline copy(s) after the preload IPC is done.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/dai.c | 13 ----- src/audio/eq_fir.c | 11 ---- src/audio/eq_iir.c | 11 ---- src/audio/host.c | 58 ++++--------------- src/audio/mixer.c | 8 --- src/audio/pipeline.c | 110 ++++++++++++++++++++++--------------- src/audio/src.c | 8 --- src/audio/tone.c | 6 -- src/audio/volume.c | 19 ------- src/include/reef/audio/buffer.h | 19 +++++-- src/include/reef/audio/component.h | 35 ------------ 11 files changed, 90 insertions(+), 208 deletions(-)
diff --git a/src/audio/dai.c b/src/audio/dai.c index 4d3f0b7..96b37ba 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -268,9 +268,6 @@ static int dai_playback_params(struct comp_dev *dev) } }
- /* set write pointer to start of buffer */ - buffer_reset_pos(dma_buffer); - return 0;
err_unwind: @@ -336,9 +333,6 @@ static int dai_capture_params(struct comp_dev *dev) } }
- /* set read pointer to start of buffer */ - buffer_reset_pos(dma_buffer); - return 0;
err_unwind: @@ -506,12 +500,6 @@ static int dai_copy(struct comp_dev *dev) return 0; }
-/* source component will preload dai */ -static int dai_preload(struct comp_dev *dev) -{ - return 0; -} - static int dai_position(struct comp_dev *dev, struct sof_ipc_stream_posn *posn) { struct dai_data *dd = comp_get_drvdata(dev); @@ -558,7 +546,6 @@ static struct comp_driver comp_dai = { .prepare = dai_prepare, .reset = dai_reset, .dai_config = dai_config, - .preload = dai_preload, .position = dai_position, }, }; diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index 8babed3..e05920f 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -317,8 +317,6 @@ static int eq_fir_params(struct comp_dev *dev) return err; }
- buffer_reset_pos(sink); - /* EQ supports only S32_LE PCM format */ if (config->frame_fmt != SOF_IPC_FRAME_S32_LE) return -EINVAL; @@ -439,9 +437,6 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data) case COMP_CMD_SET_DATA: ret = fir_cmd_set_data(dev, cdata); break; - case COMP_CMD_STOP: - comp_buffer_reset(dev); - break; }
return ret; @@ -509,11 +504,6 @@ static int eq_fir_prepare(struct comp_dev *dev) return 0; }
-static int eq_fir_preload(struct comp_dev *dev) -{ - return eq_fir_copy(dev); -} - static int eq_fir_reset(struct comp_dev *dev) { int i; @@ -542,7 +532,6 @@ struct comp_driver comp_eq_fir = { .copy = eq_fir_copy, .prepare = eq_fir_prepare, .reset = eq_fir_reset, - .preload = eq_fir_preload, }, };
diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 1c2389c..1ac8c23 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -318,8 +318,6 @@ static int eq_iir_params(struct comp_dev *dev) return err; }
- buffer_reset_pos(sink); - /* EQ supports only S32_LE PCM format */ if (config->frame_fmt != SOF_IPC_FRAME_S32_LE) return -EINVAL; @@ -437,9 +435,6 @@ static int eq_iir_cmd(struct comp_dev *dev, int cmd, void *data) case COMP_CMD_SET_DATA: ret = iir_cmd_set_data(dev, cdata); break; - case COMP_CMD_STOP: - comp_buffer_reset(dev); - break; }
return ret; @@ -510,11 +505,6 @@ static int eq_iir_prepare(struct comp_dev *dev) return 0; }
-static int eq_iir_preload(struct comp_dev *dev) -{ - return eq_iir_copy(dev); -} - static int eq_iir_reset(struct comp_dev *dev) { int i; @@ -543,7 +533,6 @@ struct comp_driver comp_eq_iir = { .copy = eq_iir_copy, .prepare = eq_iir_prepare, .reset = eq_iir_reset, - .preload = eq_iir_preload, }, };
diff --git a/src/audio/host.c b/src/audio/host.c index aa608e7..f542eec 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -51,6 +51,8 @@ #define tracev_host(__e) tracev_event(TRACE_CLASS_HOST, __e) #define trace_host_error(__e) trace_error(TRACE_CLASS_HOST, __e)
+static int host_copy(struct comp_dev *dev); + struct hc_buf { /* host buffer info */ struct list_item elem_list; @@ -439,8 +441,6 @@ static int host_params(struct comp_dev *dev) if (err < 0) return err;
- buffer_reset_pos(hd->dma_buffer); - /* set up DMA configuration */ config->src_width = sizeof(uint32_t); config->dest_width = sizeof(uint32_t); @@ -451,43 +451,9 @@ static int host_params(struct comp_dev *dev) return 0; }
-/* preload the l1 period from host to DSP */ -static int host_preload(struct comp_dev *dev) -{ - struct host_data *hd = comp_get_drvdata(dev); - struct comp_buffer *sink; - int ret; - - trace_host("PrL"); - - /* make sure there is enough space in sink buffer */ - sink = list_first_item(&dev->bsink_list, struct comp_buffer, - source_list); - if (sink->free < hd->period_bytes) - return 0; - - wait_init(&hd->complete); - - /* do DMA transfer */ - hd->complete.timeout = PLATFORM_HOST_DMA_TIMEOUT; - dma_set_config(hd->dma, hd->chan, &hd->config); - dma_start(hd->dma, hd->chan); - - /* wait for DMA to finish */ - ret = wait_for_completion_timeout(&hd->complete); - if (ret < 0) { - trace_comp_error("eHp"); - return -EIO; - } - - /* one period copied */ - return hd->period_bytes; -} - static int host_prepare(struct comp_dev *dev) { struct host_data *hd = comp_get_drvdata(dev); - struct comp_buffer *dma_buffer; int ret;
trace_host("pre"); @@ -496,14 +462,6 @@ static int host_prepare(struct comp_dev *dev) if (ret < 0) return ret;
- if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) - dma_buffer = list_first_item(&dev->bsink_list, - struct comp_buffer, source_list); - else - dma_buffer = list_first_item(&dev->bsource_list, - struct comp_buffer, sink_list); - dma_buffer->r_ptr = dma_buffer->w_ptr = dma_buffer->addr; - hd->local_pos = 0; if (hd->host_pos) *hd->host_pos = 0; @@ -537,9 +495,6 @@ static int host_stop(struct comp_dev *dev) /* reset elements, to let next start from original one */ host_elements_reset(dev);
- /* now reset downstream buffer */ - comp_buffer_reset(dev); - dev->state = COMP_STATE_PAUSED; return 0; } @@ -571,6 +526,14 @@ static int host_cmd(struct comp_dev *dev, int cmd, void *data) case COMP_CMD_STOP: ret = host_stop(dev); break; + case COMP_CMD_START: + /* preload first playback period for preloader task */ + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + ret = host_copy(dev); + if (ret == dev->frames) + ret = 0; + } + break; default: break; } @@ -675,7 +638,6 @@ struct comp_driver comp_host = { .cmd = host_cmd, .copy = host_copy, .prepare = host_prepare, - .preload = host_preload, .host_buffer = host_buffer, .position = host_position, }, diff --git a/src/audio/mixer.c b/src/audio/mixer.c index 4d50bf1..e72f1f2 100644 --- a/src/audio/mixer.c +++ b/src/audio/mixer.c @@ -149,8 +149,6 @@ static int mixer_params(struct comp_dev *dev) return ret; }
- buffer_reset_pos(sink); - return 0; }
@@ -339,11 +337,6 @@ static int mixer_prepare(struct comp_dev *dev) return downstream; }
-static int mixer_preload(struct comp_dev *dev) -{ - return mixer_copy(dev); -} - struct comp_driver comp_mixer = { .type = SOF_COMP_MIXER, .ops = { @@ -351,7 +344,6 @@ struct comp_driver comp_mixer = { .free = mixer_free, .params = mixer_params, .prepare = mixer_prepare, - .preload = mixer_preload, .cmd = mixer_cmd, .copy = mixer_copy, .reset = mixer_reset, diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 40ccdc5..947b64b 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -481,49 +481,80 @@ static int component_op_upstream(struct op_data *op_data, return err; }
-/* preload all downstream components - locks held by caller */ -static int preload_downstream(struct comp_dev *start, struct comp_dev *current) +static int component_prepare_buffers_upstream(struct comp_dev *start, + struct comp_dev *current, struct comp_buffer *buffer) { - struct sof_ipc_comp_config *config = COMP_GET_CONFIG(current); struct list_item *clist; - int i; - int total = 0; - int count = 0; + int err = 0;
- tracev_pipe("PR-"); - tracev_value(current->comp.id); + /* component copy/process to downstream */ + if (current != start && buffer != NULL) {
- /* reached endpoint ? */ - if (current != start && current->is_endpoint) - return 0; + buffer_reset_pos(buffer); + + /* stop going downstream if we reach an end point in this pipeline */ + if (current->is_endpoint) + return 0; + } + + /* travel uptream to sink end point(s) */ + list_for_item(clist, ¤t->bsource_list) {
- /* now preload the buffers */ - for (i = 0; i < config->preload_count; i++) { - count = comp_preload(current); + buffer = container_of(clist, struct comp_buffer, sink_list); + + /* dont go upstream if this component is not connected or active */ + if (!buffer->connected || buffer->source->state == COMP_STATE_ACTIVE) + continue;
- if (count < 0) - return count; - total += count; + /* continue downstream */ + err = component_prepare_buffers_upstream(start, buffer->source, + buffer); + if (err < 0) { + trace_pipe_error("eBD"); + break; + } }
- /* now run this operation downstream */ + /* return back downstream */ + return err; +} + +static int component_prepare_buffers_downstream(struct comp_dev *start, + struct comp_dev *current, struct comp_buffer *buffer) +{ + struct list_item *clist; + int err = 0; + + /* component copy/process to downstream */ + if (current != start && buffer != NULL) { + + buffer_reset_pos(buffer); + + /* stop going downstream if we reach an end point in this pipeline */ + if (current->is_endpoint) + return 0; + } + + /* travel downstream to sink end point(s) */ list_for_item(clist, ¤t->bsink_list) { - struct comp_buffer *buffer;
buffer = container_of(clist, struct comp_buffer, source_list);
- /* don't go downstream if this component is not connected */ - if (!buffer->connected) + /* dont go downstream if this component is not connected or active */ + if (!buffer->connected || buffer->sink->state == COMP_STATE_ACTIVE) continue;
- count = preload_downstream(start, buffer->sink); - if (count < 0) - return count; - - total += count; + /* continue downstream */ + err = component_prepare_buffers_downstream(start, buffer->sink, + buffer); + if (err < 0) { + trace_pipe_error("eBD"); + break; + } }
- return total; + /* return back upstream */ + return err; }
/* prepare the pipeline for usage - preload host buffers here */ @@ -531,7 +562,6 @@ int pipeline_prepare(struct pipeline *p, struct comp_dev *dev) { struct op_data op_data; int ret = -1; - int i;
trace_pipe("pre");
@@ -543,29 +573,19 @@ int pipeline_prepare(struct pipeline *p, struct comp_dev *dev) /* playback pipelines can be preloaded from host before trigger */ if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) {
- /* first of all prepare the pipeline */ ret = component_op_downstream(&op_data, dev, dev, NULL); if (ret < 0) goto out;
- /* then preload buffers - the buffers must be moved - * downstream so that every component has full buffers for - * trigger start */ - for (i = 0; i < MAX_PRELOAD_SIZE; i++) { - - ret = preload_downstream(dev, dev); - - /* errors or complete ? */ - if (ret <= 0) - break; - } - if (ret < 0) { - /* failed to preload */ - trace_pipe_error("epl"); - ret = -EIO; - } + /* set up reader and writer positions */ + component_prepare_buffers_downstream(dev, dev, NULL); } else { ret = component_op_upstream(&op_data, dev, dev, NULL); + if (ret < 0) + goto out; + + /* set up reader and writer positions */ + component_prepare_buffers_upstream(dev, dev, NULL); }
out: diff --git a/src/audio/src.c b/src/audio/src.c index d71a27a..c617038 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -450,8 +450,6 @@ static int src_params(struct comp_dev *dev) return err; }
- buffer_reset_pos(sink); - /* Check that source buffer has sufficient size */ source = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); @@ -540,11 +538,6 @@ static int src_prepare(struct comp_dev *dev) return comp_set_state(dev, COMP_CMD_PREPARE); }
-static int src_preload(struct comp_dev *dev) -{ - return src_copy(dev); -} - static int src_reset(struct comp_dev *dev) { struct comp_data *cd = comp_get_drvdata(dev); @@ -568,7 +561,6 @@ struct comp_driver comp_src = { .copy = src_copy, .prepare = src_prepare, .reset = src_reset, - .preload = src_preload, }, };
diff --git a/src/audio/tone.c b/src/audio/tone.c index ef5b7b1..8fc513d 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -649,11 +649,6 @@ static int tone_prepare(struct comp_dev * dev) return 0; }
-static int tone_preload(struct comp_dev * dev) -{ - return tone_copy(dev); -} - static int tone_reset(struct comp_dev * dev) {
@@ -681,7 +676,6 @@ struct comp_driver comp_tone = { .copy = tone_copy, .prepare = tone_prepare, .reset = tone_reset, - .preload = tone_preload, }, };
diff --git a/src/audio/volume.c b/src/audio/volume.c index b6872a7..3671046 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -666,8 +666,6 @@ static int volume_prepare(struct comp_dev *dev) goto err; }
- buffer_reset_pos(sinkb); - /* validate */ if (cd->sink_period_bytes == 0) { trace_volume_error("vp1"); @@ -713,22 +711,6 @@ found: return 0; }
-static int volume_preload(struct comp_dev *dev) -{ - struct comp_data *cd = comp_get_drvdata(dev); - struct comp_buffer *sink; - - trace_volume("PrL"); - - /* make sure there is enough space in sink buffer */ - sink = list_first_item(&dev->bsink_list, struct comp_buffer, - source_list); - if (sink->free < cd->sink_period_bytes) - return 0; - - return volume_copy(dev); -} - static int volume_reset(struct comp_dev *dev) { trace_volume("res"); @@ -747,7 +729,6 @@ struct comp_driver comp_volume = { .copy = volume_copy, .prepare = volume_prepare, .reset = volume_reset, - .preload = volume_preload, }, };
diff --git a/src/include/reef/audio/buffer.h b/src/include/reef/audio/buffer.h index d1d0869..179a84f 100644 --- a/src/include/reef/audio/buffer.h +++ b/src/include/reef/audio/buffer.h @@ -164,17 +164,28 @@ static inline uint32_t comp_buffer_get_copy_bytes(struct comp_dev *dev, return copy_bytes; }
-static inline void buffer_reset_pos(struct comp_buffer *buffer) +static inline void buffer_reset_pos(struct comp_buffer *buffer, + uint32_t sink_period_bytes) { - buffer->r_ptr = buffer->addr; + buffer->r_ptr = buffer->addr + buffer->size - sink_period_bytes; buffer->w_ptr = buffer->addr; buffer->free = buffer->size; buffer->avail = 0; }
-static inline void buffer_clear(struct comp_buffer *buffer) +static inline void buffer_reset_pos(struct comp_buffer *buffer) { - memset(buffer->addr, 0, buffer->size); + /* reset read and write pointer to buffer bas */ + buffer->w_ptr = buffer->r_ptr = buffer->addr; + + /* free space is buffer size */ + buffer->free = buffer->size; + + /* ther are no avail samples at reset */ + buffer->avail = 0; + + /* clear buffer contents */ + bzero(buffer->r_ptr, buffer->avail); }
/* set the runtime size of a buffer in bytes and improve the data cache */ diff --git a/src/include/reef/audio/component.h b/src/include/reef/audio/component.h index 0b77e87..afd5461 100644 --- a/src/include/reef/audio/component.h +++ b/src/include/reef/audio/component.h @@ -128,9 +128,6 @@ struct comp_ops { /* set component audio stream parameters */ int (*params)(struct comp_dev *dev);
- /* preload buffers */ - int (*preload)(struct comp_dev *dev); - /* set component audio stream parameters */ int (*dai_config)(struct comp_dev *dev, struct sof_ipc_dai_config *dai_config); @@ -266,12 +263,6 @@ static inline int comp_prepare(struct comp_dev *dev) return dev->drv->ops.prepare(dev); }
-/* component preload buffers -mandatory */ -static inline int comp_preload(struct comp_dev *dev) -{ - return dev->drv->ops.preload(dev); -} - /* copy component buffers - mandatory */ static inline int comp_copy(struct comp_dev *dev) { @@ -314,32 +305,6 @@ void sys_comp_tone_init(void); void sys_comp_eq_iir_init(void); void sys_comp_eq_fir_init(void);
-/* reset component downstream buffers */ -static inline int comp_buffer_reset(struct comp_dev *dev) -{ - struct list_item *clist; - - /* reset downstream buffers */ - list_for_item(clist, &dev->bsink_list) { - struct comp_buffer *buffer; - - buffer = container_of(clist, struct comp_buffer, source_list); - - /* don't reset buffer if the component is not connected */ - if (!buffer->connected) - continue; - - /* reset buffer next to the component*/ - bzero(buffer->addr, buffer->ipc_buffer.size); - buffer->w_ptr = buffer->r_ptr = buffer->addr; - buffer->end_addr = buffer->addr + buffer->ipc_buffer.size; - buffer->free = buffer->ipc_buffer.size; - buffer->avail = 0; - } - - return 0; -} - /* * Convenience functions to install upstream/downstream common params. Only * applicable to single upstream source. Components with > 1 source or sink
Fix the free/avail buffer calculations to make sure any overrun or underrun is reported in trace and returned as a negative error in copy().
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/dai.c | 12 ++++++++++ src/audio/eq_fir.c | 22 +++++++++--------- src/audio/eq_iir.c | 22 +++++++++--------- src/audio/host.c | 20 +++++++++++++---- src/audio/mixer.c | 27 ++++++++++++----------- src/audio/pipeline.c | 6 +++-- src/audio/src.c | 17 +++++++++++++- src/audio/tone.c | 9 ++++++-- src/audio/volume.c | 22 +++++++++--------- src/include/reef/audio/buffer.h | 49 ++++++++++++++--------------------------- 10 files changed, 118 insertions(+), 88 deletions(-)
diff --git a/src/audio/dai.c b/src/audio/dai.c index 96b37ba..afa4aa2 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -117,6 +117,12 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) dma_buffer->r_ptr - dma_buffer->addr; }
+ /* make sure there is availble bytes for next period */ + if (dma_buffer->avail < dd->period_bytes) { + trace_dai_error("xru"); + comp_underrun(dev, dma_buffer, copied_size, 0); + } + } else { dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); @@ -134,6 +140,12 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) *dd->dai_pos = dd->dai_pos_blks + dma_buffer->w_ptr - dma_buffer->addr; } + + /* make sure there is free bytes for next period */ + if (dma_buffer->free < dd->period_bytes) { + trace_dai_error("xro"); + comp_overrun(dev, dma_buffer, dd->period_bytes, 0); + } }
/* notify pipeline that DAI needs its buffer processed */ diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index e05920f..2dfa11c 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -448,7 +448,7 @@ static int eq_fir_copy(struct comp_dev *dev) struct comp_data *sd = comp_get_drvdata(dev); struct comp_buffer *source; struct comp_buffer *sink; - uint32_t copy_bytes; + int res;
trace_comp("EqF");
@@ -458,20 +458,20 @@ static int eq_fir_copy(struct comp_dev *dev) sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* Check that source has enough frames available and sink enough - * frames free. - */ - copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink); - - /* Run EQ if buffers have enough room */ - if (copy_bytes < sd->period_bytes) - return 0; + /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + res = comp_buffer_can_copy_bytes(source, sink, sd->period_bytes); + if (res) { + trace_eq_error("xrn"); + return -EIO; /* xrun */ + }
sd->eq_fir_func(dev, source, sink, dev->frames);
/* calc new free and available */ - comp_update_buffer_consume(source, copy_bytes); - comp_update_buffer_produce(sink, copy_bytes); + comp_update_buffer_consume(source, sd->period_bytes); + comp_update_buffer_produce(sink, sd->period_bytes);
return dev->frames; } diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 1ac8c23..eb464af 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -446,7 +446,7 @@ static int eq_iir_copy(struct comp_dev *dev) struct comp_data *cd = comp_get_drvdata(dev); struct comp_buffer *source; struct comp_buffer *sink; - uint32_t copy_bytes; + int res;
trace_comp("EqI");
@@ -456,20 +456,20 @@ static int eq_iir_copy(struct comp_dev *dev) sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* Check that source has enough frames available and sink enough - * frames free. - */ - copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink); - - /* Run EQ if buffers have enough room */ - if (copy_bytes < cd->period_bytes) - return 0; + /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + res = comp_buffer_can_copy_bytes(source, sink, cd->period_bytes); + if (res) { + trace_eq_iir_error("xrn"); + return -EIO; /* xrun */ + }
cd->eq_iir_func(dev, source, sink, dev->frames);
/* calc new free and available */ - comp_update_buffer_consume(source, copy_bytes); - comp_update_buffer_produce(sink, copy_bytes); + comp_update_buffer_consume(source, cd->period_bytes); + comp_update_buffer_produce(sink, cd->period_bytes);
return dev->frames; } diff --git a/src/audio/host.c b/src/audio/host.c index f542eec..88d45e4 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -610,15 +610,27 @@ static int host_copy(struct comp_dev *dev)
/* enough free or avail to copy ? */ if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - if (dma_buffer->free < local_elem->size) - return 0; + + if (dma_buffer->free < local_elem->size) { + /* make sure there is free bytes for next period */ + trace_host_error("xro"); + comp_overrun(dev, dma_buffer, local_elem->size, 0); + return -EIO; + } } else { + dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); - if (dma_buffer->avail < local_elem->size) - return 0; + + if (dma_buffer->avail < local_elem->size) { + /* make sure there is avail bytes for next period */ + trace_host_error("xru"); + comp_underrun(dev, dma_buffer, local_elem->size, 0); + return -EIO; + } }
/* do DMA transfer */ diff --git a/src/audio/mixer.c b/src/audio/mixer.c index e72f1f2..e3611d4 100644 --- a/src/audio/mixer.c +++ b/src/audio/mixer.c @@ -220,10 +220,12 @@ static int mixer_copy(struct comp_dev *dev) struct list_item *blist; int32_t i = 0; int32_t num_mix_sources = 0; - int32_t xru = 0; + int res;
tracev_mixer("cpy");
+ sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); + /* calculate the highest runtime component status between input streams */ list_for_item(blist, &dev->bsource_list) { source = container_of(blist, struct comp_buffer, sink_list); @@ -243,22 +245,21 @@ static int mixer_copy(struct comp_dev *dev)
/* make sure no sources have underruns */ for (i = 0; i < num_mix_sources; i++) { - if (sources[i]->avail < md->period_bytes) { + + /* make sure source component buffer has enough data available + * and that the sink component buffer has enough free bytes + * for copy. Also check for XRUNs */ + res = comp_buffer_can_copy_bytes(sources[i], sink, md->period_bytes); + if (res < 0) { + trace_mixer_error("xru"); comp_underrun(dev, sources[i], sources[i]->avail, md->period_bytes); - xru = 1; + } else if (res > 0) { + trace_mixer_error("xru"); + comp_overrun(dev, sources[i], sink->free, + md->period_bytes); } } - /* underrun ? */ - if (xru) - return 0; - - /* make sure sink has no overruns */ - sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - if (sink->free < md->period_bytes) { - comp_overrun(dev, sink, sink->free, md->period_bytes); - return 0; - }
/* mix streams */ md->mix_func(dev, sink, sources, i, dev->frames); diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 947b64b..bec7519 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -742,7 +742,8 @@ static int pipeline_copy_from_upstream(struct comp_dev *start, err = pipeline_copy_from_upstream(start, buffer->source); if (err < 0) { trace_pipe_error("ePU"); - break; + trace_value(current->comp.id); + return err; } }
@@ -800,7 +801,8 @@ static int pipeline_copy_to_downstream(struct comp_dev *start, err = pipeline_copy_to_downstream(start, buffer->sink); if (err < 0) { trace_pipe_error("ePD"); - break; + trace_value(current->comp.id); + return err; } }
diff --git a/src/audio/src.c b/src/audio/src.c index c617038..3378065 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -496,8 +496,9 @@ static int src_copy(struct comp_dev *dev) int need_sink; size_t consumed = 0; size_t produced = 0; + int res;
- trace_comp("SRC"); + trace_src("SRC");
/* src component needs 1 source and 1 sink buffer */ source = list_first_item(&dev->bsource_list, struct comp_buffer, @@ -515,6 +516,20 @@ static int src_copy(struct comp_dev *dev) need_source = cd->param.blk_in * dev->frame_bytes; need_sink = cd->param.blk_out * dev->frame_bytes;
+ /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + res = comp_buffer_can_copy_bytes(source, sink, need_source); + if (res) { + trace_src_error("xru"); + return -EIO; /* xrun */ + } + res = comp_buffer_can_copy_bytes(source, sink, need_sink); + if (res) { + trace_src_error("xro"); + return -EIO; /* xrun */ + } + /* Run SRC function if buffers avail and free allow */ if (((int) source->avail >= need_source) && ((int) sink->free >= need_sink)) { cd->src_func(dev, source, sink, &consumed, &produced); diff --git a/src/audio/tone.c b/src/audio/tone.c index 8fc513d..581f87e 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -613,9 +613,14 @@ static int tone_copy(struct comp_dev * dev)
/* calc new free and available */ comp_update_buffer_produce(sink, cd->period_bytes); - }
- return dev->frames; + return dev->frames; + } else { + /* XRUN */ + trace_tone_error("xrn"); + comp_overrun(dev, sink, cd->period_bytes, sink->free); + return -EIO; + } }
static int tone_prepare(struct comp_dev * dev) diff --git a/src/audio/volume.c b/src/audio/volume.c index 3671046..e741b1a 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -561,7 +561,6 @@ static int volume_copy(struct comp_dev *dev) struct comp_data *cd = comp_get_drvdata(dev); struct comp_buffer *sink; struct comp_buffer *source; - uint32_t copy_bytes;
tracev_volume("cpy");
@@ -569,17 +568,18 @@ static int volume_copy(struct comp_dev *dev) source = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* get max number of bytes that can be copied */ - copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink); - - /* Run volume if buffers have enough room */ - if (copy_bytes < cd->source_period_bytes) { - comp_underrun(dev, source, copy_bytes, cd->source_period_bytes); - return 0; + /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + if (source->avail < cd->source_period_bytes) { + trace_volume_error("xru"); + comp_overrun(dev, source, cd->source_period_bytes, 0); + return -EIO; /* xrun */ } - if (copy_bytes < cd->sink_period_bytes) { - comp_overrun(dev, sink, copy_bytes, cd->sink_period_bytes); - return 0; + if (sink->free < cd->sink_period_bytes) { + trace_volume_error("xro"); + comp_underrun(dev, sink, cd->sink_period_bytes, 0); + return -EIO; /* xrun */ }
/* copy and scale volume */ diff --git a/src/include/reef/audio/buffer.h b/src/include/reef/audio/buffer.h index 179a84f..2bb2033 100644 --- a/src/include/reef/audio/buffer.h +++ b/src/include/reef/audio/buffer.h @@ -81,14 +81,6 @@ void buffer_free(struct comp_buffer *buffer); static inline void comp_update_buffer_produce(struct comp_buffer *buffer, uint32_t bytes) { - /* complain loudly if component tries to overrun buffer - * components MUST check for free space first !! */ - if (bytes > buffer->free) { - trace_buffer_error("Xxo"); - trace_value(buffer->ipc_buffer.comp.id); - return; - } - buffer->w_ptr += bytes;
/* check for pointer wrap */ @@ -116,14 +108,6 @@ static inline void comp_update_buffer_produce(struct comp_buffer *buffer, static inline void comp_update_buffer_consume(struct comp_buffer *buffer, uint32_t bytes) { - /* complain loudly if component tries to underrun buffer - * components MUST check for avail space first !! */ - if (buffer->avail < bytes) { - trace_buffer_error("Xxu"); - trace_value(buffer->ipc_buffer.comp.id); - return; - } - buffer->r_ptr += bytes;
/* check for pointer wrap */ @@ -148,29 +132,28 @@ static inline void comp_update_buffer_consume(struct comp_buffer *buffer, }
/* get the max number of bytes that can be copied between sink and source */ -static inline uint32_t comp_buffer_get_copy_bytes(struct comp_dev *dev, - struct comp_buffer *source, struct comp_buffer *sink) +static inline int comp_buffer_can_copy_bytes(struct comp_buffer *source, + struct comp_buffer *sink, uint32_t bytes) { - uint32_t copy_bytes; + /* check for underrun */ + if (source->avail < bytes) + return -1;
- /* Check that source has enough frames available and sink enough - * frames free. - */ - if (source->avail > sink->free) - copy_bytes = sink->free; - else - copy_bytes = source->avail; + /* check for overrun */ + if (sink->free < bytes) + return 1;
- return copy_bytes; + /* we are good to copy */ + return 0; }
-static inline void buffer_reset_pos(struct comp_buffer *buffer, - uint32_t sink_period_bytes) +static inline uint32_t comp_buffer_get_copy_bytes(struct comp_buffer *source, + struct comp_buffer *sink) { - buffer->r_ptr = buffer->addr + buffer->size - sink_period_bytes; - buffer->w_ptr = buffer->addr; - buffer->free = buffer->size; - buffer->avail = 0; + if (source->avail < sink->free) + return sink->free; + else + return source->avail; }
static inline void buffer_reset_pos(struct comp_buffer *buffer)
Add an XRUN handler to notify components that an XRUN has occurred and to take any remedial action. The pipeline itself will handle an XRUN by re-preparing the components and then send start again from the pipeline source component.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/component.c | 1 + src/audio/pipeline.c | 70 ++++++++++++++++++++++++++++++++++++-- src/include/reef/audio/component.h | 1 + src/include/reef/audio/pipeline.h | 3 +- 4 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/src/audio/component.c b/src/audio/component.c index 5815fad..54a7da8 100644 --- a/src/audio/component.c +++ b/src/audio/component.c @@ -141,6 +141,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) } break; case COMP_CMD_STOP: + case COMP_CMD_XRUN: if (dev->state == COMP_STATE_ACTIVE) { dev->state = COMP_STATE_PREPARE; } else { diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index bec7519..ccd6094 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -76,6 +76,9 @@ static void connect_upstream(struct pipeline *p, struct comp_dev *start, /* we are an endpoint if we have 0 source components */ if (list_is_empty(¤t->bsource_list)) { current->is_endpoint = 1; + + /* pipeline source comp is current */ + p->source_comp = current; return; }
@@ -86,8 +89,13 @@ static void connect_upstream(struct pipeline *p, struct comp_dev *start, buffer = container_of(clist, struct comp_buffer, sink_list);
/* don't go upstream if this source is from another pipeline */ - if (buffer->source->comp.pipeline_id != p->ipc_pipe.pipeline_id) + if (buffer->source->comp.pipeline_id != p->ipc_pipe.pipeline_id) { + + /* pipeline source comp is current unless we go upstream */ + p->source_comp = current; + continue; + }
connect_upstream(p, start, buffer->source); } @@ -210,6 +218,7 @@ static void pipeline_cmd_update(struct pipeline *p, struct comp_dev *comp, break; case COMP_CMD_SUSPEND: case COMP_CMD_RESUME: + case COMP_CMD_XRUN: default: break; } @@ -1001,6 +1010,38 @@ void pipeline_xrun(struct pipeline *p, struct comp_dev *dev, } }
+/* recover the pipeline from a XRUN condition */ +static int pipeline_xrun_recover(struct pipeline *p) +{ + int ret; + + trace_pipe_error("pxr"); + + /* notify all pipeline comps we are in XRUN */ + ret = pipeline_cmd(p, p->source_comp, COMP_CMD_XRUN, NULL); + if (ret < 0) { + trace_pipe_error("px0"); + return ret; + } + p->xrun_bytes = 0; + + /* prepare the pipeline */ + pipeline_prepare(p, p->source_comp); + if (ret < 0) { + trace_pipe_error("px1"); + return ret; + } + + /* restart pipeline comps */ + pipeline_cmd(p, p->source_comp, COMP_CMD_START, NULL); + if (ret < 0) { + trace_pipe_error("px2"); + return ret; + } + + return 0; +} + /* notify pipeline that this component requires buffers emptied/filled */ void pipeline_schedule_copy(struct pipeline *p, uint64_t start) { @@ -1025,13 +1066,36 @@ static void pipeline_task(void *arg) { struct pipeline *p = arg; struct comp_dev *dev = p->sched_comp; + int err;
tracev_pipe("PWs");
+ /* are we in xrun ? */ + if (p->xrun_bytes) { + err = pipeline_xrun_recover(p); + if (err < 0) + return; /* failed - host will stop this pipeline */ + goto sched; + } + /* copy data from upstream source endpoints to downstream endpoints */ - pipeline_copy_from_upstream(dev, dev); - pipeline_copy_to_downstream(dev, dev); + err = pipeline_copy_from_upstream(dev, dev); + if (err < 0) { + err = pipeline_xrun_recover(p); + if (err < 0) + return; /* failed - host will stop this pipeline */ + goto sched; + } + + err = pipeline_copy_to_downstream(dev, dev); + if (err < 0) { + err = pipeline_xrun_recover(p); + if (err < 0) + return; /* failed - host will stop this pipeline */ + goto sched; + }
+sched: tracev_pipe("PWe");
/* now reschedule the task */ diff --git a/src/include/reef/audio/component.h b/src/include/reef/audio/component.h index afd5461..5d15fdb 100644 --- a/src/include/reef/audio/component.h +++ b/src/include/reef/audio/component.h @@ -80,6 +80,7 @@ #define COMP_CMD_RESUME 6 /* resume component */ #define COMP_CMD_RESET 6 /* reset component */ #define COMP_CMD_PREPARE 7 /* prepare component */ +#define COMP_CMD_XRUN 8 /* XRUN component */
/* * standard component control commands diff --git a/src/include/reef/audio/pipeline.h b/src/include/reef/audio/pipeline.h index c5ba49f..08a3f30 100644 --- a/src/include/reef/audio/pipeline.h +++ b/src/include/reef/audio/pipeline.h @@ -67,7 +67,8 @@ struct pipeline {
/* scheduling */ struct task pipe_task; /* pipeline processing task */ - struct comp_dev *sched_comp; + struct comp_dev *sched_comp; /* component that drives scheduling in this pipe */ + struct comp_dev *source_comp; /* source component for this pipe */ };
/* static pipeline */
WiP: The DAI should not be stopped and the restarted on XRUNs
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/dai.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 6 deletions(-)
diff --git a/src/audio/dai.c b/src/audio/dai.c index afa4aa2..13f7e10 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -63,6 +63,7 @@ struct dai_data { struct dma *dma; uint32_t period_bytes; completion_t complete; + int xrun; /* true if we are doing xrun recovery */
uint32_t last_bytes; /* the last bytes(<period size) it copies. */ uint32_t dai_pos_blks; /* position in bytes (nearest block) */ @@ -83,8 +84,8 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next)
tracev_dai("irq");
- /* is stream stopped or paused ? */ - if (dev->state != COMP_STATE_ACTIVE) { + /* is stream stopped or paused and we are not handling XRUN ? */ + if (dev->state != COMP_STATE_ACTIVE && dd->xrun == 0) {
/* stop the DAI */ dai_trigger(dd->dai, COMP_CMD_STOP, dev->params.direction); @@ -97,6 +98,28 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) return; }
+ /* is our pipeline handling an XRUN ? */ + if (dd->xrun) { + + /* make sure we only playback silence during an XRUN */ + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + + dma_buffer = list_first_item(&dev->bsource_list, + struct comp_buffer, sink_list); + + /* fill buffer with silence */ + bzero(dma_buffer->addr, dma_buffer->size); + + /* writeback buffer contents from cache */ + dcache_writeback_region(dma_buffer->addr, + dma_buffer->size); + } + + /* inform waiters */ + wait_completed(&dd->complete); + return; + } + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); @@ -195,6 +218,7 @@ static struct comp_dev *dai_new(struct sof_ipc_comp *comp) dd->dai_pos = NULL; dd->dai_pos_blks = 0; dd->last_bytes = 0; + dd->xrun = 0;
/* get DMA channel from DMAC1 */ dd->chan = dma_channel_get(dd->dma); @@ -423,9 +447,16 @@ static int dai_prepare(struct comp_dev *dev) dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list);
+ /* fill playback periods with silence */ + bzero(dma_buffer->r_ptr, dma_buffer->avail); + dcache_writeback_region(dma_buffer->r_ptr, dma_buffer->avail); }
+ /* dma reconfig not required if XRUN handling */ + if (dd->xrun) + return ret; + ret = dma_set_config(dd->dma, dd->chan, &dd->config); if (ret < 0) comp_set_state(dev, COMP_CMD_RESET); @@ -456,6 +487,7 @@ static int dai_reset(struct comp_dev *dev) dd->last_bytes = 0; dd->wallclock = 0; dev->position = 0; + dd->xrun = 0; comp_set_state(dev, COMP_CMD_RESET);
return 0; @@ -479,14 +511,24 @@ static int dai_cmd(struct comp_dev *dev, int cmd, void *data) switch (cmd) { case COMP_CMD_RELEASE: case COMP_CMD_START: - ret = dma_start(dd->dma, dd->chan); - if (ret < 0) - return ret; - dai_trigger(dd->dai, cmd, dev->params.direction); + + /* only start the DAI if we are not XRUN handling */ + if (dd->xrun == 0) { + + /* start the DAI */ + ret = dma_start(dd->dma, dd->chan); + if (ret < 0) + return ret; + dai_trigger(dd->dai, cmd, dev->params.direction); + } else { + dd->xrun = 0; + }
/* update starting wallclock */ platform_dai_wallclock(dev, &dd->wallclock); break; + case COMP_CMD_XRUN: + dd->xrun = 1; case COMP_CMD_PAUSE: case COMP_CMD_STOP: wait_init(&dd->complete);
On 11/16/17 4:01 PM, Liam Girdwood wrote:
WiP: The DAI should not be stopped and the restarted on XRUNs
Are you sure about this one? if you have an xrun leading to a channel swap, that's not something you could ever recover from?
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com
src/audio/dai.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 6 deletions(-)
diff --git a/src/audio/dai.c b/src/audio/dai.c index afa4aa2..13f7e10 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -63,6 +63,7 @@ struct dai_data { struct dma *dma; uint32_t period_bytes; completion_t complete;
int xrun; /* true if we are doing xrun recovery */
uint32_t last_bytes; /* the last bytes(<period size) it copies. */ uint32_t dai_pos_blks; /* position in bytes (nearest block) */
@@ -83,8 +84,8 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next)
tracev_dai("irq");
- /* is stream stopped or paused ? */
- if (dev->state != COMP_STATE_ACTIVE) {
/* is stream stopped or paused and we are not handling XRUN ? */
if (dev->state != COMP_STATE_ACTIVE && dd->xrun == 0) {
/* stop the DAI */ dai_trigger(dd->dai, COMP_CMD_STOP, dev->params.direction);
@@ -97,6 +98,28 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) return; }
- /* is our pipeline handling an XRUN ? */
- if (dd->xrun) {
/* make sure we only playback silence during an XRUN */if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) {dma_buffer = list_first_item(&dev->bsource_list,struct comp_buffer, sink_list);/* fill buffer with silence */bzero(dma_buffer->addr, dma_buffer->size);/* writeback buffer contents from cache */dcache_writeback_region(dma_buffer->addr,dma_buffer->size);}/* inform waiters */wait_completed(&dd->complete);return;- }
- if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list);
@@ -195,6 +218,7 @@ static struct comp_dev *dai_new(struct sof_ipc_comp *comp) dd->dai_pos = NULL; dd->dai_pos_blks = 0; dd->last_bytes = 0;
dd->xrun = 0;
/* get DMA channel from DMAC1 */ dd->chan = dma_channel_get(dd->dma);
@@ -423,9 +447,16 @@ static int dai_prepare(struct comp_dev *dev) dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list);
/* fill playback periods with silence */bzero(dma_buffer->r_ptr, dma_buffer->avail);dcache_writeback_region(dma_buffer->r_ptr, dma_buffer->avail); }
/* dma reconfig not required if XRUN handling */
if (dd->xrun)
return ret;ret = dma_set_config(dd->dma, dd->chan, &dd->config); if (ret < 0) comp_set_state(dev, COMP_CMD_RESET);
@@ -456,6 +487,7 @@ static int dai_reset(struct comp_dev *dev) dd->last_bytes = 0; dd->wallclock = 0; dev->position = 0;
dd->xrun = 0; comp_set_state(dev, COMP_CMD_RESET);
return 0;
@@ -479,14 +511,24 @@ static int dai_cmd(struct comp_dev *dev, int cmd, void *data) switch (cmd) { case COMP_CMD_RELEASE: case COMP_CMD_START:
ret = dma_start(dd->dma, dd->chan);if (ret < 0)return ret;dai_trigger(dd->dai, cmd, dev->params.direction);
/* only start the DAI if we are not XRUN handling */if (dd->xrun == 0) {/* start the DAI */ret = dma_start(dd->dma, dd->chan);if (ret < 0)return ret;dai_trigger(dd->dai, cmd, dev->params.direction);} else {dd->xrun = 0;}/* update starting wallclock */ platform_dai_wallclock(dev, &dd->wallclock); break;
case COMP_CMD_XRUN:
dd->xrun = 1;case COMP_CMD_PAUSE: case COMP_CMD_STOP: wait_init(&dd->complete);
On Fri, 2017-11-17 at 08:35 -0600, Pierre-Louis Bossart wrote:
On 11/16/17 4:01 PM, Liam Girdwood wrote:
WiP: The DAI should not be stopped and the restarted on XRUNs
Are you sure about this one? if you have an xrun leading to a channel swap, that's not something you could ever recover from?
This was why it was WiP :)
I'm planning a patch for the SSP part later, but in general the DAI component would pass the XRUN onto the SSP driver and await a return on how to handle it. e.g. SSP driver error status could indicate whether we need to restart SSP (due to BCE or underrun or overruns).
Flow would go something like :-
1) SSP IRQ fires and status indicates xrun/BCE. SSP IRQ handler notifies pipeline of XRUN.
2) Pipeline XRUN handler runs through all components, DAI component takes XRUN and check action with SSP driver. SSP indicates whether restart is needed.
3) DAI layer takes any XRUN actions.
4) Playback continues.
Liam
On 11/17/17 9:08 AM, Liam Girdwood wrote:
On Fri, 2017-11-17 at 08:35 -0600, Pierre-Louis Bossart wrote:
On 11/16/17 4:01 PM, Liam Girdwood wrote:
WiP: The DAI should not be stopped and the restarted on XRUNs
Are you sure about this one? if you have an xrun leading to a channel swap, that's not something you could ever recover from?
This was why it was WiP :)
ok, the commit title is also WiP then.
I'm planning a patch for the SSP part later, but in general the DAI component would pass the XRUN onto the SSP driver and await a return on how to handle it. e.g. SSP driver error status could indicate whether we need to restart SSP (due to BCE or underrun or overruns).
Flow would go something like :-
- SSP IRQ fires and status indicates xrun/BCE. SSP IRQ handler notifies
pipeline of XRUN.
- Pipeline XRUN handler runs through all components, DAI component
takes XRUN and check action with SSP driver. SSP indicates whether restart is needed.
DAI layer takes any XRUN actions.
Playback continues.
humm, this flow looks reasonable but will need some platform/debug configurability. There are quite a few issues with xruns and sometimes the SSP guarantees that the channel swap won't happen, but in others an xrun on RX leads to issues on TX, with all sorts of chicken bits disabling or enabling work-arounds. So for your point #2 the SSP driver would need to be quite smart or be told through an IPC config what to do in the event of an xrun (conceal, report, stop, interface reset, DSP reset, etc) depending on the paranoia level tolerated on the platform. As the paranoid-in-residence I'd like to make sure there are zero underflows tolerated during integration stages to force issues to pop-up in test reports...
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
participants (2)
-
Liam Girdwood -
Pierre-Louis Bossart