[alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Ughreja, Rakesh A]

>-----Original Message-----
>From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-bounces at alsa-
>project.org] On Behalf Of Takashi Iwai
>Sent: Friday, March 24, 2017 11:50 AM
>To: Ughreja, Rakesh A <rakesh.a.ughreja at intel.com>
>Cc: alsa-devel at alsa-project.org; R, Dharageswari <dharageswari.r at intel.com>;
>Patches Audio <patches.audio at intel.com>; Shah, Hardik T
><hardik.t.shah at intel.com>; broonie at kernel.org; Girdwood, Liam R
><liam.r.girdwood at intel.com>; Kp, Jeeja <jeeja.kp at intel.com>
>Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA
>position reporting for capture stream
>
>On Fri, 24 Mar 2017 19:43:52 +0100,
>Ughreja, Rakesh A wrote:
>>
>>
>>
>> >-----Original Message-----
>> >From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-
>bounces at alsa-
>> >project.org] On Behalf Of Takashi Iwai
>> >Sent: Friday, March 24, 2017 11:38 AM
>> >To: Kp, Jeeja <jeeja.kp at intel.com>
>> >Cc: alsa-devel at alsa-project.org; R, Dharageswari
><dharageswari.r at intel.com>;
>> >Patches Audio <patches.audio at intel.com>; Shah, Hardik T
>> ><hardik.t.shah at intel.com>; broonie at kernel.org; Girdwood, Liam R
>> ><liam.r.girdwood at intel.com>
>> >Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA
>> >position reporting for capture stream
>> >
>> >On Fri, 24 Mar 2017 18:40:32 +0100,
>> >jeeja.kp at intel.com wrote:
>> >>
>> >> From: Hardik T Shah <hardik.t.shah at 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.
>> >
>> >Are these workarounds needed for the legacy driver?
>> >If yes, which chips require it?
>> >
>>
>> Yes, these are needed in legacy driver as well.
>> From SKL and BXT onwards, it is needed.
>
>OK, thanks for confirmation.
>
>Now, from what I read in the above, is the workaround required *only*
>after the interrupt is generated?  20us delay isn't so cheap, and we
>tend to inquire PCM positions often.  If the workaround is needed only
>after the PCM period elapse, we can set some flag in the irq handler
>and apply the workaround only when necessary.
>

Yes, Takashi the workaround is required only in the period elapsed
interrupt. In some cases the DMA Position updates are delayed and so
when the period elapsed interrupt occurs the wait_for_avail thinks that
one period worth of data is not available and so returns only on the 
next period elapsed interrupt. This creates problem for 2 periods
playback/capture streams.

So even in the period elapsed interrupt the wait is required only if the
position is less than the period boundary.

>
>Takashi
>
>>
>> >
>> >thanks,
>> >
>> >Takashi
>> >
>> >>
>> >> Signed-off-by: Dharageswari R <dharageswari.r at intel.com>
>> >> Signed-off-by: Hardik T Shah <hardik.t.shah at intel.com>
>> >> Signed-off-by: Jeeja KP <jeeja.kp at intel.com>
>> >> ---
>> >>  sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++--
>> >>  1 file changed, 21 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/sound/soc/intel/skylake/skl-pcm.c
>> >b/sound/soc/intel/skylake/skl-pcm.c
>> >> index ef440d8..1823197 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,31 @@ 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 as update happens
>> >> +	 * on the HDA frame boundary i.e. 20.833uSec.
>> >> +	 * 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;
>> >> --
>> >> 2.5.0
>> >>
>> >> _______________________________________________
>> >> Alsa-devel mailing list
>> >> Alsa-devel at alsa-project.org
>> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>> >>
>> >_______________________________________________
>> >Alsa-devel mailing list
>> >Alsa-devel at alsa-project.org
>> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>_______________________________________________
>Alsa-devel mailing list
>Alsa-devel at alsa-project.org
>http://mailman.alsa-project.org/mailman/listinfo/alsa-devel