[PATCH 1/5] pcm: direct: Add generic hw_ptr_alignment function for dmix, dshare and dsnoop

[PATCH 1/5] pcm: direct: Add generic hw_ptr_alignment function for dmix, dshare and dsnoop

[vanitha.channaiah]

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

This patch is same as the fix in below commit
commit 6b058fda9dce ("pcm: dmix: Add option to allow alignment of slave
pointers")

Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
---
 src/pcm/pcm_direct.c | 19 +++++++++++++++++++
 src/pcm/pcm_direct.h |  8 ++++++++
 src/pcm/pcm_dmix.c   | 25 ++-----------------------
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 666a8ce..411a035 100644
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -2040,3 +2040,22 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
 
 	return 0;
 }
+
+void snd_pcm_direct_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 (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))
+		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_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDDOWN ||
+		(dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_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);
+}
diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h
index da5e280..a71aab1 100644
--- a/src/pcm/pcm_direct.h
+++ b/src/pcm/pcm_direct.h
@@ -24,6 +24,11 @@
 
 #define DIRECT_IPC_SEMS         1
 #define DIRECT_IPC_SEM_CLIENT   0
+/* Seconds representing in Milli seconds */
+#define SEC_TO_MS               1000
+/* slave_period time for low latency requirements in ms */
+#define LOW_LATENCY_PERIOD_TIME 10
+
 
 typedef void (mix_areas_t)(unsigned int size,
 			   volatile void *dst, void *src,
@@ -257,6 +262,8 @@ struct snd_pcm_direct {
 	snd1_pcm_direct_get_chmap
 #define snd_pcm_direct_set_chmap \
 	snd1_pcm_direct_set_chmap
+#define snd_pcm_direct_reset_slave_ptr \
+	snd1_pcm_direct_reset_slave_ptr
 
 int snd_pcm_direct_semaphore_create_or_connect(snd_pcm_direct_t *dmix);
 
@@ -341,6 +348,7 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct);
 int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm);
 int snd_timer_async(snd_timer_t *timer, int sig, pid_t pid);
 struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm);
+void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix);
 
 struct snd_pcm_direct_open_conf {
 	key_t ipc_key;
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index c5592cd..dcde40d 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -55,9 +55,6 @@ 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,30 +557,12 @@ 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 (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))
-		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_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDDOWN ||
-		 (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_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)
 {
 	snd_pcm_direct_t *dmix = pcm->private_data;
 	dmix->hw_ptr %= pcm->period_size;
 	dmix->appl_ptr = dmix->last_appl_ptr = dmix->hw_ptr;
-	reset_slave_ptr(pcm, dmix);
+	snd_pcm_direct_reset_slave_ptr(pcm, dmix);
 	return 0;
 }
 
@@ -592,7 +571,7 @@ static int snd_pcm_dmix_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dmix)
 	int err;
 
 	snd_pcm_hwsync(dmix->spcm);
-	reset_slave_ptr(pcm, dmix);
+	snd_pcm_direct_reset_slave_ptr(pcm, dmix);
 	err = snd_timer_start(dmix->timer);
 	if (err < 0)
 		return err;
-- 
2.7.4

[PATCH 2/5] pcm: dshare: Added "hw_ptr_alignment" option in configuration for alignment of slave pointers

[vanitha.channaiah]

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

snd_pcm_wait() is waiting for longer time more than two periodic size as
avail is less than avail_min by few frames.
This is because the hw_ptr read from the kernel during snd_pcm_start()
is not period aligned which is ahead of few frames.

These changes are adaptation of same fix as given for dmix
commit ("6b058fda9dce8f416774ae54975f5706f3f5a6da")
("pcm-dmix-Add-option-to-allow-alignment-of-slave-poin.patch")

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

diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
index f135b5d..45e2597 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:
-- 
2.7.4

[PATCH 3/5] pcm: dsnoop: Added "hw_ptr_alignment" option in configuration for slave pointer alignment

[vanitha.channaiah]

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

snd_pcm_wait() is waiting for longer time more than two periodic size as
avail is less than avail_min by few frames.
This is because the hw_ptr read from the kernel during snd_pcm_start()
is not period aligned which is ahead of few (Nperiod + 1)frames.

Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
---
 src/pcm/pcm_direct.c | 7 ++++---
 src/pcm/pcm_dmix.c   | 2 ++
 src/pcm/pcm_dshare.c | 2 ++
 src/pcm/pcm_dsnoop.c | 5 ++++-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 411a035..b56da85 100644
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -2043,11 +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)
 {
-	dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;
-
+	/* 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;
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index dcde40d..274726e 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -562,6 +562,7 @@ static int snd_pcm_dmix_reset(snd_pcm_t *pcm)
 	snd_pcm_direct_t *dmix = pcm->private_data;
 	dmix->hw_ptr %= pcm->period_size;
 	dmix->appl_ptr = dmix->last_appl_ptr = dmix->hw_ptr;
+	dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;
 	snd_pcm_direct_reset_slave_ptr(pcm, dmix);
 	return 0;
 }
@@ -571,6 +572,7 @@ static int snd_pcm_dmix_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dmix)
 	int err;
 
 	snd_pcm_hwsync(dmix->spcm);
+	dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;
 	snd_pcm_direct_reset_slave_ptr(pcm, dmix);
 	err = snd_timer_start(dmix->timer);
 	if (err < 0)
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
index 45e2597..3847645 100644
--- a/src/pcm/pcm_dshare.c
+++ b/src/pcm/pcm_dshare.c
@@ -333,6 +333,7 @@ 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;
 }
@@ -342,6 +343,7 @@ 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)
diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
index d08b624..2bbbc2d 100644
--- a/src/pcm/pcm_dsnoop.c
+++ b/src/pcm/pcm_dsnoop.c
@@ -278,6 +278,7 @@ static int snd_pcm_dsnoop_reset(snd_pcm_t *pcm)
 	dsnoop->hw_ptr %= pcm->period_size;
 	dsnoop->appl_ptr = dsnoop->hw_ptr;
 	dsnoop->slave_appl_ptr = dsnoop->slave_hw_ptr;
+	snd_pcm_direct_reset_slave_ptr(pcm, dsnoop);
 	return 0;
 }
 
@@ -285,12 +286,13 @@ static int snd_pcm_dsnoop_start(snd_pcm_t *pcm)
 {
 	snd_pcm_direct_t *dsnoop = pcm->private_data;
 	int err;
-	
+
 	if (dsnoop->state != SND_PCM_STATE_PREPARED)
 		return -EBADFD;
 	snd_pcm_hwsync(dsnoop->spcm);
 	snoop_timestamp(pcm);
 	dsnoop->slave_appl_ptr = dsnoop->slave_hw_ptr;
+	snd_pcm_direct_reset_slave_ptr(pcm, dsnoop);
 	err = snd_timer_start(dsnoop->timer);
 	if (err < 0)
 		return err;
@@ -627,6 +629,7 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
 	dsnoop->max_periods = opts->max_periods;
 	dsnoop->var_periodsize = opts->var_periodsize;
 	dsnoop->sync_ptr = snd_pcm_dsnoop_sync_ptr;
+	dsnoop->hw_ptr_alignment = opts->hw_ptr_alignment;
 
  retry:
 	if (first_instance) {
-- 
2.7.4

[PATCH 4/5] pcm: restructuring sw params function

[vanitha.channaiah]

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

snd_pcm_sw_params() is reformatted by using
_snd_pcm_sw_params_internal() function

Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
---
 src/pcm/pcm.c        | 12 +-----------
 src/pcm/pcm_local.h  |  1 +
 src/pcm/pcm_params.c | 21 +++++++++++++++++++++
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 3a71d79..f0db545 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -968,21 +968,11 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
 	}
 #endif
 	__snd_pcm_lock(pcm); /* forced lock due to pcm field change */
-	err = pcm->ops->sw_params(pcm->op_arg, params);
+	err = _snd_pcm_sw_params_internal(pcm, params);
 	if (err < 0) {
 		__snd_pcm_unlock(pcm);
 		return err;
 	}
-	pcm->tstamp_mode = params->tstamp_mode;
-	pcm->tstamp_type = params->tstamp_type;
-	pcm->period_step = params->period_step;
-	pcm->avail_min = params->avail_min;
-	pcm->period_event = sw_get_period_event(params);
-	pcm->start_threshold = params->start_threshold;
-	pcm->stop_threshold = params->stop_threshold;
-	pcm->silence_threshold = params->silence_threshold;
-	pcm->silence_size = params->silence_size;
-	pcm->boundary = params->boundary;
 	__snd_pcm_unlock(pcm);
 	return 0;
 }
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index d52229d..e103f72 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -661,6 +661,7 @@ static inline int muldiv_near(int a, int b, int c)
 	return n;
 }
 
+int _snd_pcm_sw_params_internal(snd_pcm_t *pcm, snd_pcm_sw_params_t *params);
 int snd_pcm_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params);
 int _snd_pcm_hw_params_internal(snd_pcm_t *pcm, snd_pcm_hw_params_t *params);
 #undef _snd_pcm_hw_params
diff --git a/src/pcm/pcm_params.c b/src/pcm/pcm_params.c
index 8826bc3..3ba05fb 100644
--- a/src/pcm/pcm_params.c
+++ b/src/pcm/pcm_params.c
@@ -2439,3 +2439,24 @@ int _snd_pcm_hw_params_internal(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
 	return 0;
 }
 
+int _snd_pcm_sw_params_internal(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
+{
+	int err;
+
+	assert(pcm && params);
+	assert(pcm->setup);
+	err = pcm->ops->sw_params(pcm->op_arg, params);
+	if (err < 0)
+		return err;
+	pcm->tstamp_mode = params->tstamp_mode;
+	pcm->tstamp_type = params->tstamp_type;
+	pcm->period_step = params->period_step;
+	pcm->avail_min = params->avail_min;
+	pcm->period_event = sw_get_period_event(params);
+	pcm->start_threshold = params->start_threshold;
+	pcm->stop_threshold = params->stop_threshold;
+	pcm->silence_threshold = params->silence_threshold;
+	pcm->silence_size = params->silence_size;
+	pcm->boundary = params->boundary;
+	return 0;
+}
-- 
2.7.4

[PATCH 5/5] pcm: Update pcm->avail_min with needed_slave_avail_min, after reading unaligned frames

[vanitha.channaiah]

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

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

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

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

Re: [ALSA patch] [PATCH 1/5] pcm: direct: Add generic hw_ptr_alignment function for dmix, dshare and dsnoop

[Takashi Iwai]

On Tue, 30 Apr 2019 09:38:39 +0200,
<vanitha.channaiah@in.bosch.com> wrote:
> 
> From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
> 
> This patch is same as the fix in below commit
> commit 6b058fda9dce ("pcm: dmix: Add option to allow alignment of slave
> pointers")

The patch description needs rephrasing.  What actually this does is to
move the code from pcm_dmix.c to pcm_direct.c and its header so that
the helper code can be re-used by other direct-PCM plugins.


thanks,

Takashi

Re: [ALSA patch] [PATCH 2/5] pcm: dshare: Added "hw_ptr_alignment" option in configuration for alignment of slave pointers

[Takashi Iwai]

On Tue, 30 Apr 2019 09:38:40 +0200,
<vanitha.channaiah@in.bosch.com> wrote:
> 
> From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
> 
> snd_pcm_wait() is waiting for longer time more than two periodic size as
> avail is less than avail_min by few frames.
> This is because the hw_ptr read from the kernel during snd_pcm_start()
> is not period aligned which is ahead of few frames.
> 
> These changes are adaptation of same fix as given for dmix
> commit ("6b058fda9dce8f416774ae54975f5706f3f5a6da")
> ("pcm-dmix-Add-option-to-allow-alignment-of-slave-poin.patch")
> 
> Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>

Again, this patch description is too ambiguous.

And, if it enables the hw_ptr_alignment option, update the
documentation as well.


thanks,

Takashi

> ---
>  src/pcm/pcm_dshare.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
> index f135b5d..45e2597 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:
> -- 
> 2.7.4
> 
> _______________________________________________
> Patch mailing list
> Patch@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/patch
> 

Re: [ALSA patch] [PATCH 3/5] pcm: dsnoop: Added "hw_ptr_alignment" option in configuration for slave pointer alignment

[Takashi Iwai]

On Tue, 30 Apr 2019 09:38:41 +0200,
<vanitha.channaiah@in.bosch.com> wrote:
> 
> From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
> 
> snd_pcm_wait() is waiting for longer time more than two periodic size as
> avail is less than avail_min by few frames.
> This is because the hw_ptr read from the kernel during snd_pcm_start()
> is not period aligned which is ahead of few (Nperiod + 1)frames.
> 
> Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>

Ditto as patch 2, the description is too ambiguous, and the update of
documentation is missing.

And...

> --- a/src/pcm/pcm_direct.c
> +++ b/src/pcm/pcm_direct.c
> @@ -2043,11 +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)
>  {
> -	dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;
> -
> +	/* 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;

It's not good to change the helper function semantics out of sudden,
even without any description.


thanks,

Takashi

Re: [ALSA patch] [PATCH 4/5] pcm: restructuring sw params function

[Takashi Iwai]

On Tue, 30 Apr 2019 09:38:42 +0200,
<vanitha.channaiah@in.bosch.com> wrote:
> 
> From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
> 
> snd_pcm_sw_params() is reformatted by using
> _snd_pcm_sw_params_internal() function
> 
> Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>

I see no reason to do that.  Please describe.


thanks,

Takashi

Re: [ALSA patch] [PATCH 5/5] pcm: Update pcm->avail_min with needed_slave_avail_min, after reading unaligned frames

[Takashi Iwai]

On Tue, 30 Apr 2019 09:38:43 +0200,
<vanitha.channaiah@in.bosch.com> wrote:
> 
> From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>
> 
> Issue: After partial read of unaligned frames, snd_pcm_wait()
> would block for the pcm->avail_min which would result in
> blocking of snd_pcm_wait() for longer periods i.e more than
> one period as specified by avail_min
> 
> Fix: After reading unaligned frames, set the pcm->avail_min
> to the needed available frames so that snd_pcm_wait() blocks till
> needed available frames.
> Once needed available frames are read, set back the original
> pcm->avail_min
> 
> Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com>

This kind of changes in the core code should be avoided as much as
possible, especially if it's only relevant with the specific plugins.

Sorry, this isn't convincing enough.  If this is a MUST, please
clarify better.


thanks,

Takashi