[alsa-devel] [PATCH] Fix buffer position for ATI SB4x0
Takashi Iwai
tiwai at suse.de
Mon Jun 9 15:17:41 CEST 2008
At Sat, 07 Jun 2008 10:27:54 +0200,
I wrote:
>
> At Fri, 6 Jun 2008 15:31:45 -0300,
> Mauro Carvalho Chehab wrote:
> >
> > On Fri, 06 Jun 2008 18:49:28 +0200
> > Takashi Iwai <tiwai at suse.de> wrote:
> >
> > > At Fri, 6 Jun 2008 11:52:32 -0300,
> > > Mauro Carvalho Chehab wrote:
> > > >
> > > > On Thu, 29 May 2008 16:20:21 +0200
> > > > Takashi Iwai <tiwai at suse.de> wrote:
> > > >
> > > > > At Thu, 29 May 2008 11:10:22 -0300,
> > > > > Mauro Carvalho Chehab wrote:
> > > > > >
> > > > > > ATI SB4x0 doesn't need any fix at position.
> > > > >
> > > > > It's not about the position fixing but whether to use the
> > > > > position-buffer. The devices on the blacklist are the ones that have
> > > > > no position buffer. So, it would fall into LPIB mode, and this list
> > > > > avoids it from the beginning.
> > > > >
> > > > > > This patch solves the issue of receiving several clicks during capture on those
> > > > > > devices.
> > > > > >
> > > > > > Tested with a Gateway Notebook MX-6453.
> > > > >
> > > > > The click noise is often a different problem. Did you already try
> > > > > the patch below?
> > > >
> > > > The click seems associated to some residual samples inside the buffer. Here it
> > > > is a sample of the king of click noise I'm hearing here (captured from CD input entry):
> > > > http://linuxtv.org/~mchehab/snd.ogg
> > >
> > > (I didn't check the ogg file yet, and just a wild guess)
> > >
> > > Is it with dsnoop plugin? With "default" PCM, ALSA uses dsnoop for
> > > capture to allow multiplexing for HD-audio. Does it happen with "hw"
> > > or "plughw" PCM?
> >
> > Results with the cdplay:
> >
> > With "default" PCM (both with and without mmap):
> > several clicks per second, very high clicks;
> > (like a very risky analog disk)
>
> Hm, it's obviously a problem. I couldn't reproduce it on my machine
> with HD-audio, so it might be controller/codec-specific, though.
>
> > With "hw" or "plughw" (no mmap):
> > less clicks (something like two clicks or three per second), with less volume.
> > both hw and plughw produces the same effect.
>
> If it works with hw, plughw should have no effect (i.e. no conversion
> is done). Thus it's logical that both hw and plughw have the same
> result.
>
> > With "hw" or "plughw" and mmap (-M):
> > high quality. no noticeable clicks.
> >
> > So, it seems that there are two different issues: one with dsnoop and another
> > with non-mmapped captures.
>
> Interesting. The fact that even "hw" without mmap causes occasional
> click noises implies that there is certainly a problem with the DMA
> position calculation. The dsnoop has more problems likely because it
> uses smaller period size and more periods than hw, I guess.
> Still not sure why the mmap mode works.
>
> Anyway, it means that the capture position is wrongly reported, maybe
> just in a reverse way of the playback position -- a few samples ahead
> than the real position. Sigh, another workaround is needed.
The patch below adds another workaround for the DMA buffer position.
Could you give it a try? It adds as default one sample delay for
issuing the interrupt so that the DMA pointer gets the right
position. The delay can be changed (even dynamically) via bdl_pos_adj
module option.
thanks,
Takashi
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index dc68709..401a4fd 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -58,6 +58,7 @@ static int position_fix[SNDRV_CARDS];
static int probe_mask[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1};
static int single_cmd;
static int enable_msi;
+static int bdl_pos_adj = 1;
module_param_array(index, int, NULL, 0444);
MODULE_PARM_DESC(index, "Index value for Intel HD audio interface.");
@@ -77,6 +78,8 @@ MODULE_PARM_DESC(single_cmd, "Use single command to communicate with codecs "
"(for debugging only).");
module_param(enable_msi, int, 0444);
MODULE_PARM_DESC(enable_msi, "Enable Message Signaled Interrupt (MSI)");
+module_param(bdl_pos_adj, int, 0644);
+MODULE_PARM_DESC(bdl_pos_adj, "BDL position adjustment offset");
#ifdef CONFIG_SND_HDA_POWER_SAVE
/* power_save option is defined in hda_codec.c */
@@ -310,6 +313,7 @@ struct azx_dev {
unsigned int opened :1;
unsigned int running :1;
unsigned int irq_pending: 1;
+ unsigned int irq_ignore: 1;
};
/* CORB/RIRB */
@@ -943,6 +947,11 @@ static irqreturn_t azx_interrupt(int irq, void *dev_id)
azx_sd_writeb(azx_dev, SD_STS, SD_INT_MASK);
if (!azx_dev->substream || !azx_dev->running)
continue;
+ /* ignore the first dummy IRQ (due to pos_adj) */
+ if (azx_dev->irq_ignore) {
+ azx_dev->irq_ignore = 0;
+ continue;
+ }
/* check whether this IRQ is really acceptable */
if (azx_position_ok(chip, azx_dev)) {
azx_dev->irq_pending = 0;
@@ -977,14 +986,53 @@ static irqreturn_t azx_interrupt(int irq, void *dev_id)
/*
+ * set up a BDL entry
+ */
+static int setup_bdle(struct snd_pcm_substream *substream,
+ struct azx_dev *azx_dev, u32 **bdlp,
+ int ofs, int size, int with_ioc)
+{
+ struct snd_sg_buf *sgbuf = snd_pcm_substream_sgbuf(substream);
+ u32 *bdl = *bdlp;
+
+ while (size > 0) {
+ dma_addr_t addr;
+ int chunk;
+
+ if (azx_dev->frags >= AZX_MAX_BDL_ENTRIES)
+ return -EINVAL;
+
+ addr = snd_pcm_sgbuf_get_addr(sgbuf, ofs);
+ /* program the address field of the BDL entry */
+ bdl[0] = cpu_to_le32((u32)addr);
+ bdl[1] = cpu_to_le32(upper_32bit(addr));
+ /* program the size field of the BDL entry */
+ chunk = PAGE_SIZE - (ofs % PAGE_SIZE);
+ if (size < chunk)
+ chunk = size;
+ bdl[2] = cpu_to_le32(chunk);
+ /* program the IOC to enable interrupt
+ * only when the whole fragment is processed
+ */
+ size -= chunk;
+ bdl[3] = (size || !with_ioc) ? 0 : cpu_to_le32(0x01);
+ bdl += 4;
+ azx_dev->frags++;
+ ofs += chunk;
+ }
+ *bdlp = bdl;
+ return ofs;
+}
+
+/*
* set up BDL entries
*/
static int azx_setup_periods(struct snd_pcm_substream *substream,
struct azx_dev *azx_dev)
{
- struct snd_sg_buf *sgbuf = snd_pcm_substream_sgbuf(substream);
u32 *bdl;
int i, ofs, periods, period_bytes;
+ int pos_adj = 0;
/* reset BDL address */
azx_sd_writel(azx_dev, SD_BDLPL, 0);
@@ -998,39 +1046,44 @@ static int azx_setup_periods(struct snd_pcm_substream *substream,
bdl = (u32 *)azx_dev->bdl.area;
ofs = 0;
azx_dev->frags = 0;
- for (i = 0; i < periods; i++) {
- int size, rest;
- if (i >= AZX_MAX_BDL_ENTRIES) {
- snd_printk(KERN_ERR "Too many BDL entries: "
- "buffer=%d, period=%d\n",
- azx_dev->bufsize, period_bytes);
- /* reset */
- azx_sd_writel(azx_dev, SD_BDLPL, 0);
- azx_sd_writel(azx_dev, SD_BDLPU, 0);
- return -EINVAL;
+ azx_dev->irq_ignore = 0;
+ if (bdl_pos_adj > 0) {
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ pos_adj = (bdl_pos_adj * runtime->rate + 47999) / 48000;
+ if (!pos_adj)
+ pos_adj = 1;
+ pos_adj = frames_to_bytes(runtime, pos_adj);
+ if (pos_adj >= period_bytes) {
+ snd_printk(KERN_WARNING "Too big adjustment %d\n",
+ bdl_pos_adj);
+ pos_adj = 0;
+ } else {
+ ofs = setup_bdle(substream, azx_dev,
+ &bdl, ofs, pos_adj, 1);
+ if (ofs < 0)
+ goto error;
+ azx_dev->irq_ignore = 1;
}
- rest = period_bytes;
- do {
- dma_addr_t addr = snd_pcm_sgbuf_get_addr(sgbuf, ofs);
- /* program the address field of the BDL entry */
- bdl[0] = cpu_to_le32((u32)addr);
- bdl[1] = cpu_to_le32(upper_32bit(addr));
- /* program the size field of the BDL entry */
- size = PAGE_SIZE - (ofs % PAGE_SIZE);
- if (rest < size)
- size = rest;
- bdl[2] = cpu_to_le32(size);
- /* program the IOC to enable interrupt
- * only when the whole fragment is processed
- */
- rest -= size;
- bdl[3] = rest ? 0 : cpu_to_le32(0x01);
- bdl += 4;
- azx_dev->frags++;
- ofs += size;
- } while (rest > 0);
+ }
+ for (i = 0; i < periods; i++) {
+ if (i == periods - 1 && pos_adj)
+ ofs = setup_bdle(substream, azx_dev, &bdl, ofs,
+ period_bytes - pos_adj, 0);
+ else
+ ofs = setup_bdle(substream, azx_dev, &bdl, ofs,
+ period_bytes, 1);
+ if (ofs < 0)
+ goto error;
}
return 0;
+
+ error:
+ snd_printk(KERN_ERR "Too many BDL entries: buffer=%d, period=%d\n",
+ azx_dev->bufsize, period_bytes);
+ /* reset */
+ azx_sd_writel(azx_dev, SD_BDLPL, 0);
+ azx_sd_writel(azx_dev, SD_BDLPU, 0);
+ return -EINVAL;
}
/*
More information about the Alsa-devel
mailing list