| Message ID | 87bml4c5e0.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Oct 19, 2017 at 3:15 AM, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > From: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > > SYS/RT/Audio DMAC includes independent data buffers for reading > and writing. Therefore, the read transfer counter and write transfer > counter have different values. > TCR indicates read counter, and TCRB indicates write counter. > The relationship is like below. > > TCR TCRB > [SOURCE] -> [DMAC] -> [SINK] > > In the MEM_TO_DEV direction, what really matters is how much data has > been written to the device. If the DMA is interrupted between read and > write, then, the data doesn't end up in the destination, so shouldn't > be counted. TCRB is thus the register we should use in this cases. > > In the DEV_TO_MEM direction, the situation is more complex. Both the > read and write side are important. What matters from a data consumer > point of view is how much data has been written to memory. > On the other hand, if the transfer is interrupted between read and > write, we'll end up losing data. It can also be important to report. > > In the MEM_TO_MEM direction, what matters is of course how much data > has been written to memory from data consumer point of view. > Here, because read and write have independent data buffers, it will > take a while for TCR and TCRB to become equal. Thus we should check > TCRB in this case, too. > > Thus, all cases we should check TCRB instead of TCR. > > Without this patch, Sound Capture has noise after PluseAudio support > (= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because > the recorder will use wrong residue counter which indicates transferred > from sound device, but in reality the data was not yet put to memory > and recorder will record it. > > Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > [Kuninori: added detail information in log] > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > v2 -> v3 > > - Code is back to same as v1 > - log has more detail explanation > - From: is back to Yokoyama-san again Thanks for the update! Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Morimoto-san, Thank you for the patch. On Thursday, 19 October 2017 04:15:13 EEST Kuninori Morimoto wrote: > From: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > > SYS/RT/Audio DMAC includes independent data buffers for reading > and writing. Therefore, the read transfer counter and write transfer > counter have different values. > TCR indicates read counter, and TCRB indicates write counter. > The relationship is like below. > > TCR TCRB > [SOURCE] -> [DMAC] -> [SINK] > > In the MEM_TO_DEV direction, what really matters is how much data has > been written to the device. If the DMA is interrupted between read and > write, then, the data doesn't end up in the destination, so shouldn't > be counted. TCRB is thus the register we should use in this cases. > > In the DEV_TO_MEM direction, the situation is more complex. Both the > read and write side are important. What matters from a data consumer > point of view is how much data has been written to memory. > On the other hand, if the transfer is interrupted between read and > write, we'll end up losing data. It can also be important to report. > > In the MEM_TO_MEM direction, what matters is of course how much data > has been written to memory from data consumer point of view. > Here, because read and write have independent data buffers, it will > take a while for TCR and TCRB to become equal. Thus we should check > TCRB in this case, too. > > Thus, all cases we should check TCRB instead of TCR. > > Without this patch, Sound Capture has noise after PluseAudio support > (= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because > the recorder will use wrong residue counter which indicates transferred > from sound device, but in reality the data was not yet put to memory > and recorder will record it. > > Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > [Kuninori: added detail information in log] > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > v2 -> v3 > > - Code is back to same as v1 > - log has more detail explanation > - From: is back to Yokoyama-san again > > drivers/dma/sh/rcar-dmac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index 2b2c7db..50c4950 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -1310,7 +1310,7 @@ static unsigned int rcar_dmac_chan_get_residue(struct > rcar_dmac_chan *chan, } > > /* Add the residue for the current chunk. */ > - residue += rcar_dmac_chan_read(chan, RCAR_DMATCR) << desc->xfer_shift; > + residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift; > > return residue; > }
On Thu, Oct 19, 2017 at 01:15:13AM +0000, Kuninori Morimoto wrote: > From: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > > SYS/RT/Audio DMAC includes independent data buffers for reading > and writing. Therefore, the read transfer counter and write transfer > counter have different values. > TCR indicates read counter, and TCRB indicates write counter. > The relationship is like below. > > TCR TCRB > [SOURCE] -> [DMAC] -> [SINK] > > In the MEM_TO_DEV direction, what really matters is how much data has > been written to the device. If the DMA is interrupted between read and > write, then, the data doesn't end up in the destination, so shouldn't > be counted. TCRB is thus the register we should use in this cases. > > In the DEV_TO_MEM direction, the situation is more complex. Both the > read and write side are important. What matters from a data consumer > point of view is how much data has been written to memory. > On the other hand, if the transfer is interrupted between read and > write, we'll end up losing data. It can also be important to report. > > In the MEM_TO_MEM direction, what matters is of course how much data > has been written to memory from data consumer point of view. > Here, because read and write have independent data buffers, it will > take a while for TCR and TCRB to become equal. Thus we should check > TCRB in this case, too. > > Thus, all cases we should check TCRB instead of TCR. > > Without this patch, Sound Capture has noise after PluseAudio support > (= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because > the recorder will use wrong residue counter which indicates transferred > from sound device, but in reality the data was not yet put to memory > and recorder will record it. Applied, thanks.
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index 2b2c7db..50c4950 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -1310,7 +1310,7 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan, } /* Add the residue for the current chunk. */ - residue += rcar_dmac_chan_read(chan, RCAR_DMATCR) << desc->xfer_shift; + residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift; return residue; }