More DMA API junk

More DMA API junk

[Benjamin Herrenschmidt]

While fixing ppc for DaveM new stuff, I found a couple of things
that look strange (not directly related to the patch though)

 - dma_is_consistent(dma_addr_t address) : Did somebody actually expect
anything useful out of this function ? AFAIK, it makes no sense. There
is simply no way an arch like ppc that use per-page cache-inhibit
mappings to do consistent memory will be able to tell you if a given
_DMA_ address is consistent... that makes no sense.

 - dma_cache_sync(): What is that function supposed to do ? It's a
duplicate of the other sync() functions and David didn't even bother
turning it into _for_device/for_cpu... which is fine since it doesn't
even takes a struct device argument.

I vote for removing the 2 functions above. I suspect something using
them is broken anyway.

Ben.

Re: More DMA API junk

[James Bottomley]

On Mon, 2004-03-15 at 19:53, Benjamin Herrenschmidt wrote:
>  - dma_is_consistent(dma_addr_t address) : Did somebody actually expect
> anything useful out of this function ? AFAIK, it makes no sense. There
> is simply no way an arch like ppc that use per-page cache-inhibit
> mappings to do consistent memory will be able to tell you if a given
> _DMA_ address is consistent... that makes no sense.

>  - dma_cache_sync(): What is that function supposed to do ? It's a
> duplicate of the other sync() functions and David didn't even bother
> turning it into _for_device/for_cpu... which is fine since it doesn't
> even takes a struct device argument.

They are both used.  Look at section II of DMA-API.txt.cd ..

They have to do with incoherent platforms which I assume you don't have,
but just leave them alone for those of use who are so burdened.

Actually, I was planning to update dma_cache_sync to take a device and
to conform to the new style (except I was thinking of an ownership flag
rather than introducing two more APIs).  The lack of device to
dma_cache_sync is probably irrelevant since they only apply to older
arch's which don't have bus caches.

James

Re: More DMA API junk

[Benjamin Herrenschmidt]

On Tue, 2004-03-16 at 12:12, James Bottomley wrote:
> On Mon, 2004-03-15 at 19:53, Benjamin Herrenschmidt wrote:
> >  - dma_is_consistent(dma_addr_t address) : Did somebody actually expect
> > anything useful out of this function ? AFAIK, it makes no sense. There
> > is simply no way an arch like ppc that use per-page cache-inhibit
> > mappings to do consistent memory will be able to tell you if a given
> > _DMA_ address is consistent... that makes no sense.
> 
> >  - dma_cache_sync(): What is that function supposed to do ? It's a
> > duplicate of the other sync() functions and David didn't even bother
> > turning it into _for_device/for_cpu... which is fine since it doesn't
> > even takes a struct device argument.
> 
> They are both used.  Look at section II of DMA-API.txt.cd ..

I've looked and I couldn't find a proper meaning for them.

> They have to do with incoherent platforms which I assume you don't have,
> but just leave them alone for those of use who are so burdened.

We do have incoherent PPCs (the embedded ones)

> Actually, I was planning to update dma_cache_sync to take a device and
> to conform to the new style (except I was thinking of an ownership flag
> rather than introducing two more APIs).  The lack of device to
> dma_cache_sync is probably irrelevant since they only apply to older
> arch's which don't have bus caches.

How can dma_is_consistent() be implemented at all on an arch that
implement consistency by allocating non-cacheable space using PTE
bits ? we just cannot know if a given dma_addr_t is mapped by the
kernel using a cacheable or non-cacheable (or both) mapping.

Regarding dma_cache_sync(), I still don't understand what it means
and what it should be used for. We have dma_consistent_sync_*,
care to explain in which cases those aren't useable and one would
have to call dma_cache_sync() ?

Ben.

Re: More DMA API junk

[James Bottomley]

On Mon, 2004-03-15 at 20:32, Benjamin Herrenschmidt wrote:
> On Tue, 2004-03-16 at 12:12, James Bottomley wrote:
> > They are both used.  Look at section II of DMA-API.txt.cd ..
> 
> I've looked and I couldn't find a proper meaning for them.

OK, the idea is that drivers that run on platforms which may not be able
to generate coherent memory use this (that's why no device in the API. 
I assumed it would be per-platform).

You call dma_alloc_noncoherent() to get the memory and you guarantee to
sync it correctly from the CPU's perspective using dma_cache_sync().

If you compile the driver on a coherent platform,
dma_alloc_noncoherent() becomes dma_alloc_coherent() and the
dma_cache_sync()s become nops.

If you compile it on a coherent incapable platform,
dma_alloc_noncoherent becomes an aligned kmalloc and the
dma_cache_sync() points map to the correct cpu caching instructions.

A driver has to be specially constructed to use this API ... and
obviously it needs to be used on the problem platforms.  The only
examples I know of are the scsi driver 53c700.c and the network driver
lasi_82596.


> How can dma_is_consistent() be implemented at all on an arch that
> implement consistency by allocating non-cacheable space using PTE
> bits ? we just cannot know if a given dma_addr_t is mapped by the
> kernel using a cacheable or non-cacheable (or both) mapping.

The idea was that either the platform can or cannot generate coherent
memory.  dma_is_consistent() returns true if dma_alloc_noncoherent()
returns coherent memory.

For a nop implementation see asm-i386/dma-mapping.h.  For a used
implementation see asm-parisc/dma-mapping.h.

> Regarding dma_cache_sync(), I still don't understand what it means
> and what it should be used for. We have dma_consistent_sync_*,
> care to explain in which cases those aren't useable and one would
> have to call dma_cache_sync() ?

It is used to do the CPU cache writeback and invalidates in the driver
on memory used for device mailboxes.  It's syntax is closely modelled on
dma_cache_wback() and friends (in asm/io.h).

I've never heard of dma_consistent_sync_* before.

James

Re: More DMA API junk

[Benjamin Herrenschmidt]

> OK, the idea is that drivers that run on platforms which may not be able
> to generate coherent memory use this (that's why no device in the API. 
> I assumed it would be per-platform).
> 
> You call dma_alloc_noncoherent() to get the memory and you guarantee to
> sync it correctly from the CPU's perspective using dma_cache_sync().

Ok, so a platform like ppc 4xx that will generate coherent memory
by allocating non-cacheable memory in dma_alloc_coherent isn't
concerned ? That dma_cache_sync() isn't doing anything different
than what dma_consistent_sync() or dma_sync_single_range() is
doing as far as I understand.

My problem is that we already have at least 6 different "sync"
functions, I don't like having one more, which is not clear and
DMA-api.txt really doesn't care explaining the difference with
dma_sync_single_range() (which was adapted by DaveM for the
to_device/to_cpu change, while dma_sync_cache was not).

> If you compile the driver on a coherent platform,
> dma_alloc_noncoherent() becomes dma_alloc_coherent() and the
> dma_cache_sync()s become nops.

So dma_cache_sync() is specifically for use on space returned
by dma_alloc_noncoherent (and not dma_map_*). So on a platform
like mine where we use non-cacheable memory for allocating coherent
space, we can indeed nop this one out. But then, is this also the
case for dma_sync_single_range() which seem to be documented as
beeing part of the "non-coherent" extension to the DMA API ? In
which case, where is the difference between
dma_sync_single_range() and dma_cache_sync() 

> If you compile it on a coherent incapable platform,
> dma_alloc_noncoherent becomes an aligned kmalloc and the
> dma_cache_sync() points map to the correct cpu caching instructions.
>
> A driver has to be specially constructed to use this API ... and
> obviously it needs to be used on the problem platforms.  The only
> examples I know of are the scsi driver 53c700.c and the network driver
> lasi_82596.

Ok, but see my point about
dma_sync_single_range() vs. dma_cache_sync() 

> > How can dma_is_consistent() be implemented at all on an arch that
> > implement consistency by allocating non-cacheable space using PTE
> > bits ? we just cannot know if a given dma_addr_t is mapped by the
> > kernel using a cacheable or non-cacheable (or both) mapping.
> 
> The idea was that either the platform can or cannot generate coherent
> memory.  dma_is_consistent() returns true if dma_alloc_noncoherent()
> returns coherent memory.

That is confusing then. We should remove this dma_addr_t argument,
as there is no simple way for me to tell from a physical address if
that was mapped in kernel space as cacheable or non-cacheable.

> For a nop implementation see asm-i386/dma-mapping.h.  For a used
> implementation see asm-parisc/dma-mapping.h.

I don't need a nop mapping. I can have non-coherent memory (kmalloc,
or anything not specifically allocated with dma_alloc_coherent)

> > Regarding dma_cache_sync(), I still don't understand what it means
> > and what it should be used for. We have dma_consistent_sync_*,
> > care to explain in which cases those aren't useable and one would
> > have to call dma_cache_sync() ?
> 
> It is used to do the CPU cache writeback and invalidates in the driver
> on memory used for device mailboxes.  It's syntax is closely modelled on
> dma_cache_wback() and friends (in asm/io.h).
> 
> I've never heard of dma_consistent_sync_* before.

I meant dma_sync_single/sg, sorry, and specifically dma_sync_single_range()
which is documented along with the non-coherent stuff.

Ben.

Re: More DMA API junk

[James Bottomley]

On Mon, 2004-03-15 at 22:32, Benjamin Herrenschmidt wrote:
> So dma_cache_sync() is specifically for use on space returned
> by dma_alloc_noncoherent (and not dma_map_*).

Yes.

> So on a platform
> like mine where we use non-cacheable memory for allocating coherent
> space, we can indeed nop this one out. But then, is this also the
> case for dma_sync_single_range() which seem to be documented as
> beeing part of the "non-coherent" extension to the DMA API ? In
> which case, where is the difference between
> dma_sync_single_range() and dma_cache_sync()

dma_sync_single_range is an extension of dma_sync_single that allows you
only to do a partial sync (because on platforms like PA, syncs grow in
expense linearly with size).  So, if you're snooping data in a ten page
mapping, and you're only interested in sixteen bytes, you only need sync
those sixteen bytes. 

On your platform, dma_sync_single_range() has a defined meaning for
streaming mappings.  dma_cache_sync will be a nop, so they're not
interchangeable.

> That is confusing then. We should remove this dma_addr_t argument,
> as there is no simple way for me to tell from a physical address if
> that was mapped in kernel space as cacheable or non-cacheable.

You're confused about what it's used for.  It's designed only to be
called on memory allocated by dma_alloc_noncoherent() and tells you if
that API actually returned coherent memory or not.  This was designed
for ARM which has a limited range of allocateable coherent memory and
then would need to fail dma_alloc_coherent() or, in the case of
dma_alloc_noncoherent() begin handing out ordinary kmalloc'd memory.

James

Re: More DMA API junk

[Benjamin Herrenschmidt]

> On your platform, dma_sync_single_range() has a defined meaning for
> streaming mappings.  dma_cache_sync will be a nop, so they're not
> interchangeable.

Ok, so we should make that clear in DMA-API.txt, currently it's
definitely not ;) I'll propose a patch later on if nobody beats me
on this.

> You're confused about what it's used for.  It's designed only to be
> called on memory allocated by dma_alloc_noncoherent() and tells you if
> that API actually returned coherent memory or not.  This was designed
> for ARM which has a limited range of allocateable coherent memory and
> then would need to fail dma_alloc_coherent() or, in the case of
> dma_alloc_noncoherent() begin handing out ordinary kmalloc'd memory.

I still have a problem with the argument. We should pass at least both
the virtual and physical address then. Or also make it clear in the
DMA-API.txt that it is only valid on the result of dma_alloc_noncoherent
in which case it becomes legal for me to hard-wire a result of 1.

Ben

Re: More DMA API junk

[Ralf Baechle]

On Mon, Mar 15, 2004 at 10:48:48PM -0500, James Bottomley wrote:

> You're confused about what it's used for.  It's designed only to be
> called on memory allocated by dma_alloc_noncoherent() and tells you if
> that API actually returned coherent memory or not.  This was designed

... but what is coherent?  The API's function naming is completly confusing
at least from a MIPS perspective.  MIPS calls something coherent if
cache coherency is maintained by hardware and that's (hey, what caches?!?)
not the case for uncached memory.

So I would like to rename functions to something less confusing.  Where
did the original terminology come from?  Would anybody object simply
swapping the coherent and noncoherent parts of the function names?

  Ralf