[alsa-devel] Should we really use POS_FIX_VIACOMBO for AMD/ATI chipsets?
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 :-/
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.
Takashi
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.
Thanks for remark!
Takashi
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@canonical.com Cc: stable@kernel.org Signed-off-by: Takashi Iwai tiwai@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;
On 2011-05-20 15:46, 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.
Hey, you're faster than I am :-) Ok, so there actually is a FIFO register (I read the wrong table). But the position buffer is dead and always read as zero.
I did some debug print which is pasted here: http://paste.ubuntu.com/610637/
If LPIB only gives you occasional recording noises, perhaps we need a new type of position-fix that adds some margin (e g round down to nearest multiple of fifo-size, then subtract one fifo-size or something)?
At Fri, 20 May 2011 16:51:13 +0200, David Henningsson wrote:
On 2011-05-20 15:46, 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.
Hey, you're faster than I am :-) Ok, so there actually is a FIFO register (I read the wrong table). But the position buffer is dead and always read as zero.
I did some debug print which is pasted here: http://paste.ubuntu.com/610637/
If LPIB only gives you occasional recording noises, perhaps we need a new type of position-fix that adds some margin (e g round down to nearest multiple of fifo-size, then subtract one fifo-size or something)?
The latter sounds more appropriate (although it's just my wild guess).
We are also checking the position via jiffies, so big jumps should have been filtered out. The problem is more about the small amount of difference from the real position.
thanks,
Takashi
participants (2)
-
David Henningsson -
Takashi Iwai