[alsa-devel] Should we really use POS_FIX_VIACOMBO for AMD/ATI chipsets?

[Takashi Iwai]

At Fri, 20 May 2011 16:19:57 +0200,
Takashi Iwai wrote:
> 
> At Fri, 20 May 2011 15:46:37 +0200,
> Takashi Iwai wrote:
> > 
> > At Fri, 20 May 2011 15:33:56 +0200,
> > David Henningsson wrote:
> > > 
> > > I'm far from sure, but I could be on to something...
> > > 
> > > Looking at the code for azx_via_get_position, it seems that for 
> > > recording streams, it accesses a register at position 
> > > VIA_IN_STREAM0_FIFO_SIZE_OFFSET = 0x90. Looking at the datasheet for 
> > > SB700, that register is not even present. This does not seem right to me.
> > > 
> > > Could it be that we should use POS_FIX_LPIB by default for AMD/ATI 
> > > chipsets instead? It seems more reasonable to me.
> > > In addition, I looked through some of the commits for quirking to 
> > > POS_FIX_LPIB and the four I checked were all ATI chipsets.
> > > 
> > > So; I've tried adding:
> > > options snd-hda-intel position_fix=1
> > > to /etc/modprobe.d/alsa-base.conf
> > > 
> > > ...to my machine with the infamous [1002:4383] controller (and 
> > > rebooted), and the few times I've tested, recording worked successfully 
> > > with pulseaudio. So could you please follow this up with testing on your 
> > > machines to see if we have actually managed to solve this long-standing bug?
> > > 
> > > The only annoying thing is that I didn't realise this earlier :-/
> > 
> > Hmm, I took this as default as it seems fixing the issue on machines,
> > indeed.  Actually I had to use this option first for removing
> > recording noises on AMD Hudson.  But, maybe I need to double-check
> > again after the snoop bit and other workarounds.
> > 
> > The FIFO size might got wrong on AMD.  In that case, the driver
> > calculates as if FIFO = 0.  But, it can still help for correcting the
> > position via position-buffer vs LPIB comparison.
> > 
> > For the real check, we should put some debug prints on real machines.
> 
> Actually the values read there look not working with posbuf.
> Let's fix them to use POS_FIX_LPIB now.

The patch is below, commit 50e3bbf9898840eead86f90a43b3625a2b2f4112
as below.


Takashi

---
Subject: [PATCH] ALSA: hda - Use LPIB for ATI/AMD chipsets as default

ATI and AMD chipsets seem not providing the proper position-buffer
information, and it also doesn't provide FIFO register required by
VIACOMBO fix.  It's better to use LPIB for these.

Reported-by: David Henningsson <david.henningsson at canonical.com>
Cc: <stable at kernel.org>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/pci/hda/hda_intel.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 0c1996d..43a0367 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2367,9 +2367,16 @@ static int __devinit check_position_fix(struct azx *chip, int fix)
 	/* Check VIA/ATI HD Audio Controller exist */
 	switch (chip->driver_type) {
 	case AZX_DRIVER_VIA:
-	case AZX_DRIVER_ATI:
 		/* Use link position directly, avoid any transfer problem. */
 		return POS_FIX_VIACOMBO;
+	case AZX_DRIVER_ATI:
+		/* ATI chipsets don't work well with position-buffer */
+		return POS_FIX_LPIB;
+	case AZX_DRIVER_GENERIC:
+		/* AMD chipsets also don't work with position-buffer */
+		if (chip->pci->vendor == PCI_VENDOR_ID_AMD)
+			return POS_FIX_LPIB;
+		break;
 	}
 
 	return POS_FIX_AUTO;
-- 
1.7.4.2