From: Vanitha Channaiah vanitha.channaiah@in.bosch.com

This change adapt the fix commit 6b058fda9dce ("pcm: dmix: Add option to allow alignment of slave pointers") for dshare plugin

Issue is that snd_pcm_wait() goes back to waiting because the hw_ptr is not period aligned. Therefore snd_pcm_wait() will block for a longer time as required.

With these rcar driver changes the exact position of the dma is returned. During snd_pcm_start they read hw_ptr as reference, and this hw_ptr is now not period aligned, and is a little ahead over the period while it is read. Therefore when the avail is calculated during snd_pcm_wait(), it is missing the avail_min by a few frames.

An additional option hw_ptr_alignment is provided to dshare configuration, to allow the user to configure the slave application and hw pointer alignment at startup

Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm_dshare.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index f135b5d..cf8a863 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -333,16 +333,16 @@ static int snd_pcm_dshare_reset(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; dshare->hw_ptr %= pcm->period_size; dshare->appl_ptr = dshare->last_appl_ptr = dshare->hw_ptr; - dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; + snd_pcm_direct_reset_slave_ptr(pcm, dshare); return 0; }

-static int snd_pcm_dshare_start_timer(snd_pcm_direct_t *dshare) +static int snd_pcm_dshare_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dshare) { int err;

snd_pcm_hwsync(dshare->spcm); - dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; + snd_pcm_direct_reset_slave_ptr(pcm, dshare); err = snd_timer_start(dshare->timer); if (err < 0) return err; @@ -364,7 +364,8 @@ static int snd_pcm_dshare_start(snd_pcm_t *pcm) else if (avail < 0) return 0; else { - if ((err = snd_pcm_dshare_start_timer(dshare)) < 0) + err = snd_pcm_dshare_start_timer(pcm, dshare); + if (err < 0) return err; snd_pcm_dshare_sync_area(pcm); } @@ -547,7 +548,8 @@ static snd_pcm_sframes_t snd_pcm_dshare_mmap_commit(snd_pcm_t *pcm, return 0; snd_pcm_mmap_appl_forward(pcm, size); if (dshare->state == STATE_RUN_PENDING) { - if ((err = snd_pcm_dshare_start_timer(dshare)) < 0) + err = snd_pcm_dshare_start_timer(pcm, dshare); + if (err < 0) return err; } else if (dshare->state == SND_PCM_STATE_RUNNING || dshare->state == SND_PCM_STATE_DRAINING) { @@ -755,6 +757,7 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name, dshare->slowptr = opts->slowptr; dshare->max_periods = opts->max_periods; dshare->var_periodsize = opts->var_periodsize; + dshare->hw_ptr_alignment = opts->hw_ptr_alignment; dshare->sync_ptr = snd_pcm_dshare_sync_ptr;

retry: @@ -912,6 +915,12 @@ pcm.name { ipc_key INT # unique IPC key ipc_key_add_uid BOOL # add current uid to unique IPC key ipc_perm INT # IPC permissions (octal, default 0600) + hw_ptr_alignment STR # Slave application and hw pointer alignment type + # STR can be one of the below strings : + # no + # roundup + # rounddown + # auto (default) slave STR # or slave { # Slave definition @@ -936,6 +945,27 @@ pcm.name { } \endcode

+<code>hw_ptr_alignment</code> specifies slave application and hw +pointer alignment type. By default hw_ptr_alignment is auto. Below are +the possible configurations: +- no: minimal latency with minimal frames dropped at startup. But + wakeup of application (return from snd_pcm_wait() or poll()) can + take up to 2 * period. +- roundup: It is guaranteed that all frames will be played at + startup. But the latency will increase upto period-1 frames. +- rounddown: It is guaranteed that a wakeup will happen for each + period and frames can be written from application. But on startup + upto period-1 frames will be dropped. +- auto: Selects the best approach depending on the used period and + buffer size. + If the application buffer size is < 2 * application period, + "roundup" will be selected to avoid under runs. If the slave_period + is < 10ms we could expect that there are low latency + requirements. Therefore "rounddown" will be chosen to avoid long + wakeup times. Such wakeup delay could otherwise end up with Xruns in + case of a dependency to another sound device (e.g. forwarding of + microphone to speaker). Else "no" will be chosen. + \subsection pcm_plugins_dshare_funcref Function reference

<UL>

From: Vanitha Channaiah vanitha.channaiah@in.bosch.com

For buffer size less than two period size, the start position of slave_app_ptr is rounded up in order to avoid xruns For e.g.: Considering below parameters and its values Period size = 96 Buffer size = 191 slave_appl_ptr = slave_hw_ptr = unaligned value

Issue: - During the start of the stream, app_ptr = hw_ptr = 0 - Application writes one period of data in the buffer i.e app_ptr = 96, hw_ptr = 0 - Now, the avail is just period-1 frames available. avail = hw_ptr + buffer_size - app_ptr = 95 i.e. shortage of 1 frame space - so application is waiting for the 1frame space to be available. - slave_app_ptr and slave_hw_ptr would get updated to lower values - This could lead to under run to occur.

Fix: If we round Up the slave_app_ptr pointer, - During the start of the stream, app_ptr = hw_ptr = 0 - Application writes one period of data in the buffer i.e app_ptr = 96, hw_ptr = 0 - Round Up of slave_app_ptr pointer leads to below calculation: - slave_app_ptr rounded to 96 - slave_app_ptr and slave_hw_ptr would get updated to larger value nearing to 2 period size - avail = greater than period size. - Here, there is a lower chance of under run.

Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm_direct.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 54d9900..b56da85 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -2043,10 +2043,12 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,

void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) { - + /* For buffer size less than two period size, the start position + * of slave app ptr is rounded up in order to avoid xruns + */ if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDUP || (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO && - pcm->buffer_size <= pcm->period_size * 2)) + pcm->buffer_size < pcm->period_size * 2)) dmix->slave_appl_ptr = ((dmix->slave_appl_ptr + dmix->slave_period_size - 1) / dmix->slave_period_size) * dmix->slave_period_size;

From: Vanitha Channaiah vanitha.channaiah@in.bosch.com

This Fix was analyzed for below usecase :

alsa configuration: pcm.line_in { type dsnoop ipc_key INT slave { pcm hardware channels 2 period_time 8000 rate 48000 format S16_LE } bindings { 0 0 1 1 } } pcm.hardware { type hw card "gmd-card" device 0 subdevice 0 channels 2 period_time 2000 rate 48000 format S16_LE }

command: $arecord -v -D line_in -r 48000 -c 2 -f S16_LE recordfile.wav Direct Snoop PCM Its setup is: stream : CAPTURE access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 1536 period_size : 384 period_time : 8000 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 384 period_event : 0 start_threshold : 1 stop_threshold : 1536 silence_threshold: 0 silence_size : 0 boundary : huge value Hardware PCM card 3 'gmd-card' device 0 subdevice 0 Its setup is: stream : CAPTURE access : MMAP_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 1536 period_size : 96 period_time : 2000 tstamp_mode : ENABLE tstamp_type : MONOTONIC period_step : 1 avail_min : 96 period_event : 0 start_threshold : 1 stop_threshold : huge value silence_threshold: 0 silence_size : 0 boundary : huge value appl_ptr : 0 hw_ptr : 576

Here, there are no other plugins apart from Dsnoop.

Issue: After partial read of unaligned frames(not one period frames), snd_pcm_wait() would block for the pcm->avail_min which would result in blocking for longer periods i.e more than one period as specified by pcm->avail_min

For e.g.: Slave period size = 0x60 Client period-size=0x180 No. of Ticks = 4 pcm->avail_min = one period size = 0x180

Issue: - During the start of streaming, the position of slave hw_ptr returned from the driver is 0x20. - avail is 0x20 - hw_ptr is calculated by new_slave_hw_ptr - old_slave_hw_ptr i.e. 0x20 - 0 = 0x20 - hw_ptr updated to 0x20 - avail is 0x20 - app_ptr updated to 0x20 - Now, avail = 0 - snd_pcm_wait() waits till avail=0x180 because pcm->avail_min = 0x180 - After 4 snd_pcm_elapsed(), slave_hw_ptr = 0x180 - Since app_ptr has updated with 0x20, avail becomes 0x160 There is a shortage of 0x20 frames and hence snd_pcm_wait() goes back to wait again. - Now, snd_pcm_wait is locked. - After another 4 snd_pcm_elapsed() slave_hw_ptr = 0x300 - avail = 0x2e0 - snd_pcm_wait() unlocks. So, here snd_pcm_wait() is locked for more than 1 period(exactly 2 periods)

Also, this issue can be seen without dsnoop plugin, when HW reports unaligned hw_ptr For e.g. period size = 0x60 pcm->avail_min = 0x60 - During the start of streaming, the position of slave hw_ptr returned from the driver is 0x20. - hw_ptr is calculated by new_slave_hw_ptr - old_slave_hw_ptr i.e. 0x20 - 0 = 0x20 - hw_ptr updated to 0x20 - avail is 0x20 - app_ptr updated to 0x20 - Now, avail = 0 - snd_pcm_wait() waits till avail=0x60 because pcm->avail_min=0x60 - After 1 snd_pcm_elapsed(), slave_hw_ptr = 0x60 - Since app_ptr has updated with 0x20, avail becomes 0x40 There is a shortage of 0x20 frames and hence snd_pcm_wait() goes back to wait again. - Now, snd_pcm_wait is locked. - After another 1 snd_pcm_elapsed(), slave_hw_ptr = 0x120 - avail = 0xe0 - snd_pcm_wait() unlocks. So, here snd_pcm_wait() is locked for more than 1 period (exactly 2 periods)

Fix: After reading unaligned frames(not one period frames), set the pcm->avail_min to the needed_avail_slave_min frames so that snd_pcm_wait() blocks till needed_avail_slave_min available Once needed_avail_slave_min frames are read, set back the original pcm->avail_min

For ex: Slave period size = 0x60 Client period-size=0x180 No. of Ticks = 4 pcm->avail_min = one period size = 0x180

Fix: - During the start of streaming, the position of slave_hw_ptr returned from the driver is 0x20. - hw_ptr is calculated by new_slave_hw_ptr - old_slave_hw_ptr i.e. 0x20 - 0 = 0x20 - hw_ptr updated to 0x20 - avail is 0x20 - app_ptr updated to 0x20 - Now, avail = 0 - calculate needed_avail_slave_min = 0x160 - update the needed_avail_slave_min to pcm->avail_min i.e. pcm->avail_min = 0x160 - snd_pcm_wait() waits till avail=0x160 - After 4 snd_pcm_elapsed(), slave_hw_ptr = 0x180 - snd_pcm_wait() unlocks. - Once needed_avail_slave_min frames are read, set back the original pcm->avail_min to 0x180 So, here snd_pcm_wait() is locked for 1 period only.

Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm.c | 21 +++++++++++++++++++++ src/pcm/pcm_local.h | 2 ++ 2 files changed, 23 insertions(+)

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index f0db545..f361eb1 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -973,6 +973,7 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params) __snd_pcm_unlock(pcm); return err; } + pcm->original_avail_min = pcm->avail_min; __snd_pcm_unlock(pcm); return 0; } @@ -7267,6 +7268,17 @@ void snd_pcm_areas_from_bufs(snd_pcm_t *pcm, snd_pcm_channel_area_t *areas, snd_pcm_unlock(pcm); }

+static void snd_pcm_set_avail_min(snd_pcm_t *pcm, snd_pcm_uframes_t avail) +{ + if (avail != pcm->avail_min) { + snd_pcm_sw_params_t swparams; + + snd_pcm_sw_params_current(pcm, &swparams); + swparams.avail_min = avail; + _snd_pcm_sw_params_internal(pcm, &swparams); + } +} + snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_t *areas, snd_pcm_uframes_t offset, snd_pcm_uframes_t size, snd_pcm_xfer_areas_func_t func) @@ -7274,6 +7286,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_ snd_pcm_uframes_t xfer = 0; snd_pcm_sframes_t err = 0; snd_pcm_state_t state; + snd_pcm_uframes_t needed_slave_avail_min = 0;

if (size == 0) return 0; @@ -7332,6 +7345,14 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_ if (err < 0) break; frames = err; + pcm->unaligned_frames += frames; + pcm->unaligned_frames %= pcm->period_size; + if (pcm->unaligned_frames) { + needed_slave_avail_min = pcm->period_size - pcm->unaligned_frames; + snd_pcm_set_avail_min(pcm, needed_slave_avail_min); + } else { + snd_pcm_set_avail_min(pcm, pcm->original_avail_min); + } offset += frames; size -= frames; xfer += frames; diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index e103f72..3fdffb4 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -210,6 +210,8 @@ struct _snd_pcm { snd_pcm_tstamp_type_t tstamp_type; /* timestamp type */ unsigned int period_step; snd_pcm_uframes_t avail_min; /* min avail frames for wakeup */ + snd_pcm_uframes_t unaligned_frames; + snd_pcm_uframes_t original_avail_min; int period_event; snd_pcm_uframes_t start_threshold; snd_pcm_uframes_t stop_threshold;