Messages in this thread | | | Subject | Re: [PATCH 6/7] k3dma: Fix occasional DMA ERR issue by using proper dma api | From | Andy Green <> | Date | Fri, 22 Jul 2016 03:57:24 +0800 |
| |
On July 22, 2016 12:18:48 AM GMT+08:00, John Stultz <john.stultz@linaro.org> wrote: >On Wed, Jul 20, 2016 at 11:27 PM, Andy Green <andy@warmcat.com> wrote: >> On July 21, 2016 1:22:02 PM GMT+08:00, John Stultz ><john.stultz@linaro.org> wrote: >>>On Wed, Jul 20, 2016 at 9:26 PM, zhangfei <zhangfei.gao@linaro.org> >>>wrote: >>>> >>>> >>>> On 07/21/2016 11:53 AM, John Stultz wrote: >>>>> >>>>> After lots of debugging on an occasional DMA ERR issue, I realized >>>>> that the desc structures which we point the dma hardware are being >>>>> allocated out of regular memory. This means when we fill the desc >>>>> structures, that data doesn't always get flushed out to memory by >>>>> the time we start the dma transfer, resulting in the dma engine >>>getting >>>>> some null values, resulting in a DMA ERR on the first irq. >>>> >>>> >>>> How about using wmb() flush before start dma to sync desc? >>> >>>So I'm not going to pretend to be an expert here, but my >understanding >>>is that wmb() syncrhonizes cpu write ordering operations across cpus, >> >> IIUI what the memory barrier does is tell the *compiler* to actually >do any writes that the code asked for, but which otherwise might >actually be deferred past that point. The compiler doesn't know that >buffer area has other hardware snooping it, so by default it feels it >can play tricks with what seems to it like just generally deferring >spilling registers to memory. wmb makes sure the compiler's pending >writes actually happen right there. (writel() etc definitions have one >built-in, so they always do what you asked when you asked). >> >> That can be of interest when syncing with other cores but also to >other IPs that intend to use the written-to area immediately, which is >what's happening here. Without the barrier the write may not be issued >anywhere, even to local cpu cache, until after the hw is asked to go >read the buffer, corrupting what the hw sees. >> >>>so the cpus see all the changes before the wmb() before they see any >>>changes after. But I'm not sure what effect wmb() has across cpu >>>cache to device ordering. I don't think it works as a cache flush >to >>>memory. >>> >>>Andy's patch introducing the cyclic support actually had a wmb() in >it >>>that I removed as I couldn't understand clearly why it was there (and >>>there wasn't a comment explaining, as required by checkpatch :). >But >>>even with that wmb(), the DMA ERR was still seen. >> >> The rule about the comment is there partially because there's a >general tendancy for throwing voodoo smbs on broken things in case it >helps. But writing out memory descriptors that other hw is going to >read is a legit use for it I think. If the compiler you use wasn't >deferring the write, you won't notice any difference removing it, but >that doesn't mean it shouldn't be there. >> > >Though from your comments above, wouldn't using writel() instead of >writel_relaxed(), followed by a wmb() be sufficient?
Yes, since on Arm writel() sticks a wmb (ultimately a dsb instruction + outer_sync()) after every write it does.
-Andy
>thanks >-john
| |