| Message ID | 1490377234-30257-10-git-send-email-jeeja.kp@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fdd85a054b850db43c6abe39c1da28b581be5e93 |
| Headers | show |
On Fri, 24 Mar 2017 18:40:32 +0100, 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. Are these workarounds needed for the legacy driver? If yes, which chips require it? thanks, Takashi > > 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 | 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@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
>-----Original Message----- >From: alsa-devel-bounces@alsa-project.org [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@intel.com> >Cc: alsa-devel@alsa-project.org; R, Dharageswari <dharageswari.r@intel.com>; >Patches Audio <patches.audio@intel.com>; Shah, Hardik T ><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R ><liam.r.girdwood@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@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. > >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. > >thanks, > >Takashi > >> >> 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 | 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@alsa-project.org >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >> >_______________________________________________ >Alsa-devel mailing list >Alsa-devel@alsa-project.org >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Fri, 24 Mar 2017 19:43:52 +0100, Ughreja, Rakesh A wrote: > > > > >-----Original Message----- > >From: alsa-devel-bounces@alsa-project.org [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@intel.com> > >Cc: alsa-devel@alsa-project.org; R, Dharageswari <dharageswari.r@intel.com>; > >Patches Audio <patches.audio@intel.com>; Shah, Hardik T > ><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R > ><liam.r.girdwood@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@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. > > > >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@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 | 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@alsa-project.org > >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > >> > >_______________________________________________ > >Alsa-devel mailing list > >Alsa-devel@alsa-project.org > >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
>-----Original Message----- >From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- >project.org] On Behalf Of Takashi Iwai >Sent: Friday, March 24, 2017 11:50 AM >To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com> >Cc: alsa-devel@alsa-project.org; R, Dharageswari <dharageswari.r@intel.com>; >Patches Audio <patches.audio@intel.com>; Shah, Hardik T ><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R ><liam.r.girdwood@intel.com>; Kp, Jeeja <jeeja.kp@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@alsa-project.org [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@intel.com> >> >Cc: alsa-devel@alsa-project.org; R, Dharageswari ><dharageswari.r@intel.com>; >> >Patches Audio <patches.audio@intel.com>; Shah, Hardik T >> ><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R >> ><liam.r.girdwood@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@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. >> > >> >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@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 | 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@alsa-project.org >> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >> >> >> >_______________________________________________ >> >Alsa-devel mailing list >> >Alsa-devel@alsa-project.org >> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >> >_______________________________________________ >Alsa-devel mailing list >Alsa-devel@alsa-project.org >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote: > >> >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. Hi Takashi, So we need this for 2 periods when the device is in irq mode. Not for other cases. ie SKL_PLUS.. But have you seen any user reports on this till now.
On Mon, 27 Mar 2017 12:29:54 +0200, Vinod Koul wrote: > > On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote: > > >> >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. > > Hi Takashi, > > So we need this for 2 periods when the device is in irq mode. Not for other > cases. ie SKL_PLUS.. Yeah, thanks. I'd cook a couple of patches to do that for the legacy driver. But I still wonder whether the wait is always needed. Judging from the description, does the discrepancy of posbuf read happen *only* when the DMA position goes across the BD boundary? Or does it happen at any time? When you trace, you can see that the apps frequently inquires the position. So, an unconditional wait should be really avoided. > But have you seen any user reports on this till now. I've seen some bug reports mentioning about crackling audio capture on SKL (I forgot URLs). It might be triggered by that. thanks, Takashi
On Mon, Mar 27, 2017 at 03:12:02PM +0200, Takashi Iwai wrote: > On Mon, 27 Mar 2017 12:29:54 +0200, > Vinod Koul wrote: > > > > On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote: > > > >> >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. > > > > Hi Takashi, > > > > So we need this for 2 periods when the device is in irq mode. Not for other > > cases. ie SKL_PLUS.. > > Yeah, thanks. I'd cook a couple of patches to do that for the legacy > driver. But I still wonder whether the wait is always needed. > > Judging from the description, does the discrepancy of posbuf read > happen *only* when the DMA position goes across the BD boundary? > Or does it happen at any time? I think it can happen anytime, but the impact is not felt unless we have a 2 period case. The update is in-flight, so read returns a value lesser than period boundary. We will sleep till next period ie next wake, which results in overrun. For more than 2 periods it doesn't impact much as the overrun case should not happen > When you trace, you can see that the apps frequently inquires the > position. So, an unconditional wait should be really avoided. If they enquire after a while then we should be okay, but if they are written nicely with good power behaviour then we may have issue, as writes are typically done after period boundaries. > > > But have you seen any user reports on this till now. > > I've seen some bug reports mentioning about crackling audio capture on > SKL (I forgot URLs). It might be triggered by that. Quiet possible..
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;