Subject: auich quirk for ICH5 audio
To: None <current-users@NetBSD.org>
From: Thomas Bieg <tomsbsd03@t-email.de>
List: current-users
Date: 09/04/2003 20:56:22
I tested my shiny (and blinking...) new mainboard (MSI 865PE Neo2-FIS2R)
under a -current kernel (sources from 20030830) to see what's supported.

Besides other things, I got a recognized but non-working AC-97 codec,
like this (no audio attachment):

> auich0 at pci0 dev 31 function 5: i82801EB (ICH5) AC-97 Audio
> auich0: interrupting at apic 2 int 17 (irq 12)
> auich0: auich_reset_codec: time out

(According to MSI, it's a C-Media 9739A audio codec.)

A look into src/sys/dev/pci/auich.c showed that there's already a quirk
entry present (QUIRK_IGNORE_CODEC_READY) which supposedly should take
care of this (but obviously doesn't).

Changing it to QUIRK_IGNORE_CODEC_READY_MAYBE did the trick, and I got:

> auich0 at pci0 dev 31 function 5: i82801EB (ICH5) AC-97 Audio
> auich0: interrupting at apic 2 int 17 (irq 12)
> auich0: auich_reset_codec: time out
> auich0: 20 bit precision support
> auich0: auich_reset_codec: time out
> auich0: CMI97 codec; no 3D stereo
> auich0: double rate output, S/PDIF, center DAC, surround DAC, LFE DAC
> audio0 at auich0: full duplex, independent

(A short test with mpg123 afterwards went fine.)

But I don't think that's the proper solution. If I understand things
correctly, shouldn't QUIRK_IGNORE_CODEC_READY already imply ..._MAYBE?

If so, then a better fix would be:


--- auich.c.orig	Wed Aug 20 03:11:03 2003
+++ auich.c	Thu Sep  4 14:27:50 2003
@@ -477,13 +477,13 @@
  	DPRINTF(ICH_DEBUG_DMA, ("auich_attach: lists %p %p %p\n",
  	    sc->dmalist_pcmo, sc->dmalist_pcmi, sc->dmalist_mici));

  	/* Reset codec and AC'97 */
  	auich_reset_codec(sc);
  	status = bus_space_read_4(sc->iot, sc->aud_ioh, ICH_GSTS);
-	if (!(status & ICH_PCR)) { /* reset failure */
+	if (!sc->sc_ignore_codecready && !(status & ICH_PCR)) { /* reset failure */
  			/* It never return ICH_PCR in some cases */
  		if (d->quirks & QUIRK_IGNORE_CODEC_READY_MAYBE) {
  			sc->sc_ignore_codecready = TRUE;
  		} else {
  			return;
  		}


The next thing may be questionable, but, in my opinion, the timeout
message in auich_reset_codec() should also be suppressed when it's
already known that this will happen.

Here's my suggestion: (previous diff continued)


@@ -604,13 +604,13 @@
  	control |= (control & ICH_CRESET) ? ICH_WRESET : ICH_CRESET;
  	bus_space_write_4(sc->iot, sc->aud_ioh, ICH_GCTRL, control);

  	for (i = 500000; i-- &&
  	       !(bus_space_read_4(sc->iot, sc->aud_ioh, ICH_GSTS) & ICH_PCR);
  	     DELAY(1));					/*       or ICH_SCR? */
-	if (i <= 0)
+	if (!sc->sc_ignore_codecready && (i <= 0))
  		printf("%s: auich_reset_codec: time out\n", sc->sc_dev.dv_xname);
  }

  int
  auich_open(void *v, int flags)
  {


This conforms to the other occasions in auich.c where ICH_PCR is
checked.


Does anyone like to pick this up or should I file a PR on this?


There is one more thing I am wondering about: Does the delay in
auich_reset_codec() (see above) really have to be that large (500000)?
No doubt that it's needed (considering
http://mail-index.netbsd.org/port-i386/2002/03/18/0000.html), but half
a second seems much to me, in times where CPU speeds are measured in
GHz...


Regards,
Tom