[alsa-devel] [PATCH] snd_dma_pointer workaround for chipsets with buggy DMA

[Takashi Iwai]

At Sun, 11 Oct 2009 12:48:00 +0200,
Krzysztof Helt wrote:
> 
> From: Krzysztof Helt <krzysztof.h1 at wp.pl>
> 
> The chipsets with the isa_dma_bridge_buggy set do not stop DMA during
> DMA counter reads. The DMA counter is read in two 8-bit read steps
> on x86 platform. Sometimes, such reads happen during higher byte
> change so the lower byte is already decremented (rolled over) but
> the higher byte is not. It introduces an error that position is
> moved 256 bytes ahead of the true position. Thus, the next DMA
> position read can return a lower value then the previous read.
> If the DMA position is decreased (reversed) the ALSA subsystem is
> tricked into the playback underrun error and resets the playback.
> It results in a "pop" during a playback.
> 
> Work around the issue by reading the counter twice and choosing a higher
> value.
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1 at wp.pl>
> ---
> This issue can be tested with the pcm tool from the alsa-lib's test package. 
> The command "pcm -c 2 -b 350000 -p 5800" can easily trigger the original error
> every ten seconds on my system.
> This work around greatly reduces the underrun errors for ogg123 and mpg321 
> on VIA Apollo (586) chipset + K6-2 533 MHz (from every few seconds to one per about
> 15 minutes - few songs).

Applied now.  The only affecting change is the call of enable_dma(),
but this should be OK.


thanks,

Takashi

> 
> 
>  sound/core/isadma.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/core/isadma.c b/sound/core/isadma.c
> index 79f0f16..950e19b 100644
> --- a/sound/core/isadma.c
> +++ b/sound/core/isadma.c
> @@ -85,16 +85,24 @@ EXPORT_SYMBOL(snd_dma_disable);
>  unsigned int snd_dma_pointer(unsigned long dma, unsigned int size)
>  {
>  	unsigned long flags;
> -	unsigned int result;
> +	unsigned int result, result1;
>  
>  	flags = claim_dma_lock();
>  	clear_dma_ff(dma);
>  	if (!isa_dma_bridge_buggy)
>  		disable_dma(dma);
>  	result = get_dma_residue(dma);
> +	/*
> +	 * HACK - read the counter again and choose higher value in order to
> +	 * avoid reading during counter lower byte roll over if the
> +	 * isa_dma_bridge_buggy is set.
> +	 */
> +	result1 = get_dma_residue(dma);
>  	if (!isa_dma_bridge_buggy)
>  		enable_dma(dma);
>  	release_dma_lock(flags);
> +	if (unlikely(result < result1))
> +		result = result1;
>  #ifdef CONFIG_SND_DEBUG
>  	if (result > size)
>  		snd_printk(KERN_ERR "pointer (0x%x) for DMA #%ld is greater than transfer size (0x%x)\n", result, dma, size);
> -- 
> 1.6.0.3
> 
> 
> 
> ---------------------------------------------------------------
> Zobacz jak pracuje sie na wysokosciach.
> Kliknij >>> http://link.interia.pl/f2384
>