All of lore.kernel.org
 help / color / mirror / Atom feed
From: broonie@opensource.wolfsonmicro.com (Mark Brown)
To: linux-arm-kernel@lists.infradead.org
Subject: Cyclic DMA - callback properties and tx_status residue
Date: Wed, 9 May 2012 12:16:15 +0100	[thread overview]
Message-ID: <20120509111614.GH3955@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <20120509093334.GS26481@n2100.arm.linux.org.uk>

On Wed, May 09, 2012 at 10:33:34AM +0100, Russell King - ARM Linux wrote:
> On Wed, May 09, 2012 at 11:27:17AM +0200, Linus Walleij wrote:

> > There is a DMAengine helper in ALSA SoC these days,
> > sound/soc/soc-dmaengine-pcm.c which does not yet include
> > get position calls, but I expect that's where the ALSA glue will
> > end up.

> I'm more or less ignoring that because with current DMA engine stuff, it's
> buggy.  The presence of such a user doesn't really define how this
> interface supposed to work either.

We need to at least include it in the fixes; I'm currently insisting
that any dmaengine based device uses it.  If we can't come up with a
standard library to map the dmaengine based DMA controllers to the ALSA
userspace API something is seriously wrong.  We definitely shouldn't be
treating it as an API reference but we should be ensuring that it does
the best possible job with the APIs that do exist and enhancing the
underlying APIs if that's needed.

Looking at the code the current usage doesn't seem obviously wrong so
there's some work to do - we get a callback assigned on each descriptor
we submit so it's a bit surprising that this might not get delivered,
though as has been discussed further up the thread that's something that
might actually happen and perhaps we need to clarify the interfaces
here.

> Given that counting the number of period callbacks to determine the
> position is buggy, soc-dmaengine-pcm.c is buggy as it currently stands.
> I'm willing to bet that if ALSA requests a small period size, and you
> load a system up with lots of activity, you'll eventually hear
> corrupted audio.

> It's difficult to test because the corruption will probably only be
> occasional and it'll be purely audio based - it'll basically play the
> bit of the buffer that's currently being loaded with new data from time
> to time because ALSAs idea of where the DMA is vs where the hardware
> is will gradually slip over time.

Right, any driver relying on this code must currently be confident that the
completion callbacks can actually be delivered before the next period
expires (for example, by requiring a large enough period size and
ensuring that the completion gets delivered in hard IRQ context on the
DMA side so there's less impact from load).  This might break down under
extreme loads, though if it does I rather suspect that we'd underflow
anyway.

If we can resolve the issues with reading the current position in
dmaengine then this should be fairly simple to address more
comprehensively of course, though there will be some hardware imposed
limits on what we can do.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120509/1bc521af/attachment-0001.sig>

  reply	other threads:[~2012-05-09 11:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-02 14:45 Cyclic DMA - callback properties and tx_status residue Russell King - ARM Linux
2012-05-02 16:01 ` Vinod Koul
2012-05-02 16:27   ` Russell King - ARM Linux
2012-05-04 12:26     ` Russell King - ARM Linux
2012-05-04 12:45       ` Vinod Koul
2012-05-10 22:54         ` Russell King - ARM Linux
2012-05-11  3:00           ` Vinod Koul
2012-05-11 12:24             ` Linus Walleij
2012-05-11 13:03               ` Russell King - ARM Linux
2012-05-15  5:02                 ` Vinod Koul
2012-05-09  9:27     ` Linus Walleij
2012-05-09  9:33       ` Russell King - ARM Linux
2012-05-09 11:16         ` Mark Brown [this message]
2012-05-09 12:19           ` Russell King - ARM Linux
2012-05-09 12:49             ` Lars-Peter Clausen
2012-05-09 14:03             ` Mark Brown
2012-05-10  3:44             ` Vinod Koul
2012-05-10  7:44               ` Russell King - ARM Linux
2012-05-10 10:58                 ` Vinod Koul
2012-05-10 13:19                   ` Huang Shijie
2012-05-10 14:54                     ` Vinod Koul
2012-05-10  9:42               ` Mark Brown
2012-05-10 11:01                 ` Vinod Koul
2012-05-11 14:02                   ` Mark Brown
2012-05-11 14:07                   ` Russell King - ARM Linux
2012-05-11 14:18                     ` Mark Brown
2012-05-11 14:29                       ` Russell King - ARM Linux
2012-05-11 15:07                         ` Mark Brown
2012-05-15  5:07                     ` Vinod Koul
2012-05-15  7:37                       ` Russell King - ARM Linux
2012-05-15  8:58                         ` Vinod Koul
2012-05-09 12:35       ` Lars-Peter Clausen
2012-05-07 10:40 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120509111614.GH3955@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.