[alsa-devel] [PATCH] Fix buffer position for ATI SB4x0
ATI SB4x0 doesn't need any fix at position.
This patch solves the issue of receiving several clicks during capture on those devices.
Tested with a Gateway Notebook MX-6453.
Signed-off-by: Mauro Carvalho Chehab mchehab@infradead.org
diff -r 33d7308ade64 pci/hda/hda_intel.c --- a/pci/hda/hda_intel.c Fri May 23 17:36:59 2008 -0300 +++ b/pci/hda/hda_intel.c Thu May 29 11:04:56 2008 -0300 @@ -1860,6 +1860,7 @@ SND_PCI_QUIRK(0x1028, 0x01cc, "Dell D820", POS_FIX_NONE), SND_PCI_QUIRK(0x1028, 0x01de, "Dell Precision 390", POS_FIX_NONE), SND_PCI_QUIRK(0x1043, 0x813d, "ASUS P5AD2", POS_FIX_NONE), + SND_PCI_QUIRK(0x1002, 0x437b, "ATI SB4x0 HDA", POS_FIX_NONE), {} };
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?
thanks,
Takashi
===
commit 6bf70d9d25f35b41d7d04274aa22baebca5496ea Author: Takashi Iwai tiwai@suse.de Date: Fri May 16 12:34:47 2008 +0200
[ALSA] hda - Fix DMA position inaccuracy
Many HD-audio controllers seem inaccurate about the IRQ timing of PCM period updates. This has caused problems on audio quality; e.g. JACK doesn't work with two periods.
This patch fixes the problem by checking the current DMA position at IRQ handler and delays the period-update via a workq if it's inaccurate.
Signed-off-by: Takashi Iwai tiwai@suse.de
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index b3a618e..6ba7ac0 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -285,6 +285,7 @@ struct azx_dev { u32 *posbuf; /* position buffer pointer */
unsigned int bufsize; /* size of the play buffer in bytes */ + unsigned int period_bytes; /* size of the period in bytes */ unsigned int frags; /* number for period in the play buffer */ unsigned int fifo_size; /* FIFO size */
@@ -301,11 +302,10 @@ struct azx_dev { */ unsigned char stream_tag; /* assigned stream */ unsigned char index; /* stream index */ - /* for sanity check of position buffer */ - unsigned int period_intr;
unsigned int opened :1; unsigned int running :1; + unsigned int irq_pending: 1; };
/* CORB/RIRB */ @@ -369,6 +369,9 @@ struct azx {
/* for debugging */ unsigned int last_cmd; /* last issued command (to sync) */ + + /* for pending irqs */ + struct work_struct irq_pending_work; };
/* driver types */ @@ -908,6 +911,8 @@ static void azx_init_pci(struct azx *chip) }
+static int azx_position_ok(struct azx *chip, struct azx_dev *azx_dev); + /* * interrupt handler */ @@ -930,11 +935,18 @@ static irqreturn_t azx_interrupt(int irq, void *dev_id) azx_dev = &chip->azx_dev[i]; if (status & azx_dev->sd_int_sta_mask) { azx_sd_writeb(azx_dev, SD_STS, SD_INT_MASK); - if (azx_dev->substream && azx_dev->running) { - azx_dev->period_intr++; + if (!azx_dev->substream || !azx_dev->running) + continue; + /* check whether this IRQ is really acceptable */ + if (azx_position_ok(chip, azx_dev)) { + azx_dev->irq_pending = 0; spin_unlock(&chip->reg_lock); snd_pcm_period_elapsed(azx_dev->substream); spin_lock(&chip->reg_lock); + } else { + /* bogus IRQ, process it later */ + azx_dev->irq_pending = 1; + schedule_work(&chip->irq_pending_work); } } } @@ -973,6 +985,7 @@ static int azx_setup_periods(struct snd_pcm_substream *substream, azx_sd_writel(azx_dev, SD_BDLPU, 0);
period_bytes = snd_pcm_lib_period_bytes(substream); + azx_dev->period_bytes = period_bytes; periods = azx_dev->bufsize / period_bytes;
/* program the initial BDL entries */ @@ -1421,27 +1434,16 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return 0; }
-static snd_pcm_uframes_t azx_pcm_pointer(struct snd_pcm_substream *substream) +static unsigned int azx_get_position(struct azx *chip, + struct azx_dev *azx_dev) { - struct azx_pcm *apcm = snd_pcm_substream_chip(substream); - struct azx *chip = apcm->chip; - struct azx_dev *azx_dev = get_azx_dev(substream); unsigned int pos;
if (chip->position_fix == POS_FIX_POSBUF || chip->position_fix == POS_FIX_AUTO) { /* use the position buffer */ pos = le32_to_cpu(*azx_dev->posbuf); - if (chip->position_fix == POS_FIX_AUTO && - azx_dev->period_intr == 1 && !pos) { - printk(KERN_WARNING - "hda-intel: Invalid position buffer, " - "using LPIB read method instead.\n"); - chip->position_fix = POS_FIX_NONE; - goto read_lpib; - } } else { - read_lpib: /* read LPIB */ pos = azx_sd_readl(azx_dev, SD_LPIB); if (chip->position_fix == POS_FIX_FIFO) @@ -1449,7 +1451,90 @@ static snd_pcm_uframes_t azx_pcm_pointer(struct snd_pcm_substream *substream) } if (pos >= azx_dev->bufsize) pos = 0; - return bytes_to_frames(substream->runtime, pos); + return pos; +} + +static snd_pcm_uframes_t azx_pcm_pointer(struct snd_pcm_substream *substream) +{ + struct azx_pcm *apcm = snd_pcm_substream_chip(substream); + struct azx *chip = apcm->chip; + struct azx_dev *azx_dev = get_azx_dev(substream); + return bytes_to_frames(substream->runtime, + azx_get_position(chip, azx_dev)); +} + +/* + * Check whether the current DMA position is acceptable for updating + * periods. Returns non-zero if it's OK. + * + * Many HD-audio controllers appear pretty inaccurate about + * the update-IRQ timing. The IRQ is issued before actually the + * data is processed. So, we need to process it afterwords in a + * workqueue. + */ +static int azx_position_ok(struct azx *chip, struct azx_dev *azx_dev) +{ + unsigned int pos; + + pos = azx_get_position(chip, azx_dev); + if (chip->position_fix == POS_FIX_AUTO) { + if (!pos) { + printk(KERN_WARNING + "hda-intel: Invalid position buffer, " + "using LPIB read method instead.\n"); + chip->position_fix = POS_FIX_NONE; + pos = azx_get_position(chip, azx_dev); + } else + chip->position_fix = POS_FIX_POSBUF; + } + + if (pos % azx_dev->period_bytes > azx_dev->period_bytes / 2) + return 0; /* NG - it's below the period boundary */ + return 1; /* OK, it's fine */ +} + +/* + * The work for pending PCM period updates. + */ +static void azx_irq_pending_work(struct work_struct *work) +{ + struct azx *chip = container_of(work, struct azx, irq_pending_work); + int i, pending; + + for (;;) { + pending = 0; + spin_lock_irq(&chip->reg_lock); + for (i = 0; i < chip->num_streams; i++) { + struct azx_dev *azx_dev = &chip->azx_dev[i]; + if (!azx_dev->irq_pending || + !azx_dev->substream || + !azx_dev->running) + continue; + if (azx_position_ok(chip, azx_dev)) { + azx_dev->irq_pending = 0; + spin_unlock(&chip->reg_lock); + snd_pcm_period_elapsed(azx_dev->substream); + spin_lock(&chip->reg_lock); + } else + pending++; + } + spin_unlock_irq(&chip->reg_lock); + if (!pending) + return; + cond_resched(); + } +} + +/* clear irq_pending flags and assure no on-going workq */ +static void azx_clear_irq_pending(struct azx *chip) +{ + int i; + + spin_lock_irq(&chip->reg_lock); + for (i = 0; i < chip->num_streams; i++) + chip->azx_dev[i].irq_pending = 0; + spin_unlock_irq(&chip->reg_lock); + flush_scheduled_work(); }
static struct snd_pcm_ops azx_pcm_ops = { @@ -1676,6 +1761,7 @@ static int azx_suspend(struct pci_dev *pci, pm_message_t state) int i;
snd_power_change_state(card, SNDRV_CTL_POWER_D3hot); + azx_clear_irq_pending(chip); for (i = 0; i < AZX_MAX_PCMS; i++) snd_pcm_suspend_all(chip->pcm[i]); if (chip->initialized) @@ -1732,6 +1818,7 @@ static int azx_free(struct azx *chip) int i;
if (chip->initialized) { + azx_clear_irq_pending(chip); for (i = 0; i < chip->num_streams; i++) azx_stream_stop(chip, &chip->azx_dev[i]); azx_stop_chip(chip); @@ -1857,6 +1944,7 @@ static int __devinit azx_create(struct snd_card *card, struct pci_dev *pci, chip->irq = -1; chip->driver_type = driver_type; chip->msi = enable_msi; + INIT_WORK(&chip->irq_pending_work, azx_irq_pending_work);
chip->position_fix = check_position_fix(chip, position_fix[dev]); check_probe_mask(chip, dev);
On Thu, 29 May 2008 16:20:21 +0200 Takashi Iwai tiwai@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.
Sometimes, I get this message:
[ 857.014011] hda-intel: Invalid position buffer, using LPIB read method instead.
It is weird, since this line doesn't appear always.
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?
This patch is already here on my testing environment.
There are a few reports I found at the net about Gateway devices with SB450 ATI chipset. Some suggestions for working with MIC pointed the need of using position_fix=0 to avoid the clicks on those devices.
One weird thing is that I've did this procedure, when I generated the patch: - tested without the patch - clicks at mic; - applied the patch I sent - no clicks; - removed the patch after your email - no clicks; - reapplied/removed several times - no clicks.
I'll try today to test with and without the position fix after cold boot/soft boot/resume. Maybe there are some registers that aren't properly initialized on some cases.
Btw, after a suspend/resume, some widgets aren't properly restored (at mercurial tree version). For example, if I suspend with master Mute on, after resume, mute is off (but the kmix still shows it as muted). I'm not sure if the clicks happen after resume. I'll double check it.
Cheers, Mauro
On Thu, 29 May 2008 16:20:21 +0200 Takashi Iwai tiwai@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've started/stopped the CD some times (with cdplay). When volume goes to zero, after an "echoed click", it disappears.
I've did some trials with position_fix changes, but nothing solved. So, my previous patch were wrong. The click is somewhere else (still, I suspect that it is linked to the buffer handling).
Cheers, Mauro
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@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?
Takashi
On Fri, 06 Jun 2008 18:49:28 +0200 Takashi Iwai tiwai@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@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)
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.
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.
Cheers, Mauro
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@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@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.
thanks,
Takashi
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@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@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; }
/*
On Mon, 09 Jun 2008 15:17:41 +0200 Takashi Iwai tiwai@suse.de wrote:
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@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@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.
I tried it here, but I didn't noticed any results. I've ranged the value from 1 to 317 (I had to stop/start record each time). I tried with the default PCM input, since this way is easier to notice the click.
Cheers, Mauro
participants (2)
-
Mauro Carvalho Chehab -
Takashi Iwai