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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 24 Mar 2017 19:43:52 +0100,
Ughreja, Rakesh A wrote:
> 
> 
> 
> >-----Original Message-----
> >From: alsa-devel-bounces@xxxxxxxxxxxxxxxx [mailto:alsa-devel-bounces@alsa-
> >project.org] On Behalf Of Takashi Iwai
> >Sent: Friday, March 24, 2017 11:38 AM
> >To: Kp, Jeeja <jeeja.kp@xxxxxxxxx>
> >Cc: alsa-devel@xxxxxxxxxxxxxxxx; R, Dharageswari <dharageswari.r@xxxxxxxxx>;
> >Patches Audio <patches.audio@xxxxxxxxx>; Shah, Hardik T
> ><hardik.t.shah@xxxxxxxxx>; broonie@xxxxxxxxxx; Girdwood, Liam R
> ><liam.r.girdwood@xxxxxxxxx>
> >Subject: Re:  [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@xxxxxxxxx wrote:
> >>
> >> From: Hardik T Shah <hardik.t.shah@xxxxxxxxx>
> >>
> >> 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.


Takashi

> 
> >
> >thanks,
> >
> >Takashi
> >
> >>
> >> Signed-off-by: Dharageswari R <dharageswari.r@xxxxxxxxx>
> >> Signed-off-by: Hardik T Shah <hardik.t.shah@xxxxxxxxx>
> >> Signed-off-by: Jeeja KP <jeeja.kp@xxxxxxxxx>
> >> ---
> >>  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@xxxxxxxxxxxxxxxx
> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >>
> >_______________________________________________
> >Alsa-devel mailing list
> >Alsa-devel@xxxxxxxxxxxxxxxx
> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux