On Mon, 12 Nov 2018 13:47:18 +0100, twischer@de.adit-jv.com wrote:

From: Laxmi Devi Laxmi.Devi@in.bosch.com

These changes are required due to the kernel commit 07b7acb51d283d8469696c906b91f1882696a4d4 ("ASoC: rsnd: update pointer more accurate")

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_filter is provided to dmix configuration, to allow the user to configure the slave application and hw pointer alignment at startup

Signed-off-by: Laxmi Devi Laxmi.Devi@in.bosch.com

Please provide your own sign-off when you submitting a patch, too.

The subject line should be adjusted better to match with the change; it's about adding an option to allow alignment of hwptr reporting.

About the changes: the term "hw_ptr_filter" might sound confusing. IMO, this is about the (hw)ptr alignment, not about filtering. Also, a too long option string can easily lead to a misspell :)

After all, we do either "no" alignment, "roundup" or "rounddown", or "auto" choice.

thanks,

Takashi

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 091fd78..1ff7527 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -1883,6 +1883,7 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf, rec->max_periods = 0; rec->var_periodsize = 0; rec->direct_memory_access = 1;

• rec->hw_ptr_filter = SND_PCM_HW_PTR_FLTR_AUTO;

  /* read defaults */ if (snd_config_search(root, "defaults.pcm.dmix_max_periods", &n) >= 0) {

@@ -1924,6 +1925,28 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf, rec->ipc_perm = perm; continue; }

• if (strcmp(id, "hw_ptr_filter") == 0) {
  

• 	const char *str;
  

• 	err = snd_config_get_string(n, &str);
  

• 	if (err < 0) {
  

• 		SNDERR("Invalid type for %s", id);
  

• 		return -EINVAL;
  

• 	}
  

• 	if (strcmp(str, "min_latency_min_frame_drop") == 0)
  

• 		rec->hw_ptr_filter = SND_PCM_HW_PTR_FLTR_MIN_LATENCY_MIN_FRAME_DROP;
  

• 	else if (strcmp(str, "no_frame_drop") == 0)
  

• 		rec->hw_ptr_filter = SND_PCM_HW_PTR_FLTR_NO_FRAME_DROP;
  

• 	else if (strcmp(str, "exact_wakeup") == 0)
  

• 		rec->hw_ptr_filter = SND_PCM_HW_PTR_FLTR_EXACT_WAKEUP;
  

• 	else if (strcmp(str, "auto") == 0)
  

• 		rec->hw_ptr_filter = SND_PCM_HW_PTR_FLTR_AUTO;
  

• 	else {
  

• 		SNDERR("The field hw_ptr_filter is invalid : %s", str);
  

• 		return -EINVAL;
  

• 	}
  

• 	continue;
  

• }
  

  if (strcmp(id, "ipc_gid") == 0) { char *group; char *endp;

diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h index 582f055..bd0d0a1 100644 --- a/src/pcm/pcm_direct.h +++ b/src/pcm/pcm_direct.h @@ -49,6 +49,13 @@ typedef void (mix_areas_u8_t)(unsigned int size, volatile signed int *sum, size_t dst_step, size_t src_step, size_t sum_step);

+typedef enum snd_pcm_direct_hw_ptr_filter {

• SND_PCM_HW_PTR_FLTR_MIN_LATENCY_MIN_FRAME_DROP = 0, /* use the hw_ptr as is and do no rounding */
• SND_PCM_HW_PTR_FLTR_NO_FRAME_DROP = 1, /* round the slave_appl_ptr up to slave_period */
• SND_PCM_HW_PTR_FLTR_EXACT_WAKEUP = 2, /* round slave_hw_ptr and slave_appl_ptr down to slave_period */
• SND_PCM_HW_PTR_FLTR_AUTO = 3 /* automatic selection */

+} snd_pcm_direct_hw_ptr_filter_t;

struct slave_params { snd_pcm_format_t format; int rate; @@ -159,6 +166,7 @@ struct snd_pcm_direct { unsigned int *bindings; unsigned int recoveries; /* mirror of executed recoveries on slave */ int direct_memory_access; /* use arch-optimized buffer RW */

• snd_pcm_direct_hw_ptr_filter_t hw_ptr_filter; union { struct { int shmid_sum; /* IPC global sum ring buffer memory identification */

@@ -341,6 +349,7 @@ struct snd_pcm_direct_open_conf { int max_periods; int var_periodsize; int direct_memory_access;

• snd_pcm_direct_hw_ptr_filter_t hw_ptr_filter; snd_config_t *slave; snd_config_t *bindings;

}; diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index 881154e..ad1bd90 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -55,6 +55,9 @@ const char *_snd_module_pcm_dmix = ""; #define STATE_RUN_PENDING 1024 #endif

+#define SEC_TO_MS 1000 /* Seconds representing in Milli seconds */ +#define LOW_LATENCY_PERIOD_TIME 10 /* slave_period time for low latency requirements in ms */

/*

*/ @@ -560,14 +563,17 @@ static int snd_pcm_dmix_hwsync(snd_pcm_t *pcm) static void reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) { dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;

• if (pcm->buffer_size > pcm->period_size * 2)

• return;
  

• /* If we have too litte periods, better to align the start position

• * to the period boundary so that the interrupt can be handled properly
  

• * at the right time.
  

• */
  

• dmix->slave_appl_ptr = ((dmix->slave_appl_ptr + dmix->slave_period_size - 1)

• if (dmix->hw_ptr_filter == SND_PCM_HW_PTR_FLTR_NO_FRAME_DROP ||

• 	(dmix->hw_ptr_filter == SND_PCM_HW_PTR_FLTR_AUTO &&
  

• 	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;
  

• else if (dmix->hw_ptr_filter == SND_PCM_HW_PTR_FLTR_EXACT_WAKEUP ||

• 	(dmix->hw_ptr_filter == SND_PCM_HW_PTR_FLTR_AUTO &&
  

• 	(((dmix->slave_period_size * SEC_TO_MS) / pcm->rate) < LOW_LATENCY_PERIOD_TIME)))
  

• dmix->slave_appl_ptr = dmix->slave_hw_ptr = ((dmix->slave_hw_ptr
  

• 		/ dmix->slave_period_size) * dmix->slave_period_size);
  

}

static int snd_pcm_dmix_reset(snd_pcm_t *pcm) @@ -1086,6 +1092,7 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name, dmix->slowptr = opts->slowptr; dmix->max_periods = opts->max_periods; dmix->var_periodsize = opts->var_periodsize;

• dmix->hw_ptr_filter = opts->hw_ptr_filter; dmix->sync_ptr = snd_pcm_dmix_sync_ptr; dmix->direct_memory_access = opts->direct_memory_access;

@@ -1241,6 +1248,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_filter STR # Slave application and hw pointer alignment type

• 				# STR can be one of the below strings :
  

• 				# min_latency_min_frame_drop
  

• 				# no_frame_drop
  

• 				# exact_wakeup
  

• 				# auto (default)
  

  slave STR # or slave { # Slave definition

@@ -1273,6 +1286,25 @@ added to the value set in <code>ipc_key</code>. This will avoid the confliction of the same IPC key with different users concurrently.

+<code>hw_ptr_filter</code> specifies slave application and hw pointer alignment +type. By default hw_ptr_filter is auto. Below are the possible configurations : +- min_latency_min_frame_drop : 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.

+- no_frame_drop : It is guaranteed that all frames will be played at startup. But

• the latency will increase upto period-1 frames.

+- exact_wakeup : 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, no_frame_drop
• will be selected to avoid under runs. If the slave_period is < 10ms we
• could expect that there are low latency requirements. Therefore exact_wakeup
• 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 min_latency_min_frame_drop
• will be chosen.

Note that the dmix plugin itself supports only a single configuration. That is, it supports only the fixed rate (default 48000), format (\c S16), channels (2), and period_time (125000). -- 2.7.4