[alsa-devel] [PATCH 4/6] ASoC Blackfin: fix bug - Audio Latency on AD1981 with MMAP enabled

[Takashi Iwai]

Hi,

I already pulled it, but below are some comments.

At Mon, 29 Sep 2008 14:44:31 +0100,
Mark Brown wrote:
> 
> From: Cliff Cai <cliff.cai at analog.com>
> 
> With MMAP enabled (DMA mode) on the AD1981, there is +/- 250ms of delay between
> writing data to alsa and audio starts coming out of the AD1981.
> 
> Copy more data to local buffer before starting DMA
> 
> Signed-off-by: Cliff Cai <cliff.cai at analog.com>
> Signed-off-by: Bryan Wu <cooloney at kernel.org>
> Signed-off-by: Mark Brown <broonie at opensource.wolfsonmicro.com>
>  	snd_pcm_period_elapsed(pcm);
>  }
> @@ -114,6 +123,10 @@ static int bf5xx_pcm_hw_params(struct snd_pcm_substream *substream,
>  
>  static int bf5xx_pcm_hw_free(struct snd_pcm_substream *substream)
>  {
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +	memset(runtime->dma_area, 0, runtime->buffer_size);

Missing indentation here.

> @@ -164,8 +172,12 @@ static int bf5xx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
>  	pr_debug("%s enter\n", __func__);
>  	switch (cmd) {
>  	case SNDRV_PCM_TRIGGER_START:
> -		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +			bf5xx_mmap_copy(substream, runtime->period_size);
> +			snd_pcm_period_elapsed(substream);

Calling this in the trigger callback is pretty tricky.
I guess this could be implemented without involving
snd_pcm_period_elapsed(), but rather only inside this machine driver,
if it's only the question of dummy data handling at the beginning.


> @@ -237,6 +249,21 @@ static int bf5xx_pcm_open(struct snd_pcm_substream *substream)
>  	return ret;
>  }
>  
> +static int bf5xx_pcm_close(struct snd_pcm_substream *substream)
> +{
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	struct sport_device *sport = runtime->private_data;
> +
> +	pr_debug("%s enter\n", __func__);
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		sport->once = 0;
> +		memset(sport->tx_dma_buf, 0, runtime->buffer_size * sizeof(struct ac97_frame));
> +	} else
> +		memset(sport->rx_dma_buf, 0, runtime->buffer_size * sizeof(struct ac97_frame));
> +
> +	return 0;
> +}

Better to clear buffers in hw_free in both playback and capture cases
(unless you have any issue against it).
The hw_free is supposed to be called anyway before closing, so you
don't need the same thing in multiple places.

As mentioned, I already pulled the patches, so please make changes
against the latest tree.


thanks,

Takashi