| Message ID | 20230403154800.215924-1-paul@crapouillou.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | iio: new DMABUF based API, v3 | expand |
Hi Hillf, Le mardi 04 avril 2023 à 09:59 +0800, Hillf Danton a écrit : > On 3 Apr 2023 17:47:50 +0200 Paul Cercueil <paul@crapouillou.net> > > This function can be used to initiate a scatter-gather DMA transfer > > where the DMA addresses and lengths are located inside arrays. > > > > The major difference with dmaengine_prep_slave_sg() is that it > > supports > > specifying the lengths of each DMA transfer; as trying to override > > the > > length of the transfer with dmaengine_prep_slave_sg() is a very > > tedious > > process. The introduction of a new API function is also justified > > by the > > fact that scatterlists are on their way out. > > Given sg's wayout and conceptually iovec and kvec (in > include/linux/uio.h), > what you add should have been dma_vec to ease people making use of > it. > > struct dma_vec { > dma_addr_t addr; > size_t len; > }; Well it's not too late ;) Thanks for the feedback. Cheers, -Paul > > > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > > > > --- > > v3: New patch > > --- > > include/linux/dmaengine.h | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > > index c3656e590213..62efa28c009a 100644 > > --- a/include/linux/dmaengine.h > > +++ b/include/linux/dmaengine.h > > @@ -912,6 +912,11 @@ struct dma_device { > > struct dma_async_tx_descriptor > > *(*device_prep_dma_interrupt)( > > struct dma_chan *chan, unsigned long flags); > > > > + struct dma_async_tx_descriptor > > *(*device_prep_slave_dma_array)( > > + struct dma_chan *chan, dma_addr_t *addrs, > > + size_t *lengths, size_t nb, > > + enum dma_transfer_direction direction, > > + unsigned long flags); > > Then the callback looks like > > struct dma_async_tx_descriptor *(*device_prep_slave_vec)( > struct dma_chan *chan, > struct dma_vec *vec, > int nvec, > enum dma_transfer_direction direction, > unsigned long flags);
On Mon, 2023-04-03 at 17:47 +0200, Paul Cercueil wrote: > Hi Jonathan, > > Here's the v3 of my patchset that introduces a new interface based on > DMABUF objects to complement the fileio API, and adds write() support to > the existing fileio API. > > It changed quite a lot since V2; the IIO subsystem is now just a DMABUF > importer, and all the complexity related to handling creation, deletion > and export of DMABUFs (including DMA mapping etc.) is gone. > > This new interface will be used by Libiio. The code is ready[1] and will > be merged to the main branch as soon as the kernel bits are accepted > upstream. > > Note that Libiio (and its server counterpart, iiod) use this new > interface in two different ways: > - by memory-mapping the DMABUFs to access the sample data directly, > which is much faster than using the existing fileio API as the sample > data does not need to be copied; > - by passing the DMABUFs around directly to the USB stack, in a > device-to-device zero-copy fashion, using a new DMABUF interface for > the USB (FunctionFS to be exact) stack, which is being upstreamed in > parallel of this patchset [2]. > > As for write() support, Nuno (Cc'd) said he will work on upstreaming the > DAC counterpart of adc/adi-axi-adc.c in the next few weeks, so there > will be a user for the buffer write() support. I hope you are okay with > this - otherwise, we can just wait until this work is done, and I still > benefit from sending this patchset early to get feedback. > Indeed, I already started a discussion [1] since what we have now for adc/adi-axi-adc.c has some major flaws (IMHO). So I'm hopping to get some feedback/discussion to get "righter" from the beginning. [1]: https://lore.kernel.org/linux-iio/dac3967805d7ddbd4653ead6d50e614844e0b70b.camel@gmail.com/T/#u - Nuno Sá
Am 04.04.23 um 09:42 schrieb Paul Cercueil: > Hi Hillf, > > Le mardi 04 avril 2023 à 09:59 +0800, Hillf Danton a écrit : >> On 3 Apr 2023 17:47:50 +0200 Paul Cercueil <paul@crapouillou.net> >>> This function can be used to initiate a scatter-gather DMA transfer >>> where the DMA addresses and lengths are located inside arrays. >>> >>> The major difference with dmaengine_prep_slave_sg() is that it >>> supports >>> specifying the lengths of each DMA transfer; as trying to override >>> the >>> length of the transfer with dmaengine_prep_slave_sg() is a very >>> tedious >>> process. The introduction of a new API function is also justified >>> by the >>> fact that scatterlists are on their way out. >> Given sg's wayout and conceptually iovec and kvec (in >> include/linux/uio.h), >> what you add should have been dma_vec to ease people making use of >> it. >> >> struct dma_vec { >> dma_addr_t addr; >> size_t len; >> }; > Well it's not too late ;) Yeah adding that is pretty much the job I have on my TODO list for quite some time. I wouldn't mind if you start adding that and provide helper functions in DMA-buf to convert from/to an sg_table. This way we can migrate the interface over to a new design over time. Regards, Christian. > > Thanks for the feedback. > > Cheers, > -Paul > >>> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >>> >>> --- >>> v3: New patch >>> --- >>> include/linux/dmaengine.h | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h >>> index c3656e590213..62efa28c009a 100644 >>> --- a/include/linux/dmaengine.h >>> +++ b/include/linux/dmaengine.h >>> @@ -912,6 +912,11 @@ struct dma_device { >>> struct dma_async_tx_descriptor >>> *(*device_prep_dma_interrupt)( >>> struct dma_chan *chan, unsigned long flags); >>> >>> + struct dma_async_tx_descriptor >>> *(*device_prep_slave_dma_array)( >>> + struct dma_chan *chan, dma_addr_t *addrs, >>> + size_t *lengths, size_t nb, >>> + enum dma_transfer_direction direction, >>> + unsigned long flags); >> Then the callback looks like >> >> struct dma_async_tx_descriptor *(*device_prep_slave_vec)( >> struct dma_chan *chan, >> struct dma_vec *vec, >> int nvec, >> enum dma_transfer_direction direction, >> unsigned long flags);
Hi Jonathan, Here's the v3 of my patchset that introduces a new interface based on DMABUF objects to complement the fileio API, and adds write() support to the existing fileio API. It changed quite a lot since V2; the IIO subsystem is now just a DMABUF importer, and all the complexity related to handling creation, deletion and export of DMABUFs (including DMA mapping etc.) is gone. This new interface will be used by Libiio. The code is ready[1] and will be merged to the main branch as soon as the kernel bits are accepted upstream. Note that Libiio (and its server counterpart, iiod) use this new interface in two different ways: - by memory-mapping the DMABUFs to access the sample data directly, which is much faster than using the existing fileio API as the sample data does not need to be copied; - by passing the DMABUFs around directly to the USB stack, in a device-to-device zero-copy fashion, using a new DMABUF interface for the USB (FunctionFS to be exact) stack, which is being upstreamed in parallel of this patchset [2]. As for write() support, Nuno (Cc'd) said he will work on upstreaming the DAC counterpart of adc/adi-axi-adc.c in the next few weeks, so there will be a user for the buffer write() support. I hope you are okay with this - otherwise, we can just wait until this work is done, and I still benefit from sending this patchset early to get feedback. Finally, the dmaengine implementation for this new interface requires a new dmaengine API function, since dmaengine_prep_slave_sg() will always transfer the full scatterlist unconditionally, while we want to be able to transfer an arbitrary amount of bytes from/to the DMABUF. Since scatterlists seem to be going away soon, the new API function will take an array of DMA addresses + lengths. I am open to suggestions if anybody (especially Vinod) have a better design in mind. Cheers, -Paul [1]: https://github.com/analogdevicesinc/libiio/pull/928 [2]: https://lore.kernel.org/linux-usb/425c1b8ea20002c6344a574cd094b4c715c67ba6.camel@crapouillou.net/T/#t --- Changelog: * Patches 01-02 are new; * Patches [03/11], [05/11] didn't change; * Patch [04/11]: - Reorganize arguments to iio_dma_buffer_io() - Change 'is_write' argument to 'is_from_user' - Change (__force char *) to (__force __user char *), in iio_dma_buffer_write(), since we only want to drop the "const". * Patch [07/11]: - Get rid of the old IOCTLs. The IIO subsystem does not create or manage DMABUFs anymore, and only attaches/detaches externally created DMABUFs. - Add IIO_BUFFER_DMABUF_CYCLIC to the supported flags. * Patch [09/11]: Update code to provide the functions that will be used as callbacks for the new IOCTLs. * Patch [10/11]: Use the new dmaengine_prep_slave_dma_array(), and adapt the code to work with the new functions introduced in industrialio-buffer-dma.c. * Patch [11/11]: Update the documentation to reflect the new API. --- Alexandru Ardelean (1): iio: buffer-dma: split iio_dma_buffer_fileio_free() function Paul Cercueil (10): dmaengine: Add API function dmaengine_prep_slave_dma_array() dmaengine: dma-axi-dmac: Implement device_prep_slave_dma_array iio: buffer-dma: Get rid of outgoing queue iio: buffer-dma: Enable buffer write support iio: buffer-dmaengine: Support specifying buffer direction iio: buffer-dmaengine: Enable write support iio: core: Add new DMABUF interface infrastructure iio: buffer-dma: Enable support for DMABUFs iio: buffer-dmaengine: Support new DMABUF based userspace API Documentation: iio: Document high-speed DMABUF based API Documentation/iio/dmabuf_api.rst | 59 +++ Documentation/iio/index.rst | 2 + drivers/dma/dma-axi-dmac.c | 41 ++ drivers/iio/adc/adi-axi-adc.c | 3 +- drivers/iio/buffer/industrialio-buffer-dma.c | 331 +++++++++++--- .../buffer/industrialio-buffer-dmaengine.c | 77 +++- drivers/iio/industrialio-buffer.c | 402 ++++++++++++++++++ include/linux/dmaengine.h | 16 + include/linux/iio/buffer-dma.h | 40 +- include/linux/iio/buffer-dmaengine.h | 5 +- include/linux/iio/buffer_impl.h | 22 + include/uapi/linux/iio/buffer.h | 22 + 12 files changed, 939 insertions(+), 81 deletions(-) create mode 100644 Documentation/iio/dmabuf_api.rst