lkml.org 
[lkml]   [2016]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 6/7] k3dma: Fix occasional DMA ERR issue by using proper dma api
From
Date


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

\
 
 \ /
  Last update: 2016-07-21 22:41    [W:1.420 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site