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

[Pierre-Louis Bossart]

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(-)
>