Re: [Sound-open-firmware] [PATCH v3 00/27] add R/W pointers check for pipeline buffers

On 2/10/17 8:17 PM, Keyon Jie wrote:

This series adding the R/W buffer pointers check and make sure we won't copy wrong/missed/repeated data for the whole pipeline.

For ALSA applications, usually we have two kinds of trigger command sequences:

1. (e.g. aplay) prefill datas ==> trigger start ==> refill buffer(when buffer free is enough) ==> ... ==> data filling finished ==> trigger drain ==> when all datas are rendered(via read pointer feedback) ==> trigger stop;
2. (e.g. speaker-test with large buffer size > wav file size): prefill all datas one time ==> trigger start ==> wait until all datas are rendered(via feedback read pointer) ==> trigger stop ==> prefill another wav file ==> trigger start ==> ...

So the feedback from firmware about the read pointer is very important for the pipeline's running, to make the firmware can handle these two kinds of sequences, this series design/assume with preconditions:

1. We do down stream full preload(full fill all pipeline buffers, except host

side one); 2. Using pull mode at running stage and pulling one period per time; 3. When in normal running status, if buffer a->avail is 0, then all those upstream buffers are empty;

For those two sequence, we use a same logic to handle it:

1. We put all components in running status even some of them are empty(dai is

an exception); 2. We stop dai/ssp once we found there is no more data for it to render, and notify this last read position to host side; 3. Host side will trigger stop and we will reset all buffer pointers and related resources; 4. For case #1, trigger reset will reset/free more resources(allocated/init in set_params()); For case #2, another start cmd will trigger preload and start in firmware, before that all pointers are reset to original ones.

TODOs: It is only verified for playback, for capture, we may need some vivid code to implement them.

I may be missing something here - the cover letter isn't very clear - , but I see mentions of 'pull one period at a time' and no mention of the start/stop threshold. Is this really going to work in no-irq mode where periods are irrelevant and with start/stop thresholds that are smaller than a period. If you really want to make the transfers reliable then you should also assume that there are no rewinds and compare read/write pointers to avoid fetching stale data. that's a useful mode to have

Changes in v3:

1. add one preload patch for pipeline full preload, which make playback works.
2. fix DMA channel status wrong after trigger stop issue.

Changes in v2:

1. extract some common code to separate functions to save code;
2. add comments for component status;
3. other slight changes per Liam's suggestion.

Keyon Jie (26): host: handle more possibility for playback spitting dai: reset dai_pos at MMAP_PPOS command pipleline: op_sink: don't need clear the buffer for each prepare host: start: don't real start for start command ipc: presentation_posn should be uint64_t type intel-ipc: add application buffer pointer update from host side intel-ipc: add stream operation IPC_INTEL_STR_STOP host: add host_update_buffer for produce and consume host: rename host_pos_blks to host_pos_read host: for the last bytes/period, send read notification later host: add R/W pointers check for playback host: add a completion work to notify host side host: reset pointers at new() and reset() host: cleanup host_cmd() cases host: host_copy: only copy when there is data and wait until finish volume: volume_params(): set the sink buffer params depending on its sink component: status: rename COMP_STATE_STOPPED to COMP_STATE_SETUP volume: volume_cmd(): only stop/pause at running volume: add R/W pointer check for volume_copy() mixer: mixer_params(): set the sink buffer params to STREAM_FORMAT_S32_LE. mixer: add R/W pointer check for mixer_copy() mixer: add dd->last_bytes for the last bytes(< period) caculation dai: add R/W pointer check for dai copy dw-dma: handle different cases in irq handler component: host/volume/mixer: reset buffers for stop cmd host: create host_elements_reset() and host_pointer_reset() for common use

Liam Girdwood (1): pipeline: Add support for full pipeline pre-loading.

src/audio/dai.c | 71 ++++- src/audio/host.c | 313 ++++++++++++++++------ src/audio/mixer.c | 85 +++++- src/audio/pipeline.c | 107 +++++++- src/audio/volume.c | 59 +++- src/drivers/dw-dma.c | 30 ++- src/include/reef/audio/component.h | 59 +++- src/include/reef/audio/pipeline.h | 2 + src/include/reef/dma.h | 7 + src/include/uapi/intel-ipc.h | 3 +- src/ipc/intel-ipc.c | 69 +++++ src/platform/baytrail/include/platform/platform.h | 6 + 12 files changed, 661 insertions(+), 150 deletions(-)