public inbox for alsa-devel@alsa-project.org
 help / color / mirror / Atom feed
From: Shine Liu <shinel@foxmail.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: ben@simtec.co.uk, alsa-devel@alsa-project.org
Subject: [PATCH] ASoC: S3C24XX : Align the peroid size to the buffer size
Date: Thu, 20 Aug 2009 23:02:23 +0800	[thread overview]
Message-ID: <1250780543.1404.57.camel@sl> (raw)
In-Reply-To: <s5h7hx0c8l2.wl%tiwai@suse.de>

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@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);

  parent reply	other threads:[~2009-08-20 15:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-19 10:22 [PATCH] pcm_lib.c: Fixed inaccurate calculation of hw_ptr_interrupt in snd_pcm_update_hw_ptr_interrupt function Shine Liu
2009-08-19 10:34 ` Takashi Iwai
2009-08-19 10:59   ` Shine Liu
2009-08-19 12:05     ` Takashi Iwai
2009-08-19 12:48       ` Shine Liu
2009-08-19 12:53         ` Takashi Iwai
2009-08-19 13:01           ` Shine Liu
2009-08-20  7:20           ` Shine Liu
2009-08-20  8:42             ` Clemens Ladisch
2009-08-20 15:02       ` Shine Liu [this message]
2009-08-20 18:44         ` [PATCH] ASoC: S3C24XX : Align the peroid size to the buffer size Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1250780543.1404.57.camel@sl \
    --to=shinel@foxmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=ben@simtec.co.uk \
    --cc=broonie@opensource.wolfsonmicro.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox