[v3,00/11] iio: new DMABUF based API, v3

Message ID 20230403154800.215924-1-paul@crapouillou.net (mailing list archive)
Headers
Series iio: new DMABUF based API, v3 |

Message

Paul Cercueil April 3, 2023, 3:47 p.m. UTC
  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
  

Comments

Paul Cercueil April 4, 2023, 7:42 a.m. UTC | #1
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);
  
Nuno Sá April 4, 2023, 7:44 a.m. UTC | #2
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á
  
Christian König April 4, 2023, 8:54 a.m. UTC | #3
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);