linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [rfc] i810_audio: offset LVI from CIV to avoid stalled start
@ 2005-01-17 18:37 John W. Linville
  2005-01-17 18:46 ` [patch 2.4.29-rc1] " John W. Linville
  2005-01-17 20:39 ` [rfc] " Herbert Xu
  0 siblings, 2 replies; 9+ messages in thread
From: John W. Linville @ 2005-01-17 18:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: herbert, jgarzik

"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 herbert@gondor.apana.org.au 
#   [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 herbert@gondor.apana.org.au +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))) ) ;
 		}
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch 2.4.29-rc1] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-17 18:37 [rfc] i810_audio: offset LVI from CIV to avoid stalled start John W. Linville
@ 2005-01-17 18:46 ` John W. Linville
  2005-01-17 22:54   ` Thomas Voegtle
  2005-01-17 20:39 ` [rfc] " Herbert Xu
  1 sibling, 1 reply; 9+ messages in thread
From: John W. Linville @ 2005-01-17 18:46 UTC (permalink / raw)
  To: linux-kernel, herbert, jgarzik

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(+)

--- linux-test/drivers/sound/i810_audio.c.orig	2005-01-14 17:21:20.000000000 -0500
+++ linux-test/drivers/sound/i810_audio.c	2005-01-17 13:11:31.000000000 -0500
@@ -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))))
 			;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfc] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-17 18:37 [rfc] i810_audio: offset LVI from CIV to avoid stalled start John W. Linville
  2005-01-17 18:46 ` [patch 2.4.29-rc1] " John W. Linville
@ 2005-01-17 20:39 ` Herbert Xu
  2005-01-17 21:44   ` John W. Linville
  1 sibling, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2005-01-17 20:39 UTC (permalink / raw)
  To: linux-kernel, jgarzik; +Cc: John W. Linville

On Mon, Jan 17, 2005 at 01:37:08PM -0500, John W. Linville wrote:
> "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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfc] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-17 20:39 ` [rfc] " Herbert Xu
@ 2005-01-17 21:44   ` John W. Linville
  2005-01-17 23:23     ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: John W. Linville @ 2005-01-17 21:44 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-kernel, jgarzik

On Tue, Jan 18, 2005 at 07:39:30AM +1100, Herbert Xu wrote:
> On Mon, Jan 17, 2005 at 01:37:08PM -0500, John W. Linville wrote:
> > "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
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch 2.4.29-rc1] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-17 18:46 ` [patch 2.4.29-rc1] " John W. Linville
@ 2005-01-17 22:54   ` Thomas Voegtle
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Voegtle @ 2005-01-17 22:54 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-kernel, herbert, jgarzik

On Mon, 17 Jan 2005, John W. Linville wrote:

> 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

-- 
 Thomas Vögtle    email: thomas@voegtle-clan.de
 ----- http://www.voegtle-clan.de/thomas ------

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfc] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-17 21:44   ` John W. Linville
@ 2005-01-17 23:23     ` Herbert Xu
  2005-01-18 18:07       ` John W. Linville
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2005-01-17 23:23 UTC (permalink / raw)
  To: linux-kernel, jgarzik

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

On Mon, Jan 17, 2005 at 04:44:22PM -0500, John W. Linville wrote:
> 
> 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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 867 bytes --]

===== sound/oss/i810_audio.c 1.76 vs edited =====
--- 1.76/sound/oss/i810_audio.c	2005-01-08 16:44:18 +11:00
+++ edited/sound/oss/i810_audio.c	2005-01-18 10:20:42 +11:00
@@ -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);
 }
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfc] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-17 23:23     ` Herbert Xu
@ 2005-01-18 18:07       ` John W. Linville
  2005-01-18 22:42         ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: John W. Linville @ 2005-01-18 18:07 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-kernel, jgarzik

On Tue, Jan 18, 2005 at 10:23:23AM +1100, Herbert Xu wrote:
> On Mon, Jan 17, 2005 at 04:44:22PM -0500, John W. Linville wrote:
> > 
> > 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
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfc] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-18 18:07       ` John W. Linville
@ 2005-01-18 22:42         ` Herbert Xu
  2005-01-19  8:59           ` Thomas Voegtle
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2005-01-18 22:42 UTC (permalink / raw)
  To: linux-kernel, jgarzik

On Tue, Jan 18, 2005 at 01:07:47PM -0500, John W. Linville wrote:
> 
> 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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfc] i810_audio: offset LVI from CIV to avoid stalled start
  2005-01-18 22:42         ` Herbert Xu
@ 2005-01-19  8:59           ` Thomas Voegtle
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Voegtle @ 2005-01-19  8:59 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-kernel, jgarzik

On Wed, 19 Jan 2005, Herbert Xu wrote:

> On Tue, Jan 18, 2005 at 01:07:47PM -0500, John W. Linville wrote:
> > 
> > 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.


--- linux-2.6.11-rc1-bk6/sound/oss/i810_audio.c.old	2005-01-19 09:47:20.438345600 +0100
+++ linux-2.6.11-rc1-bk6/sound/oss/i810_audio.c	2005-01-19 09:48:43.618700264 +0100
@@ -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))))
 			;






-- 
 Thomas Vögtle    email: thomas@voegtle-clan.de
 ----- http://www.voegtle-clan.de/thomas ------

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2005-01-19  9:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-17 18:37 [rfc] i810_audio: offset LVI from CIV to avoid stalled start John W. Linville
2005-01-17 18:46 ` [patch 2.4.29-rc1] " John W. Linville
2005-01-17 22:54   ` Thomas Voegtle
2005-01-17 20:39 ` [rfc] " Herbert Xu
2005-01-17 21:44   ` John W. Linville
2005-01-17 23:23     ` Herbert Xu
2005-01-18 18:07       ` John W. Linville
2005-01-18 22:42         ` Herbert Xu
2005-01-19  8:59           ` Thomas Voegtle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).