Re: [PATCH 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 3/23/17 8:32 AM, jeeja.kp@intel.com wrote:
> From: Hardik T Shah <hardik.t.shah@intel.com>
>
> As per hardware recommendation, for every capture stream completion
> following operations need to be done in order to reflect the actual
> data that is received in position buffer.
>
> 1. Wait for 20us before reading the DMA position in buffer once the
> interrupt is generated for stream completion.
> 2. Read any of the register to flush the DMA position value. This is
> dummy read operation.
>
> Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> index ef440d8..149221c 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -21,6 +21,7 @@
>
>  #include <linux/pci.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/delay.h>
>  #include <sound/pcm_params.h>
>  #include <sound/soc.h>
>  #include "skl.h"
> @@ -1063,13 +1064,30 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer
>  	 * HAD space reflects the actual data that is transferred.
>  	 * Use the position buffer for capture, as DPIB write gets
>  	 * completed earlier than the actual data written to the DDR.
> +	 *
> +	 * For capture stream following workaround is required to fix the
> +	 * incorrect position reporting.
> +	 *
> +	 * 1. Wait for 20us before reading the DMA position in buffer once
> +	 * the interrupt is generated for stream completion.

is this really 20us regardless of the sampling frequency/channel count?
20us is one sample at 48kHz so wondering how generic this work-around is...

> +	 * 2. Read DPIB register to flush the DMA position value. This dummy
> +	 * read is required to flush DMA position value.
> +	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
> +	 * or greater than period boundary.
>  	 */
> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE +
>  				(AZX_REG_VS_SDXDPIB_XINTERVAL *
>  				hdac_stream(hstream)->index));
> -	else
> +	} else {
> +		udelay(20);
> +		readl(ebus->bus.remap_addr +
> +				AZX_REG_VS_SDXDPIB_XBASE +
> +				(AZX_REG_VS_SDXDPIB_XINTERVAL *
> +				 hdac_stream(hstream)->index));
>  		pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
> +	}
>
>  	if (pos >= hdac_stream(hstream)->bufsize)
>  		pos = 0;
>