Re: [PATCH v2 00/12] iio: buffer-dma: write() and new DMABUF based API

Daniel Vetter
2022-03-29T08:33:44Z

On Tue, Feb 15, 2022 at 05:43:35PM +0000, Paul Cercueil wrote:
> Hi Jonathan,
>
> Le dim., févr. 13 2022 at 18:46:16 +0000, Jonathan Cameron
> <jic23@kernel.org> a écrit :
> > On Mon, 7 Feb 2022 12:59:21 +0000
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >
> > > Hi Jonathan,
> > >
> > > This is the V2 of my patchset that introduces a new userspace
> > > interface
> > > based on DMABUF objects to complement the fileio API, and adds
> > > write()
> > > support to the existing fileio API.
> >
> > Hi Paul,
> >
> > It's been a little while. Perhaps you could summarize the various view
> > points around the appropriateness of using DMABUF for this?
> > I appreciate it is a tricky topic to distil into a brief summary but
> > I know I would find it useful even if no one else does!
>
> So we want to have a high-speed interface where buffers of samples are
> passed around between IIO devices and other devices (e.g. USB or network),
> or made available to userspace without copying the data.
>
> DMABUF is, at least in theory, exactly what we need. Quoting the
> documentation
> (https://www.kernel.org/doc/html/v5.15/driver-api/dma-buf.html):
> "The dma-buf subsystem provides the framework for sharing buffers for
> hardware (DMA) access across multiple device drivers and subsystems, and for
> synchronizing asynchronous hardware access. This is used, for example, by
> drm “prime” multi-GPU support, but is of course not limited to GPU use
> cases."
>
> The problem is that right now DMABUF is only really used by DRM, and to
> quote Daniel, "dma-buf looks like something super generic and useful, until
> you realize that there's a metric ton of gpu/accelerator bagage piled in".
>
> Still, it seems to be the only viable option. We could add a custom
> buffer-passing interface, but that would mean implementing the same
> buffer-passing interface on the network and USB stacks, and before we know
> it we re-invented DMABUFs.

dma-buf also doesn't support sharing with network and usb stacks, so I'm a
bit confused why exactly this is useful?

So yeah unless there's some sharing going on with gpu stuff (for data
processing maybe) I'm not sure this makes a lot of sense really. Or at
least some zero-copy sharing between drivers, but even that would
minimally require a dma-buf import ioctl of some sorts. Which I either
missed or doesn't exist.

If there's none of that then just hand-roll your buffer handling code
(xarray is cheap to use in terms of code for this), you can always add
dma-buf import/export later on when the need arises.

Scrolling through patches you only have dma-buf export, but no importing,
so the use-case that works is with one of the existing subsystems that
supporting dma-buf importing.

I think minimally we need the use-case (in form of code) that needs the
buffer sharing here.
-Daniel

> Cheers,
> -Paul
>
>
> > >
> > > Changes since v1:
> > >
> > > - the patches that were merged in v1 have been (obviously) dropped
> > > from
> > > this patchset;
> > > - the patch that was setting the write-combine cache setting has
> > > been
> > > dropped as well, as it was simply not useful.
> > > - [01/12]:
> > > * Only remove the outgoing queue, and keep the incoming queue,
> > > as we
> > > want the buffer to start streaming data as soon as it is
> > > enabled.
> > > * Remove IIO_BLOCK_STATE_DEQUEUED, since it is now functionally
> > > the
> > > same as IIO_BLOCK_STATE_DONE.
> > > - [02/12]:
> > > * Fix block->state not being reset in
> > > iio_dma_buffer_request_update() for output buffers.
> > > * Only update block->bytes_used once and add a comment about
> > > why we
> > > update it.
> > > * Add a comment about why we're setting a different state for
> > > output
> > > buffers in iio_dma_buffer_request_update()
> > > * Remove useless cast to bool (!!) in iio_dma_buffer_io()
> > > - [05/12]:
> > > Only allow the new IOCTLs on the buffer FD created with
> > > IIO_BUFFER_GET_FD_IOCTL().
> > > - [12/12]:
> > > * Explicitly state that the new interface is optional and is
> > > not implemented by all drivers.
> > > * The IOCTLs can now only be called on the buffer FD returned by
> > > IIO_BUFFER_GET_FD_IOCTL.
> > > * Move the page up a bit in the index since it is core stuff
> > > and not
> > > driver-specific.
> > >
> > > The patches not listed here have not been modified since v1.
> > >
> > > Cheers,
> > > -Paul
> > >
> > > Alexandru Ardelean (1):
> > > iio: buffer-dma: split iio_dma_buffer_fileio_free() function
> > >
> > > Paul Cercueil (11):
> > > 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: Use DMABUFs instead of custom solution
> > > iio: buffer-dma: Implement new DMABUF based userspace API
> > > iio: buffer-dmaengine: Support new DMABUF based userspace API
> > > iio: core: Add support for cyclic buffers
> > > iio: buffer-dmaengine: Add support for cyclic buffers
> > > Documentation: iio: Document high-speed DMABUF based API
> > >
> > > Documentation/driver-api/dma-buf.rst | 2 +
> > > Documentation/iio/dmabuf_api.rst | 94 +++
> > > Documentation/iio/index.rst | 2 +
> > > drivers/iio/adc/adi-axi-adc.c | 3 +-
> > > drivers/iio/buffer/industrialio-buffer-dma.c | 610
> > > ++++++++++++++----
> > > .../buffer/industrialio-buffer-dmaengine.c | 42 +-
> > > drivers/iio/industrialio-buffer.c | 60 ++
> > > include/linux/iio/buffer-dma.h | 38 +-
> > > include/linux/iio/buffer-dmaengine.h | 5 +-
> > > include/linux/iio/buffer_impl.h | 8 +
> > > include/uapi/linux/iio/buffer.h | 30 +
> > > 11 files changed, 749 insertions(+), 145 deletions(-)
> > > create mode 100644 Documentation/iio/dmabuf_api.rst
> > >
> >
>
>

--
Daniel Vetter
Software Engineer, Intel Corporation