i810 audio patch

i810 audio patch

[Nathan Bryant]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

this patch is slightly differerent from the last one i posted.

it's still diffed against 2.4.17pre1. one obvious thinko is fixed, and a 
couple lines that looked bad to me have been changed (goto end in 
i810_read seems to be necessary to clean up the wait queue unless I'm 
mistaken), and I'm no longer reproducing any OOPSes.

however, i am seeing artsd segfault occasionally. this also seems to 
happen with the 4Front driver, however, at least if I load 4Front's 
module after unloading this patched driver. I'm not sure if this is a 
bug in artsd or specific to my setup or something nasty i've done to my 
kernel's data structures ;) so maybe somebody else who's still having 
problems with 2.4.17pre1 and KDE can take a look and see how it works 
for them.

[-- Attachment #2: i810.diff2 --]
[-- Type: text/plain, Size: 13951 bytes --]

--- i810_audio.c.17p1	Mon Dec  3 14:14:40 2001
+++ linux/drivers/sound/i810_audio.c	Mon Dec  3 19:06:33 2001
@@ -197,7 +197,7 @@
 #define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)
 
 
-#define DRIVER_VERSION "0.04"
+#define DRIVER_VERSION "0.05b"
 
 /* magic numbers to protect our data structures */
 #define I810_CARD_MAGIC		0x5072696E /* "Prin" */
@@ -357,6 +357,10 @@
 	struct i810_channel *(*alloc_rec_pcm_channel)(struct i810_card *);
 	struct i810_channel *(*alloc_rec_mic_channel)(struct i810_card *);
 	void (*free_pcm_channel)(struct i810_card *, int chan);
+
+        /* We have a *very* long init time possibly, so use this to block */
+        /* attempts to open our devices before we are ready (stops oops'es) */
+	int initializing;
 };
 
 static struct i810_card *devs = NULL;
@@ -364,32 +368,6 @@
 static int i810_open_mixdev(struct inode *inode, struct file *file);
 static int i810_ioctl_mixdev(struct inode *inode, struct file *file, unsigned int cmd,
 				unsigned long arg);
-
-static inline unsigned ld2(unsigned int x)
-{
-	unsigned r = 0;
-	
-	if (x >= 0x10000) {
-		x >>= 16;
-		r += 16;
-	}
-	if (x >= 0x100) {
-		x >>= 8;
-		r += 8;
-	}
-	if (x >= 0x10) {
-		x >>= 4;
-		r += 4;
-	}
-	if (x >= 4) {
-		x >>= 2;
-		r += 2;
-	}
-	if (x >= 2)
-		r++;
-	return r;
-}
-
 static u16 i810_ac97_get(struct ac97_codec *dev, u8 reg);
 static void i810_ac97_set(struct ac97_codec *dev, u8 reg, u16 data);
 
@@ -969,14 +947,6 @@
 	else
 		port += dmabuf->write_channel->port;
 
-	if(dmabuf->mapped) {
-		if(rec)
-			dmabuf->swptr = (dmabuf->hwptr + dmabuf->dmasize
-				       	- dmabuf->count) % dmabuf->dmasize;
-		else
-			dmabuf->swptr = (dmabuf->hwptr + dmabuf->count)
-			       		% dmabuf->dmasize;
-	}
 	/*
 	 * two special cases, count == 0 on write
 	 * means no data, and count == dmasize
@@ -993,7 +963,7 @@
 	/* swptr - 1 is the tail of our transfer */
 	x = (dmabuf->dmasize + dmabuf->swptr - 1) % dmabuf->dmasize;
 	x /= dmabuf->fragsize;
-	outb(x&31, port+OFF_LVI);
+	outb(x, port+OFF_LVI);
 }
 
 static void i810_update_lvi(struct i810_state *state, int rec)
@@ -1020,7 +990,9 @@
 		/* update hardware pointer */
 		hwptr = i810_get_dma_addr(state, 1);
 		diff = (dmabuf->dmasize + hwptr - dmabuf->hwptr) % dmabuf->dmasize;
-//		printk("HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
+#ifdef DEBUG_INTERRUPTS
+		printk("ADC HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
+#endif
 		dmabuf->hwptr = hwptr;
 		dmabuf->total_bytes += diff;
 		dmabuf->count += diff;
@@ -1043,7 +1015,9 @@
 		/* update hardware pointer */
 		hwptr = i810_get_dma_addr(state, 0);
 		diff = (dmabuf->dmasize + hwptr - dmabuf->hwptr) % dmabuf->dmasize;
-//		printk("HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
+#ifdef DEBUG_INTERRUPTS
+		printk("DAC HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
+#endif
 		dmabuf->hwptr = hwptr;
 		dmabuf->total_bytes += diff;
 		dmabuf->count -= diff;
@@ -1068,6 +1042,40 @@
 	}
 }
 
+static inline int i810_get_free_write_space(struct i810_state *state)
+{
+	struct dmabuf *dmabuf = &state->dmabuf;
+	int free;
+
+	i810_update_ptr(state);
+	// catch underruns during playback
+	if (dmabuf->count < 0) {
+		dmabuf->count = 0;
+	}
+	free = dmabuf->dmasize - dmabuf->count;
+	free -= (dmabuf->hwptr % dmabuf->fragsize);
+	if(free < 0)
+		return(0);
+	return(free);
+}
+
+static inline int i810_get_available_read_data(struct i810_state *state)
+{
+	struct dmabuf *dmabuf = &state->dmabuf;
+	int avail;
+
+	i810_update_ptr(state);
+	// catch overruns during record
+	if (dmabuf->count > dmabuf->dmasize) {
+		dmabuf->count = dmabuf->dmasize;
+	}
+	avail = dmabuf->count;
+	avail -= (dmabuf->hwptr % dmabuf->fragsize);
+	if(avail < 0)
+		return(0);
+	return(avail);
+}
+
 static int drain_dac(struct i810_state *state, int nonblock)
 {
 	DECLARE_WAITQUEUE(wait, current);
@@ -1271,10 +1279,7 @@
                         continue;
                 }
 		swptr = dmabuf->swptr;
-		if (dmabuf->count > dmabuf->dmasize) {
-			dmabuf->count = dmabuf->dmasize;
-		}
-		cnt = dmabuf->count - dmabuf->fragsize;
+		cnt = i810_get_available_read_data(state);
 		// this is to make the copy_to_user simpler below
 		if(cnt > (dmabuf->dmasize - swptr))
 			cnt = dmabuf->dmasize - swptr;
@@ -1291,7 +1296,7 @@
 			i810_update_lvi(state,1);
 			if (file->f_flags & O_NONBLOCK) {
 				if (!ret) ret = -EAGAIN;
-				return ret;
+				goto done;
 			}
 			/* This isnt strictly right for the 810  but it'll do */
 			tmo = (dmabuf->dmasize * HZ) / (dmabuf->rate * 2);
@@ -1315,7 +1320,7 @@
 			}
 			if (signal_pending(current)) {
 				ret = ret ? ret : -ERESTARTSYS;
-				return ret;
+				goto done;
 			}
 			continue;
 		}
@@ -1402,10 +1407,7 @@
                 }
 
 		swptr = dmabuf->swptr;
-		if (dmabuf->count < 0) {
-			dmabuf->count = 0;
-		}
-		cnt = dmabuf->dmasize - swptr;
+		cnt = i810_get_free_write_space(state);
 		if(cnt > (dmabuf->dmasize - dmabuf->count))
 			cnt = dmabuf->dmasize - dmabuf->count;
 		spin_unlock_irqrestore(&state->card->lock, flags);
@@ -1508,13 +1510,8 @@
 			mask |= POLLIN | POLLRDNORM;
 	}
 	if (file->f_mode & FMODE_WRITE && dmabuf->enable & DAC_RUNNING) {
-		if (dmabuf->mapped) {
-			if (dmabuf->count >= (signed)dmabuf->fragsize)
-				mask |= POLLOUT | POLLWRNORM;
-		} else {
-			if ((signed)dmabuf->dmasize >= dmabuf->count + (signed)dmabuf->fragsize)
-				mask |= POLLOUT | POLLWRNORM;
-		}
+		if ((signed)dmabuf->dmasize >= dmabuf->count + (signed)dmabuf->fragsize)
+			mask |= POLLOUT | POLLWRNORM;
 	}
 	spin_unlock_irqrestore(&state->card->lock, flags);
 
@@ -1559,10 +1556,7 @@
 			     size, vma->vm_page_prot))
 		goto out;
 	dmabuf->mapped = 1;
-	if(vma->vm_flags & VM_WRITE)
-		dmabuf->count = dmabuf->dmasize;
-	else
-		dmabuf->count = 0;
+	dmabuf->count = 0;
 	ret = 0;
 #ifdef DEBUG
 	printk("i810_audio: mmap'ed %ld bytes of data space\n", size);
@@ -1580,11 +1574,9 @@
 	audio_buf_info abinfo;
 	count_info cinfo;
 	unsigned int i_glob_cnt;
-	int val = 0, mapped, ret;
+	int val = 0, ret;
 	struct ac97_codec *codec = state->card->ac97_codec[0];
 
-	mapped = ((file->f_mode & FMODE_WRITE) && dmabuf->mapped) ||
-		((file->f_mode & FMODE_READ) && dmabuf->mapped);
 #ifdef DEBUG
 	printk("i810_audio: i810_ioctl, arg=0x%x, cmd=", arg ? *(int *)arg : 0);
 #endif
@@ -1674,9 +1666,6 @@
 #ifdef DEBUG
 		printk("SNDCTL_DSP_STEREO\n");
 #endif
-		if (get_user(val, (int *)arg))
-			return -EFAULT;
-
 		if (dmabuf->enable & DAC_RUNNING) {
 			stop_dac(state);
 		}
@@ -1820,22 +1809,47 @@
 
 		dmabuf->ossfragsize = 1<<(val & 0xffff);
 		dmabuf->ossmaxfrags = (val >> 16) & 0xffff;
-		if (dmabuf->ossmaxfrags <= 4)
-			dmabuf->ossmaxfrags = 4;
-		else if (dmabuf->ossmaxfrags <= 8)
-			dmabuf->ossmaxfrags = 8;
-		else if (dmabuf->ossmaxfrags <= 16)
-			dmabuf->ossmaxfrags = 16;
-		else
-			dmabuf->ossmaxfrags = 32;
+		if (!dmabuf->ossfragsize || !dmabuf->ossmaxfrags)
+			return -EINVAL;
+		/*
+		 * Bound the frag size into our allowed range of 256 - 4096
+		 */
+		if (dmabuf->ossfragsize < 256)
+			dmabuf->ossfragsize = 256;
+		else if (dmabuf->ossfragsize > 4096)
+			dmabuf->ossfragsize = 4096;
+		/*
+		 * The numfrags could be something reasonable, or it could
+		 * be 0xffff meaning "Give me as much as possible".  So,
+		 * we check the numfrags * fragsize doesn't exceed our
+		 * 64k buffer limit, nor is it less than our 8k minimum.
+		 * If it fails either one of these checks, then adjust the
+		 * number of fragments, not the size of them.  It's OK if
+		 * our number of fragments doesn't equal 32 or anything
+		 * like our hardware based number now since we are using
+		 * a different frag count for the hardware.  Before we get
+		 * into this though, bound the maxfrags to avoid overflow
+		 * issues.  A reasonable bound would be 64k / 256 since our
+		 * maximum buffer size is 64k and our minimum frag size is
+		 * 256.  On the other end, our minimum buffer size is 8k and
+		 * our maximum frag size is 4k, so the lower bound should
+		 * be 2.
+		 */
+
+		if(dmabuf->ossmaxfrags > 256)
+			dmabuf->ossmaxfrags = 256;
+		else if (dmabuf->ossmaxfrags < 2)
+			dmabuf->ossmaxfrags = 2;
+
 		val = dmabuf->ossfragsize * dmabuf->ossmaxfrags;
-		if (val < 16384)
-			val = 16384;
-		if (val > 65536)
-			val = 65536;
-		dmabuf->ossmaxfrags = val/dmabuf->ossfragsize;
-		if(dmabuf->ossmaxfrags<4)
-			dmabuf->ossfragsize = val/4;
+		while (val < 8192) {
+		    val <<= 1;
+		    dmabuf->ossmaxfrags <<= 1;
+		}
+		while (val > 65536) {
+		    val >>= 1;
+		    dmabuf->ossmaxfrags >>= 1;
+		}
 		dmabuf->ready = 0;
 #ifdef DEBUG
 		printk("SNDCTL_DSP_SETFRAGMENT 0x%x, %d, %d\n", val,
@@ -1853,10 +1867,10 @@
 		i810_update_ptr(state);
 		abinfo.fragsize = dmabuf->userfragsize;
 		abinfo.fragstotal = dmabuf->userfrags;
-		if(dmabuf->mapped)
-			abinfo.bytes = dmabuf->count;
-		else
-			abinfo.bytes = dmabuf->dmasize - dmabuf->fragsize - dmabuf->count;
+		if (dmabuf->mapped)
+ 			abinfo.bytes = dmabuf->dmasize;
+  		else
+ 			abinfo.bytes = i810_get_free_write_space(state);
 		abinfo.fragments = abinfo.bytes / dmabuf->userfragsize;
 		spin_unlock_irqrestore(&state->card->lock, flags);
 #ifdef DEBUG
@@ -1871,13 +1885,13 @@
 		if (!dmabuf->ready && (val = prog_dmabuf(state, 0)) != 0)
 			return val;
 		spin_lock_irqsave(&state->card->lock, flags);
-		i810_update_ptr(state);
+		val = i810_get_free_write_space(state);
 		cinfo.bytes = dmabuf->total_bytes;
 		cinfo.ptr = dmabuf->hwptr;
-		cinfo.blocks = (dmabuf->dmasize - dmabuf->count)/dmabuf->userfragsize;
-		if (dmabuf->mapped) {
-			dmabuf->count = (dmabuf->dmasize - 
-					 (dmabuf->count & (dmabuf->userfragsize-1)));
+		cinfo.blocks = val/dmabuf->userfragsize;
+		if (dmabuf->mapped && dmabuf->enable && DAC_RUNNING) {
+			dmabuf->count += val;
+			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
 			__i810_update_lvi(state, 0);
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
@@ -1893,10 +1907,9 @@
 		if (!dmabuf->ready && (val = prog_dmabuf(state, 1)) != 0)
 			return val;
 		spin_lock_irqsave(&state->card->lock, flags);
-		i810_update_ptr(state);
+		abinfo.bytes = i810_get_available_read_data(state);
 		abinfo.fragsize = dmabuf->userfragsize;
 		abinfo.fragstotal = dmabuf->userfrags;
-		abinfo.bytes = dmabuf->dmasize - dmabuf->count;
 		abinfo.fragments = abinfo.bytes / dmabuf->userfragsize;
 		spin_unlock_irqrestore(&state->card->lock, flags);
 #ifdef DEBUG
@@ -1911,12 +1924,13 @@
 		if (!dmabuf->ready && (val = prog_dmabuf(state, 0)) != 0)
 			return val;
 		spin_lock_irqsave(&state->card->lock, flags);
-		i810_update_ptr(state);
+		val = i810_get_available_read_data(state);
 		cinfo.bytes = dmabuf->total_bytes;
-		cinfo.blocks = dmabuf->count/dmabuf->userfragsize;
+		cinfo.blocks = val/dmabuf->userfragsize;
 		cinfo.ptr = dmabuf->hwptr;
-		if (dmabuf->mapped) {
-			dmabuf->count &= (dmabuf->userfragsize-1);
+		if (dmabuf->mapped && dmabuf->enable && ADC_RUNNING) {
+			dmabuf->count -= val;
+			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
 			__i810_update_lvi(state, 1);
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
@@ -1970,8 +1984,12 @@
 			if (!dmabuf->ready && (ret = prog_dmabuf(state, 0)))
 				return ret;
 			if (dmabuf->mapped) {
-				dmabuf->count = dmabuf->dmasize;
-				i810_update_lvi(state,0);
+				spin_lock_irqsave(&state->card->lock, flags);
+				i810_update_ptr(state);
+				dmabuf->count = 0;
+				dmabuf->count = i810_get_free_write_space(state);
+				__i810_update_lvi(state, 0);
+				spin_unlock_irqrestore(&state->card->lock, flags);
 			}
 			if (!dmabuf->enable && dmabuf->count > dmabuf->userfragsize)
 				start_dac(state);
@@ -1985,10 +2003,6 @@
 			}
 			if (!dmabuf->ready && (ret = prog_dmabuf(state, 1)))
 				return ret;
-			if (dmabuf->mapped) {
-				dmabuf->count = 0;
-				i810_update_lvi(state,1);
-			}
 			if (!dmabuf->enable && dmabuf->count <
 			    (dmabuf->dmasize - dmabuf->userfragsize))
 				start_adc(state);
@@ -2195,7 +2209,11 @@
 
 	/* find an avaiable virtual channel (instance of /dev/dsp) */
 	while (card != NULL) {
-		for (i = 0; i < NR_HW_CH; i++) {
+		for (i = 0; i < 50 && card->initializing; i++) {
+			current->state = TASK_UNINTERRUPTIBLE;
+			schedule_timeout(HZ/20);
+		}
+		for (i = 0; i < NR_HW_CH && !card->initializing; i++) {
 			if (card->states[i] == NULL) {
 				state = card->states[i] = (struct i810_state *)
 					kmalloc(sizeof(struct i810_state), GFP_KERNEL);
@@ -2344,13 +2362,18 @@
 	int minor = MINOR(inode->i_rdev);
 	struct i810_card *card = devs;
 
-	for (card = devs; card != NULL; card = card->next)
-		for (i = 0; i < NR_AC97; i++)
+	for (card = devs; card != NULL; card = card->next) {
+		for (i = 0; i < 50 && card->initializing; i++) {
+			current->state = TASK_UNINTERRUPTIBLE;
+			schedule_timeout(HZ/20);
+		}
+		for (i = 0; i < NR_AC97 && !card->initializing; i++)
 			if (card->ac97_codec[i] != NULL &&
 			    card->ac97_codec[i]->dev_mixer == minor) {
 				file->private_data = card->ac97_codec[i];
 				return 0;
 			}
+	}
 	return -ENODEV;
 }
 
@@ -2696,6 +2719,7 @@
 	}
 	memset(card, 0, sizeof(*card));
 
+	card->initializing = 1;
 	card->iobase = pci_resource_start (pci_dev, 1);
 	card->ac97base = pci_resource_start (pci_dev, 0);
 	card->pci_dev = pci_dev;
@@ -2771,7 +2795,7 @@
 		kfree(card);
 		return -ENODEV;
 	}
-
+ 	card->initializing = 0;
 	return 0;
 }
 
@@ -2789,6 +2813,7 @@
 		if (card->ac97_codec[i] != NULL) {
 			unregister_sound_mixer(card->ac97_codec[i]->dev_mixer);
 			kfree (card->ac97_codec[i]);
+			card->ac97_codec[i] = NULL;
 		}
 	unregister_sound_dsp(card->dev_audio);
 	kfree(card);
@@ -2957,9 +2982,6 @@
 	if(ftsodell != 0) {
 		printk("i810_audio: ftsodell is now a deprecated option.\n");
 	}
-	if(clocking == 48000) {
-		i810_configure_clocking();
-	}
 	if(spdif_locked > 0 ) {
 		if(spdif_locked == 32000 || spdif_locked == 44100 || spdif_locked == 48000) {
 			printk("i810_audio: Enabling S/PDIF at sample rate %dHz.\n", spdif_locked);

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> this patch is slightly differerent from the last one i posted.
> 
> it's still diffed against 2.4.17pre1. one obvious thinko is fixed, and a 
> couple lines that looked bad to me have been changed (goto end in 
> i810_read seems to be necessary to clean up the wait queue unless I'm 
> mistaken), and I'm no longer reproducing any OOPSes.
> 
> however, i am seeing artsd segfault occasionally. this also seems to 
> happen with the 4Front driver, however, at least if I load 4Front's 
> module after unloading this patched driver. I'm not sure if this is a 
> bug in artsd or specific to my setup or something nasty i've done to my 
> kernel's data structures ;) so maybe somebody else who's still having 
> problems with 2.4.17pre1 and KDE can take a look and see how it works 
> for them.


Nathan, thanks for taking the time to merge these.  I've got some more 
stuff here that I've done just recently for ANK, and a few moments of 
spare time.  If you'll send me the last version of the .c file you had, 
I can probably do a pretty quick merge of everthing together (I need the 
.c file instead of the patch because I'm not starting from the same 
source you are).  Hopefully, between your efforts and the feedback I've 
gotten in the last few days, we can send a final version to Marcello for 
the 2.4 kernel series that actually works well.




-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Nathan Bryant wrote

> - I haven't seen those oopses again either; they may have been caused 
> by i810_configure_clocking being called twice during the 
> initialization due to a merging goof on my part... 

Ok, I spoke too soon. That piece of code couldn't cause the problem, 
because of the surrounding if (clocking==...

So there may be some VM/buffer related problem lurking under the covers 
still. Originally the oops popped up in kswapd, for me, but I can't 
trigger it again.

Re: i810 audio patch

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

Nathan Bryant wrote:

> Nathan Bryant wrote
> 
>> - I haven't seen those oopses again either; they may have been caused 
>> by i810_configure_clocking being called twice during the 
>> initialization due to a merging goof on my part... 
> 
> 
> Ok, I spoke too soon. That piece of code couldn't cause the problem, 
> because of the surrounding if (clocking==...
> 
> So there may be some VM/buffer related problem lurking under the covers 
> still. Originally the oops popped up in kswapd, for me, but I can't 
> trigger it again.
> 

Well, your second version of the file had the merge done right (my code 
didn't include S/PDIF support or PM support, so those parts were 
different, but the parts that were the same as my code were done 
correctly).  I'm attaching a patch that bumps the code from your 0.05b 
to a unified 0.06 and I'm also placing the 0.06 i810_audio.c.gz file on 
my web site in the same place that I put the 0.05 version.  If people 
could please test this and report problems back, I would like to get 
this one off my plate (aka, I don't want to hear any more about artsd 
not working ever again so I want testers to tell me that it's fixed ;-)

-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

[-- Attachment #2: i810_audio-0.06.patch --]
[-- Type: text/plain, Size: 6597 bytes --]

--- i810_audio.c.05b.orig	Tue Dec  4 00:29:18 2001
+++ i810_audio.c.06	Tue Dec  4 00:37:35 2001
@@ -105,7 +105,7 @@
 
 static int ftsodell=0;
 static int strict_clocking=0;
-static unsigned int clocking=48000;
+static unsigned int clocking=0;
 static int spdif_locked=0;
 
 //#define DEBUG
@@ -197,7 +197,7 @@
 #define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)
 
 
-#define DRIVER_VERSION "0.05b"
+#define DRIVER_VERSION "0.06"
 
 /* magic numbers to protect our data structures */
 #define I810_CARD_MAGIC		0x5072696E /* "Prin" */
@@ -358,16 +358,16 @@
 	struct i810_channel *(*alloc_rec_mic_channel)(struct i810_card *);
 	void (*free_pcm_channel)(struct i810_card *, int chan);
 
-        /* We have a *very* long init time possibly, so use this to block */
-        /* attempts to open our devices before we are ready (stops oops'es) */
+	/* We have a *very* long init time possibly, so use this to block */
+	/* attempts to open our devices before we are ready (stops oops'es) */
 	int initializing;
 };
 
 static struct i810_card *devs = NULL;
 
 static int i810_open_mixdev(struct inode *inode, struct file *file);
-static int i810_ioctl_mixdev(struct inode *inode, struct file *file, unsigned int cmd,
-				unsigned long arg);
+static int i810_ioctl_mixdev(struct inode *inode, struct file *file,
+			     unsigned int cmd, unsigned long arg);
 static u16 i810_ac97_get(struct ac97_codec *dev, u8 reg);
 static void i810_ac97_set(struct ac97_codec *dev, u8 reg, u16 data);
 
@@ -710,21 +710,27 @@
 	spin_unlock_irqrestore(&card->lock, flags);
 }
 
-static void start_adc(struct i810_state *state)
+static inline void __start_adc(struct i810_state *state)
 {
 	struct dmabuf *dmabuf = &state->dmabuf;
-	struct i810_card *card = state->card;
-	unsigned long flags;
 
 	if (dmabuf->count < dmabuf->dmasize && dmabuf->ready && !dmabuf->enable &&
 	    (dmabuf->trigger & PCM_ENABLE_INPUT)) {
-		spin_lock_irqsave(&card->lock, flags);
 		dmabuf->enable |= ADC_RUNNING;
-		outb((1<<4) | (1<<2) | 1, card->iobase + PI_CR);
-		spin_unlock_irqrestore(&card->lock, flags);
+		outb((1<<4) | (1<<2) | 1, state->card->iobase + PI_CR);
 	}
 }
 
+static void start_adc(struct i810_state *state)
+{
+	struct i810_card *card = state->card;
+	unsigned long flags;
+
+	spin_lock_irqsave(&card->lock, flags);
+	__start_adc(state);
+	spin_unlock_irqrestore(&card->lock, flags);
+}
+
 /* stop playback (lock held) */
 static inline void __stop_dac(struct i810_state *state)
 {
@@ -750,20 +756,25 @@
 	spin_unlock_irqrestore(&card->lock, flags);
 }	
 
-static void start_dac(struct i810_state *state)
+static inline void __start_dac(struct i810_state *state)
 {
 	struct dmabuf *dmabuf = &state->dmabuf;
-	struct i810_card *card = state->card;
-	unsigned long flags;
 
 	if (dmabuf->count > 0 && dmabuf->ready && !dmabuf->enable &&
 	    (dmabuf->trigger & PCM_ENABLE_OUTPUT)) {
-		spin_lock_irqsave(&card->lock, flags);
 		dmabuf->enable |= DAC_RUNNING;
-		outb((1<<4) | (1<<2) | 1, card->iobase + PO_CR);
-		spin_unlock_irqrestore(&card->lock, flags);
+		outb((1<<4) | (1<<2) | 1, state->card->iobase + PO_CR);
 	}
 }
+static void start_dac(struct i810_state *state)
+{
+	struct i810_card *card = state->card;
+	unsigned long flags;
+
+	spin_lock_irqsave(&card->lock, flags);
+	__start_dac(state);
+	spin_unlock_irqrestore(&card->lock, flags);
+}
 
 #define DMABUF_DEFAULTORDER (16-PAGE_SHIFT)
 #define DMABUF_MINORDER 1
@@ -783,6 +794,8 @@
 		dmabuf->ossfragsize = (PAGE_SIZE<<DMABUF_DEFAULTORDER)/dmabuf->ossmaxfrags;
 	size = dmabuf->ossfragsize * dmabuf->ossmaxfrags;
 
+	if(dmabuf->rawbuf && (PAGE_SIZE << dmabuf->buforder) == size)
+		return 0;
 	/* alloc enough to satisfy the oss params */
 	for (order = DMABUF_DEFAULTORDER; order >= DMABUF_MINORDER; order--) {
 		if ( (PAGE_SIZE<<order) > size )
@@ -851,14 +864,19 @@
 	dmabuf->swptr = dmabuf->hwptr = 0;
 	spin_unlock_irqrestore(&state->card->lock, flags);
 
-	/* allocate DMA buffer if not allocated yet */
-	if (dmabuf->rawbuf)
-		dealloc_dmabuf(state);
+	/* allocate DMA buffer, let alloc_dmabuf determine if we are already
+	 * allocated well enough or if we should replace the current buffer
+	 * (assuming one is already allocated, if it isn't, then allocate it).
+	 */
 	if ((ret = alloc_dmabuf(state)))
 		return ret;
 
 	/* FIXME: figure out all this OSS fragment stuff */
 	/* I did, it now does what it should according to the OSS API.  DL */
+	/* We may not have realloced our dmabuf, but the fragment size to
+	 * fragment number ratio may have changed, so go ahead and reprogram
+	 * things
+	 */
 	dmabuf->dmasize = PAGE_SIZE << dmabuf->buforder;
 	dmabuf->numfrag = SG_LEN;
 	dmabuf->fragsize = dmabuf->dmasize/dmabuf->numfrag;
@@ -1190,6 +1208,11 @@
 			if(count > 0) {
 				outb(inb(port+OFF_CR) | 1, port+OFF_CR);
 			} else {
+				if (dmabuf->enable & DAC_RUNNING)
+					__stop_dac(state);
+				if (dmabuf->enable & ADC_RUNNING)
+					__stop_adc(state);
+				dmabuf->enable = 0;
 				wake_up(&dmabuf->wait);
 #ifdef DEBUG_INTERRUPTS
 				printk("DCH - STOP ");
@@ -1698,18 +1721,7 @@
 #ifdef DEBUG
 		printk("SNDCTL_DSP_SETFMT\n");
 #endif
-		if (get_user(val, (int *)arg))
-			return -EFAULT;
-
-		switch ( val ) {
-			case AFMT_S16_LE:
-				break;
-			case AFMT_QUERY:
-			default:
-				val = AFMT_S16_LE;
-				break;
-		}
-		return put_user(val, (int *)arg);
+		return put_user(AFMT_S16_LE, (int *)arg);
 
 	case SNDCTL_DSP_CHANNELS:
 #ifdef DEBUG
@@ -1889,10 +1901,12 @@
 		cinfo.bytes = dmabuf->total_bytes;
 		cinfo.ptr = dmabuf->hwptr;
 		cinfo.blocks = val/dmabuf->userfragsize;
-		if (dmabuf->mapped && dmabuf->enable && DAC_RUNNING) {
+		if (dmabuf->mapped && dmabuf->trigger && PCM_ENABLE_OUTPUT) {
 			dmabuf->count += val;
 			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
 			__i810_update_lvi(state, 0);
+			if (!dmabuf->enable)
+				__start_dac(state);
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
 #ifdef DEBUG
@@ -1928,10 +1942,12 @@
 		cinfo.bytes = dmabuf->total_bytes;
 		cinfo.blocks = val/dmabuf->userfragsize;
 		cinfo.ptr = dmabuf->hwptr;
-		if (dmabuf->mapped && dmabuf->enable && ADC_RUNNING) {
+		if (dmabuf->mapped && dmabuf->trigger && PCM_ENABLE_INPUT) {
 			dmabuf->count -= val;
 			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
 			__i810_update_lvi(state, 1);
+			if (!dmabuf->enable)
+				__start_adc(state);
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
 #ifdef DEBUG
@@ -2776,7 +2792,7 @@
 	}
 	pci_set_drvdata(pci_dev, card);
 
-	if(clocking == 48000) {
+	if(clocking == 0) {
 		i810_configure_clocking();
 	}
 

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

>
> Well, your second version of the file had the merge done right (my 
> code didn't include S/PDIF support or PM support, so those parts were 
> different, but the parts that were the same as my code were done 
> correctly).  I'm attaching a patch that bumps the code from your 0.05b 
> to a unified 0.06 and I'm also placing the 0.06 i810_audio.c.gz file 
> on my web site in the same place that I put the 0.05 version.  If 
> people could please test this and report problems back, I would like 
> to get this one off my plate (aka, I don't want to hear any more about 
> artsd not working ever again so I want testers to tell me that it's 
> fixed ;-)

You'll need to do something like add "clocking = 48000;" right before 
the call to i810_set_dac_rate in i810_configure_clocking() to avoid a 
divide by zero.

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Well, your second version of the file had the merge done right (my 
> code didn't include S/PDIF support or PM support, so those parts were 
> different, but the parts that were the same as my code were done 
> correctly).  I'm attaching a patch that bumps the code from your 0.05b 
> to a unified 0.06 and I'm also placing the 0.06 i810_audio.c.gz file 
> on my web site in the same place that I put the 0.05 version.  If 
> people could please test this and report problems back, I would like 
> to get this one off my plate (aka, I don't want to hear any more about 
> artsd not working ever again so I want testers to tell me that it's 
> fixed ;-)

Ok, fixed the divide by zero but artsd doesn't quite work with 0.06. 
almost but not quite.. sound works normally with non-artsd stuff but 
artsd decides to stop writing because select() never signals that the fd 
is writeable. it just times out.

Re: i810 audio patch

[Alan Cox]

> So there may be some VM/buffer related problem lurking under the covers 
> still. Originally the oops popped up in kswapd, for me, but I can't 
> trigger it again.

I've seen multiple reports of random kswapd oopses in 2.4.16 so while it
could be that the i810 code introduced a bug its also quite possible you hit
an underlying flaw

Alan

Re: i810 audio patch

[Mario Mikocevic]

Hi,

> Well, your second version of the file had the merge done right (my code 
> didn't include S/PDIF support or PM support, so those parts were 
> different, but the parts that were the same as my code were done 
> correctly).  I'm attaching a patch that bumps the code from your 0.05b 
> to a unified 0.06 and I'm also placing the 0.06 i810_audio.c.gz file on 
> my web site in the same place that I put the 0.05 version.  If people 
> could please test this and report problems back, I would like to get 
> this one off my plate (aka, I don't want to hear any more about artsd 
> not working ever again so I want testers to tell me that it's fixed ;-)

Umh, problems ->

modprobe produced an oops (17-pre2), module is left in init state :

# lsmod
Module                  Size  Used by
i810_audio             19472   1  (initializing)
ac97_codec              9632   0  [i810_audio]

Intel 810 + AC97 Audio, version 0.06, 15:21:16 Dec  4 2001
PCI: Found IRQ 9 for device 00:1f.5
PCI: Sharing IRQ 9 with 00:1f.3
PCI: Setting latency timer of device 00:1f.5 to 64
i810: Intel ICH2 found at IO 0xef00 and 0xe800, IRQ 9
i810_audio: Audio Controller supports 6 channels.
ac97_codec: AC97 Audio codec, id: 0x4144:0x5360 (Analog Devices AD1885)
i810_audio: AC'97 codec 0 Unable to map surround DAC's (or DAC's not present), total channels = 2


# ksymoops < oops0 
ksymoops 2.4.1 on i686 2.4.17-pre2-mzg2.  Options used
     -V (default)
     -k /proc/ksyms (default)
     -l /proc/modules (default)
     -o /lib/modules/2.4.17-pre2-mzg2/ (default)
     -m /boot/System.map-2.4.17-pre2-mzg2 (default)

Warning: You did not tell me where to find symbol information.  I will
assume that the log matches the kernel and modules that are running
right now and I'll use the default options above for symbol resolution.
If the current kernel and/or modules do not match the log, you can get
more accurate output by telling me the kernel version and where to find
map, modules, ksyms etc.  ksymoops -h explains the options.

cpu: 0, clocks: 1329057, slice: 664528
ac97_codec: AC97 Audio codec, id: 0x4144:0x5360 (Analog Devices AD1885)
CPU:    0
EIP:    0010:[<d08f944e>]    Not tainted
Using defaults from ksymoops -t elf32-i386 -a i386
EFLAGS: 00010246
eax: 14244000   ebx: 00000000   ecx: 18000004   edx: 00000000
esi: cfe07880   edi: cfe077b0   ebp: 00000000   esp: ce817e90
ds: 0018   es: 0018   ss: 0018
Process modprobe (pid: 727, stackpage=ce817000)
Stack: 00000000 cfe07780 0000009c cfe0781c cfea2400 d08fc761 cfe07780 0000bb80 
       00000180 d08fc60f d08fda60 cfe077b0 cfe07780 cfd96800 d08fd930 cfd96800 
       d08fd0c7 cfea2400 d08fcc17 d08fd998 d08fdac0 cfea2400 00000000 c01aa55c 
Call Trace: [<d08fc761>] [<d08fc60f>] [<d08fda60>] [<d08fd930>] [<d08fd0c7>] 
   [<d08fcc17>] [<d08fd998>] [<d08fdac0>] [<c01aa55c>] [<d08fd998>] [<d08fdac0>] 
   [<c01aa5c2>] [<d08fdac0>] [<d08fcd94>] [<d08fdac0>] [<d08fd740>] [<c0117f25>] 
   [<d08fdbd8>] [<d08f9060>] [<c0106f1b>] 
Code: f7 35 28 d9 8f d0 89 07 8b 07 59 5b 5e 5f 5d c3 89 f6 55 57 

>>EIP; d08f944e <[i810_audio]i810_set_dac_rate+9e/b0>   <=====
Trace; d08fc761 <[i810_audio]i810_configure_clocking+c1/2c0>
Trace; d08fc60f <[i810_audio]i810_ac97_init+2bf/350>
Trace; d08fda60 <[i810_audio]i810_mixer_fops+0/60>
Trace; d08fd930 <[i810_audio]card_names+0/14>
Trace; d08fd0c7 <[i810_audio].rodata.end+c/11>
Trace; d08fcc17 <[i810_audio]i810_probe+2b7/360>
Trace; d08fd998 <[i810_audio]i810_pci_tbl+54/a8>
Trace; d08fdac0 <[i810_audio]i810_pci_driver+0/3f>
Trace; c01aa55c <pci_announce_device+3c/60>
Trace; d08fd998 <[i810_audio]i810_pci_tbl+54/a8>
Trace; d08fdac0 <[i810_audio]i810_pci_driver+0/3f>
Trace; c01aa5c2 <pci_register_driver+42/60>
Trace; d08fdac0 <[i810_audio]i810_pci_driver+0/3f>
Trace; d08fcd94 <[i810_audio]i810_init_module+24/a0>
Trace; d08fdac0 <[i810_audio]i810_pci_driver+0/3f>
Trace; d08fd740 <[i810_audio].LC31+28/40>
Trace; c0117f25 <sys_init_module+555/630>
Trace; d08fdbd8 <.data.end+d9/????>
Trace; d08f9060 <[i810_audio]i810_alloc_pcm_channel+0/4>
Trace; c0106f1b <system_call+33/38>
Code;  d08f944e <[i810_audio]i810_set_dac_rate+9e/b0>
00000000 <_EIP>:
Code;  d08f944e <[i810_audio]i810_set_dac_rate+9e/b0>   <=====
   0:   f7 35 28 d9 8f d0         div    0xd08fd928,%eax   <=====
Code;  d08f9454 <[i810_audio]i810_set_dac_rate+a4/b0>
   6:   89 07                     mov    %eax,(%edi)
Code;  d08f9456 <[i810_audio]i810_set_dac_rate+a6/b0>
   8:   8b 07                     mov    (%edi),%eax
Code;  d08f9458 <[i810_audio]i810_set_dac_rate+a8/b0>
   a:   59                        pop    %ecx
Code;  d08f9459 <[i810_audio]i810_set_dac_rate+a9/b0>
   b:   5b                        pop    %ebx
Code;  d08f945a <[i810_audio]i810_set_dac_rate+aa/b0>
   c:   5e                        pop    %esi
Code;  d08f945b <[i810_audio]i810_set_dac_rate+ab/b0>
   d:   5f                        pop    %edi
Code;  d08f945c <[i810_audio]i810_set_dac_rate+ac/b0>
   e:   5d                        pop    %ebp
Code;  d08f945d <[i810_audio]i810_set_dac_rate+ad/b0>
   f:   c3                        ret    
Code;  d08f945e <[i810_audio]i810_set_dac_rate+ae/b0>
  10:   89 f6                     mov    %esi,%esi
Code;  d08f9460 <[i810_audio]i810_set_adc_rate+0/b0>
  12:   55                        push   %ebp
Code;  d08f9461 <[i810_audio]i810_set_adc_rate+1/b0>
  13:   57                        push   %edi


1 warning issued.  Results may not be reliable.


-- 
Mario Mikočević (Mozgy)
mozgy at hinet dot hr
My favourite FUBAR ...

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> Well, your second version of the file had the merge done right (my 
>> code didn't include S/PDIF support or PM support, so those parts were 
>> different, but the parts that were the same as my code were done 
>> correctly).  I'm attaching a patch that bumps the code from your 0.05b 
>> to a unified 0.06 and I'm also placing the 0.06 i810_audio.c.gz file 
>> on my web site in the same place that I put the 0.05 version.  If 
>> people could please test this and report problems back, I would like 
>> to get this one off my plate (aka, I don't want to hear any more about 
>> artsd not working ever again so I want testers to tell me that it's 
>> fixed ;-)
> 
> 
> Ok, fixed the divide by zero but artsd doesn't quite work with 0.06. 
> almost but not quite.. sound works normally with non-artsd stuff but 
> artsd decides to stop writing because select() never signals that the fd 
> is writeable. it just times out.


I fixed the clocking issue in my source.  I need more details on the 
artsd problem though.  Does artsd start to work and then stop, or does 
it never get around to outputting any sound?  Also, do you have any of 
the debugging output turned on (it *drastically* changes the timings in 
the driver and can make things that normally work fail and vice versa)?




-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Mario Mikocevic wrote:

>modprobe produced an oops (17-pre2), module is left in init state :
>
Yep. In the i810_configure_clocking() function, immediately before the 
call to i810_set_dac_rate(), add a line "clocking = 48000;"

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Mario Mikocevic wrote:
> 
>> modprobe produced an oops (17-pre2), module is left in init state :
>>
> Yep. In the i810_configure_clocking() function, immediately before the 
> call to i810_set_dac_rate(), add a line "clocking = 48000;"
> 

There is a new version of the driver (0.07) on my web site.  It has this 
issue and one other issue fixed (hopefully).  The other issue is when 
using artsd with the 0.06 driver, I had a report that artsd would end up 
waiting on select forever and never getting woken up.  The 0.07 driver 
changes wait queue and lvi handling in a few strategic places, so it 
should work.  However, it's untested.  Reports welcome.

Complete c file: http://people.redhat.com/dledford/i810_audio.c.gz

-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> I fixed the clocking issue in my source.  I need more details on the 
> artsd problem though.  Does artsd start to work and then stop, or does 
> it never get around to outputting any sound?  Also, do you have any of 
> the debugging output turned on (it *drastically* changes the timings 
> in the driver and can make things that normally work fail and vice versa)?

It works for a while then stops. select() works properly for a while, 
and then starts returning timeouts.

I've tried this with DEBUG both on and off, and also with 
DEBUG_INTERRUPTS on/off... no difference.

I don't see anything too interesting in DEBUG+DEBUG2 output. Select() 
stops working after the buffer fills up (after 4 seconds with artsd set 
to 256byte fragments*32) and doesn't start working again after the 
buffer begins to drain.

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> I fixed the clocking issue in my source.  I need more details on the 
>> artsd problem though.  Does artsd start to work and then stop, or does 
>> it never get around to outputting any sound?  Also, do you have any of 
>> the debugging output turned on (it *drastically* changes the timings 
>> in the driver and can make things that normally work fail and vice 
>> versa)?
> 
> 
> It works for a while then stops. select() works properly for a while, 
> and then starts returning timeouts.
> 
> I've tried this with DEBUG both on and off, and also with 
> DEBUG_INTERRUPTS on/off... no difference.
> 
> I don't see anything too interesting in DEBUG+DEBUG2 output. Select() 
> stops working after the buffer fills up (after 4 seconds with artsd set 
> to 256byte fragments*32) and doesn't start working again after the 
> buffer begins to drain.
> 

Can you try this again with the 0.07 version I put on my web site about 
30 minutes ago?  I put code in there to (hopefully) solve this problem.

-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> There is a new version of the driver (0.07) on my web site.  It has 
> this issue and one other issue fixed (hopefully).  The other issue is 
> when using artsd with the 0.06 driver, I had a report that artsd would 
> end up waiting on select forever and never getting woken up.  The 0.07 
> driver changes wait queue and lvi handling in a few strategic places, 
> so it should work.  However, it's untested.  Reports welcome. 

With 0.07, the kernel goes into an endless sleep as soon as artsd calls 
select(). I don't think the looping changes to i810_poll are correct... 
or even necessary? though the userfragsize change is probably appropriate.

Re: i810 audio patch

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> There is a new version of the driver (0.07) on my web site.  It has 
>> this issue and one other issue fixed (hopefully).  The other issue is 
>> when using artsd with the 0.06 driver, I had a report that artsd would 
>> end up waiting on select forever and never getting woken up.  The 0.07 
>> driver changes wait queue and lvi handling in a few strategic places, 
>> so it should work.  However, it's untested.  Reports welcome. 
> 
> 
> With 0.07, the kernel goes into an endless sleep as soon as artsd calls 
> select(). I don't think the looping changes to i810_poll are correct... 
> or even necessary?


Probably not.  Although I did change it back but then change it in 
another way.  Use the attached patch to back out those changes and let 
me know if it works (for some reason, I doubt it).

> though the userfragsize change is probably appropriate.
> 



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

[-- Attachment #2: foo --]
[-- Type: text/plain, Size: 1193 bytes --]

--- i810_audio.c.07	Tue Dec  4 16:13:39 2001
+++ i810_audio.c.07b	Tue Dec  4 16:13:00 2001
@@ -1530,30 +1530,24 @@
 	struct dmabuf *dmabuf = &state->dmabuf;
 	unsigned long flags;
 	unsigned int mask = 0;
-        DECLARE_WAITQUEUE(waita, current);
+	int count;
 
 	if(!dmabuf->ready)
 		return 0;
-        add_wait_queue(&dmabuf->wait, &waita);
-again:
+	poll_wait(file, &dmabuf->wait, wait);
 	spin_lock_irqsave(&state->card->lock, flags);
 	i810_update_ptr(state);
+	count = dmabuf->count;
+	spin_unlock_irqrestore(&state->card->lock, flags);
 	if (file->f_mode & FMODE_READ && dmabuf->enable & ADC_RUNNING) {
-		if (dmabuf->count >= (signed)dmabuf->userfragsize)
+		if (count >= (signed)dmabuf->userfragsize)
 			mask |= POLLIN | POLLRDNORM;
 	}
 	if (file->f_mode & FMODE_WRITE && dmabuf->enable & DAC_RUNNING) {
 		if ((signed)dmabuf->dmasize >=
-		    dmabuf->count + (signed)dmabuf->userfragsize)
+		    count + (signed)dmabuf->userfragsize)
 			mask |= POLLOUT | POLLWRNORM;
 	}
-	spin_unlock_irqrestore(&state->card->lock, flags);
-	if (mask == 0) {
-		poll_wait(file, &dmabuf->wait, wait);
-		goto again;
-	}
-        remove_wait_queue(&dmabuf->wait, &waita);
-
 	return mask;
 }
 

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Probably not.  Although I did change it back but then change it in 
> another way.  Use the attached patch to back out those changes and let 
> me know if it works (for some reason, I doubt it). 

Let's see if I'm understanding things here. You made the looping change 
because of the userfragsize change, correct? The idea being if 
userfragsize is larger than the hardware's fragsize, we have to wait 
longer to wake up.

Except poll_wait doesn't seem designed to be called this way, at a 
glance. (Maybe it's possible to muck around with the poll table and call 
again, as a hack? but this seems ugly, have to be very careful that 
calling poll_wait twice doesn't corrupt memory, though, as implied in 
some of the notes on http://linux24.sourceforge.net/)

fs/select.c:do_poll() calls schedule_timeout() after all do_pollfd's 
have returned empty sets and there is still time remaining. so if you 
just eliminate the loop in i810_poll, it will loop back and if there's 
data available, poll(2) would return properly, but with extra latency. i 
assume sys_select behaves the same way...

so, 2 choices to eliminate latency, either hack i810_poll further, or be 
a lot more intelligent about calling wake_up. am i wrong?

Re: i810 audio patch

[Nathan Bryant]

ok, i'm learning here ;)

it seems i guessed wrong, and schedule() wakes up on a wait queue which 
has been added by poll_wait; poll_wait doesn't actually do any sleeping 
so no loop is necessary in i810_poll; it will be called again by do_poll 
or do_select after the call to schedule. so no latency and no problem.

right?

Nathan Bryant wrote:

> fs/select.c:do_poll() calls schedule_timeout() after all do_pollfd's 
> have returned empty sets and there is still time remaining. so if you 
> just eliminate the loop in i810_poll, it will loop back and if there's 
> data available, poll(2) would return properly, but with extra latency. 
> i assume sys_select behaves the same way...
>
> so, 2 choices to eliminate latency, either hack i810_poll further, or 
> be a lot more intelligent about calling wake_up. am i wrong?
>

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Probably not.  Although I did change it back but then change it in 
> another way.  Use the attached patch to back out those changes and let 
> me know if it works (for some reason, I doubt it).

Fascinating...

Your patch to i810_poll fixes the sleep of death. and with the rest of 
the patches in 0.07, select() works a lot better but still not perfectly.

xmms+artsd is likely to play sound for quite a while, *until* I do 
something that causes another process to be scheduled, like click on the 
Mozilla window that's sitting in the background. At that point, it 
reverts to the behavior where select() doesn't return properly. And 
stays that way.

this may be due to an output underrun... or i suppose lost interrupt is 
also possible.

i think it might be wise to use 
get_available_read_data/get_free_write_space from i810_poll instead of 
dmabuf->count directly. i'll try this and see if it works...

Re: i810 audio patch

[Nathan Bryant]

Nathan Bryant wrote:

> this may be due to an output underrun... or i suppose lost interrupt 
> is also possible.
>
> i think it might be wise to use 
> get_available_read_data/get_free_write_space from i810_poll instead of 
> dmabuf->count directly. i'll try this and see if it works... 

No good. I tried the following, thinking that the underrun handling in 
get_free_write_space would help, but it does the same thing.

What interrupt do we get upon underrun, are we dropping the 
dmabuf->enable or DAC_RUNNING state when that happens?

--

static unsigned int i810_poll(struct file *file, struct 
poll_table_struct *wait){
        struct i810_state *state = (struct i810_state *)file->private_data;
        struct dmabuf *dmabuf = &state->dmabuf;
        unsigned long flags;
        unsigned int mask = 0;
        int count;

        if(!dmabuf->ready)
                return 0;
        poll_wait(file, &dmabuf->wait, wait);
        spin_lock_irqsave(&state->card->lock, flags);
        if (file->f_mode & FMODE_READ && dmabuf->enable & ADC_RUNNING) {
                count = i810_get_available_read_data(state);
                if (count >= (signed)dmabuf->userfragsize)
                        mask |= POLLIN | POLLRDNORM;
        }
        if (file->f_mode & FMODE_WRITE && dmabuf->enable & DAC_RUNNING) {
                count = i810_get_free_write_space(state);
                if (count >= (signed)dmabuf->userfragsize)
                        mask |= POLLOUT | POLLWRNORM;
        }
        spin_unlock_irqrestore(&state->card->lock, flags);
        return mask;
}

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> Probably not.  Although I did change it back but then change it in 
>> another way.  Use the attached patch to back out those changes and let 
>> me know if it works (for some reason, I doubt it).
> 
> 
> Fascinating...
> 
> Your patch to i810_poll fixes the sleep of death. and with the rest of 
> the patches in 0.07, select() works a lot better but still not perfectly.


That's because my previous patch would set the poll mask and return on 
the first call in because there was space already available.  However, 
the initial call into poll() at the vfs layer evidently sets the state 
of the process to interruptible sleep, and then poll_wait() sets it back 
to running.  So, when I skipped the call to poll_wait(), I wasn't 
manually setting the task state back to running, and as a result the 
code would return, but the calling program (artsd) would be set to a 
sleeping state in the scheduler and would never get woken back up.  An 
alternative fix would have been to set the task state to running before 
leaving the poll function manually instead of relying upon poll_wait() 
to do it for us.


> xmms+artsd is likely to play sound for quite a while, *until* I do 
> something that causes another process to be scheduled, like click on the 
> Mozilla window that's sitting in the background. At that point, it 
> reverts to the behavior where select() doesn't return properly. And 
> stays that way.

 >

> this may be due to an output underrun... or i suppose lost interrupt is 
> also possible.
> 
> i think it might be wise to use 
> get_available_read_data/get_free_write_space from i810_poll instead of 
> dmabuf->count directly. i'll try this and see if it works...
> 



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Nathan Bryant wrote:
> 
>> this may be due to an output underrun... or i suppose lost interrupt 
>> is also possible.
>>
>> i think it might be wise to use 
>> get_available_read_data/get_free_write_space from i810_poll instead of 
>> dmabuf->count directly. i'll try this and see if it works... 
> 
> 
> No good. I tried the following, thinking that the underrun handling in 
> get_free_write_space would help, but it does the same thing.
> 
> What interrupt do we get upon underrun, are we dropping the 
> dmabuf->enable or DAC_RUNNING state when that happens?


Yes, on underrun the DAC is stopped and dmabuf->enable is cleared. 
That's clearly a bug in this case.  However, it should only cause your 
problem if you are in fact getting an underrun.  Anyway, here's a 
proposed fix you can try to see if that's what's causing the problem:


> -- 
> 
> static unsigned int i810_poll(struct file *file, struct 
> poll_table_struct *wait){
>        struct i810_state *state = (struct i810_state *)file->private_data;
>        struct dmabuf *dmabuf = &state->dmabuf;
>        unsigned long flags;
>        unsigned int mask = 0;
>        int count;
> 
>        if(!dmabuf->ready)
>                return 0;
>        poll_wait(file, &dmabuf->wait, wait);
>        spin_lock_irqsave(&state->card->lock, flags);
>        if (file->f_mode & FMODE_READ && dmabuf->enable & ADC_RUNNING) {


	if (file->f_mode & FMODE_READ && ((dmabuf->enable & ADC_RUNNING)
					 || (dmabuf->trigger &
					     PCM_ENABLE_INPUT))) {


>                count = i810_get_available_read_data(state);
>                if (count >= (signed)dmabuf->userfragsize)
>                        mask |= POLLIN | POLLRDNORM;
>        }
>        if (file->f_mode & FMODE_WRITE && dmabuf->enable & DAC_RUNNING) {


	if (file->f_mode & FMODE_WRITE &&((dmabuf->enable & DAC_RUNNING)
					|| (dmabuf->trigger &
					    PCM_ENABLE_OUTPUT))) {


>                count = i810_get_free_write_space(state);
>                if (count >= (signed)dmabuf->userfragsize)
>                        mask |= POLLOUT | POLLWRNORM;
>        }
>        spin_unlock_irqrestore(&state->card->lock, flags);
>        return mask;
> }
> 



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Yes, on underrun the DAC is stopped and dmabuf->enable is cleared. 
> That's clearly a bug in this case.  However, it should only cause your 
> problem if you are in fact getting an underrun.  Anyway, here's a 
> proposed fix you can try to see if that's what's causing the problem:

[snip]

That works.

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> Yes, on underrun the DAC is stopped and dmabuf->enable is cleared. 
>> That's clearly a bug in this case.  However, it should only cause your 
>> problem if you are in fact getting an underrun.  Anyway, here's a 
>> proposed fix you can try to see if that's what's causing the problem:
> 
> 
> [snip]
> 
> That works.


OK, good.  I've fixed another bug related to MMAPed stuff (for the 
people that like to play Quake on these sound cards).  I've put up a 
0.08 version of the file on my web page.  If people could please verify 
that this version works for them I would appreciate it.  Once I've 
gotten a few "It works here" reports and no "It blew my computer up" 
reports, I'll submit it to Marcello and Linus so we can finally get 
these things working a bit more reliably.

Highlights of this release:

Fix GETOPTR and GETIPTR (thinko/typo in an if statement)
Fix i810_poll() (side effect of fixing overrun/underrun handling in 0.06)
Make handling of dmabuf->trigger more consistent throughout the source code

http://people.redhat.com/dledford/i810_audio.c.gz





-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> OK, good.  I've fixed another bug related to MMAPed stuff (for the 
> people that like to play Quake on these sound cards).  I've put up a 
> 0.08 version of the file on my web page.  If people could please 
> verify that this version works for them I would appreciate it.  Once 
> I've gotten a few "It works here" reports and no "It blew my computer 
> up" reports, I'll submit it to Marcello and Linus so we can finally 
> get these things working a bit more reliably.

artsd works in 0.08
glquake.glx doesn't: "i810_audio: drain_dac, dma timeout?"

Re: i810 audio patch

[Nathan Bryant]

Nathan Bryant wrote:

> glquake.glx doesn't: "i810_audio: drain_dac, dma timeout?" 

Turns out this has been broken for a while; stock 2.4.17pre1 has a 
broken mmap too...

Re: i810 audio patch

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

Nathan Bryant wrote:

> Nathan Bryant wrote:
> 
>> glquake.glx doesn't: "i810_audio: drain_dac, dma timeout?" 
> 
> 
> Turns out this has been broken for a while; stock 2.4.17pre1 has a 
> broken mmap too...
> 

Two questions:

1) This is a timeout on close.  Does it work up until then?

2) Does the attached patch (against the 0.08 driver) help?

-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

[-- Attachment #2: foo --]
[-- Type: text/plain, Size: 3129 bytes --]

--- i810_audio.c.08	Tue Dec  4 18:43:22 2001
+++ i810_audio.c.08b	Tue Dec  4 22:04:05 2001
@@ -664,26 +664,26 @@
 	return offset;
 }
 
-//static void resync_dma_ptrs(struct i810_state *state, int rec)
-//{
-//	struct dmabuf *dmabuf = &state->dmabuf;
-//	struct i810_channel *c;
-//	int offset;
-//
-//	if(rec) {
-//		c = dmabuf->read_channel;
-//	} else {
-//		c = dmabuf->write_channel;
-//	}
-//	if(c==NULL)
-//		return;
-//	offset = inb(state->card->iobase+c->port+OFF_CIV);
-//	if(offset == inb(state->card->iobase+c->port+OFF_LVI))
-//		offset++;
-//	offset *= dmabuf->fragsize;
-//	
-//	dmabuf->hwptr=dmabuf->swptr = offset;
-//}
+static void resync_dma_ptrs(struct i810_state *state, int rec)
+{
+	struct dmabuf *dmabuf = &state->dmabuf;
+	struct i810_channel *c;
+	int offset;
+
+	if(rec) {
+		c = dmabuf->read_channel;
+	} else {
+		c = dmabuf->write_channel;
+	}
+	if(c==NULL)
+		return;
+	offset = inb(state->card->iobase+c->port+OFF_CIV);
+	if(offset == inb(state->card->iobase+c->port+OFF_LVI))
+		offset++;
+	offset *= dmabuf->fragsize;
+	
+	dmabuf->hwptr=dmabuf->swptr = offset;
+}
 	
 /* Stop recording (lock held) */
 static inline void __stop_adc(struct i810_state *state)
@@ -1094,13 +1094,13 @@
 	return(avail);
 }
 
-static int drain_dac(struct i810_state *state, int nonblock)
+static int drain_dac(struct i810_state *state)
 {
 	DECLARE_WAITQUEUE(wait, current);
 	struct dmabuf *dmabuf = &state->dmabuf;
 	unsigned long flags;
 	unsigned long tmo;
-	int count;
+	int count, timeout=0;
 
 	if (!dmabuf->ready)
 		return 0;
@@ -1119,30 +1119,29 @@
 		if (count <= 0)
 			break;
 
-		if (signal_pending(current))
-			break;
-
-		i810_update_lvi(state,0);
 		if (dmabuf->enable != DAC_RUNNING)
 			start_dac(state);
 
-		if (nonblock) {
-			remove_wait_queue(&dmabuf->wait, &wait);
-			set_current_state(TASK_RUNNING);
-			return -EBUSY;
-		}
+		if (signal_pending(current))
+			break;
 
-		tmo = (dmabuf->dmasize * HZ) / dmabuf->rate;
-		tmo >>= 1;
+		/* set the timeout to exactly twice as long as it *should*
+		 * take for the DAC to drain the DMA buffer
+		 */
+		tmo = (count * HZ * 2) / dmabuf->rate;
 		if (!schedule_timeout(tmo ? tmo : 1) && tmo){
 			printk(KERN_ERR "i810_audio: drain_dac, dma timeout?\n");
+			timeout = 1;
 			break;
 		}
 	}
-	stop_dac(state);
-	synchronize_irq();
 	remove_wait_queue(&dmabuf->wait, &wait);
 	set_current_state(TASK_RUNNING);
+	if(count <= 0 || timeout) {
+		stop_dac(state);
+		synchronize_irq();
+		resync_dma_ptrs(state, 0 /* record channel */);
+	}
 	if (signal_pending(current))
 		return -ERESTARTSYS;
 
@@ -1651,7 +1650,7 @@
 #endif
 		if (dmabuf->enable != DAC_RUNNING || file->f_flags & O_NONBLOCK)
 			return 0;
-		drain_dac(state, 0);
+		drain_dac(state);
 		dmabuf->ready = 0;
 		dmabuf->swptr = dmabuf->hwptr = 0;
 		dmabuf->count = dmabuf->total_bytes = 0;
@@ -2324,7 +2323,7 @@
 	/* stop DMA state machine and free DMA buffers/channels */
 	if(dmabuf->enable & DAC_RUNNING ||
 	   (dmabuf->count && (dmabuf->trigger & PCM_ENABLE_OUTPUT))) {
-		drain_dac(state,0);
+		drain_dac(state);
 	}
 	if(dmabuf->enable & ADC_RUNNING) {
 		stop_adc(state);

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Two questions:
>
> 1) This is a timeout on close.  Does it work up until then? 

no - no audible output. (quake doesn't use SNDCTL_DSP_SYNC anywhere 
either so it would have to be on close)

>
>
> 2) Does the attached patch (against the 0.08 driver) help?

No - timeout printk still occurs

Re: i810 audio patch

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> Two questions:
>>
>> 1) This is a timeout on close.  Does it work up until then? 
> 
> 
> no - no audible output. (quake doesn't use SNDCTL_DSP_SYNC anywhere 
> either so it would have to be on close)
> 
>>
>>
>> 2) Does the attached patch (against the 0.08 driver) help?
> 
> 
> No - timeout printk still occurs
> 



A few more tweaks to the mmap code.  This might actually work.  It 
should apply cleanly on top of what you already have.  Let me know if it 
enables Quake sound...




-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

[-- Attachment #2: foo2 --]
[-- Type: text/plain, Size: 1720 bytes --]

--- i810_audio.c.08b	Tue Dec  4 22:04:05 2001
+++ i810_audio.c.08c	Tue Dec  4 23:24:16 2001
@@ -2005,7 +2005,7 @@
 			stop_dac(state);
 		}
 		dmabuf->trigger = val;
-		if(val & PCM_ENABLE_OUTPUT) {
+		if(val & PCM_ENABLE_OUTPUT && !(dmabuf->enable & DAC_RUNNING)) {
 			if (!dmabuf->write_channel) {
 				dmabuf->ready = 0;
 				dmabuf->write_channel = state->card->alloc_pcm_channel(state->card);
@@ -2017,15 +2017,18 @@
 			if (dmabuf->mapped) {
 				spin_lock_irqsave(&state->card->lock, flags);
 				i810_update_ptr(state);
+				resync_dma_ptrs(state,0);
 				dmabuf->count = 0;
 				dmabuf->count = i810_get_free_write_space(state);
+				dmabuf->swptr = (dmabuf->swptr + dmabuf->count) % dmabuf->dmasize;
 				__i810_update_lvi(state, 0);
 				spin_unlock_irqrestore(&state->card->lock, flags);
 			}
-			if (!dmabuf->enable && dmabuf->count > dmabuf->userfragsize)
+			if (dmabuf->count > dmabuf->userfragsize ||
+			    dmabuf->mapped)
 				start_dac(state);
 		}
-		if(val & PCM_ENABLE_INPUT) {
+		if(val & PCM_ENABLE_INPUT && !(dmabuf->enable & ADC_RUNNING)) {
 			if (!dmabuf->read_channel) {
 				dmabuf->ready = 0;
 				dmabuf->read_channel = state->card->alloc_rec_pcm_channel(state->card);
@@ -2034,9 +2037,15 @@
 			}
 			if (!dmabuf->ready && (ret = prog_dmabuf(state, 1)))
 				return ret;
-			if (!dmabuf->enable && dmabuf->count <
-			    (dmabuf->dmasize - dmabuf->userfragsize))
-				start_adc(state);
+			if (dmabuf->mapped) {
+				spin_lock_irqsave(&state->card->lock, flags);
+				i810_update_ptr(state);
+				resync_dma_ptrs(state,0);
+				dmabuf->count = 0;
+				spin_unlock_irqrestore(&state->card->lock, flags);
+			}
+			i810_update_lvi(state, 1);
+			start_adc(state);
 		}
 		return 0;
 

Re: i810 audio patch

[Nathan Bryant]

got this during a quake run with only DEBUG_INTERRUPTS enabled

it seems that we aren't making it properly chase its tail during mmap 
mode. so it runs through once and then stops. the final timeout is 
waiting for a completion that's never going to come.

Dec  4 23:33:50 lasn-001 kernel: Intel 810 + AC97 Audio, version 0.07, 
23:28:08 Dec  4 2001
Dec  4 23:33:50 lasn-001 kernel: PCI: Setting latency timer of device 
00:1f.5 to 64
Dec  4 23:33:50 lasn-001 kernel: i810: Intel ICH 82801AA found at IO 
0x2400 and 0x2000, IRQ 17
Dec  4 23:33:51 lasn-001 kernel: i810_audio: Audio Controller supports 2 
channels.
Dec  4 23:33:51 lasn-001 kernel: ac97_codec: AC97 Audio codec, id: 
0x4144:0x5360 (Analog Devices AD1885)
Dec  4 23:33:51 lasn-001 kernel: i810_audio: AC'97 codec 0 Unable to map 
surround DAC's (or DAC's not present), total channels = 2
Dec  4 23:33:51 lasn-001 kernel: i810_audio: setting clocking to 41231
Dec  4 23:34:00 lasn-001 kernel: NVRM: AGPGART: allocated 257 pages
Dec  4 23:34:00 lasn-001 kernel: NVRM: AGPGART: allocated 128 pages
Dec  4 23:34:00 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST8 DAC HWP 
16384,0,16384
Dec  4 23:34:00 lasn-001 kernel: COMP 8 )
Dec  4 23:34:01 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST8 DAC HWP 
32768,16384,16384
Dec  4 23:34:01 lasn-001 kernel: COMP 16 )
Dec  4 23:34:01 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST8 DAC HWP 
49152,32768,16384
Dec  4 23:34:01 lasn-001 kernel: COMP 24 )
Dec  4 23:34:01 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST15 DAC 
HWP 65536,49152,16384
Dec  4 23:34:01 lasn-001 kernel: COMP 32 DAC HWP 65536,65536,0
Dec  4 23:34:01 lasn-001 kernel: LVI DAC HWP 65536,65536,0
Dec  4 23:34:01 lasn-001 kernel: DCH - STOP )
Dec  4 23:34:02 lasn-001 kernel: cdrom: open failed.
Dec  4 23:34:03 lasn-001 kernel: DAC HWP 65536,65536,0
Dec  4 23:34:07 lasn-001 kernel: NVRM: AGPGART: freed 128 pages
Dec  4 23:34:07 lasn-001 kernel: NVRM: AGPGART: freed 257 pages
Dec  4 23:34:07 lasn-001 kernel: DAC HWP 65536,65536,0
Dec  4 23:34:10 lasn-001 kernel: i810_audio: drain_dac, dma timeout?

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> 
> 
> got this during a quake run with only DEBUG_INTERRUPTS enabled
> 
> it seems that we aren't making it properly chase its tail during mmap 
> mode.


Yes and no.  We aren't chasing our tail, but that's because the 
application is suppossed to call the GETOPTR ioctl in order to trigger a 
pointer update.  In short, the GETOPTR ioctl is our cue that the 
application knows about the empty space and will fill it.  We can always 
make the thing chase it's tail unconditionally, but then you risk 
playing total garbage when the application falls behind :-/

> so it runs through once and then stops. the final timeout is 
> waiting for a completion that's never going to come.
> 
> Dec  4 23:33:50 lasn-001 kernel: Intel 810 + AC97 Audio, version 0.07, 
> 23:28:08 Dec  4 2001
> Dec  4 23:33:50 lasn-001 kernel: PCI: Setting latency timer of device 
> 00:1f.5 to 64
> Dec  4 23:33:50 lasn-001 kernel: i810: Intel ICH 82801AA found at IO 
> 0x2400 and 0x2000, IRQ 17
> Dec  4 23:33:51 lasn-001 kernel: i810_audio: Audio Controller supports 2 
> channels.
> Dec  4 23:33:51 lasn-001 kernel: ac97_codec: AC97 Audio codec, id: 
> 0x4144:0x5360 (Analog Devices AD1885)
> Dec  4 23:33:51 lasn-001 kernel: i810_audio: AC'97 codec 0 Unable to map 
> surround DAC's (or DAC's not present), total channels = 2
> Dec  4 23:33:51 lasn-001 kernel: i810_audio: setting clocking to 41231
> Dec  4 23:34:00 lasn-001 kernel: NVRM: AGPGART: allocated 257 pages
> Dec  4 23:34:00 lasn-001 kernel: NVRM: AGPGART: allocated 128 pages
> Dec  4 23:34:00 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST8 DAC HWP 
> 16384,0,16384
> Dec  4 23:34:00 lasn-001 kernel: COMP 8 )
> Dec  4 23:34:01 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST8 DAC HWP 
> 32768,16384,16384
> Dec  4 23:34:01 lasn-001 kernel: COMP 16 )
> Dec  4 23:34:01 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST8 DAC HWP 
> 49152,32768,16384
> Dec  4 23:34:01 lasn-001 kernel: COMP 24 )
> Dec  4 23:34:01 lasn-001 kernel: CHANNEL NUM 1 PORT 10 IRQ ( ST15 DAC 
> HWP 65536,49152,16384
> Dec  4 23:34:01 lasn-001 kernel: COMP 32 DAC HWP 65536,65536,0
> Dec  4 23:34:01 lasn-001 kernel: LVI DAC HWP 65536,65536,0
> Dec  4 23:34:01 lasn-001 kernel: DCH - STOP )
> Dec  4 23:34:02 lasn-001 kernel: cdrom: open failed.
> Dec  4 23:34:03 lasn-001 kernel: DAC HWP 65536,65536,0


Interesting...two seconds after the buffer ran out of data, quake 
*finally* calls one of the ioctls that then calls i810_update_ptrs()...


> Dec  4 23:34:07 lasn-001 kernel: NVRM: AGPGART: freed 128 pages
> Dec  4 23:34:07 lasn-001 kernel: NVRM: AGPGART: freed 257 pages
> Dec  4 23:34:07 lasn-001 kernel: DAC HWP 65536,65536,0
> Dec  4 23:34:10 lasn-001 kernel: i810_audio: drain_dac, dma timeout?


I see no reason to drain the dac on close for mmaped stuff.  The 
application can check GETOPTR to see if the last stuff has played if it 
really cares that much.  I'm going to skip the drain_dac() calls on mmap 
from now on.  That will solve that problem (and it's also the right 
thing to do if we are chasing our tail and have set the software pointer 
to God only knows where as far as the final sounds are concerned).


> 



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> A few more tweaks to the mmap code.  This might actually work.  It 
> should apply cleanly on top of what you already have.  Let me know if 
> it enables Quake sound...

No still silence, and debug output is weirder. (still using only 
DEBUG_INTERRUPTS)

Intel 810 + AC97 Audio, version 0.07, 00:07:49 Dec  5 2001
PCI: Setting latency timer of device 00:1f.5 to 64
i810: Intel ICH 82801AA found at IO 0x2400 and 0x2000, IRQ 17
i810_audio: Audio Controller supports 2 channels.
ac97_codec: AC97 Audio codec, id: 0x4144:0x5360 (Analog Devices AD1885)
i810_audio: AC'97 codec 0 Unable to map surround DAC's (or DAC's not 
present), total channels = 2
i810_audio: setting clocking to 41231
NVRM: AGPGART: allocated 257 pages
NVRM: AGPGART: allocated 128 pages
CHANNEL NUM 1 PORT 10 IRQ ( ST7 DAC HWP 2048,2048,0
LVI DAC HWP 2048,2048,0
)
cdrom: open failed.
VFS: Disk change detected on device ide1(22,0)
DAC HWP 2048,2048,0
DAC HWP 2048,2048,0
DAC HWP 2048,2048,0
[a few hundred of the above line]
DAC HWP 2048,2048,0
NVRM: AGPGART: freed 128 pages
NVRM: AGPGART: freed 257 pages
DAC HWP 2048,2048,0
i810_audio: drain_dac, dma timeout?

Re: i810 audio patch

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> A few more tweaks to the mmap code.  This might actually work.  It 
>> should apply cleanly on top of what you already have.  Let me know if 
>> it enables Quake sound...
> 
> 
> No still silence, and debug output is weirder. (still using only 
> DEBUG_INTERRUPTS)


The attached patch should get me the debugging output I need to solve 
the problem.  If you'll get me the output, then I can likely have a 
working version in short order.




-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

[-- Attachment #2: foo3 --]
[-- Type: text/plain, Size: 4500 bytes --]

--- i810_audio.c.08c	Tue Dec  4 23:24:16 2001
+++ i810_audio.c.09	Wed Dec  5 00:19:23 2001
@@ -1,3 +1,4 @@
+
 /*
  *	Intel i810 and friends ICH driver for Linux
  *	Alan Cox <alan@redhat.com>
@@ -111,6 +112,7 @@
 //#define DEBUG
 //#define DEBUG2
 //#define DEBUG_INTERRUPTS
+#define DEBUG_MMAP
 
 #define ADC_RUNNING	1
 #define DAC_RUNNING	2
@@ -197,7 +199,7 @@
 #define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)
 
 
-#define DRIVER_VERSION "0.07"
+#define DRIVER_VERSION "0.09"
 
 /* magic numbers to protect our data structures */
 #define I810_CARD_MAGIC		0x5072696E /* "Prin" */
@@ -968,7 +970,7 @@
 	/*
 	 * two special cases, count == 0 on write
 	 * means no data, and count == dmasize
-	 * means no data on read, handle appropriately
+	 * means no data on read, handle appropriately.
 	 */
 	if(!rec && dmabuf->count == 0) {
 		outb(inb(port+OFF_CIV),port+OFF_LVI);
@@ -1008,7 +1010,7 @@
 		/* update hardware pointer */
 		hwptr = i810_get_dma_addr(state, 1);
 		diff = (dmabuf->dmasize + hwptr - dmabuf->hwptr) % dmabuf->dmasize;
-#ifdef DEBUG_INTERRUPTS
+#if defined(DEBUG_INTERRUPTS) || defined(DEBUG_MMAP)
 		printk("ADC HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
 #endif
 		dmabuf->hwptr = hwptr;
@@ -1033,7 +1035,7 @@
 		/* update hardware pointer */
 		hwptr = i810_get_dma_addr(state, 0);
 		diff = (dmabuf->dmasize + hwptr - dmabuf->hwptr) % dmabuf->dmasize;
-#ifdef DEBUG_INTERRUPTS
+#if defined(DEBUG_INTERRUPTS) || defined(DEBUG_MMAP)
 		printk("DAC HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
 #endif
 		dmabuf->hwptr = hwptr;
@@ -1168,11 +1170,11 @@
 		if(!state->dmabuf.ready)
 			continue;
 		dmabuf = &state->dmabuf;
-		if(dmabuf->enable & DAC_RUNNING)
+		if(dmabuf->enable & DAC_RUNNING) {
 			c=dmabuf->write_channel;
-		else if(dmabuf->enable & ADC_RUNNING)
+		} else if(dmabuf->enable & ADC_RUNNING) {
 			c=dmabuf->read_channel;
-		else	/* This can occur going from R/W to close */
+		} else	/* This can occur going from R/W to close */
 			continue;
 		
 		port+=c->port;
@@ -1596,7 +1598,7 @@
 	dmabuf->count = 0;
 	dmabuf->trigger = 0;
 	ret = 0;
-#ifdef DEBUG
+#ifdef DEBUG_MMAP
 	printk("i810_audio: mmap'ed %ld bytes of data space\n", size);
 #endif
 out:
@@ -1650,7 +1652,10 @@
 #endif
 		if (dmabuf->enable != DAC_RUNNING || file->f_flags & O_NONBLOCK)
 			return 0;
-		drain_dac(state);
+		if(!dmabuf->mapped)
+			drain_dac(state);
+		else
+			stop_dac(state);
 		dmabuf->ready = 0;
 		dmabuf->swptr = dmabuf->hwptr = 0;
 		dmabuf->count = dmabuf->total_bytes = 0;
@@ -1900,7 +1905,7 @@
  			abinfo.bytes = i810_get_free_write_space(state);
 		abinfo.fragments = abinfo.bytes / dmabuf->userfragsize;
 		spin_unlock_irqrestore(&state->card->lock, flags);
-#ifdef DEBUG
+#if defined(DEBUG) || defined(DEBUG_MMAP)
 		printk("SNDCTL_DSP_GETOSPACE %d, %d, %d, %d\n", abinfo.bytes,
 			abinfo.fragsize, abinfo.fragments, abinfo.fragstotal);
 #endif
@@ -1924,7 +1929,7 @@
 				__start_dac(state);
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
-#ifdef DEBUG
+#if defined(DEBUG) || defined(DEBUG_MMAP)
 		printk("SNDCTL_DSP_GETOPTR %d, %d, %d, %d\n", cinfo.bytes,
 			cinfo.blocks, cinfo.ptr, dmabuf->count);
 #endif
@@ -1941,7 +1946,7 @@
 		abinfo.fragstotal = dmabuf->userfrags;
 		abinfo.fragments = abinfo.bytes / dmabuf->userfragsize;
 		spin_unlock_irqrestore(&state->card->lock, flags);
-#ifdef DEBUG
+#if defined(DEBUG) || defined(DEBUG_MMAP)
 		printk("SNDCTL_DSP_GETISPACE %d, %d, %d, %d\n", abinfo.bytes,
 			abinfo.fragsize, abinfo.fragments, abinfo.fragstotal);
 #endif
@@ -1965,7 +1970,7 @@
 				__start_adc(state);
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
-#ifdef DEBUG
+#if defined(DEBUG) || defined(DEBUG_MMAP)
 		printk("SNDCTL_DSP_GETIPTR %d, %d, %d, %d\n", cinfo.bytes,
 			cinfo.blocks, cinfo.ptr, dmabuf->count);
 #endif
@@ -1995,7 +2000,7 @@
 	case SNDCTL_DSP_SETTRIGGER:
 		if (get_user(val, (int *)arg))
 			return -EFAULT;
-#ifdef DEBUG
+#if defined(DEBUG) || defined(DEBUG_MMAP)
 		printk("SNDCTL_DSP_SETTRIGGER 0x%x\n", val);
 #endif
 		if( !(val & PCM_ENABLE_INPUT) && dmabuf->enable == ADC_RUNNING) {
@@ -2332,7 +2337,12 @@
 	/* stop DMA state machine and free DMA buffers/channels */
 	if(dmabuf->enable & DAC_RUNNING ||
 	   (dmabuf->count && (dmabuf->trigger & PCM_ENABLE_OUTPUT))) {
-		drain_dac(state);
+		if(!dmabuf->mapped)
+			drain_dac(state);
+		else {
+			stop_dac(state);
+			synchronize_irqs();
+		}
 	}
 	if(dmabuf->enable & ADC_RUNNING) {
 		stop_adc(state);

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Interesting...two seconds after the buffer ran out of data, quake 
> *finally* calls one of the ioctls that then calls i810_update_ptrs()...

mmap followed by SETTRIGGER is part of the sound init routine, in 
WinQuake/snd_linux.c. they probably call that on startup and don't get 
around to playing any sound until the rest of the game has booted up.

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Yes and no.  We aren't chasing our tail, but that's because the 
> application is suppossed to call the GETOPTR ioctl in order to trigger 
> a pointer update.  In short, the GETOPTR ioctl is our cue that the 
> application knows about the empty space and will fill it.  We can 
> always make the thing chase it's tail unconditionally, but then you 
> risk playing total garbage when the application falls behind :-/

This is what happens under most of the drivers i've played quake with 
(under DOS too, I think). OSS and OSS/Free, and most of the other PCI 
drivers I've looked at, I believe they don't actually program the DMA 
engine during GETOPTR calls. Actually one could argue that in the case 
of quake it's better to let it chase its tail indefinitely (that's what 
they seem to have assumed when they wrote the game anyway according to 
comments in the source code) because that allows ambient sounds to 
continue uninterrupted during some slowdown, eg possible network 
congestion. (The side effect is that sometimes you end up with endlessly 
repeating weapon sound effects if you fire during congestion but of 
course that coincidence happens less often than congestion itself)

> I see no reason to drain the dac on close for mmaped stuff.  The 
> application can check GETOPTR to see if the last stuff has played if 
> it really cares that much.  I'm going to skip the drain_dac() calls on 
> mmap from now on.  That will solve that problem (and it's also the 
> right thing to do if we are chasing our tail and have set the software 
> pointer to God only knows where as far as the final sounds are 
> concerned). 

yep...

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> The attached patch should get me the debugging output I need to solve 
> the problem.  If you'll get me the output, then I can likely have a 
> working version in short order.

Here is a fix. It is diffed against your original 0.08 version, Doug.

It makes GETOPTR set the LVI to the hardware fragment preceding the one 
that's currently playing. In the case of Quake, that means Quake must 
call GETOPTR at least every 3/4ths of a DMA buffer. Hopefully that 
requirement should be relaxed enough. The alternate fix is to modify the 
completion handlers.

I don't see anything obvious in the databook about how to make the 
hardware loop infinitely without taking any additional input from us.

Comments, please.

Re: i810 audio patch

[Nathan Bryant]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

Umm, duh, here's the actual patch. :-)

Nathan Bryant wrote:

> Doug Ledford wrote:
>
>> The attached patch should get me the debugging output I need to solve 
>> the problem.  If you'll get me the output, then I can likely have a 
>> working version in short order.
>
>
> Here is a fix. It is diffed against your original 0.08 version, Doug.
>
> It makes GETOPTR set the LVI to the hardware fragment preceding the 
> one that's currently playing. In the case of Quake, that means Quake 
> must call GETOPTR at least every 3/4ths of a DMA buffer. Hopefully 
> that requirement should be relaxed enough. The alternate fix is to 
> modify the completion handlers.
>
> I don't see anything obvious in the databook about how to make the 
> hardware loop infinitely without taking any additional input from us.
>
> Comments, please.
>



[-- Attachment #2: 8n.diff --]
[-- Type: text/plain, Size: 2455 bytes --]

--- i810_audio.c.08	Tue Dec  4 19:43:21 2001
+++ linux/drivers/sound/i810_audio.c	Wed Dec  5 14:52:28 2001
@@ -197,7 +197,7 @@
 #define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)
 
 
-#define DRIVER_VERSION "0.07"
+#define DRIVER_VERSION "0.08n"
 
 /* magic numbers to protect our data structures */
 #define I810_CARD_MAGIC		0x5072696E /* "Prin" */
@@ -1651,7 +1651,10 @@
 #endif
 		if (dmabuf->enable != DAC_RUNNING || file->f_flags & O_NONBLOCK)
 			return 0;
-		drain_dac(state, 0);
+		if (!dmabuf->mapped)
+			drain_dac(state, 0);
+		else
+			stop_dac(state);
 		dmabuf->ready = 0;
 		dmabuf->swptr = dmabuf->hwptr = 0;
 		dmabuf->count = dmabuf->total_bytes = 0;
@@ -1913,16 +1916,31 @@
 		if (!dmabuf->ready && (val = prog_dmabuf(state, 0)) != 0)
 			return val;
 		spin_lock_irqsave(&state->card->lock, flags);
-		val = i810_get_free_write_space(state);
+		i810_update_ptr(state);
 		cinfo.bytes = dmabuf->total_bytes;
 		cinfo.ptr = dmabuf->hwptr;
-		cinfo.blocks = val/dmabuf->userfragsize;
+		/* blocks is only valid in mmap mode, according to API doc */
+		cinfo.blocks = 0;
 		if (dmabuf->mapped && (dmabuf->trigger & PCM_ENABLE_OUTPUT)) {
-			dmabuf->count += val;
-			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
+			/* blocks is supposed to reset to 0 on every call to GETOPTR */
+			/* hopefully nobody else destroys count so we can use it for this purpose
+			   in mmap mode */
+			cinfo.blocks = (dmabuf->dmasize - dmabuf->count - dmabuf->fragsize) /
+				dmabuf->userfragsize;
+			dmabuf->count = dmabuf->dmasize - dmabuf->fragsize;
+			dmabuf->swptr = (dmabuf->dmasize + dmabuf->hwptr - dmabuf->fragsize) 
+				% dmabuf->dmasize;
+#ifdef DEBUG
+			printk("SNDCTL_DSP_GETOPTR: calling __i810_update_lvi for swptr %d\n",
+				dmabuf->swptr);
+#endif
 			__i810_update_lvi(state, 0);
-			if (!dmabuf->enable)
+			if (!dmabuf->enable) {
+#ifdef DEBUG
+				printk("SNDCTL_DSP_GETOPTR: calling __start_dac\n");
+#endif
 				__start_dac(state);
+			}
 		}
 		spin_unlock_irqrestore(&state->card->lock, flags);
 #ifdef DEBUG
@@ -2324,7 +2342,12 @@
 	/* stop DMA state machine and free DMA buffers/channels */
 	if(dmabuf->enable & DAC_RUNNING ||
 	   (dmabuf->count && (dmabuf->trigger & PCM_ENABLE_OUTPUT))) {
-		drain_dac(state,0);
+		if (!dmabuf->mapped)
+			drain_dac(state,0);
+		else {
+			stop_dac(state);
+			synchronize_irq();
+		}
 	}
 	if(dmabuf->enable & ADC_RUNNING) {
 		stop_adc(state);

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Umm, duh, here's the actual patch. :-)
> 
> Nathan Bryant wrote:
> 
>> Doug Ledford wrote:
>>
>>> The attached patch should get me the debugging output I need to solve 
>>> the problem.  If you'll get me the output, then I can likely have a 
>>> working version in short order.
>>
>>
>>
>> Here is a fix. It is diffed against your original 0.08 version, Doug.
>>
>> It makes GETOPTR set the LVI to the hardware fragment preceding the 
>> one that's currently playing. In the case of Quake, that means Quake 
>> must call GETOPTR at least every 3/4ths of a DMA buffer. Hopefully 
>> that requirement should be relaxed enough. The alternate fix is to 
>> modify the completion handlers.
>>
>> I don't see anything obvious in the databook about how to make the 
>> hardware loop infinitely without taking any additional input from us.
>>
>> Comments, please.


If that fixes it, then the real fix is to find the bug in 
i810_get_free_Wwrite_space() and i810_update_lvi().  By using the first 
function to find the available data in the GETOPTR ioctl, then using 
update_lvi(), we *should* be setting the lvi fragment to exactly 31 sg 
segments away from the current index.  If that's failing, and we are 
instead setting lvi == civ, then things will stop and not work.


> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- i810_audio.c.08	Tue Dec  4 19:43:21 2001
> +++ linux/drivers/sound/i810_audio.c	Wed Dec  5 14:52:28 2001
> @@ -197,7 +197,7 @@
>  #define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)
>  
>  
> -#define DRIVER_VERSION "0.07"
> +#define DRIVER_VERSION "0.08n"
>  
>  /* magic numbers to protect our data structures */
>  #define I810_CARD_MAGIC		0x5072696E /* "Prin" */
> @@ -1651,7 +1651,10 @@
>  #endif
>  		if (dmabuf->enable != DAC_RUNNING || file->f_flags & O_NONBLOCK)
>  			return 0;
> -		drain_dac(state, 0);
> +		if (!dmabuf->mapped)
> +			drain_dac(state, 0);
> +		else
> +			stop_dac(state);
>  		dmabuf->ready = 0;
>  		dmabuf->swptr = dmabuf->hwptr = 0;
>  		dmabuf->count = dmabuf->total_bytes = 0;
> @@ -1913,16 +1916,31 @@
>  		if (!dmabuf->ready && (val = prog_dmabuf(state, 0)) != 0)
>  			return val;
>  		spin_lock_irqsave(&state->card->lock, flags);
> -		val = i810_get_free_write_space(state);
> +		i810_update_ptr(state);
>  		cinfo.bytes = dmabuf->total_bytes;
>  		cinfo.ptr = dmabuf->hwptr;
> -		cinfo.blocks = val/dmabuf->userfragsize;
> +		/* blocks is only valid in mmap mode, according to API doc */
> +		cinfo.blocks = 0;
>  		if (dmabuf->mapped && (dmabuf->trigger & PCM_ENABLE_OUTPUT)) {
> -			dmabuf->count += val;
> -			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
> +			/* blocks is supposed to reset to 0 on every call to GETOPTR */
> +			/* hopefully nobody else destroys count so we can use it for this purpose
> +			   in mmap mode */
> +			cinfo.blocks = (dmabuf->dmasize - dmabuf->count - dmabuf->fragsize) /
> +				dmabuf->userfragsize;
> +			dmabuf->count = dmabuf->dmasize - dmabuf->fragsize;
> +			dmabuf->swptr = (dmabuf->dmasize + dmabuf->hwptr - dmabuf->fragsize) 
> +				% dmabuf->dmasize;
> +#ifdef DEBUG
> +			printk("SNDCTL_DSP_GETOPTR: calling __i810_update_lvi for swptr %d\n",
> +				dmabuf->swptr);
> +#endif
>  			__i810_update_lvi(state, 0);
> -			if (!dmabuf->enable)
> +			if (!dmabuf->enable) {
> +#ifdef DEBUG
> +				printk("SNDCTL_DSP_GETOPTR: calling __start_dac\n");
> +#endif
>  				__start_dac(state);
> +			}
>  		}
>  		spin_unlock_irqrestore(&state->card->lock, flags);
>  #ifdef DEBUG
> @@ -2324,7 +2342,12 @@
>  	/* stop DMA state machine and free DMA buffers/channels */
>  	if(dmabuf->enable & DAC_RUNNING ||
>  	   (dmabuf->count && (dmabuf->trigger & PCM_ENABLE_OUTPUT))) {
> -		drain_dac(state,0);
> +		if (!dmabuf->mapped)
> +			drain_dac(state,0);
> +		else {
> +			stop_dac(state);
> +			synchronize_irq();
> +		}
>  	}
>  	if(dmabuf->enable & ADC_RUNNING) {
>  		stop_adc(state);
> 



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> If that fixes it, then the real fix is to find the bug in 
> i810_get_free_Wwrite_space() and i810_update_lvi(). 

It does fix it.

By the way, I'm confused about something. i810_write appears to overrun 
the end of the DMA buffer instead of wrapping around to the beginning 
when it does copy_from_user. which makes no sense to me.

so ok, the correct number is 31/32nds, not 3/4ths - not so bad.

> By using the first function to find the available data in the GETOPTR 
> ioctl, then using update_lvi(), we *should* be setting the lvi 
> fragment to exactly 31 sg segments away from the current index.  If 
> that's failing, and we are instead setting lvi == civ, then things 
> will stop and not work.

yeah, i think that's what's happening

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> If that fixes it, then the real fix is to find the bug in 
>> i810_get_free_Wwrite_space() and i810_update_lvi(). 
> 
> 
> It does fix it.
> 
> By the way, I'm confused about something. i810_write appears to overrun 
> the end of the DMA buffer instead of wrapping around to the beginning 
> when it does copy_from_user. which makes no sense to me.


This didn't used to be a bug, but it is now (all these changes and 
merges have let some thigns slip in I'm afraid, and I'm not just 
referring to the work in the last few days, I'm also referring to 
changes that were lost in the time of the S/PDIF and PM merges).

Anyway, in i810_write, the lines that read:

if (cnt > (dmabuf->dmasize - dmabuf->count))
	cnt = dmabuf->dmasize - dmabuf->count;

should read:

if (cnt > (dmabuf->dmasize - swptr))
	cnt = dmabuf->dmasize - swptr;



> so ok, the correct number is 31/32nds, not 3/4ths - not so bad.
> 
>> By using the first function to find the available data in the GETOPTR 
>> ioctl, then using update_lvi(), we *should* be setting the lvi 
>> fragment to exactly 31 sg segments away from the current index.  If 
>> that's failing, and we are instead setting lvi == civ, then things 
>> will stop and not work.
> 
> 
> yeah, i think that's what's happening


Can you add a debug check to update_lvi()?  Something like:

#ifdef DEBUG_MMAP
         if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
                 printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == LVI\n");
#endif

and see if that triggers with the original mmap code?





-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> This didn't used to be a bug, but it is now (all these changes and 
> merges have let some thigns slip in I'm afraid, and I'm not just 
> referring to the work in the last few days, I'm also referring to 
> changes that were lost in the time of the S/PDIF and PM merges). 

Agreed. There are some uglies...

>
>
> Anyway, in i810_write, the lines that read:
>
> if (cnt > (dmabuf->dmasize - dmabuf->count))
>     cnt = dmabuf->dmasize - dmabuf->count;
>
> should read:
>
> if (cnt > (dmabuf->dmasize - swptr))
>     cnt = dmabuf->dmasize - swptr; 

Okay. I thought I was being stupid. :-)

> Can you add a debug check to update_lvi()?  Something like:
>
> #ifdef DEBUG_MMAP
>         if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
>                 printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == 
> LVI\n");
> #endif
>
> and see if that triggers with the original mmap code? 

Actually, I think I *may* have found the problem, in update ptr, it 
should look like this: going to test in a moment

        /* error handling and process wake up for DAC */
        if (dmabuf->enable == DAC_RUNNING) {
                /* update hardware pointer */
                hwptr = i810_get_dma_addr(state, 0);
                diff = hwptr - dmabuf->hwptr;
                if (hwptr < dmabuf->hwptr)
                        diff += dmabuf->dmasize;
#if defined(DEBUG_INTERRUPTS) || defined(DEBUG_MMAP)
                printk("DAC HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
#endif

reason: hwptr hits 65536 which is indistinguishable from 0 in mod arithmetic

also, I ran your other DEBUG_MMAP patch and the news is that count just 
sits at 65536 ad nauseum.

Re: i810 audio patch

[Nathan Bryant]

Nathan Bryant wrote:

>
> also, I ran your other DEBUG_MMAP patch and the news is that count 
> just sits at 65536 ad nauseum.
>

this is because settrigger in 0.9 doesn't: it's setting LVI=CVI so it 
goes nowhere. this was introduced by the second patch against 0.08 that 
you sent me...

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Nathan Bryant wrote:
> 
>>
>> also, I ran your other DEBUG_MMAP patch and the news is that count 
>> just sits at 65536 ad nauseum.
>>
> 
> this is because settrigger in 0.9 doesn't: it's setting LVI=CVI so it 
> goes nowhere.


Not true.  SETTRIGGER is (generally) only called once to start things 
(and things are getting started or else it would set at 0 instead of at 
65536).  After that, SETTRIGGER isn't typically called again unless you 
are turning the device off.  It's calls to GETOPTR that are suppossed to 
keep the LVI-CIV tail chasing going on.

> this was introduced by the second patch against 0.08 that 
> you sent me...
> 



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

>
> Actually, I think I *may* have found the problem, in update ptr, it 
> should look like this: going to test in a moment
> 
>        /* error handling and process wake up for DAC */
>        if (dmabuf->enable == DAC_RUNNING) {
>                /* update hardware pointer */
>                hwptr = i810_get_dma_addr(state, 0);
>                diff = hwptr - dmabuf->hwptr;
>                if (hwptr < dmabuf->hwptr)
>                        diff += dmabuf->dmasize;


This is mathematically equivelant to what was already there.


> #if defined(DEBUG_INTERRUPTS) || defined(DEBUG_MMAP)
>                printk("DAC HWP %d,%d,%d\n", hwptr, dmabuf->hwptr, diff);
> #endif
> 
> reason: hwptr hits 65536 which is indistinguishable from 0 in mod 
> arithmetic


The mod doesn't happen until all the rest is already done, and then it's 
just cancelling out the dmabuf->dmasize portion (instead of using an if 
statement to do the same thing).


> also, I ran your other DEBUG_MMAP patch and the news is that count just 
> sits at 65536 ad nauseum.


Yes, but if I have the stuff leading up to it sitting at 65536 forever I 
might be able to diagnose the problem (without installing quake myself 
and taking the time to set it up anyway) ;-)





-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Doug Ledford wrote:

> Can you add a debug check to update_lvi()?  Something like:
>
> #ifdef DEBUG_MMAP
>         if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
>                 printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == 
> LVI\n");
> #endif
>
> and see if that triggers with the original mmap code? 

I've narrowed it down a little more.. the above does not trigger. but 
i've added some other printk's and the sequence of events goes like this:

open, mmap
SETTRIGGER : works properly with the resync_dma removed; count 
initialized to 65536
we take 6 interrupts; 4 comp + lvi + dch; count drains to 0 properly
[quake finishes initializing]
GETOPTR:
at the beginning of the call, count is 0; by the end of the call, count 
has been set to 64K

but, we take no interrupts after this point.

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> Can you add a debug check to update_lvi()?  Something like:
>>
>> #ifdef DEBUG_MMAP
>>         if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
>>                 printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == 
>> LVI\n");
>> #endif
>>
>> and see if that triggers with the original mmap code? 
> 
> 
> I've narrowed it down a little more.. the above does not trigger. but 
> i've added some other printk's and the sequence of events goes like this:
> 
> open, mmap
> SETTRIGGER : works properly with the resync_dma removed; count 
> initialized to 65536
> we take 6 interrupts; 4 comp + lvi + dch; count drains to 0 properly
> [quake finishes initializing]
> GETOPTR:
> at the beginning of the call, count is 0; by the end of the call, count 
> has been set to 64K
> 
> but, we take no interrupts after this point.
> 

In __i810_update_lvi(), try changing the line that reads:

x = (dmabuf->dmasize + dmabuf->swptr - 1) % dmabuf->dmasize;

to use - 2 instead of -1 and see if that solves the problem (I've been 
suspecting an off-by-one error, I just haven't found it for sure.  The 
whole 65536 vs. 0 could be providing the off-by-one though.)

-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

Nathan Bryant wrote:

> Doug Ledford wrote:
>
>> Can you add a debug check to update_lvi()?  Something like:
>>
>> #ifdef DEBUG_MMAP
>>         if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
>>                 printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == 
>> LVI\n");
>> #endif
>>
>> and see if that triggers with the original mmap code? 
>
>
> I've narrowed it down a little more.. the above does not trigger. but 
> i've added some other printk's and the sequence of events goes like this: 

sorry... syslog.conf was not configured for DEBUG level...

it actually looks like this

Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR: count = 0
Dec  5 18:55:17 lasn-001 kernel: GETOPTR: setting swptr 0, hwptr=65536
Dec  5 18:55:17 lasn-001 kernel: i810_audio: update_lvi - CIV == LVI
Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR 65536, 4, 65536, 65536
Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR: count = 65536
Dec  5 18:55:17 lasn-001 kernel: DAC HWP 65536,65536,0
Dec  5 18:55:17 lasn-001 kernel: GETOPTR: setting swptr 0, hwptr=65536
Dec  5 18:55:17 lasn-001 kernel: i810_audio: update_lvi - CIV == LVI
Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR 65536, 0, 65536, 65536
Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR: count = 65536
Dec  5 18:55:17 lasn-001 kernel: DAC HWP 65536,65536,0
Dec  5 18:55:17 lasn-001 kernel: GETOPTR: setting swptr 0, hwptr=65536
Dec  5 18:55:17 lasn-001 kernel: i810_audio: update_lvi - CIV == LVI
Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR 65536, 0, 65536, 65536
Dec  5 18:55:17 lasn-001 kernel: SNDCTL_DSP_GETOPTR: count = 65536
Dec  5 18:55:17 lasn-001 kernel: DAC HWP 65536,65536,0

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

OK, on my site there is now a 0.10 version of the driver.  This one 
stands a reasonable chance of working in mmap mode.  Please give it a 
try and let me know what happens (see my comments in __i810_update_lvi() 
to see what I think the actual problem is).

http://people.redhat.com/dledford/i810_audio.c.gz




-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

Doug Ledford wrote:

> Nathan Bryant wrote:
>
> OK, on my site there is now a 0.10 version of the driver.  This one 
> stands a reasonable chance of working in mmap mode.  Please give it a 
> try and let me know what happens (see my comments in 
> __i810_update_lvi() to see what I think the actual problem is).
>
> http://people.redhat.com/dledford/i810_audio.c.gz
>
>
>
>
some fixes needed, attached


[-- Attachment #2: foo --]
[-- Type: text/plain, Size: 1926 bytes --]

--- /home/nbryant/i810_audio.c	Wed Dec  5 19:49:38 2001
+++ drivers/sound/i810_audio.c	Wed Dec  5 19:49:00 2001
@@ -953,7 +953,7 @@
 	 * the CIV value to the next sg segment to be played so that when
 	 * we call start_{dac,adc}, things will operate properly
 	 */
-	if (!dmabuf->enabled)
+	if (!dmabuf->enable)
 		outb((inb(port+OFF_CIV)+1)&31, port+OFF_CIV);
 
 	/* swptr - 1 is the tail of our transfer */
@@ -961,7 +961,7 @@
 	x /= dmabuf->fragsize;
 #ifdef DEBUG_MMAP
 	if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
-		printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == LVI\n");
+		printk(KERN_DEBUG "i810_audio: update_lvi - CIV == LVI\n");
 #endif
 	outb(x, port+OFF_LVI);
 }
@@ -1124,7 +1124,6 @@
 	if(count <= 0 || timeout) {
 		stop_dac(state);
 		synchronize_irq();
-		resync_dma_ptrs(state, 0 /* record channel */);
 	}
 	if (signal_pending(current))
 		return -ERESTARTSYS;
@@ -1595,7 +1594,7 @@
 static int i810_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct i810_state *state = (struct i810_state *)file->private_data;
-	struct i810_channel *c;
+	struct i810_channel *c = NULL;
 	struct dmabuf *dmabuf = &state->dmabuf;
 	unsigned long flags;
 	audio_buf_info abinfo;
@@ -1629,10 +1628,12 @@
 			c = dmabuf->read_channel;
 			__stop_adc(state);
 		}
-		outb(2, state->card->iobase+c->port+OFF_CR);   /* reset DMA machine */
-		outl(virt_to_bus(&c->sg[0]), state->card->iobase+c->port+OFF_BDBAR);
-		outb(0, state->card->iobase+c->port+OFF_CIV);
-		outb(0, state->card->iobase+c->port+OFF_LVI);
+		if (c != NULL) {
+			outb(2, state->card->iobase+c->port+OFF_CR);   /* reset DMA machine */
+			outl(virt_to_bus(&c->sg[0]), state->card->iobase+c->port+OFF_BDBAR);
+			outb(0, state->card->iobase+c->port+OFF_CIV);
+			outb(0, state->card->iobase+c->port+OFF_LVI);
+		}
 
 		spin_unlock_irqrestore(&state->card->lock, flags);
 		synchronize_irq();

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> Nathan Bryant wrote:
>>
>> OK, on my site there is now a 0.10 version of the driver.  This one 
>> stands a reasonable chance of working in mmap mode.  Please give it a 
>> try and let me know what happens (see my comments in 
>> __i810_update_lvi() to see what I think the actual problem is).
>>
>> http://people.redhat.com/dledford/i810_audio.c.gz
>>
>>
>>
>>
> some fixes needed, attached


Thanks, applied.  (I haven't been compile testing these patches because 
I would have to set up another compile environment just to get it to 
work, my current ones won't work for it)





-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

as evidenced by,


        /* if we are currently stopped, then our CIV is actually set to our
         * *last* sg segment and we are ready to wrap to the next.  However,
         * if we set our LVI to the last sg segment, then it won't wrap to
         * the next sg segment, it won't even get a start.  So, instead, 
when
         * we are stopped, we set both the LVI value and also we increment
         * the CIV value to the next sg segment to be played so that when
         * we call start_{dac,adc}, things will operate properly
         */
        if (!dmabuf->enable) {
#ifdef DEBUG_MMAP
                printk("fnord? %d\n", inb(port+OFF_CIV));
#endif
                /* maybe we have to increment LVI to make room before 
incrementing CIV,
                   (databook says CELV isn't cleared until new val written
                    to LVI.)
                   we'll set LVI where we really want it down below. */
                //outb((inb(port+OFF_LVI)+1)&31, port+OFF_LVI);
                newciv = (inb(port+OFF_CIV)+1)&31;
                outb(newciv, port+OFF_CIV);
                for (x = 0; inb(port+OFF_CIV) != newciv && x< 5000000; x++);
                if (x==5000000) printk("foo! civ != %d\n", newciv);
        }

and

Dec  5 21:34:32 lasn-001 kernel: foo! civ != 1

CIV doesn't seem to respond to writes on my chipset, at least not in 
this situation. you'll note that nowhere in the driver are we 
initializing CIV to anything other than 0 anyway.

two alternatives I can think of:

first fix:
set LVI to desired value minus one SG
start_dac (which should have side effect of incrementing CIV by one)
increment LVI to desired value

2nd fix:
back off and perform DMA reset.

3rd fix (really fix 1a ;)
just don't allow use of the last SG when restarting

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> as evidenced by,
> 
> 
>        /* if we are currently stopped, then our CIV is actually set to our
>         * *last* sg segment and we are ready to wrap to the next.  However,
>         * if we set our LVI to the last sg segment, then it won't wrap to
>         * the next sg segment, it won't even get a start.  So, instead, 
> when
>         * we are stopped, we set both the LVI value and also we increment
>         * the CIV value to the next sg segment to be played so that when
>         * we call start_{dac,adc}, things will operate properly
>         */
>        if (!dmabuf->enable) {
> #ifdef DEBUG_MMAP
>                printk("fnord? %d\n", inb(port+OFF_CIV));
> #endif
>                /* maybe we have to increment LVI to make room before 
> incrementing CIV,
>                   (databook says CELV isn't cleared until new val written
>                    to LVI.)
>                   we'll set LVI where we really want it down below. */
>                //outb((inb(port+OFF_LVI)+1)&31, port+OFF_LVI);
>                newciv = (inb(port+OFF_CIV)+1)&31;
>                outb(newciv, port+OFF_CIV);
>                for (x = 0; inb(port+OFF_CIV) != newciv && x< 5000000; x++);
>                if (x==5000000) printk("foo! civ != %d\n", newciv);
>        }
> 
> and
> 
> Dec  5 21:34:32 lasn-001 kernel: foo! civ != 1
> 
> CIV doesn't seem to respond to writes on my chipset, at least not in 
> this situation. you'll note that nowhere in the driver are we 
> initializing CIV to anything other than 0 anyway.
> 
> two alternatives I can think of:
> 
> first fix:
> set LVI to desired value minus one SG
> start_dac (which should have side effect of incrementing CIV by one)
> increment LVI to desired value
> 
> 2nd fix:
> back off and perform DMA reset.
> 
> 3rd fix (really fix 1a ;)
> just don't allow use of the last SG when restarting
> 

Try this out:
         /* if we are currently stopped, then our CIV is actually set to our
          * *last* sg segment and we are ready to wrap to the next. 
However,
          * if we set our LVI to the last sg segment, then it won't wrap to
          * the next sg segment, it won't even get a start.  So, 
instead, when
          * we are stopped, we set both the LVI value and also we increment
          * the CIV value to the next sg segment to be played so that when
          * we call start_{dac,adc}, things will operate properly
          */
         if (!dmabuf->enable) {
                 outb((inb(port+OFF_CIV)+1)&31, port+OFF_LVI);
                 if(rec) {
                         __start_adc(state);
                         while( !(inb(port + OFF_CR) & ((1<<4) | (1<<2))) )
                                 barrier();
                 } else {
                         __start_dac(state);
                         while( !(inb(port + OFF_CR) & ((1<<4) | (1<<2))) )
                                 barrier();
                 }
         }



-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch

[Nathan Bryant]

ok that works

Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> ok that works
> 

OK, I've reloaded a new 0.10 version of the driver to my web site.  At 
this point, I'm calling this driver done until I hear otherwise.  If you 
have problems, or suspect you might have problems with this driver, or 
just want to make sure you *won't* have problems with this driver, test 
and speak now or possibly end up having to hack it yourself later ;-)

http://people.redhat.com/dledford/i810_audio.c.gz

-- 

  Doug Ledford <dledford@redhat.com>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems

Re: i810 audio patch (it _works_ for me :)

[Mario Mikocevic]

Hi,

> OK, I've reloaded a new 0.10 version of the driver to my web site.  At 
> this point, I'm calling this driver done until I hear otherwise.  If you 
> have problems, or suspect you might have problems with this driver, or 
> just want to make sure you *won't* have problems with this driver, test 
> and speak now or possibly end up having to hack it yourself later ;-)
> 
> http://people.redhat.com/dledford/i810_audio.c.gz

Well, 0.10 works for me ->

Dec  6 16:04:27 videotest kernel: Intel 810 + AC97 Audio, version 0.10, 16:01:33 Dec  6 2001
Dec  6 16:04:27 videotest kernel: PCI: Found IRQ 9 for device 00:1f.5
Dec  6 16:04:27 videotest kernel: PCI: Sharing IRQ 9 with 00:1f.3
Dec  6 16:04:27 videotest kernel: PCI: Setting latency timer of device 00:1f.5 to 64
Dec  6 16:04:27 videotest kernel: i810: Intel ICH2 found at IO 0xef00 and 0xe800, IRQ 9
Dec  6 16:04:27 videotest kernel: i810_audio: Audio Controller supports 6 channels.
Dec  6 16:04:27 videotest kernel: ac97_codec: AC97 Audio codec, id: 0x4144:0x5360 (Analog Devices AD1885)
Dec  6 16:04:27 videotest kernel: i810_audio: AC'97 codec 0 Unable to map surround DAC's (or DAC's not present), total channels = 2
Dec  6 16:04:27 videotest kernel: i810_audio: setting clocking to 41319

RealProducer finally works on _all_ codecs and _all_ bitrates, no more oops,
xmms works and artsd also works.


thanks,

-- 
Mario Mikočević (Mozgy)
mozgy at hinet dot hr
My favourite FUBAR ...

Re: i810 audio patch

[Pascal Junod]

Hi !

I'm trying to get sound working on my Sony Vaio PCG-GR114SK laptop with
0.10 patch:

kernel: Intel 810 + AC97 Audio, version 0.10, 17:31:53 Dec  6 2001
kernel: PCI: Setting latency timer of device 00:1f.5 to 64
kernel: i810: Intel ICH3 found at IO 0x18c0 and 0x1c00, IRQ 9
kernel: i810_audio: Audio Controller supports 6 channels.
kernel: ac97_codec: AC97 Audio codec, id: 0x4144:0x5348 (Analog Devices AD1881A)
kernel: i810_audio: AC'97 codec 0 Unable to map surround DAC's (or DAC's not present), total channels = 2
kernel: ac97_codec: AC97 Modem codec, id: 0x4358:0x5421 (Unknown)
kernel: i810_audio: timed out waiting for codec 1 analog ready

I have tried alsa drivers 0.9.0beta10 with a little bit more success: I
get some sound, but it's looping forever.

A+

Pascal

-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Pascal Junod, pascal.junod@epfl.ch                                 *
* Security and Cryptography Laboratory (LASEC)                       *
* INF 240, EPFL, CH-1015 Lausanne, Switzerland  ++41 (0)21 693 76 17 *
* Montétan 13, CH-1004 Lausanne                 ++41 (0)79 617 28 57 *
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: i810 audio patch

[Nathan Bryant]

That timeout is for the modem codec. The primary, audio codec seems to have initialized properly.

> kernel: Intel 810 + AC97 Audio, version 0.10, 17:31:53 Dec  6 2001
> kernel: PCI: Setting latency timer of device 00:1f.5 to 64
> kernel: i810: Intel ICH3 found at IO 0x18c0 and 0x1c00, IRQ 9
> kernel: i810_audio: Audio Controller supports 6 channels.
> kernel: ac97_codec: AC97 Audio codec, id: 0x4144:0x5348 (Analog Devices AD1881A)
> kernel: i810_audio: AC'97 codec 0 Unable to map surround DAC's (or DAC's not present), total channels = 2
> kernel: ac97_codec: AC97 Modem codec, id: 0x4358:0x5421 (Unknown)
> kernel: i810_audio: timed out waiting for codec 1 analog ready