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; }
/*