[alsa-devel] VT1708 clicks & pops

Takashi Iwai tiwai at suse.de
Tue Sep 27 14:57:16 CEST 2011


At Wed, 21 Sep 2011 13:55:53 -0400,
Forest Bond wrote:
> 
> Hi Takashi,
> 
> On Wed, Sep 21, 2011 at 04:43:15PM +0200, Takashi Iwai wrote:
> > At Wed, 21 Sep 2011 10:35:10 -0400, Forest Bond wrote:
> > > On Wed, Sep 21, 2011 at 10:39:44AM +0200, Takashi Iwai wrote:
> > > > At Tue, 20 Sep 2011 15:05:34 -0400, Forest Bond wrote:
> > > > > On Tue, Sep 20, 2011 at 09:22:23AM +0200, Takashi Iwai wrote:
> > > > > > At Mon, 19 Sep 2011 19:14:13 -0400, Forest Bond wrote:
> > > > > > > On Fri, Sep 16, 2011 at 04:36:01PM -0400, Forest Bond wrote:
> > > > > > > > I have two boards with a VT1708:
> > > > > > > > 
> > > > > > > > * VIA EPIA EX15000G
> > > > > > > > * VIA VB8002
> > > > > > > > 
> > > > > > > > I'm using VLC to play audio via PulseAudio to the analog outputs.  The VB8002
> > > > > > > > sounds great, but the EX15000G has random pops and clicks in the output.  Both
> > > > > > > > machines are running exactly the same configuration (they are using the same
> > > > > > > > pre-built OS image).
> > > > > The crackling with PCM volume set to 0 persists with tsched=0.
> 
> [...]
> 
> > > > Hrm, which PA version are you using?  I remember vaguely a buggy PA
> > > > SIMD operations in some old PA versions.
> > > 
> > > 1:0.9.22+stable-queue-24-g67d18-0ubuntu3 from Ubuntu Lucid.
> > > 
> > > > Also, when you mute the mixer (nor the PA's mixer), e.g. via
> > > > "alsamixer -c0", the noise goes away, right?
> > > 
> > > I do not use the PA mixer.  The noise is present with ALSA's PCM control set to
> > > zero (-51dB).  If I move the PCM control up one tick (to -50.8dB) the noise goes
> > > away.
> > 
> > OK, then it's an issue in the sound driver.
> > Could you give alsa-info.sh outputs both mute-with-noise and without
> > noise for comparison?
> 
> I guess we can ignore this one for now.  I do not have this problem using a
> recent alsa-driver snapshot.  The PCM control does not go below -40.25dB with
> this version.  It's possible the new driver simply doesn't allow the control to
> be set to a problematic value.
> 
> I will note, however, that the master control with the snapshot has a minimum
> value of 0dB and a maximum of 6.75dB,

That's odd.

> and that the audio is barely audible at
> 0dB.  I have to crank the master control all the way up to get the same volume
> as the old driver set to 0dB.  Maybe I can come up with a fix for this.

For debugging, I'd need anyway the alsa-info.sh output from the latest
alsa-driver snapshot.

Also, I'm working now on non-snooping mode support, and this might
help for fixing some problems like yours.  Could you try the patch
below (with the snapshot version)?

FYI, the snapshot tarballs are found in
	ftp://ftp.suse.com/pub/people/tiwai/snapshot/
while kernel.org is down.


thanks,

Takashi

---
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index dc36f75..3e7fda6 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1037,6 +1037,8 @@ static inline void snd_pcm_mmap_data_close(struct vm_area_struct *area)
 	atomic_dec(&substream->mmap_count);
 }
 
+int snd_pcm_lib_default_mmap(struct snd_pcm_substream *substream,
+			     struct vm_area_struct *area);
 /* mmap for io-memory area */
 #if defined(CONFIG_X86) || defined(CONFIG_PPC) || defined(CONFIG_ALPHA)
 #define SNDRV_PCM_INFO_MMAP_IOMEM	SNDRV_PCM_INFO_MMAP
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 1c6be91..b4bf4a4 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3156,8 +3156,8 @@ static const struct vm_operations_struct snd_pcm_vm_ops_data_fault = {
 /*
  * mmap the DMA buffer on RAM
  */
-static int snd_pcm_default_mmap(struct snd_pcm_substream *substream,
-				struct vm_area_struct *area)
+int snd_pcm_lib_default_mmap(struct snd_pcm_substream *substream,
+			     struct vm_area_struct *area)
 {
 	area->vm_flags |= VM_RESERVED;
 #ifdef ARCH_HAS_DMA_MMAP_COHERENT
@@ -3177,6 +3177,7 @@ static int snd_pcm_default_mmap(struct snd_pcm_substream *substream,
 	area->vm_ops = &snd_pcm_vm_ops_data_fault;
 	return 0;
 }
+EXPORT_SYMBOL_GPL(snd_pcm_lib_default_mmap);
 
 /*
  * mmap the DMA buffer on I/O memory area
@@ -3242,7 +3243,7 @@ int snd_pcm_mmap_data(struct snd_pcm_substream *substream, struct file *file,
 	if (substream->ops->mmap)
 		err = substream->ops->mmap(substream, area);
 	else
-		err = snd_pcm_default_mmap(substream, area);
+		err = snd_pcm_lib_default_mmap(substream, area);
 	if (!err)
 		atomic_inc(&substream->mmap_count);
 	return err;
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 2a8bed9..bfec401 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -34,7 +34,6 @@
  * 
  */
 
-#include <asm/io.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -46,6 +45,12 @@
 #include <linux/pci.h>
 #include <linux/mutex.h>
 #include <linux/reboot.h>
+#include <linux/io.h>
+#ifdef CONFIG_X86
+/* for snoop control */
+#include <asm/pgtable.h>
+#include <asm/cacheflush.h>
+#endif
 #include <sound/core.h>
 #include <sound/initval.h>
 #include "hda_codec.h"
@@ -121,6 +126,15 @@ module_param(align_buffer_size, bool, 0644);
 MODULE_PARM_DESC(align_buffer_size,
 		"Force buffer and period sizes to be multiple of 128 bytes.");
 
+#ifdef CONFIG_X86
+static bool hda_snoop;
+module_param_named(snoop, hda_snoop, bool, 0444);
+MODULE_PARM_DESC(snoop, "Enable/disable snooping");
+#else
+#define hda_snoop	true
+#endif
+
+
 MODULE_LICENSE("GPL");
 MODULE_SUPPORTED_DEVICE("{{Intel, ICH6},"
 			 "{Intel, ICH6M},"
@@ -376,6 +390,7 @@ struct azx_dev {
 	 *  when link position is not greater than FIFO size
 	 */
 	unsigned int insufficient :1;
+	unsigned int wc_marked:1;
 };
 
 /* CORB/RIRB */
@@ -548,6 +563,45 @@ static char *driver_short_names[] __devinitdata = {
 /* for pcm support */
 #define get_azx_dev(substream) (substream->runtime->private_data)
 
+#ifdef CONFIG_X86
+static void __mark_pages_wc(struct azx *chip, void *addr, size_t size, bool on)
+{
+	if (hda_snoop)
+		return;
+	if (addr && size) {
+		int pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+		if (on)
+			set_memory_wc((unsigned long)addr, pages);
+		else
+			set_memory_wb((unsigned long)addr, pages);
+	}
+}
+
+static inline void mark_pages_wc(struct azx *chip, struct snd_dma_buffer *buf,
+				 bool on)
+{
+	__mark_pages_wc(chip, buf->area, buf->bytes, on);
+}
+static inline void mark_runtime_wc(struct azx *chip, struct azx_dev *azx_dev,
+				   struct snd_pcm_runtime *runtime, bool on)
+{
+	if (azx_dev->wc_marked != on) {
+		__mark_pages_wc(chip, runtime->dma_area, runtime->dma_bytes, on);
+		azx_dev->wc_marked = on;
+	}
+}
+#else
+/* NOP for other archs */
+static inline void mark_pages_wc(struct azx *chip, struct snd_dma_buffer *buf,
+				 bool on)
+{
+}
+static inline void mark_runtime_wc(struct azx *chip, struct azx_dev *azx_dev,
+				   struct snd_pcm_runtime *runtime, bool on)
+{
+}
+#endif
+
 static int azx_acquire_irq(struct azx *chip, int do_disconnect);
 static int azx_send_cmd(struct hda_bus *bus, unsigned int val);
 /*
@@ -569,6 +623,7 @@ static int azx_alloc_cmd_io(struct azx *chip)
 		snd_printk(KERN_ERR SFX "cannot allocate CORB/RIRB\n");
 		return err;
 	}
+	mark_pages_wc(chip, &chip->rb, true);
 	return 0;
 }
 
@@ -1104,8 +1159,8 @@ static void azx_init_pci(struct azx *chip)
 	if (chip->driver_caps & AZX_DCAPS_ATI_SNOOP) {
 		snd_printdd(SFX "Enabling ATI snoop\n");
 		update_pci_byte(chip->pci,
-				ATI_SB450_HDAUDIO_MISC_CNTR2_ADDR, 
-				0x07, ATI_SB450_HDAUDIO_ENABLE_SNOOP);
+				ATI_SB450_HDAUDIO_MISC_CNTR2_ADDR, 0x07,
+				hda_snoop ? ATI_SB450_HDAUDIO_ENABLE_SNOOP : 0);
 	}
 
 	/* For NVIDIA HDA, enable snoop */
@@ -1125,15 +1180,18 @@ static void azx_init_pci(struct azx *chip)
 	/* Enable SCH/PCH snoop if needed */
 	if (chip->driver_caps & AZX_DCAPS_SCH_SNOOP) {
 		pci_read_config_word(chip->pci, INTEL_SCH_HDA_DEVC, &snoop);
-		if (snoop & INTEL_SCH_HDA_DEVC_NOSNOOP) {
-			pci_write_config_word(chip->pci, INTEL_SCH_HDA_DEVC,
-				snoop & (~INTEL_SCH_HDA_DEVC_NOSNOOP));
+		if ((!hda_snoop && !(snoop & INTEL_SCH_HDA_DEVC_NOSNOOP)) ||
+		    (hda_snoop && (snoop & INTEL_SCH_HDA_DEVC_NOSNOOP))) {
+			snoop &= ~INTEL_SCH_HDA_DEVC_NOSNOOP;
+			if (!hda_snoop)
+				snoop |= INTEL_SCH_HDA_DEVC_NOSNOOP;
+			pci_write_config_word(chip->pci, INTEL_SCH_HDA_DEVC, snoop);
 			pci_read_config_word(chip->pci,
 				INTEL_SCH_HDA_DEVC, &snoop);
-			snd_printdd(SFX "HDA snoop disabled, enabling ... %s\n",
-				(snoop & INTEL_SCH_HDA_DEVC_NOSNOOP)
-				? "Failed" : "OK");
 		}
+		snd_printdd(SFX "HDA snoop %s\n",
+				(snoop & INTEL_SCH_HDA_DEVC_NOSNOOP)
+				? "Disabled" : "Enabled");
         }
 }
 
@@ -1340,12 +1398,16 @@ static void azx_stream_reset(struct azx *chip, struct azx_dev *azx_dev)
  */
 static int azx_setup_controller(struct azx *chip, struct azx_dev *azx_dev)
 {
+	unsigned int val;
 	/* make sure the run bit is zero for SD */
 	azx_stream_clear(chip, azx_dev);
 	/* program the stream_tag */
-	azx_sd_writel(azx_dev, SD_CTL,
-		      (azx_sd_readl(azx_dev, SD_CTL) & ~SD_CTL_STREAM_TAG_MASK)|
-		      (azx_dev->stream_tag << SD_CTL_STREAM_TAG_SHIFT));
+	val = azx_sd_readl(azx_dev, SD_CTL);
+	val = (val & ~SD_CTL_STREAM_TAG_MASK) |
+		(azx_dev->stream_tag << SD_CTL_STREAM_TAG_SHIFT);
+	if (!hda_snoop)
+		val |= SD_CTL_TRAFFIC_PRIO;
+	azx_sd_writel(azx_dev, SD_CTL, val);
 
 	/* program the length of samples in cyclic buffer */
 	azx_sd_writel(azx_dev, SD_CBL, azx_dev->bufsize);
@@ -1693,19 +1755,30 @@ static int azx_pcm_close(struct snd_pcm_substream *substream)
 static int azx_pcm_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *hw_params)
 {
+	struct azx_pcm *apcm = snd_pcm_substream_chip(substream);
+	struct azx *chip = apcm->chip;
+	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct azx_dev *azx_dev = get_azx_dev(substream);
+	int ret;
 
+	mark_runtime_wc(chip, azx_dev, runtime, false);
 	azx_dev->bufsize = 0;
 	azx_dev->period_bytes = 0;
 	azx_dev->format_val = 0;
-	return snd_pcm_lib_malloc_pages(substream,
+	ret = snd_pcm_lib_malloc_pages(substream,
 					params_buffer_bytes(hw_params));
+	if (ret < 0)
+		return ret;
+	mark_runtime_wc(chip, azx_dev, runtime, true);
+	return ret;
 }
 
 static int azx_pcm_hw_free(struct snd_pcm_substream *substream)
 {
 	struct azx_pcm *apcm = snd_pcm_substream_chip(substream);
 	struct azx_dev *azx_dev = get_azx_dev(substream);
+	struct azx *chip = apcm->chip;
+	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct hda_pcm_stream *hinfo = apcm->hinfo[substream->stream];
 
 	/* reset BDL address */
@@ -1718,6 +1791,7 @@ static int azx_pcm_hw_free(struct snd_pcm_substream *substream)
 
 	snd_hda_codec_cleanup(apcm->codec, hinfo, substream);
 
+	mark_runtime_wc(chip, azx_dev, runtime, false);
 	return snd_pcm_lib_free_pages(substream);
 }
 
@@ -2076,6 +2150,18 @@ static void azx_clear_irq_pending(struct azx *chip)
 	spin_unlock_irq(&chip->reg_lock);
 }
 
+#ifdef CONFIG_X86
+static int azx_pcm_mmap(struct snd_pcm_substream *substream,
+			struct vm_area_struct *area)
+{
+	if (!hda_snoop)
+		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
+	return snd_pcm_lib_default_mmap(substream, area);
+}
+#else
+#define azx_pcm_mmap	NULL
+#endif
+
 static struct snd_pcm_ops azx_pcm_ops = {
 	.open = azx_pcm_open,
 	.close = azx_pcm_close,
@@ -2085,6 +2171,7 @@ static struct snd_pcm_ops azx_pcm_ops = {
 	.prepare = azx_pcm_prepare,
 	.trigger = azx_pcm_trigger,
 	.pointer = azx_pcm_pointer,
+	.mmap = azx_pcm_mmap,
 	.page = snd_pcm_sgbuf_ops_page,
 };
 
@@ -2365,13 +2452,19 @@ static int azx_free(struct azx *chip)
 
 	if (chip->azx_dev) {
 		for (i = 0; i < chip->num_streams; i++)
-			if (chip->azx_dev[i].bdl.area)
+			if (chip->azx_dev[i].bdl.area) {
+				mark_pages_wc(chip, &chip->azx_dev[i].bdl, false);
 				snd_dma_free_pages(&chip->azx_dev[i].bdl);
+			}
 	}
-	if (chip->rb.area)
+	if (chip->rb.area) {
+		mark_pages_wc(chip, &chip->rb, false);
 		snd_dma_free_pages(&chip->rb);
-	if (chip->posbuf.area)
+	}
+	if (chip->posbuf.area) {
+		mark_pages_wc(chip, &chip->posbuf, false);
 		snd_dma_free_pages(&chip->posbuf);
+	}
 	pci_release_regions(chip->pci);
 	pci_disable_device(chip->pci);
 	kfree(chip->azx_dev);
@@ -2693,6 +2786,7 @@ static int __devinit azx_create(struct snd_card *card, struct pci_dev *pci,
 			snd_printk(KERN_ERR SFX "cannot allocate BDL\n");
 			goto errout;
 		}
+		mark_pages_wc(chip, &chip->azx_dev[i].bdl, true);
 	}
 	/* allocate memory for the position buffer */
 	err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
@@ -2702,6 +2796,7 @@ static int __devinit azx_create(struct snd_card *card, struct pci_dev *pci,
 		snd_printk(KERN_ERR SFX "cannot allocate posbuf\n");
 		goto errout;
 	}
+	mark_pages_wc(chip, &chip->posbuf, true);
 	/* allocate CORB/RIRB */
 	err = azx_alloc_cmd_io(chip);
 	if (err < 0)


More information about the Alsa-devel mailing list