linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ohad Ben-Cohen <ohad@wizery.com>
To: linux-omap@vger.kernel.org
Cc: Kanigeri Hari <h-kanigeri2@ti.com>,
	Omar Ramirez Luna <omar.ramirez@ti.com>,
	Guzman Lugo Fernando <x0095840@ti.com>,
	Menon Nishanth <nm@ti.com>, Hiroshi Doyu <Hiroshi.DOYU@nokia.com>,
	Ohad Ben-Cohen <ohad@wizery.com>
Subject: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
Date: Sat,  1 May 2010 23:44:25 +0300	[thread overview]
Message-ID: <1272746671-13423-1-git-send-email-ohad@wizery.com> (raw)

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.

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).
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..).
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.

   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.

Notes:
1. During my tests, a few testsuite scenarios failed due to the
   fact that the test cases called flush/invalidate on a memory
   buffer it didn't map beforehand.
   I consider that as a misuse of the API, and thus a testsuite error.
2. The global bridge device struct is used by adding an 'extern'
   to proc. This issue should be handled in a different patch series
   (the struct should not be global. instead, it should be accessible
   to the dspbridge code via one of the context objects. This way we 
   will also be able to transform pr_* prints to dev_* prints).
3. The patchset is not yet rebased on the latest dspbridge commits.
   It's currently based on 13e2573f2162b76d45313e790fc67a0d7672930b.
4. The patchset was tested with the bridge testsuite and the dmm sample
   application running on ZOOM3 / 2.6.33.

I'd like to thank Hari and Fernando for initial review of this patchset.

Please review and let me know your comments.

Thanks,
Ohad.

---
If you want, you can also reach me at <  ohadb at ti dot com  >.

Ohad Ben-Cohen (6):
  DSPBRIDGE: add memory_map_info to PROC
  DSPBRIDGE: remember mapping and page info in proc_map
  DSPBRIDGE: remove mapping information in proc_unmap
  DSPBRIDGE: do not call follow_page
  DSPBRIDGE: do not use low level cache manipulation API
  DSPBRIDGE: add dspbridge API to mark end of DMA

 arch/arm/plat-omap/include/dspbridge/_dcd.h     |    6 +-
 arch/arm/plat-omap/include/dspbridge/proc.h     |    8 +-
 arch/arm/plat-omap/include/dspbridge/wcdioctl.h |    6 +-
 arch/arm/plat-omap/include/dspbridge/wmd.h      |    3 +-
 drivers/dsp/bridge/pmgr/wcd.c                   |   39 ++-
 drivers/dsp/bridge/rmgr/proc.c                  |  440 ++++++++++++++++++++---
 drivers/dsp/bridge/wmd/io_sm.c                  |   10 +-
 drivers/dsp/bridge/wmd/tiomap3430.c             |   11 +-
 8 files changed, 448 insertions(+), 75 deletions(-)


             reply	other threads:[~2010-05-01 20:47 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-01 20:44 Ohad Ben-Cohen [this message]
2010-05-01 20:44 ` [RFC/PATCH 1/6] DSPBRIDGE: add memory_map_info to PROC Ohad Ben-Cohen
2010-05-01 20:44 ` [RFC/PATCH 2/6] DSPBRIDGE: remember mapping and page info in proc_map Ohad Ben-Cohen
2010-05-15  8:34   ` Felipe Contreras
2010-05-16 23:00     ` Ohad Ben-Cohen
2010-05-01 20:44 ` [RFC/PATCH 3/6] DSPBRIDGE: remove mapping information in proc_unmap Ohad Ben-Cohen
2010-05-15  8:38   ` Felipe Contreras
2010-05-16 23:02     ` Ohad Ben-Cohen
2010-05-01 20:44 ` [RFC/PATCH 4/6] DSPBRIDGE: do not call follow_page Ohad Ben-Cohen
2010-05-01 20:44 ` [RFC/PATCH 5/6] DSPBRIDGE: do not use low level cache manipulation API Ohad Ben-Cohen
2010-05-02 11:56   ` Ohad Ben-Cohen
2010-05-01 20:44 ` [RFC/PATCH 6/6] DSPBRIDGE: add dspbridge API to mark end of DMA Ohad Ben-Cohen
2010-05-02 13:17 ` [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues Felipe Contreras
2010-05-02 17:47   ` Ohad Ben-Cohen
2010-05-14 19:27     ` Felipe Contreras
2010-05-14 19:49       ` Omar Ramirez Luna
2010-05-15  8:26         ` Felipe Contreras
2010-05-15  9:08           ` Felipe Contreras
2010-05-16 16:08             ` Felipe Contreras
2010-05-16 17:35       ` Felipe Contreras
2010-05-16 22:57         ` Ohad Ben-Cohen
2010-05-16 23:51           ` Felipe Contreras
2010-05-18  8:05             ` Ohad Ben-Cohen
2010-05-18 11:02               ` Felipe Contreras
2010-05-18 11:14                 ` Ohad Ben-Cohen
2010-05-18 11:43                   ` Felipe Contreras
2010-05-18 11:57                     ` Ohad Ben-Cohen
2010-05-18 12:24                       ` Felipe Contreras
2010-05-18 12:53                         ` Ohad Ben-Cohen
2010-05-19 16:50                           ` Felipe Contreras
2010-05-20 21:22                             ` Ohad Ben-Cohen
2010-05-20 21:23                               ` Ohad Ben-Cohen
2010-05-21  6:14                               ` Felipe Contreras
2010-05-21  8:22                                 ` Ohad Ben-Cohen
2010-05-21  9:42                                   ` Felipe Contreras
2010-05-24 16:19                                     ` Ohad Ben-Cohen
2010-05-24 19:21                                       ` Felipe Contreras
2010-05-24 19:49                                         ` Ohad Ben-Cohen
2010-05-16 22:35       ` Ohad Ben-Cohen
2010-05-16 23:15         ` Felipe Contreras
2010-05-16 23:21           ` Ohad Ben-Cohen
2010-05-16 23:25           ` Ohad Ben-Cohen
2010-05-17  0:05             ` Felipe Contreras
2010-05-18  6:20               ` Ohad Ben-Cohen
2010-05-20 21:18       ` Ohad Ben-Cohen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1272746671-13423-1-git-send-email-ohad@wizery.com \
    --to=ohad@wizery.com \
    --cc=Hiroshi.DOYU@nokia.com \
    --cc=h-kanigeri2@ti.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=omar.ramirez@ti.com \
    --cc=x0095840@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).