On Fri, Apr 20, 2018 at 05:46:25AM -0700, Christoph Hellwig wrote:
On Fri, Apr 20, 2018 at 12:44:01PM +0200, Christian König wrote:
What we need is an sg_alloc_table_from_resources(dev, resources, num_resources) which does the handling common to all drivers.
A structure that contains
{page,offset,len} + {dma_addr+dma_len}
is not a good container for storing
{virt addr, dma_addr, len}
no matter what interface you build arond it.
Why not? I mean at least for my use case we actually don't need the virtual address.
If you don't need the virtual address you need scatterlist even list.
What we need is {dma_addr+dma_len} in a consistent interface which can come from both {page,offset,len} as well as {resource, len}.
Ok.
What I actually don't need is separate handling for system memory and resources, but that would we get exactly when we don't use sg_table.
At the very lowest level they will need to be handled differently for many architectures, the questions is at what point we'll do the branching out.
Having at least struct page also in that list with (dma_addr_t, lenght) pairs has a bunch of benefits for drivers in unifying buffer handling code. You just pass that one single list around, use the dma_addr_t side for gpu access (generally bashing it into gpu ptes). And the struct page (if present) for cpu access, using kmap or vm_insert_*. We generally ignore virt, if we do need a full mapping then we construct a vmap for that buffer of our own.
If (and that would be serious amounts of work all over the tree, with lots of drivers) we come up with a new struct for gpu buffers, then I'd also add "force page alignement for everything" to the requirements list. That's another mismatch we have, since gpu buffer objects (and dma-buf) are always full pages. That mismatch motived the addition of the page-oriented sg iterators.
So maybe a list of (struct page *, dma_addr_t, num_pages) would suit best, with struct page * being optional (if it's a resource, or something else that the kernel core mm isn't aware of). But that only has benefits if we really roll it out everywhere, in all the subsystems and drivers, since if we don't we've made the struct pages ** <-> sgt conversion fun only worse by adding a 3 representation of gpu buffer object backing storage. -Daniel