On 21/11/2018 16:13, Ben Dooks wrote: > The dma_desc->bytes_transferred counter tracks the number of bytes > moved by the DMA channel. This is then used to calculate the information > passed back in the in the tegra_dma_tx_status callback, which is usually > fine. > > When the DMA channel is configured as continous, then the bytes_transferred > counter will increase over time and eventually overflow to become negative > so the residue count will become invalid and the ALSA sound-dma code will > report invalid hardware pointer values to the application. This results in > some users becoming confused about the playout position and putting audio > data in the wrong place. > > To fix this issue, always ensure the bytes_transferred field is modulo the > size of the request. We only do this for the case of the cyclic transfer > done ISR as anyone attempting to move 2GiB of DMA data in one transfer > is unlikely. > > Note, we don't fix the issue that we should /never/ transfer a negative > number of bytes so we could make those fields unsigned. > > Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx> > Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> > --- > drivers/dma/tegra20-apb-dma.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > index 9a558e30c461..8219ab88a507 100644 > --- a/drivers/dma/tegra20-apb-dma.c > +++ b/drivers/dma/tegra20-apb-dma.c > @@ -636,7 +636,10 @@ static void handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc, > > sgreq = list_first_entry(&tdc->pending_sg_req, typeof(*sgreq), node); > dma_desc = sgreq->dma_desc; > - dma_desc->bytes_transferred += sgreq->req_len; > + /* if we dma for long enough the transfer count will wrap */ > + dma_desc->bytes_transferred = > + (dma_desc->bytes_transferred + sgreq->req_len) % > + dma_desc->bytes_requested; > > /* Callback need to be call */ > if (!dma_desc->cb_count) Acked-by: Jon Hunter <jonathanh@xxxxxxxxxx> Thanks! Jon -- nvpublic
