Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues

[Felipe Contreras <felipe.contreras@gmail.com>]

On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> This patchset introduces an approach to eliminate the direct calls
> to follow_page and to the low level cache APIs.
>
> The patchset works by caching the page information while memory
> is mapped, and then using that information later when needed
> instead of calling follow_page. The low level cache API is then replaced
> by the standard DMA API.

Finally! Very interesting patch indeed.

> A few key points in the current approach that I'd be happy to hear
> your feedback about:
> 1. The new mapping + page information is currently cached in the
>   proc object, but it might fit better inside dmm map objects
>   (by enhancing the DMM layer to support the required data caching,
>   storing and retrieving).

Sounds like a good idea.

> 2. The information is stored in a linked list. That's pretty fine
>   as long as the number of memory mappings per application is not
>   extremely high. If that assumption is wrong, a different underlying
>   data structure might be better (hash table, priority tree, etc..).

I think a linked list is fine for now. AFAIK only a limited number of
mmaps happen at the same time, usually 4.

> 3. Moving to standard DMA API completely changes the user's point
>   of view; users should no longer think in terms of which cache
>   manipulation is required, but instead, they should just tell dspbridge
>   before a DMA transfer begins, and after it ends. Between the begin
>   and end calls, the buffer "belongs" to the DSP and should not
>   be accessed by the user.

This is really nice. That API should have been that way since the beginning.

>   The patchset renames the flush ioctl to begin_dma_to_dsp and
>   the invalidate ioctl to begin_dma_from_dsp. Both functions
>   eventually call dma_map_sg, with the former requesting a
>   DMA_BIDIRECTIONAL direction, and the latter requesting a
>   DMA_FROM_DEVICE direction.
>   In addition, the patchset adds two new APIs which calls dma_unmap_sg:
>   end_dma_to_dsp and end_dma_from_dsp.
>
>   Ideally, there would be only a single begin_dma command and a single
>   end_dma one, which would accept an additional parameter that will
>   determine the direction of the transfer. Such an approach would be more
>   versatile and cleaner, but it would also break all user space apps that
>   use dspbridge today.

If I understand correctly all user-space apps would be broken anyway
because they are not issuing the end_dma calls. At the very least they
need to be updated to use them.

Also, in Nokia we patched the bridgedriver to be able to have the 3
operations available from user-space (clean, inv, and flush), so we
would be very interested in having the direction of the transfer
available.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html