[alsa-devel] [PATCH] ASoC: S3C24XX : Align the peroid size to the buffer size

[Shine Liu]

On Wed, 2009-08-19 at 14:05 +0200, Takashi Iwai wrote:
> At Wed, 19 Aug 2009 18:59:31 +0800,
> Shine Liu wrote:
> > 
> > On Wed, 2009-08-19 at 12:34 +0200, Takashi Iwai wrote:
> > > At Wed, 19 Aug 2009 18:22:19 +0800,
> > > Shine Liu wrote:
> > > > 
> > > > There's no restriction on that the runtime->period_size should be the
> > > > multiplicator of the runtime->buffer_size in current kernel code.
> > > > 
> > > > So "hw_ptr_interrupt = runtime->hw_ptr_interrupt +
> > > > runtime->period_size;" is not always the trueth.
> > > 
> > > The hw_ptr_interrupt is the expected position where the current irq is
> > > issued.  So, the current code should be correct -- it's simply
> > > increased in the size of period_size.
> > 
> > Yes, the period_size and buffer_size I used was 0x800 and 0x3e80.
> > 
> > The log from the kernel is :
> > 
> > period_update: pcmC0D0p:0: pos=0x809/0x800/0x3e80, hwptr=0x1c, hw_base=0x0, hw_intr=0x0
> > period_update: pcmC0D0p:0: pos=0x1008/0x800/0x3e80, hwptr=0x809, hw_base=0x0, hw_intr=0x800
> > period_update: pcmC0D0p:0: pos=0x1808/0x800/0x3e80, hwptr=0x10e3, hw_base=0x0, hw_intr=0x1000
> > period_update: pcmC0D0p:0: pos=0x2006/0x800/0x3e80, hwptr=0x1808, hw_base=0x0, hw_intr=0x1800
> > period_update: pcmC0D0p:0: pos=0x2808/0x800/0x3e80, hwptr=0x203f, hw_base=0x0, hw_intr=0x2000
> > period_update: pcmC0D0p:0: pos=0x3008/0x800/0x3e80, hwptr=0x2808, hw_base=0x0, hw_intr=0x2800
> > period_update: pcmC0D0p:0: pos=0x3808/0x800/0x3e80, hwptr=0x30e9, hw_base=0x0, hw_intr=0x3000
> > period_update: pcmC0D0p:0: pos=0x8/0x800/0x3e80, hwptr=0x3808, hw_base=0x0, hw_intr=0x3800
> > PCM: Unexpected hw_pointer value (stream=0, pos=8, intr_ptr=16384)
> > 
> > It's true that irq was issued when hw_ptr_interrupt is
> > 0x800,0x1000,0x1800,...,0x3800,0x3e80.
> > 
> > But the irq last issued should be at 0x3e80, but not 0x3800 + 0x800 =
> > 0x4000. The DMA engine loaded 0x680 frame not 0x800 frame at the last
> > time.
> 
> Then it's a driver bug.  If unaligned period size is allowed, it means
> that the irq is really generated in that period, not at the buffer
> boundary.  Otherwise, it must have a proper hw-constraint to align the
> period size to the buffer size.
> 
> What hardware is it?
> 
> 
> thanks,
> 
> Takashi
> 

This patch will fix the bug metioned in the above mail. Force the peroid
size to be aligned with the buffer size.

Based and tested on linux-2.6.31-rc6.

Signed-off-by: Shine Liu <shinel at foxmail.com>
---------------------------------------------------------------------------

--- a/sound/soc/s3c24xx/s3c24xx-pcm.c	2009-08-14 06:43:34.000000000 +0800
+++ b/sound/soc/s3c24xx/s3c24xx-pcm.c	2009-08-20 22:33:58.000000000 +0800
@@ -318,6 +318,7 @@
 
 	pr_debug("Entered %s\n", __func__);
 
+	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 	snd_soc_set_runtime_hwparams(substream, &s3c24xx_pcm_hardware);
 
 	prtd = kzalloc(sizeof(struct s3c24xx_runtime_data), GFP_KERNEL);