[PATCH v3 0/3] ASoC: davinci: perparation for eDMA dmaengine PCM

[PATCH v3 0/3] ASoC: davinci: perparation for eDMA dmaengine PCM

[Peter Ujfalusi]

Hi,

Changes since v2:
- Constraint only placed for the period size [1]
- Patch for davinci-pcm.h to add empty function in case of !DAVINCI platforms

When the edma arch and dmaengine patches has been taken only one patch is needed
to switch AM335x/AM437x to use the edma-pcm.

[1]. I was not able to reporoduce the issue I have seen with mplayer anymore.
     As it was thought constraint for the period size is enough and there's no
     need to place the same constraint on buffer size.

Regards,
Peter
---
Peter Ujfalusi (3):
  ASoC: davinci-mcasp: Constraint on the period and buffer size based on
    FIFO usage
  ASoC: davinci-mcasp: Assign the dma_data earlier in dai_probe callback
  ASoC: davinci-pcm: Add empty functions for !CONFIG_SND_DAVINCI_SOC
    builds

 sound/soc/davinci/davinci-mcasp.c | 33 +++++++++++++++++++++++++++++----
 sound/soc/davinci/davinci-pcm.h   | 10 ++++++++++
 2 files changed, 39 insertions(+), 4 deletions(-)

-- 
1.9.0

[PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Peter Ujfalusi]

We need to place constraint on the period if the read or write AFIFO
is enabled and it is configured for more than one word otherwise
the DMA will fail in buffer configuration where the sizes are not
aligned with the requested FIFO configuration.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index a01ae97c90aa..df067a836c4d 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -720,6 +720,7 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
 				 struct snd_soc_dai *dai)
 {
 	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
+	int afifo_numevt;
 
 	if (mcasp->version == MCASP_VERSION_4)
 		snd_soc_dai_set_dma_data(dai, substream,
@@ -727,6 +728,16 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
 	else
 		snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
 
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		afifo_numevt = mcasp->txnumevt;
+	else
+		afifo_numevt = mcasp->rxnumevt;
+
+	if (afifo_numevt > 1)
+		snd_pcm_hw_constraint_step(substream->runtime, 0,
+					   SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+					   afifo_numevt);
+
 	return 0;
 }
 
-- 
1.9.0

[PATCH v3 2/3] ASoC: davinci-mcasp: Assign the dma_data earlier in dai_probe callback

[Peter Ujfalusi]

Set up the playback_dma_data/capture_dma_data for the dai at probe
time since the generic dmaengine PCM stack needs to have access to
this information early.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index df067a836c4d..d7f1a50d6bc1 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -722,12 +722,6 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
 	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
 	int afifo_numevt;
 
-	if (mcasp->version == MCASP_VERSION_4)
-		snd_soc_dai_set_dma_data(dai, substream,
-					&mcasp->dma_data[substream->stream]);
-	else
-		snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
-
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		afifo_numevt = mcasp->txnumevt;
 	else
@@ -750,6 +744,25 @@ static const struct snd_soc_dai_ops davinci_mcasp_dai_ops = {
 	.set_sysclk	= davinci_mcasp_set_sysclk,
 };
 
+static int davinci_mcasp_dai_probe(struct snd_soc_dai *dai)
+{
+	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
+
+	if (mcasp->version == MCASP_VERSION_4) {
+		/* Using dmaengine PCM */
+		dai->playback_dma_data =
+				&mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
+		dai->capture_dma_data =
+				&mcasp->dma_data[SNDRV_PCM_STREAM_CAPTURE];
+	} else {
+		/* Using davinci-pcm */
+		dai->playback_dma_data = mcasp->dma_params;
+		dai->capture_dma_data = mcasp->dma_params;
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int davinci_mcasp_suspend(struct snd_soc_dai *dai)
 {
@@ -803,6 +816,7 @@ static int davinci_mcasp_resume(struct snd_soc_dai *dai)
 static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
 	{
 		.name		= "davinci-mcasp.0",
+		.probe		= davinci_mcasp_dai_probe,
 		.suspend	= davinci_mcasp_suspend,
 		.resume		= davinci_mcasp_resume,
 		.playback	= {
-- 
1.9.0

[PATCH v3 3/3] ASoC: davinci-pcm: Add empty functions for !CONFIG_SND_DAVINCI_SOC builds

[Peter Ujfalusi]

To save drivers using davinci-pcm and edma-pcm the need to fiddle with
!CONFIG_SND_DAVINCI_SOC in their code.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-pcm.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
index fbb710c76c08..5fd4737ab398 100644
--- a/sound/soc/davinci/davinci-pcm.h
+++ b/sound/soc/davinci/davinci-pcm.h
@@ -29,7 +29,17 @@ struct davinci_pcm_dma_params {
 	unsigned int fifo_level;
 };
 
+#if IS_ENABLED(CONFIG_SND_DAVINCI_SOC)
 int davinci_soc_platform_register(struct device *dev);
 void davinci_soc_platform_unregister(struct device *dev);
+#else
+static inline int davinci_soc_platform_register(struct device *dev)
+{
+	return 0;
+}
+static inline void davinci_soc_platform_unregister(struct device *dev)
+{
+}
+#endif /* CONFIG_SND_DAVINCI_SOC */
 
 #endif
-- 
1.9.0

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Peter Ujfalusi]

On 03/18/2014 03:20 PM, Peter Ujfalusi wrote:
> We need to place constraint on the period if the read or write AFIFO
> is enabled and it is configured for more than one word otherwise
> the DMA will fail in buffer configuration where the sizes are not
> aligned with the requested FIFO configuration.

Oh, I edited the commit message and removed the text related to buffer size
but I failed to do that for the commit title :(

Mark: Do you want me to resend with fixed commit title?

-- 
Péter

> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  sound/soc/davinci/davinci-mcasp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
> index a01ae97c90aa..df067a836c4d 100644
> --- a/sound/soc/davinci/davinci-mcasp.c
> +++ b/sound/soc/davinci/davinci-mcasp.c
> @@ -720,6 +720,7 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
>  				 struct snd_soc_dai *dai)
>  {
>  	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
> +	int afifo_numevt;
>  
>  	if (mcasp->version == MCASP_VERSION_4)
>  		snd_soc_dai_set_dma_data(dai, substream,
> @@ -727,6 +728,16 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
>  	else
>  		snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
>  
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		afifo_numevt = mcasp->txnumevt;
> +	else
> +		afifo_numevt = mcasp->rxnumevt;
> +
> +	if (afifo_numevt > 1)
> +		snd_pcm_hw_constraint_step(substream->runtime, 0,
> +					   SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> +					   afifo_numevt);
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Peter Ujfalusi]

On 03/18/2014 03:23 PM, Peter Ujfalusi wrote:
> On 03/18/2014 03:20 PM, Peter Ujfalusi wrote:
>> We need to place constraint on the period if the read or write AFIFO
>> is enabled and it is configured for more than one word otherwise
>> the DMA will fail in buffer configuration where the sizes are not
>> aligned with the requested FIFO configuration.
> 
> Oh, I edited the commit message and removed the text related to buffer size
> but I failed to do that for the commit title :(

Actually I still have issue with the constraint.
If I play a 48KHz sample with mplayer it is fine:
Trying preferred audio driver 'alsa', options '[none]'
alsa-init: requested format: 48000 Hz, 2 channels, 9
alsa-init: using ALSA 1.0.27.2
alsa-init: setup for 1/2 channel(s)
alsa-init: using device default
alsa-init: opening device in blocking mode
alsa-init: device reopened in blocking mode
alsa-init: got buffersize=96000
alsa-init: got period size 1600
alsa: 48000 Hz/2 channels/4 bpf/96000 bytes buffer/Signed 16 bit Little Endian
AO: [alsa] 48000Hz 2ch s16le (2 bytes per sample)
AO: Description: ALSA-0.9.x-1.x audio output
AO: Author: Alex Beregszaszi, Zsolt Barat <joy@streamminister.de>
AO: Comment: under development
Building audio filter chain for 48000Hz/2ch/s16le -> 48000Hz/2ch/s16le...


However if I play 44.1KHz sample with mplayer:
Trying preferred audio driver 'alsa', options '[none]'
alsa-init: requested format: 44100 Hz, 2 channels, 9
alsa-init: using ALSA 1.0.27.2
alsa-init: setup for 1/2 channel(s)
alsa-init: using device default
alsa-init: opening device in blocking mode
alsa-init: device reopened in blocking mode
ALSA ERROR hw_params: set_near (BUFFER_TIME)
           value = 500000 : Invalid argument
ACCESS:  RW_INTERLEAVED
FORMAT:  S16_LE
SUBFORMAT:  STD
SAMPLE_BITS: 16
FRAME_BITS: 32
CHANNELS: 2
RATE: 44100
PERIOD_TIME: 50000
PERIOD_SIZE: NONE
PERIOD_BYTES: [4736 65536]
PERIODS: (1 19)
BUFFER_TIME: 500000
BUFFER_SIZE: 22050
BUFFER_BYTES: 88200
TICK_TIME: ALL
[AO_ALSA] Unable to set buffer time near: Invalid argument
Failed to initialize audio driver 'alsa'


Now if I place the same constraint to the buffer size as well and playing the
same 44.1KHz sample:
Trying preferred audio driver 'alsa', options '[none]'
alsa-init: requested format: 44100 Hz, 2 channels, 9
alsa-init: using ALSA 1.0.27.2
alsa-init: setup for 1/2 channel(s)
alsa-init: using device default
alsa-init: opening device in blocking mode
alsa-init: device reopened in blocking mode
alsa-init: got buffersize=88192
alsa-init: got period size 1696
alsa: 44100 Hz/2 channels/4 bpf/88192 bytes buffer/Signed 16 bit Little Endian
AO: [alsa] 44100Hz 2ch s16le (2 bytes per sample)
AO: Description: ALSA-0.9.x-1.x audio output
AO: Author: Alex Beregszaszi, Zsolt Barat <joy@streamminister.de>
AO: Comment: under development
Building audio filter chain for 44100Hz/2ch/s16le -> 44100Hz/2ch/s16le...

It seams that I need to place the same constraint step to both period size and
buffer size.
For some reason snd_pcm_hw_params_set_buffer_time_near() fails in case of
44.1KHz when asking for 0.5s buffer when I placed step 32 constraint to period
size. Placing the same 32 steps constraint to the buffer size as well will
make things working (for mplayer at least)

-- 
Péter

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1074 bytes --]

On Tue, Mar 18, 2014 at 04:28:55PM +0200, Peter Ujfalusi wrote:
> On 03/18/2014 03:23 PM, Peter Ujfalusi wrote:

> > Oh, I edited the commit message and removed the text related to buffer size
> > but I failed to do that for the commit title :(

> Actually I still have issue with the constraint.

OK, so is the patch an improvement or not?  If it fixes some cases it's
probably worth applying even if further fixes are still needed.  No need
to resend for the subject, I can fix that up.

> It seams that I need to place the same constraint step to both period size and
> buffer size.
> For some reason snd_pcm_hw_params_set_buffer_time_near() fails in case of
> 44.1KHz when asking for 0.5s buffer when I placed step 32 constraint to period
> size. Placing the same 32 steps constraint to the buffer size as well will
> make things working (for mplayer at least)

That seems like the constraint code is failing, at a guess probably
looking for too near a value of near if you see what I mean.  I don't
really have the time/enthusiasm to investigate this right now, sorry.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Peter Ujfalusi]

On 03/18/2014 08:07 PM, Mark Brown wrote:
> On Tue, Mar 18, 2014 at 04:28:55PM +0200, Peter Ujfalusi wrote:
>> On 03/18/2014 03:23 PM, Peter Ujfalusi wrote:
> 
>>> Oh, I edited the commit message and removed the text related to buffer size
>>> but I failed to do that for the commit title :(
> 
>> Actually I still have issue with the constraint.
> 
> OK, so is the patch an improvement or not?  If it fixes some cases it's
> probably worth applying even if further fixes are still needed.

Some application might fail (like mplayer with 44.1KHz) with constraint on the
period size only. Without the constraint we will have constant pops at every
period when non aligned size has been selected - if the FIFO depth is
configured to more than 1, which is not yet the case in upstream.

> No need to resend for the subject, I can fix that up.
> 
>> It seams that I need to place the same constraint step to both period size and
>> buffer size.
>> For some reason snd_pcm_hw_params_set_buffer_time_near() fails in case of
>> 44.1KHz when asking for 0.5s buffer when I placed step 32 constraint to period
>> size. Placing the same 32 steps constraint to the buffer size as well will
>> make things working (for mplayer at least)
> 
> That seems like the constraint code is failing, at a guess probably
> looking for too near a value of near if you see what I mean.  I don't
> really have the time/enthusiasm to investigate this right now, sorry.

Yep, this needs to be investigated. Me or Jyri will take a look at this for
sure since it is somehow odd.

-- 
Péter

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 664 bytes --]

On Wed, Mar 19, 2014 at 01:13:50PM +0200, Peter Ujfalusi wrote:
> On 03/18/2014 08:07 PM, Mark Brown wrote:

> > OK, so is the patch an improvement or not?  If it fixes some cases it's
> > probably worth applying even if further fixes are still needed.

> Some application might fail (like mplayer with 44.1KHz) with constraint on the
> period size only. Without the constraint we will have constant pops at every
> period when non aligned size has been selected - if the FIFO depth is
> configured to more than 1, which is not yet the case in upstream.

OK, given that the FIFO depth change isn't yet upstream it'd be a step
backwards so I'll leave this for now.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 3/3] ASoC: davinci-pcm: Add empty functions for !CONFIG_SND_DAVINCI_SOC builds

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 196 bytes --]

On Tue, Mar 18, 2014 at 03:20:37PM +0200, Peter Ujfalusi wrote:
> To save drivers using davinci-pcm and edma-pcm the need to fiddle with
> !CONFIG_SND_DAVINCI_SOC in their code.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 2/3] ASoC: davinci-mcasp: Assign the dma_data earlier in dai_probe callback

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 309 bytes --]

On Tue, Mar 18, 2014 at 03:20:36PM +0200, Peter Ujfalusi wrote:
> Set up the playback_dma_data/capture_dma_data for the dai at probe
> time since the generic dmaengine PCM stack needs to have access to
> this information early.

This looks good but seems to depend on the first patch so I'll skip it
for now.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Peter Ujfalusi]

Hi Mark,

On 03/19/2014 03:14 PM, Mark Brown wrote:
> On Wed, Mar 19, 2014 at 01:13:50PM +0200, Peter Ujfalusi wrote:
>> On 03/18/2014 08:07 PM, Mark Brown wrote:
> 
>>> OK, so is the patch an improvement or not?  If it fixes some cases it's
>>> probably worth applying even if further fixes are still needed.
> 
>> Some application might fail (like mplayer with 44.1KHz) with constraint on the
>> period size only. Without the constraint we will have constant pops at every
>> period when non aligned size has been selected - if the FIFO depth is
>> configured to more than 1, which is not yet the case in upstream.
> 
> OK, given that the FIFO depth change isn't yet upstream it'd be a step
> backwards so I'll leave this for now.

I have looked at the issue and I found some clues on why mplayer fails:
it try to set the snd_pcm_hw_params_set_buffer_time_near() first followed by
the snd_pcm_hw_params_set_periods_near(). And this fails in some cases when we
have constraint step on the period only.
Other applications set the period param first followed by the buffer (aplay
and various alsa test tools as well).
PA however have a fallback mechanism:
1. buffer first followed by period
2. period first followed by buffer
3. buffer only
4. period only
5. no period, no buffer params

When I have step constraint on period PA fails with the first, second (because
there's a bug in the PA code here) and third method but going to succeed with
four.

I have looked around in the kernel and other drivers do set both period and
buffer step constraint, for example:

sound/arm/pxa2xx-pcm-lib.c (_BYTES)
sound/drivers/vx/vx_pcm.c (_BYTES)
sound/pci/echoaudio/echoaudio.c (_SIZE)
sound/pci/ice1712/ice1724.c (_BYTES)
sound/pci/lola/lola_pcm.c (_SIZE)
sound/soc/kirkwood/kirkwood-dma.c (_BYTES)
sound/soc/s6000/s6000-pcm.c (_BYTES)
...

I still don't know why buffer size fails (in some cases) if we only have step
constraint on the period size, but it looks to me that others also encountered
with similar issues and the fix they have is the same what I had in the first
version of the patch.

-- 
Péter

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Lars-Peter Clausen]

On 03/20/2014 02:47 PM, Peter Ujfalusi wrote:
> Hi Mark,
>
> On 03/19/2014 03:14 PM, Mark Brown wrote:
>> On Wed, Mar 19, 2014 at 01:13:50PM +0200, Peter Ujfalusi wrote:
>>> On 03/18/2014 08:07 PM, Mark Brown wrote:
>>
>>>> OK, so is the patch an improvement or not?  If it fixes some cases it's
>>>> probably worth applying even if further fixes are still needed.
>>
>>> Some application might fail (like mplayer with 44.1KHz) with constraint on the
>>> period size only. Without the constraint we will have constant pops at every
>>> period when non aligned size has been selected - if the FIFO depth is
>>> configured to more than 1, which is not yet the case in upstream.
>>
>> OK, given that the FIFO depth change isn't yet upstream it'd be a step
>> backwards so I'll leave this for now.
>
> I have looked at the issue and I found some clues on why mplayer fails:
> it try to set the snd_pcm_hw_params_set_buffer_time_near() first followed by
> the snd_pcm_hw_params_set_periods_near(). And this fails in some cases when we
> have constraint step on the period only.
> Other applications set the period param first followed by the buffer (aplay
> and various alsa test tools as well).
> PA however have a fallback mechanism:
> 1. buffer first followed by period
> 2. period first followed by buffer
> 3. buffer only
> 4. period only
> 5. no period, no buffer params
>
> When I have step constraint on period PA fails with the first, second (because
> there's a bug in the PA code here) and third method but going to succeed with
> four.
>
> I have looked around in the kernel and other drivers do set both period and
> buffer step constraint, for example:
>
> sound/arm/pxa2xx-pcm-lib.c (_BYTES)
> sound/drivers/vx/vx_pcm.c (_BYTES)
> sound/pci/echoaudio/echoaudio.c (_SIZE)
> sound/pci/ice1712/ice1724.c (_BYTES)
> sound/pci/lola/lola_pcm.c (_SIZE)
> sound/soc/kirkwood/kirkwood-dma.c (_BYTES)
> sound/soc/s6000/s6000-pcm.c (_BYTES)
> ...
>
> I still don't know why buffer size fails (in some cases) if we only have step
> constraint on the period size, but it looks to me that others also encountered
> with similar issues and the fix they have is the same what I had in the first
> version of the patch.

That sounds like a bug in either the kernel or alsa-lib. We do have a rule 
in place that specifies that the buffer size needs to be a integer multiple 
of the period size and we have a rule in place that the period size needs to 
be a multiple of a constant C. Hence ALSA should be able to deduce that the 
buffer size needs to be at least a multiple of min_periods * C. We probably 
should fix this for good and not workaround it in individual drivers. Do you 
think you can put together a small standalone test application that shows 
the issue?

- Lars

Re: [PATCH v3 1/3] ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage

[Peter Ujfalusi]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

On 03/20/2014 04:15 PM, Lars-Peter Clausen wrote:
> That sounds like a bug in either the kernel or alsa-lib. We do have a rule in
> place that specifies that the buffer size needs to be a integer multiple of
> the period size and we have a rule in place that the period size needs to be a
> multiple of a constant C. Hence ALSA should be able to deduce that the buffer
> size needs to be at least a multiple of min_periods * C. We probably should
> fix this for good and not workaround it in individual drivers. Do you think
> you can put together a small standalone test application that shows the issue?

Now that I have 'wasted' quite some time with this I ended up writing the tool
to test the issue.

It is a simple tool which:
opens the hw:0,0 (you can pass another PCM to open).
Goes and tests 44.1, 88.2, 48 and 96 KHz from 0.1s to 1s buffer time with
0.005s steps.
It prints the running test and tells if the combination failed or not. If it
is OK, it is going to print the resulting buffer time.

Since the output is long for email they are in pastebin.

On am335x-evmsk only period_step = 32 constraint is placed by the McASP driver:
http://pastebin.com/C81uQkJd

When both period_step and buffer_step is set to 32:
http://pastebin.com/D8hr3bQ1

As a note: if I run this tool on my desktop/laptop it does fail in some
combination there as well with hd_intel. What is even more interesting is that
I have less failure cases with hda_intel on 64bit machines then on my old
macbook1,1 which is 32bit Linux. Basically the mcabook,1,1 behaves similarly
like my am335x (when I change the hda_intel driver to place only period_step =
32 constraint)

-- 
Péter

[-- Attachment #2: constraint.c --]
[-- Type: text/x-csrc, Size: 2285 bytes --]

#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif

#include <stdio.h>
#include <alsa/asoundlib.h>

#define BUFFER_TIME_MIN		(100000)
#define BUFFER_TIME_MAX		(1000000)
#define BUFFER_TIME_STEP	(5000)

int try_alsa_setup(snd_pcm_t *pcm, unsigned int rate, unsigned int *buffer_time,
		   unsigned int *periods)
{
	int r;
	snd_pcm_hw_params_t *hwparams;

	snd_pcm_hw_params_alloca (&hwparams);

	r = snd_pcm_hw_params_any(pcm, hwparams);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_rate_resample(pcm, hwparams, 0);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_access(pcm, hwparams,
	                                  SND_PCM_ACCESS_RW_INTERLEAVED);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_format(pcm, hwparams,SND_PCM_FORMAT_S16_LE);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_rate_near(pcm, hwparams, &rate, NULL);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_channels(pcm, hwparams, 2);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_buffer_time_near(pcm, hwparams, buffer_time,
	                NULL);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_set_periods_near(pcm, hwparams, periods, NULL);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params(pcm, hwparams);
	if (r < 0)
		return r;

	r = snd_pcm_hw_params_current(pcm, hwparams);
	if (r < 0)
		return r;

	return 0;
}

int main(int argc, char *argv[])
{
	const char *dev;
	snd_pcm_t *pcm;
	unsigned int rates[] = {44100, 88200, 48000, 96000, 0 };
	unsigned int periods = 10;
	unsigned int i;
	int r;

	dev = argc > 1 ? argv[1] : "hw:0,0";

	printf("Opening %s\n\n", dev);
	for (i = 0; rates[i]; i++) {
		unsigned int time = BUFFER_TIME_MIN;
		do {
			unsigned int buffer_time = time;
			r = snd_pcm_open(&pcm, dev, SND_PCM_STREAM_PLAYBACK, 0);
			if (r < 0) {
				printf("Failed to open %s\n", dev);
				return -1;
			}

			printf("Try of %dHz, %u us buffer: ", rates[i],
			       buffer_time);

			r = try_alsa_setup(pcm, rates[i], &buffer_time,
					   &periods);

			if (r < 0)
				printf("FAILED\n");
			else
				printf("OK (got %u us)\n", buffer_time);

			r = snd_pcm_close(pcm);
			if (r < 0) {
				printf("snd_pcm_close() failed %d\n", r);
				return r;
			}
			time += BUFFER_TIME_STEP;
		} while (time <= BUFFER_TIME_MAX);
	}

	printf("Done with %s\n\n", dev);
	return 0;
}

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]