i810_audio: offset LVI from CIV to avoid stalled start

RFC In review John W. Linville 3 participants 9 messages Jan 17, 2005 – Jan 19, 2005 Sound

Share & tools

John W. Linville

author

2005-01-17 18:45 UTC #0 lore raw

"Some" OSS applications have trouble with later versions of the i810_audio driver. Wolfenstein Enemy Territory from idSoftware is one such application.

I did a little legwork in BK and tracked-down the exact change which caused the break. The changelog comments are dismissive to the original code. However, I find that recreating a patch equivalent to what was removed restores sound to the game.

Anyone have any suggestions for a patch that a) works; and, b) accounts for the concerns expressed in the changelog?

John

P.S. Here is the problematic patch: (DO NOT TRY TO APPLY THIS)

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2004/05/11 03:51:54-04:00 [email protected]
# [sound i810] remove bogus CIV_TO_LVI
#

#   This patch removes a pair of bogus LVI assignments.  The explanation in
#   the comment is wrong because the value of PCIB tells the hardware that
#   the DMA buffer can be processed even if LVI == CIV.

#
# Setting LVI to CIV + 1 causes overruns when with short writes
# (something that vmware is very fond of).
#

# drivers/sound/i810_audio.c
#   2004/05/11 03:51:52-04:00 [email protected] +0 -10
#   [sound i810] remove bogus CIV_TO_LVI
#   
#   This patch removes a pair of bogus LVI assignments.  The explanation in
#   the comment is wrong because the value of PCIB tells the hardware that
#   the DMA buffer can be processed even if LVI == CIV.
#   
#   Setting LVI to CIV + 1 causes overruns when with short writes
#   (something that vmware is very fond of).
# 
diff -Nru a/drivers/sound/i810_audio.c b/drivers/sound/i810_audio.c
--- a/drivers/sound/i810_audio.c	2005-01-14 16:20:27 -05:00
+++ b/drivers/sound/i810_audio.c	2005-01-14 16:20:27 -05:00
@@ -1079,25 +1079,15 @@
 	else
 		port += dmabuf->write_channel->port;

- /* 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 && dmabuf->ready) {
if(rec && dmabuf->count < dmabuf->dmasize &&
(dmabuf->trigger & PCM_ENABLE_INPUT))
{
- CIV_TO_LVI(port, 1);
__start_adc(state);
while( !(inb(port + OFF_CR) & ((1<<4) | (1<<2))) ) ;
} else if (!rec && dmabuf->count &&
(dmabuf->trigger & PCM_ENABLE_OUTPUT))
{
- CIV_TO_LVI(port, 1);
__start_dac(state);
while( !(inb(port + OFF_CR) & ((1<<4) | (1<<2))) ) ;
}

Signature · 2 lines

John W. Linville
[email protected]

John W. Linville

author

2005-01-17 18:46 UTC #1 lore raw

PATCH 1/1 i810_audio: offset LVI from CIV to avoid stalled start

Offset LVI past CIV when starting DAC/ADC in order to prevent stalled start.
---
Here is the (working) patch I'm using against a later 2.4. This makes sound work fine with Enemy Territory.

 drivers/sound/i810_audio.c |   10 ++++++++++
 1 files changed, 10 insertions(+)

drivers/sound/i810_audio.c renamed from drivers/sound/i810_audio.c.orig +10 -0

@@ -1081,10 +1081,20 @@ static void __i810_update_lvi(struct i81
 	if (count < fragsize)
 		return;
 
+	/* 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, things will operate properly
+	 */
 	if (!dmabuf->enable && dmabuf->ready) {
 		if (!(dmabuf->trigger & trigger))
 			return;
 
+		CIV_TO_LVI(state->card, port, 1);
+
 		start(state);
 		while (!(I810_IOREADB(state->card, port + OFF_CR) & ((1<<4) | (1<<2))))
 			;

Thomas Voegtle

2005-01-17 22:59 UTC #2 lore raw

Quoted 6 lines · John W. Linville

Offset LVI past CIV when starting DAC/ADC in order to prevent stalled start.
---
Here is the (working) patch I'm using against a later 2.4. This makes sound work fine with Enemy Territory.

This patch, hand-modified for 2.6.10 enabled sound again with i810 and quake3. (Q3 1.32b linux-i386 Nov 14 2002)

The problem was that in the opening of quake3 sound was there and then suddenly the sound stopped.

Thank you John for tracking down this problem.

Thomas

Signature · 2 lines

 Thomas Vögtle    email: [email protected]
 ----- http://www.voegtle-clan.de/thomas ------

Herbert Xu

2005-01-17 20:43 UTC #3 lore raw

In reply to John W. Linville

Quoted 3 lines · John W. Linville

"Some" OSS applications have trouble with later versions of the i810_audio driver. Wolfenstein Enemy Territory from idSoftware is one such application.

Would it be possible to create a minimal program (something that triggers a start through __i810_update_lvi) that reproduces this problem?

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

John W. Linville

author

2005-01-17 21:45 UTC #4 lore raw

Quoted 7 lines · Herbert Xu

Quoted 3 lines · John W. Linville
"Some" OSS applications have trouble with later versions of the i810_audio driver. Wolfenstein Enemy Territory from idSoftware is one such application.
Would it be possible to create a minimal program (something that triggers a start through __i810_update_lvi) that reproduces this problem?

Possible is, of course, somewhat relative... :-) I'm not immediately equipped to produce such a program.

Enemy Territory is available for free (as in beer) download from www.enemy-territory.com. Sound plays almost immediately once the game is started.

Is this sufficient?

John

Signature · 2 lines

John W. Linville
[email protected]

Herbert Xu

2005-01-17 23:33 UTC #5 lore raw

Quoted 6 lines · John W. Linville

Enemy Territory is available for free (as in beer) download from www.enemy-territory.com. Sound plays almost immediately once the game is started.
Is this sufficient?

Sure, I don't mind trying it out :)

In the mean time, does this patch fix your problem as well?

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

===== sound/oss/i810_audio.c 1.76 vs edited =====

sound/oss/i810_audio.c +6 -3

@@ -1196,18 +1196,21 @@
 	if (count < fragsize)
 		return;
 
+	/* MASKP2(swptr, fragsize) - 1 is the tail of our transfer */
+	x = MODULOP2(MASKP2(dmabuf->swptr, fragsize) - 1, dmabuf->dmasize);
+	x >>= dmabuf->fragshift;
+
 	if (!dmabuf->enable && dmabuf->ready) {
 		if (!(dmabuf->trigger & trigger))
 			return;
 
+		I810_IOWRITEB(x, state->card, port + OFF_LVI);
 		start(state);
 		while (!(I810_IOREADB(state->card, port + OFF_CR) & ((1<<4) | (1<<2))))
 			;
+		return;
 	}
 
-	/* MASKP2(swptr, fragsize) - 1 is the tail of our transfer */
-	x = MODULOP2(MASKP2(dmabuf->swptr, fragsize) - 1, dmabuf->dmasize);
-	x >>= dmabuf->fragshift;
 	I810_IOWRITEB(x, state->card, port + OFF_LVI);
 }

John W. Linville

author

2005-01-18 18:08 UTC #6 lore raw

Quoted 7 lines · Herbert Xu

Quoted 2 lines · John W. Linville
Enemy Territory is available for free (as in beer) download from
Sure, I don't mind trying it out :)
In the mean time, does this patch fix your problem as well?

Herbert,

No, that does not fix it. :-( In fact, it doesn't seem to alter the problem at all...

John

Signature · 2 lines

John W. Linville
[email protected]

Herbert Xu

2005-01-18 22:45 UTC #7 lore raw

Quoted 3 lines · John W. Linville

No, that does not fix it. :-( In fact, it doesn't seem to alter the problem at all...

OK. In that case I agree with your patch. The overruns that I attributed to it were probably caused by other bugs that's been fixed since.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Thomas Voegtle

2005-01-19 09:03 UTC #8 lore raw

Quoted 11 lines · Herbert Xu

Quoted 3 lines · John W. Linville
No, that does not fix it. :-( In fact, it doesn't seem to alter the problem at all...
OK. In that case I agree with your patch. The overruns that I attributed to it were probably caused by other bugs that's been fixed since.
Cheers,

Here is the same patch against 2.6.11-rc1-bk6. Works for me.

sound/oss/i810_audio.c renamed from sound/oss/i810_audio.c.old +10 -0

@@ -1196,10 +1196,20 @@
 	if (count < fragsize)
 		return;
 
+	/* 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, things will operate properly
+	 */
 	if (!dmabuf->enable && dmabuf->ready) {
 		if (!(dmabuf->trigger & trigger))
 			return;
 
+		CIV_TO_LVI(state->card, port, 1);
+
 		start(state);
 		while (!(I810_IOREADB(state->card, port + OFF_CR) & ((1<<4) | (1<<2))))
 			;

Signature · 2 lines

 Thomas Vögtle    email: [email protected]
 ----- http://www.voegtle-clan.de/thomas ------