Re: [RFCv3 2/2] dma-buf: add helpers for sharing attacher constraints with dma-parms

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Tue, Feb 03, 2015 at 09:44:57AM -0500, Rob Clark wrote:
> On Tue, Feb 3, 2015 at 9:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Feb 03, 2015 at 09:04:03AM -0500, Rob Clark wrote:
> >> Since I'm stuck w/ an iommu, instead of built in mmu, my plan was to
> >> drop use of dma-mapping entirely (incl the current call to dma_map_sg,
> >> which I just need until we can use drm_cflush on arm), and
> >> attach/detach iommu domains directly to implement context switches.
> >> At that point, dma_addr_t really has no sensible meaning for me.
> >
> > So how do you intend to import from a subsystem which only gives you
> > the dma_addr_t?
> >
> > If you aren't passing system memory, you have no struct page.  You can't
> > fake up a struct page.  What this means is that struct scatterlist can't
> > represent it any other way.
> 
> Tell the exporter to stop using carveouts, and give me proper memory
> instead.. ;-)
> 
> Well, at least on these SoC's, I think the only valid use for carveout
> memory is the bootloader splashscreen.  And I was planning on just
> hanging on to that for myself for fbdev scanout buffer or other
> internal (non shared) usage..

I wasn't thinking about carveouts - as I already mentioned earlier in this
thread, it may be memory which couldn't possibly ever be system memory -
for example, a separate chunk of memory which is tightly coupled to the
graphics system but not so to the CPU.

In such a case, we wouldn't want to use that as normal system memory, but
we would want to allocate framebuffers and the like from it, and maybe
pass them around.

While it may not be appropriate for MSM, it's still something that needs
to be considered, because there may be (and I know there are) dmabuf
users which do pass memory this way.

So, what I'm saying is that for the purposes of the dmabuf API, we can't
mandate that the scatterlists will contain a valid struct page pointer.
It'd probably be a good idea for the importer to validate the scatterlist
at import time if it has this requirement.

However, thinking about this more, I think that from a generic design
point of view, we really should limit the "struct page" usage to a
special MSM-ism - something which should definitely not be copied by
other drivers.  As has been mentioned previously, if there is a system
MMU which needs to be programmed to map system memory onto the bus, the
struct page becomes absolutely useless, and the only thing that gives
you the correct "handle" to that memory is the dma_addr_t.

Finally, note that n_mapped = dma_map_sg(dev, sg, n_ent, dir) - n_mapped
can be less than n_ent when there's the presence of an IOMMU, since an
IOMMU is permitted to coalesce individual scatterlist entries if it
so chooses, and when walking the scatterlist for DMA purposes, the
scatterlist sg_dma_*() accessors should be used, and it should be
iterated over from 0 to n_mapped, not 0 to n_ent.  It's important to
realise that in driver code, sg->length may not be the same as
sg_dma_len(sg) for exactly this reason:

#ifdef CONFIG_NEED_SG_DMA_LENGTH
#define sg_dma_len(sg)          ((sg)->dma_length)
#else
#define sg_dma_len(sg)          ((sg)->length)
#endif

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.