All of lore.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.