Russell King's dma_map_single 's answer

 

Linux kernel development

headers

karl.beldan | 30 Jul 22:24 2012

About dma_sync_single_for_{cpu,device}

Hi,

On our board we've got an MV78200 and a network device between which we
xfer memory chunks via the ddram with an external dma controller.
To handle these xfers we're using the dma API.

To tx a chunk of data from the SoC => network device, we :
- prepare a buffer with a leading header embedding a pattern, 
- trigger the xfer and wait for an irq
// The device updates the pattern and then triggers an irq
- upon irq we check the pattern for the xfer completion

I was expecting the following to work:
	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
	dev_send(buffer);
	// wait for irq (don't peek in the buffer) ... got irq
	dma_sync_single_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
	if (!xfer_done(buffer)) // not RAM value
		dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
	[...]

But this does not work (the buffer pattern does not reflect the ddram
value).
On the other hand, the following works:
	[...]
	// wait for irq (don't peek in the buffer) ... got irq
	dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
	if (!xfer_done(buffer)) // RAM value
	[...]

(Continue reading)

Permalink| Reply|

headers

karl.beldan | 30 Jul 22:34 2012

Re: About dma_sync_single_for_{cpu,device}

On Mon, Jul 30, 2012 at 10:24:01PM +0200, karl.beldan <at> gmail.com wrote:
> I was expecting the following to work:
> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);

Sorry, I forgot this (invalidate):
 	dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);

> 	dev_send(buffer);
> 	// wait for irq (don't peek in the buffer) ... got irq
> 	dma_sync_single_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> 	if (!xfer_done(buffer)) // not RAM value
> 		dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> 	[...]

 
Karl

Permalink| Reply|

headers

Karl Beldan | 31 Jul 08:45 2012

Re: About dma_sync_single_for_{cpu,device}

Hi,

(This is an email originally addressed to the linux-kernel
mailing-list.)

On our board we've got an MV78200 and a network device between which we
xfer memory chunks via the ddram with an external dma controller.
To handle these xfers we're using the dma API.

To tx a chunk of data from the SoC => network device, we :
- prepare a buffer with a leading header embedding a pattern, 
- trigger the xfer and wait for an irq
// The device updates the pattern and then triggers an irq
- upon irq we check the pattern for the xfer completion

I was expecting the following to work:
	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
	dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
	dev_send(buffer);
	// wait for irq (don't peek in the buffer) ... got irq
	dma_sync_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
	if (!xfer_done(buffer)) // not RAM value
		dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
	[...]

But this does not work (the buffer pattern does not reflect the ddram
value).
On the other hand, the following works:
	[...]
	// wait for irq (don't peek in the buffer) ... got irq

(Continue reading)

Permalink| Reply|

headers

Clemens Ladisch | 31 Jul 08:59 2012

Re: About dma_sync_single_for_{cpu,device}

Karl Beldan wrote:
> To tx a chunk of data from the SoC => network device, we :
> - prepare a buffer with a leading header embedding a pattern,
> - trigger the xfer and wait for an irq
> // The device updates the pattern and then triggers an irq
> - upon irq we check the pattern for the xfer completion
>
> I was expecting the following to work:
> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);

Of both the CPU and the device write to the buffer, you must use
DMA_BIDIRECTIONAL.

Regards,
Clemens

Permalink| Reply|

headers

Karl Beldan | 31 Jul 09:27 2012

Re: About dma_sync_single_for_{cpu,device}

On 7/31/12, Clemens Ladisch <clemens <at> ladisch.de> wrote:
> Karl Beldan wrote:
>> To tx a chunk of data from the SoC => network device, we :
>> - prepare a buffer with a leading header embedding a pattern,
>> - trigger the xfer and wait for an irq
>> // The device updates the pattern and then triggers an irq
>> - upon irq we check the pattern for the xfer completion
>>
>> I was expecting the following to work:
>> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
>
> Of both the CPU and the device write to the buffer, you must use
> DMA_BIDIRECTIONAL.
>
Hi Clemens,

This does not work (tested) :  seems to me BIDIRECTIONAL would just
add invalidate, and invalidate before the ram has been updated, as
stated, does not work.
In fact the immediately following sync_for_device is intended to cater
for what DMA_BIDIRECTIONAL would provide (though it is not
implementation agnostic), only  invalidating a smaller address range.

Regards,

Karl

Permalink| Reply|

headers

Clemens Ladisch | 31 Jul 09:34 2012

Re: About dma_sync_single_for_{cpu,device}

Karl Beldan wrote:
> On 7/31/12, Clemens Ladisch <clemens <at> ladisch.de> wrote:
>> Karl Beldan wrote:
>>> To tx a chunk of data from the SoC => network device, we :
>>> - prepare a buffer with a leading header embedding a pattern,
>>> - trigger the xfer and wait for an irq
>>> // The device updates the pattern and then triggers an irq
>>> - upon irq we check the pattern for the xfer completion
>>>
>>> I was expecting the following to work:
>>> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
>>
>> Of both the CPU and the device write to the buffer, you must use
>> DMA_BIDIRECTIONAL.
>
> This does not work (tested) :  seems to me BIDIRECTIONAL would just
> add invalidate, and invalidate before the ram has been updated, as
> stated, does not work.

Please show the exact sequence of dma_* calls, and also show when and
how the CPU and the device access the buffer.

Regards,
Clemens

Permalink| Reply|

headers

Karl Beldan | 31 Jul 10:30 2012

Re: About dma_sync_single_for_{cpu,device}

On Tue, Jul 31, 2012 at 09:34:01AM +0200, Clemens Ladisch wrote:
> Karl Beldan wrote:
> > On 7/31/12, Clemens Ladisch <clemens <at> ladisch.de> wrote:
> >> Karl Beldan wrote:
> >>> To tx a chunk of data from the SoC => network device, we :
> >>> - prepare a buffer with a leading header embedding a pattern,
> >>> - trigger the xfer and wait for an irq
> >>> // The device updates the pattern and then triggers an irq
> >>> - upon irq we check the pattern for the xfer completion
> >>>
> >>> I was expecting the following to work:
> >>> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
> >>
> >> Of both the CPU and the device write to the buffer, you must use
> >> DMA_BIDIRECTIONAL.
> >
> > This does not work (tested) :  seems to me BIDIRECTIONAL would just
> > add invalidate, and invalidate before the ram has been updated, as
> > stated, does not work.
> 
> Please show the exact sequence of dma_* calls, and also show when and
> how the CPU and the device access the buffer.
> 
Hmm, so I just spotted a line where we peek in the buffer after
invalidating .. cannot believe I missed it .. so sorry for the noise ..
now it's working.
I felt I would find the culprit right after posting ;)
Thanks Clemens !

Permalink| Reply|

headers

Russell King - ARM Linux | 31 Jul 11:09 2012

Re: About dma_sync_single_for_{cpu,device}

On Tue, Jul 31, 2012 at 08:45:57AM +0200, Karl Beldan wrote:
> I was expecting the following to work:
> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
> 	dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> 	dev_send(buffer);
> 	// wait for irq (don't peek in the buffer) ... got irq
> 	dma_sync_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> 	if (!xfer_done(buffer)) // not RAM value
> 		dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> 	[...]

First point is that you clearly do not understand the DMA API at all.  The
DMA API has the idea of buffer ownership.  Only the owner may access the
buffer:

*CPU OWNS THE BUFFER*
dma_map_single()
*DEVICE OWNS THE BUFFER*
dma_sync_single_for_cpu()
*CPU OWNS THE BUFFER*
dma_sync_single_for_device()
*DEVICE OWNS THE BUFFER*
dma_unmap_single()
*CPU OWNS THE BUFFER*

So, there is absolutely no noeed what so ever to follow dma_map_single()
with dma_sync_single_for_device().

Second point is that you should not change the 'direction' argument while
a buffer is mapped.

(Continue reading)

Permalink| Reply|

headers

Karl Beldan | 31 Jul 21:31 2012

Re: About dma_sync_single_for_{cpu,device}

On 7/31/12, Russell King - ARM Linux <linux <at> arm.linux.org.uk> wrote:
> On Tue, Jul 31, 2012 at 08:45:57AM +0200, Karl Beldan wrote:
>> I was expecting the following to work:
>> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
>> 	dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
>> 	dev_send(buffer);
>> 	// wait for irq (don't peek in the buffer) ... got irq
>> 	dma_sync_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
>> 	if (!xfer_done(buffer)) // not RAM value
>> 		dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
>> 	[...]
>

Hi Russell,

> First point is that you clearly do not understand the DMA API at all.  The
> DMA API has the idea of buffer ownership.  Only the owner may access the
> buffer:
>
Are you saying that this scenario does not work ?
We are taking some liberties with the DMA API, we're more using some
of its funcs rather than _using_ it ;).
The question was not whether this was a proper usage of the API, but
why that scenario would not lead to the expected results .. and now
I've found the culprit peek I am happy.

[...]
> So, there is absolutely no noeed what so ever to follow dma_map_single()
> with dma_sync_single_for_device().
>

(Continue reading)

Permalink| Reply|

headers

Russell King - ARM Linux | 31 Jul 22:08 2012

Re: About dma_sync_single_for_{cpu,device}

On Tue, Jul 31, 2012 at 09:31:13PM +0200, Karl Beldan wrote:
> On 7/31/12, Russell King - ARM Linux <linux <at> arm.linux.org.uk> wrote:
> > On Tue, Jul 31, 2012 at 08:45:57AM +0200, Karl Beldan wrote:
> >> I was expecting the following to work:
> >> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
> >> 	dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> >> 	dev_send(buffer);
> >> 	// wait for irq (don't peek in the buffer) ... got irq
> >> 	dma_sync_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> >> 	if (!xfer_done(buffer)) // not RAM value
> >> 		dma_sync_single_for_device(dev, buffer, pattern_size, DMA_FROM_DEVICE);
> >> 	[...]
> >
> 
> Hi Russell,
> 
> 
> > First point is that you clearly do not understand the DMA API at all.  The
> > DMA API has the idea of buffer ownership.  Only the owner may access the
> > buffer:
> >
> Are you saying that this scenario does not work ?
> We are taking some liberties with the DMA API, we're more using some
> of its funcs rather than _using_ it ;).
> The question was not whether this was a proper usage of the API, but
> why that scenario would not lead to the expected results .. and now
> I've found the culprit peek I am happy.

If you abuse the API don't expect your stuff to work in future kernel
versions.

(Continue reading)

Permalink| Reply|

headers

Karl Beldan | 1 Aug 08:50 2012

Re: About dma_sync_single_for_{cpu,device}

On 7/31/12, Russell King - ARM Linux <linux <at> arm.linux.org.uk> wrote:
> On Tue, Jul 31, 2012 at 09:31:13PM +0200, Karl Beldan wrote:
>> On 7/31/12, Russell King - ARM Linux <linux <at> arm.linux.org.uk> wrote:
>> > On Tue, Jul 31, 2012 at 08:45:57AM +0200, Karl Beldan wrote:
>> >> I was expecting the following to work:
>> >> 	addr = dma_map_single(dev, buffer, size, DMA_TO_DEVICE);
>> >> 	dma_sync_single_for_device(dev, buffer, pattern_size,
>> >> DMA_FROM_DEVICE);
>> >> 	dev_send(buffer);
>> >> 	// wait for irq (don't peek in the buffer) ... got irq
>> >> 	dma_sync_single_for_cpu(dev, buffer, pattern_size, DMA_FROM_DEVICE);
>> >> 	if (!xfer_done(buffer)) // not RAM value
>> >> 		dma_sync_single_for_device(dev, buffer, pattern_size,
>> >> DMA_FROM_DEVICE);
>> >> 	[...]
>> >
>>
>> Hi Russell,
>>
>>
>> > First point is that you clearly do not understand the DMA API at all.
>> > The
>> > DMA API has the idea of buffer ownership.  Only the owner may access
>> > the
>> > buffer:
>> >
>> Are you saying that this scenario does not work ?
>> We are taking some liberties with the DMA API, we're more using some
>> of its funcs rather than _using_ it ;).
>> The question was not whether this was a proper usage of the API, but

(Continue reading)

Permalink| Reply|

Return

Return to gmane.linux.kernel.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
 RSS Feed
List Information

About Gmane

Gmane

http://comments.gmane.org/gmane.linux.kernel/1335309