Subject: kern/18745: autri0: Codec timeout. Busy reading AC'97 codec
To: None <gnats-bugs@gnats.netbsd.org>
From: Klaus Heinz <k.heinz.okt.zwei@onlinehome.de>
List: netbsd-bugs
Date: 10/21/2002 00:09:37
>Number:         18745
>Category:       kern
>Synopsis:       autri0: Codec timeout. Busy reading AC'97 codec
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 20 17:17:01 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Klaus Heinz
>Release:        NetBSD 1.6
>Organization:
none
>Environment:
System: NetBSD/i386 1.6. For more details please ask Hubert Feyrer.


>Description:
	There was a discussion on tech-kern about problems with the ac97
	code and the autri driver. A fix was proposed but noone seemed
	to care enough to send-pr this, so I do it now.

	Original problem description by
	hubert.feyrer at informatik.fh-regensburg.de :

--------
When playing a movie in mplayer, it works fine for about 15 minutes.
After that, the movie plays in 1 second skips only. What seems to happen
the mplayer process gets stuck while interacting with /dev/audio.  
dmesg shows lots of these errors then: 
  
        autri0: Codec timeout. Busy reading AC'97 codec.
  
I'm using NetBSD 1.6/i386, and also tried the latest autri* code from
-current - no change. ^C'ing mplayer and restarting doesn't help, the
movie still skips. Rebooting fixes things.   
  
One interesting thing is that I can still play mp3s fine (with mpg123)
when the hangs happen in mplayer.

Does anyone have a clue on what to do?

 - Hubert
--------



	Answer from grendel at zeitbombe.org:

--------
On Tue, 10 Sep 2002, Hubert Feyrer wrote:
  
>       autri0: Codec timeout. Busy reading AC'97 codec.
> 
> I'm using NetBSD 1.6/i386, and also tried the latest autri* code from
> -current - no change. ^C'ing mplayer and restarting doesn't help, the
> movie still skips. Rebooting fixes things.
  
mplayer polls the mixer to determine the volume over and over.  The autri
chip can't keep up and goes nuts.  You can fix the ac97.c file so it
doesn't read from the device unnecessarily.
http://www.zeitbombe.org/ac97.diff
That's against OpenBSD, but it's simple enough to do by hand if it doesn't
apply.

> One interesting thing is that I can still play mp3s fine (with mpg123)
> when the hangs happen in mplayer.

mpg123 doesn't touch the mixer.  if you start up aumix it would start
stuttering again.

--------



	hubert.feyrer at informatik.fh-regensburg.de:
	
--------
On Mon, 9 Sep 2002, tedu wrote: 
> mplayer polls the mixer to determine the volume over and over.  The
> autri 
> chip can't keep up and goes nuts.  You can fix the ac97.c file so it
> doesn't read from the device unnecessarily.
> http://www.zeitbombe.org/ac97.diff
> That's against OpenBSD, but it's simple enough to do by hand if it
> doesn't
> apply. 

I tried the patch below, but it seems that ac97_write() never gets invoked
with ac97_writethrough=1. While playing a divx w/ mplayer, there don't
seem to be any calls to ac97_write(), at least no printf traces are in
dmesg. I get a few at bootup, but none when using mplayer. No idea what's
going on, I don't know the guts of the driver.

I tried looking at the OpenBSD ac97.c, but it seems quite different from
ours. ;(

  - Hubert
Index: ac97.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/ac97.c,v
retrieving revision 1.24
diff -u -r1.24 ac97.c
--- ac97.c      2002/01/23 14:50:45     1.24
+++ ac97.c      2002/09/11 16:47:34
@@ -392,6 +392,8 @@  
 #define DPRINTFN(n,x)
 #endif 
 
+static int ac97_writethrough;
+       
 void
 ac97_read(as, reg, val)
        struct ac97_softc *as;
@@ -418,6 +420,11 @@
        u_int8_t reg;
        u_int16_t val;
 {
+       printf ("ac97_write: ac97_writethrough=%d, val=%d, reg=%d\n",
+ac97_writethrough, val, reg); /*HF*/
+
+       if(ac97_writethrough == 0 && as->shadow_reg[reg >> 1] == val) {
+               return 0;
+       }

        as->shadow_reg[reg >> 1] = val;

@@ -433,10 +440,12 @@

        memset(as->shadow_reg, 0, sizeof(as->shadow_reg));

+       ac97_writethrough = 1;
        for (idx = 0; idx < SOURCE_INFO_SIZE; idx++) {
                si = &source_info[idx];
                ac97_write(as, si->reg, si->default_value);
        }
+       ac97_writethrough = 0;
 }

 void
@@ -447,10 +456,12 @@
        int idx;
        const struct ac97_source_info *si;

+       ac97_writethrough = 1;
        for (idx = 0; idx < SOURCE_INFO_SIZE; idx++) {
                si = &source_info[idx];
                ac97_write(as, si->reg, as->shadow_reg[si->reg >> 1]);
        }
+       ac97_writethrough = 0;
 }

 int
--------




	grendel at zeitbombe.org:

--------
On Wed, 11 Sep 2002, Hubert Feyrer wrote:

> I tried the patch below, but it seems that ac97_write() never gets
> invoked
> with ac97_writethrough=1. While playing a divx w/ mplayer, there don't  
> seem to be any calls to ac97_write(), at least no printf traces are in
> dmesg. I get a few at bootup, but none when using mplayer. No idea
> what's
> going on, I don't know the guts of the driver.

The problem isn't actually mplayer writing to the mixer, it's reading.
Whenever possible, you want ac97 to read the shadow registers.

I think you picked the wrong part of the patch to try. :)  You want the
one-liner
-       if (((as->host_flags & AC97_HOST_DONT_READ) &&
+       if ((
in ac97_read().

The real fix is to have as->host_flags set to AC97_HOST_DONT_READ, but the
autri driver doesn't set any flags.  I was interested in making sound work
more than doing it right. :(


ted
--------



        hubert.feyrer at informatik.fh-regensburg.de:

--------
On Wed, 11 Sep 2002, tedu wrote:
> I think you picked the wrong part of the patch to try. :)  You want
> the
> one-liner
> -       if (((as->host_flags & AC97_HOST_DONT_READ) &&
> +       if (( 
> in ac97_read().
> 
> The real fix is to have as->host_flags set to AC97_HOST_DONT_READ, but
> the
> autri driver doesn't set any flags.  I was interested in making sound
> work
> more than doing it right. :(
  
That indeed did the trick, thanks a lot!
Anyone want to comment on committing that change?


 - Hubert
--------


	grendel at zeitbombe.org:
	
--------
The correct fix for this issue is included below.  There's some fuzz
(openBSD patch) but it applies clean.
  
--- autri.c.orig        Sun Sep 29 05:14:28 2002
+++ autri.c     Sun Sep 29 05:19:17 2002
@@ -97,6 +97,7 @@
 int    autri_read_codec(void *sc, u_int8_t a, u_int16_t *d);
 int    autri_write_codec(void *sc, u_int8_t a, u_int16_t d);
 void   autri_reset_codec(void *sc);
+enum ac97_host_flags   autri_flags_codec(void *);
  
 void autri_powerhook(int why,void *addr);
 int  autri_init(void *sc);
@@ -456,6 +457,12 @@
                    sc->sc_dev.dv_xname);
 }

+enum ac97_host_flags
+autri_flags_codec(void *v)
+{
+       return (AC97_HOST_DONT_READ);
+}
+
 /*
  *
  */
@@ -563,6 +570,7 @@
        codec->host_if.reset = autri_reset_codec;
        codec->host_if.read = autri_read_codec;
        codec->host_if.write = autri_write_codec;
+       codec->host_if.flags = autri_flags_codec;

        if ((r = ac97_attach(&codec->host_if)) != 0) {
                printf("%s: can't attach codec (error 0x%X)\n",
--------


>How-To-Repeat:
	I am not sure about this, more input from Hubert is required.
>Fix:
	As far as I understood it, the last message from
	grendel at zeitbombe.org (see above) contains a fix for the problem.
>Release-Note:
>Audit-Trail:
>Unformatted: