[Sound-open-firmware] [PATCH] pipeline: preloader: simplify preloader by reusing pipeline copy()

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, &current->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, &current->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