Re: [PATCH 07/15] iio: core: Add new DMABUF interface infrastructure

2021-11-21 Thread Jonathan Cameron
On Mon, 15 Nov 2021 14:19:17 +
Paul Cercueil  wrote:

> Add the necessary infrastructure to the IIO core to support a new DMABUF
> based interface.
> 
> The advantage of this new DMABUF based interface vs. the read()
> interface, is that it avoids an extra copy of the data between the
> kernel and userspace. This is particularly userful for high-speed
> devices which produce several megabytes or even gigabytes of data per
> second.
> 
> The data in this new DMABUF interface is managed at the granularity of
> DMABUF objects. Reducing the granularity from byte level to block level
> is done to reduce the userspace-kernelspace synchronization overhead
> since performing syscalls for each byte at a few Mbps is just not
> feasible.
> 
> This of course leads to a slightly increased latency. For this reason an
> application can choose the size of the DMABUFs as well as how many it
> allocates. E.g. two DMABUFs would be a traditional double buffering
> scheme. But using a higher number might be necessary to avoid
> underflow/overflow situations in the presence of scheduling latencies.
> 
> As part of the interface, 2 new IOCTLs have been added:
> 
> IIO_BUFFER_DMABUF_ALLOC_IOCTL(struct iio_dmabuf_alloc_req *):
>  Each call will allocate a new DMABUF object. The return value (if not
>  a negative errno value as error) will be the file descriptor of the new
>  DMABUF.
> 
> IIO_BUFFER_DMABUF_ENQUEUE_IOCTL(struct iio_dmabuf *):
>  Place the DMABUF object into the queue pending for hardware process.
> 
> These two IOCTLs have to be performed on the IIO buffer's file
> descriptor (either opened from the corresponding /dev/iio:deviceX, or
> obtained using the IIO_BUFFER_GET_FD_IOCTL() ioctl).

Unrelated to this patch except tangentially.  Maybe we should enable
new buffer features only on the IIO_BUFFER_GET_FD_IOCTL() route as
we probably want to deprecate the old interfaces due to the it only
supporting a single buffer / datastream per device.

Possibly something for another day...

Nothing to add on actual code...

Jonathan

> 
> To access the data stored in a block by userspace the block must be
> mapped to the process's memory. This is done by calling mmap() on the
> DMABUF's file descriptor.
> 
> Before accessing the data through the map, you must use the
> DMA_BUF_IOCTL_SYNC(struct dma_buf_sync *) ioctl, with the
> DMA_BUF_SYNC_START flag, to make sure that the data is available.
> This call may block until the hardware is done with this block. Once
> you are done reading or writing the data, you must use this ioctl again
> with the DMA_BUF_SYNC_END flag, before enqueueing the DMABUF to the
> kernel's queue.
> 
> If you need to know when the hardware is done with a DMABUF, you can
> poll its file descriptor for the EPOLLOUT event.
> 
> Finally, to destroy a DMABUF object, simply call close() on its file
> descriptor.
> 
> A typical workflow for the new interface is:
> 
>   for block in blocks:
> DMABUF_ALLOC block
> mmap block
> 
>   enable buffer
> 
>   while !done
> for block in blocks:
>   DMABUF_ENQUEUE block
> 
>   DMABUF_SYNC_START block
>   process data
>   DMABUF_SYNC_END block
> 
>   disable buffer
> 
>   for block in blocks:
> close block
> 
> Signed-off-by: Paul Cercueil 
> ---
>  drivers/iio/industrialio-buffer.c | 44 +++
>  include/linux/iio/buffer_impl.h   |  8 ++
>  include/uapi/linux/iio/buffer.h   | 29 
>  3 files changed, 81 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c 
> b/drivers/iio/industrialio-buffer.c
> index e180728914c0..30910e6c2346 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -1585,12 +1586,55 @@ static long iio_device_buffer_getfd(struct iio_dev 
> *indio_dev, unsigned long arg
>   return ret;
>  }
>  
> +static int iio_buffer_enqueue_dmabuf(struct iio_buffer *buffer,
> +  struct iio_dmabuf __user *user_buf)
> +{
> + struct iio_dmabuf dmabuf;
> +
> + if (!buffer->access->enqueue_dmabuf)
> + return -EPERM;
> +
> + if (copy_from_user(, user_buf, sizeof(dmabuf)))
> + return -EFAULT;
> +
> + if (dmabuf.flags & ~IIO_BUFFER_DMABUF_SUPPORTED_FLAGS)
> + return -EINVAL;
> +
> + return buffer->access->enqueue_dmabuf(buffer, );
> +}
> +
> +static int iio_buffer_alloc_dmabuf(struct iio_buffer *buffer,
> +struct iio_dmabuf_alloc_req __user *user_req)
> +{
> + struct iio_dmabuf_alloc_req req;
> +
> + if (!buffer->access->alloc_dmabuf)
> + return -EPERM;
> +
> + if (copy_from_user(, user_req, sizeof(req)))
> + return -EFAULT;
> +
> + if (req.resv)
> + return -EINVAL;
> +
> + return buffer->access->alloc_dmabuf(buffer, );
> +}
> +
>  static long 

[PATCH 07/15] iio: core: Add new DMABUF interface infrastructure

2021-11-15 Thread Paul Cercueil
Add the necessary infrastructure to the IIO core to support a new DMABUF
based interface.

The advantage of this new DMABUF based interface vs. the read()
interface, is that it avoids an extra copy of the data between the
kernel and userspace. This is particularly userful for high-speed
devices which produce several megabytes or even gigabytes of data per
second.

The data in this new DMABUF interface is managed at the granularity of
DMABUF objects. Reducing the granularity from byte level to block level
is done to reduce the userspace-kernelspace synchronization overhead
since performing syscalls for each byte at a few Mbps is just not
feasible.

This of course leads to a slightly increased latency. For this reason an
application can choose the size of the DMABUFs as well as how many it
allocates. E.g. two DMABUFs would be a traditional double buffering
scheme. But using a higher number might be necessary to avoid
underflow/overflow situations in the presence of scheduling latencies.

As part of the interface, 2 new IOCTLs have been added:

IIO_BUFFER_DMABUF_ALLOC_IOCTL(struct iio_dmabuf_alloc_req *):
 Each call will allocate a new DMABUF object. The return value (if not
 a negative errno value as error) will be the file descriptor of the new
 DMABUF.

IIO_BUFFER_DMABUF_ENQUEUE_IOCTL(struct iio_dmabuf *):
 Place the DMABUF object into the queue pending for hardware process.

These two IOCTLs have to be performed on the IIO buffer's file
descriptor (either opened from the corresponding /dev/iio:deviceX, or
obtained using the IIO_BUFFER_GET_FD_IOCTL() ioctl).

To access the data stored in a block by userspace the block must be
mapped to the process's memory. This is done by calling mmap() on the
DMABUF's file descriptor.

Before accessing the data through the map, you must use the
DMA_BUF_IOCTL_SYNC(struct dma_buf_sync *) ioctl, with the
DMA_BUF_SYNC_START flag, to make sure that the data is available.
This call may block until the hardware is done with this block. Once
you are done reading or writing the data, you must use this ioctl again
with the DMA_BUF_SYNC_END flag, before enqueueing the DMABUF to the
kernel's queue.

If you need to know when the hardware is done with a DMABUF, you can
poll its file descriptor for the EPOLLOUT event.

Finally, to destroy a DMABUF object, simply call close() on its file
descriptor.

A typical workflow for the new interface is:

  for block in blocks:
DMABUF_ALLOC block
mmap block

  enable buffer

  while !done
for block in blocks:
  DMABUF_ENQUEUE block

  DMABUF_SYNC_START block
  process data
  DMABUF_SYNC_END block

  disable buffer

  for block in blocks:
close block

Signed-off-by: Paul Cercueil 
---
 drivers/iio/industrialio-buffer.c | 44 +++
 include/linux/iio/buffer_impl.h   |  8 ++
 include/uapi/linux/iio/buffer.h   | 29 
 3 files changed, 81 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c 
b/drivers/iio/industrialio-buffer.c
index e180728914c0..30910e6c2346 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -1585,12 +1586,55 @@ static long iio_device_buffer_getfd(struct iio_dev 
*indio_dev, unsigned long arg
return ret;
 }
 
+static int iio_buffer_enqueue_dmabuf(struct iio_buffer *buffer,
+struct iio_dmabuf __user *user_buf)
+{
+   struct iio_dmabuf dmabuf;
+
+   if (!buffer->access->enqueue_dmabuf)
+   return -EPERM;
+
+   if (copy_from_user(, user_buf, sizeof(dmabuf)))
+   return -EFAULT;
+
+   if (dmabuf.flags & ~IIO_BUFFER_DMABUF_SUPPORTED_FLAGS)
+   return -EINVAL;
+
+   return buffer->access->enqueue_dmabuf(buffer, );
+}
+
+static int iio_buffer_alloc_dmabuf(struct iio_buffer *buffer,
+  struct iio_dmabuf_alloc_req __user *user_req)
+{
+   struct iio_dmabuf_alloc_req req;
+
+   if (!buffer->access->alloc_dmabuf)
+   return -EPERM;
+
+   if (copy_from_user(, user_req, sizeof(req)))
+   return -EFAULT;
+
+   if (req.resv)
+   return -EINVAL;
+
+   return buffer->access->alloc_dmabuf(buffer, );
+}
+
 static long iio_device_buffer_ioctl(struct iio_dev *indio_dev, struct file 
*filp,
unsigned int cmd, unsigned long arg)
 {
+   struct iio_dev_buffer_pair *ib = filp->private_data;
+   struct iio_buffer *buffer = ib->buffer;
+   void __user *_arg = (void __user *)arg;
+
switch (cmd) {
case IIO_BUFFER_GET_FD_IOCTL:
return iio_device_buffer_getfd(indio_dev, arg);
+   case IIO_BUFFER_DMABUF_ALLOC_IOCTL:
+   return iio_buffer_alloc_dmabuf(buffer, _arg);
+   case IIO_BUFFER_DMABUF_ENQUEUE_IOCTL:
+   /* TODO: support non-blocking enqueue operation */
+