2021-02-12 10:12:30

by Alexandru Ardelean

[permalink] [raw]
Subject: [PATCH v2 0/3] iio: core,buffer-dma: add mmap support

Changelog v1 -> v2:
* https://lore.kernel.org/linux-iio/[email protected]/T/#t
* removed IIO_BUFFER_BLOCK_FLAG_CYCLIC flag; will be added in a later
patch
* removed extra line in tools/iio/iio_generic_buffer.c
* patch 'iio: core: Add mmap interface infrastructure'
added docstrings for new hooks (alloc_blocks, mmap, etc)

This is basically Lars' work adapted from branch:
https://github.com/larsclausen/linux/commits/iio-high-speed-5.10
[hopefully i got the stuff correctly from that branch]

What is different, is that this one is adapted on top of the multibuffer
support (currently at v5) discussed here:
https://lore.kernel.org/linux-iio/[email protected]/T/#t

Also, adapted an example for high-speed/mmap support in
'tools/iio/iio_generic_buffer.c'

The example is adapted from libiio:
https://github.com/analogdevicesinc/libiio/blob/master/local.c#L51
but will all the ioctl()s organized after the one that are reserved
(hopefully) for IIO

Tested that mmap() works.
Moved (artifically) valid buffer0 as buffer2 and the operation still
works.

Alexandru Ardelean (1):
tools: iio: add example for high-speed buffer support

Lars-Peter Clausen (2):
iio: core: Add mmap interface infrastructure
iio: buffer-dma: Add mmap support

drivers/iio/buffer/industrialio-buffer-dma.c | 314 ++++++++++++++++--
.../buffer/industrialio-buffer-dmaengine.c | 22 +-
drivers/iio/industrialio-buffer.c | 158 +++++++++
include/linux/iio/buffer-dma.h | 25 +-
include/linux/iio/buffer_impl.h | 23 ++
include/uapi/linux/iio/buffer.h | 26 ++
tools/iio/iio_generic_buffer.c | 183 +++++++++-
7 files changed, 704 insertions(+), 47 deletions(-)

--
2.17.1


2021-02-12 10:13:52

by Alexandru Ardelean

[permalink] [raw]
Subject: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support

Following a recent update to the IIO buffer infrastructure, this change
adds a basic example on how to access an IIO buffer via the new mmap()
interface.

The ioctl() for the high-speed mode needs to be enabled right from the
start, before setting any parameters via sysfs (length, enable, etc), to
make sure that the mmap mode is used and not the fileio mode.

Signed-off-by: Alexandru Ardelean <[email protected]>
---
tools/iio/iio_generic_buffer.c | 183 +++++++++++++++++++++++++++++++--
1 file changed, 177 insertions(+), 6 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index fdd08514d556..675a7e6047e0 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -31,6 +31,7 @@
#include <stdbool.h>
#include <signal.h>
#include <sys/ioctl.h>
+#include <sys/mman.h>
#include <linux/iio/buffer.h>
#include "iio_utils.h"

@@ -239,6 +240,132 @@ static int enable_disable_all_channels(char *dev_dir_name, int buffer_idx, int e
return 0;
}

+struct mmap_block {
+ struct iio_buffer_block block;
+ void *addr;
+};
+
+static struct mmap_block *enable_high_speed(int buf_fd, unsigned int block_size,
+ int nblocks)
+{
+ struct iio_buffer_block_alloc_req req = { 0 };
+ struct mmap_block *mmaps = NULL;
+ int mmaps_cnt = 0;
+ int i, ret;
+
+ /**
+ * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
+ * If using just BLOCK_ALLOC it's distinguish between ENOSYS
+ * and other error types.
+ */
+ ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
+ if (ret < 0) {
+ errno = ENOSYS;
+ return NULL;
+ }
+
+ /* for now, this */
+ req.id = 0;
+ req.type = 0;
+ req.size = block_size;
+ req.count = nblocks;
+
+ ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, &req);
+ if (ret < 0)
+ return NULL;
+
+ if (req.count == 0) {
+ errno = ENOMEM;
+ return NULL;
+ }
+
+ if (req.count < nblocks) {
+ fprintf(stderr, "Requested %d blocks, got %d\n",
+ nblocks, req.count);
+ errno = ENOMEM;
+ return NULL;
+ }
+
+ mmaps = calloc(req.count, sizeof(*mmaps));
+ if (!mmaps) {
+ errno = ENOMEM;
+ return NULL;
+ }
+
+ for (i = 0; i < req.count; i++) {
+ mmaps[i].block.id = i;
+ ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL, &mmaps[i].block);
+ if (ret < 0)
+ goto error;
+
+ ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, &mmaps[i].block);
+ if (ret < 0)
+ goto error;
+
+ mmaps[i].addr = mmap(0, mmaps[i].block.size,
+ PROT_READ | PROT_WRITE, MAP_SHARED,
+ buf_fd, mmaps[i].block.data.offset);
+
+ if (mmaps[i].addr == MAP_FAILED)
+ goto error;
+
+ mmaps_cnt++;
+ }
+
+ return mmaps;
+
+error:
+ for (i = 0; i < mmaps_cnt; i++)
+ munmap(mmaps[i].addr, mmaps[i].block.size);
+ free(mmaps);
+ return NULL;
+}
+
+static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
+ struct mmap_block *mmaps, unsigned int mmaps_cnt)
+{
+ struct iio_buffer_block block;
+ int ret;
+
+ /**
+ * This is where some buffer-pool management can do wonders,
+ * but for the sake of this sample-code, we're just going to
+ * copy the data and re-enqueue it back
+ */
+ memset(&block, 0, sizeof(block));
+ ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, &block);
+ if (ret < 0)
+ return ret;
+
+ /* check for weird conditions */
+ if (block.bytes_used > block_size) {
+ fprintf(stderr,
+ "Got a bigger block (%u) than expected (%u)\n",
+ block.bytes_used, block_size);
+ return -EFBIG;
+ }
+
+ if (block.bytes_used < block_size) {
+ /**
+ * This can be normal, with some real-world data
+ * terminating abruptly. But log it.
+ */
+ fprintf(stderr,
+ "Got a smaller block (%u) than expected (%u)\n",
+ block.bytes_used, block_size);
+ }
+
+ /* memcpy() the data, we lose some more performance here :p */
+ memcpy(data, mmaps[block.id].addr, block.bytes_used);
+
+ /* and re-queue this back */
+ ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, &mmaps[block.id].block);
+ if (ret < 0)
+ return ret;
+
+ return block.bytes_used;
+}
+
static void print_usage(void)
{
fprintf(stderr, "Usage: generic_buffer [options]...\n"
@@ -249,6 +376,7 @@ static void print_usage(void)
" -c <n> Do n conversions, or loop forever if n < 0\n"
" -e Disable wait for event (new data)\n"
" -g Use trigger-less mode\n"
+ " -h Use high-speed buffer access\n"
" -l <n> Set buffer length to n samples\n"
" --device-name -n <name>\n"
" --device-num -N <num>\n"
@@ -356,9 +484,15 @@ int main(int argc, char **argv)

struct iio_channel_info *channels = NULL;

+ static bool use_high_speed = false;
+ unsigned int block_size;
+ int nblocks = 16; /* default */
+ int mmaps_cnt = 0;
+ struct mmap_block *mmaps = NULL;
+
register_cleanup();

- while ((c = getopt_long(argc, argv, "aAb:c:egl:n:N:t:T:w:?", longopts,
+ while ((c = getopt_long(argc, argv, "aAb:c:eghl:n:N:t:T:w:?", longopts,
NULL)) != -1) {
switch (c) {
case 'a':
@@ -396,6 +530,9 @@ int main(int argc, char **argv)
case 'g':
notrigger = 1;
break;
+ case 'h':
+ use_high_speed = true;
+ break;
case 'l':
errno = 0;
buf_len = strtoul(optarg, &dummy, 10);
@@ -661,6 +798,29 @@ int main(int argc, char **argv)
goto error;
}

+ scan_size = size_from_channelarray(channels, num_channels);
+ block_size = scan_size * buf_len;
+ /**
+ * Need to enable high-speed before configuring length/enable.
+ * Otherwise, the DMA buffer will work in fileio mode,
+ * and mmap won't work.
+ */
+ if (use_high_speed) {
+ /**
+ * The block_size for one block is the same as 'data', but it
+ * doesn't need to be the same size. It is easier for the sake
+ * of this example.
+ */
+ mmaps = enable_high_speed(buf_fd, block_size, nblocks);
+ if (!mmaps) {
+ fprintf(stderr, "Could not enable high-speed mode\n");
+ ret = -errno;
+ goto error;
+ }
+ mmaps_cnt = nblocks;
+ printf("Using high-speed mode\n");
+ }
+
/* Setup ring buffer parameters */
ret = write_sysfs_int("length", buf_dir_name, buf_len);
if (ret < 0)
@@ -675,8 +835,7 @@ int main(int argc, char **argv)
goto error;
}

- scan_size = size_from_channelarray(channels, num_channels);
- data = malloc(scan_size * buf_len);
+ data = malloc(block_size);
if (!data) {
ret = -ENOMEM;
goto error;
@@ -719,7 +878,13 @@ int main(int argc, char **argv)
toread = 64;
}

- read_size = read(buf_fd, data, toread * scan_size);
+ if (use_high_speed) {
+ read_size = read_high_speed(buf_fd, data, block_size,
+ mmaps, mmaps_cnt);
+ } else {
+ read_size = read(buf_fd, data, toread * scan_size);
+ }
+
if (read_size < 0) {
if (errno == EAGAIN) {
fprintf(stderr, "nothing available\n");
@@ -738,8 +903,14 @@ int main(int argc, char **argv)

if (fd >= 0 && close(fd) == -1)
perror("Failed to close character device");
- if (buf_fd >= 0 && close(buf_fd) == -1)
- perror("Failed to close buffer");
+ for (i = 0; i < mmaps_cnt; i++)
+ munmap(mmaps[i].addr, mmaps[i].block.size);
+ free(mmaps);
+ if (buf_fd >= 0) {
+ ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
+ if (close(buf_fd) == -1)
+ perror("Failed to close buffer");
+ }
free(buffer_access);
free(data);
free(buf_dir_name);
--
2.17.1

2021-02-12 10:14:23

by Alexandru Ardelean

[permalink] [raw]
Subject: [PATCH v2 1/3] iio: core: Add mmap interface infrastructure

From: Lars-Peter Clausen <[email protected]>

Add the necessary infrastructure to the IIO core to support an mmap based
interface to access the capture data.

The advantage of the mmap based interface compared to the read() based
interface is that it avoids an extra copy of the data between kernel and
userspace. This is particular useful for high-speed devices which produce
several megabytes or even gigabytes of data per second.

The data for the mmap interface is managed at the granularity of so called
blocks. A block is a contiguous region of memory (at the moment both
physically and virtually contiguous). 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
data-rate of a few megabytes is not feasible.

This of course leads to a slightly increased latency. For this reason an
application can choose the size of the blocks as well as how many blocks it
allocates. E.g. two blocks 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.

A block can either be owned by kernel space or userspace. When owned by
userspace it save to access the data in the block and process it. When
owned by kernel space the block can be in one of 3 states.

It can be in the incoming queue where all blocks submitted from userspace
are placed and are waiting to be processed by the kernel driver.

It can be currently being processed by the kernel driver, this means it is
actively placing capturing data in it (usually using DMA).

Or it can be in the outgoing queue where all blocks that have been
processed by the kernel are placed. Userspace can dequeue the blocks as
necessary.

As part of the interface 5 new IOCTLs to manage the blocks and exchange
them between userspace and kernelspace. The IOCTLs can be accessed through
a open file descriptor to a IIO device.

IIO_BUFFER_BLOCK_ALLOC_IOCTL(struct iio_buffer_block_alloc_req *):
Allocates new blocks. Can be called multiple times if necessary. A newly
allocated block is initially owned by userspace.

IIO_BUFFER_BLOCK_FREE_IOCTL(void):
Frees all previously allocated blocks. If the backing memory of a block is
still in use by a kernel driver (i.e. active DMA transfer) it will be
freed once the kernel driver has released it.

IIO_BUFFER_BLOCK_QUERY_IOCTL(struct iio_buffer_block *):
Queries information about a block. The id of the block about which
information is to be queried needs to be set by userspace.

IIO_BUFFER_BLOCK_ENQUEUE_IOCTL(struct iio_buffer_block *):
Places a block on the incoming queue. This transfers ownership of the
block from userspace to kernelspace. Userspace must populate the id field
of the block to indicate which block to enqueue.

IIO_BUFFER_BLOCK_DEQUEUE_IOCTL(struct iio_buffer_block *):
Removes the first block from the outgoing queue. This transfers ownership
of the block from kernelspace to userspace. Kernelspace will populate all
fields of the block. If the queue is empty and the file descriptor is set
to blocking the IOCTL will block until a new block is available on the
outgoing queue.

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 IIO device
file descriptor. Each block has a unique offset assigned to it which should
be passed to the mmap interface. E.g.

mmap(0, block.size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
block.offset);

A typical workflow for the new interface is:

BLOCK_ALLOC

foreach block
BLOCK_QUERY block
mmap block.data.offset
BLOCK_ENQUEUE block

enable buffer

while !done
BLOCK_DEQUEUE block
process data
BLOCK_ENQUEUE block

disable buffer

BLOCK_FREE

Signed-off-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Alexandru Ardelean <[email protected]>
---
drivers/iio/industrialio-buffer.c | 157 ++++++++++++++++++++++++++++++
include/linux/iio/buffer-dma.h | 5 -
include/linux/iio/buffer_impl.h | 23 +++++
include/uapi/linux/iio/buffer.h | 26 +++++
4 files changed, 206 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 3aa6702a5811..50228df0b09f 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -16,6 +16,7 @@
#include <linux/fs.h>
#include <linux/cdev.h>
#include <linux/slab.h>
+#include <linux/mm.h>
#include <linux/poll.h>
#include <linux/sched/signal.h>

@@ -1370,6 +1371,12 @@ static void iio_buffer_unregister_legacy_sysfs_groups(struct iio_dev *indio_dev)
kfree(iio_dev_opaque->legacy_scan_el_group.attrs);
}

+static void iio_buffer_free_blocks(struct iio_buffer *buffer)
+{
+ if (buffer->access->free_blocks)
+ buffer->access->free_blocks(buffer);
+}
+
static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)
{
struct iio_dev_buffer_pair *ib = filep->private_data;
@@ -1378,18 +1385,24 @@ static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)

wake_up(&buffer->pollq);
clear_bit(IIO_BUSY_BIT_POS, &buffer->flags);
+ iio_buffer_free_blocks(buffer);
iio_device_put(indio_dev);
kfree(ib);

return 0;
}

+static long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
+static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma);
+
static const struct file_operations iio_buffer_chrdev_fileops = {
.owner = THIS_MODULE,
.llseek = noop_llseek,
.read = iio_buffer_read,
.poll = iio_buffer_poll,
+ .unlocked_ioctl = iio_buffer_ioctl,
.compat_ioctl = compat_ptr_ioctl,
+ .mmap = iio_buffer_mmap,
.release = iio_buffer_chrdev_release,
};

@@ -1762,6 +1775,150 @@ void iio_buffer_put(struct iio_buffer *buffer)
}
EXPORT_SYMBOL_GPL(iio_buffer_put);

+static int iio_buffer_query_block(struct iio_buffer *buffer,
+ struct iio_buffer_block __user *user_block)
+{
+ struct iio_buffer_block block;
+ int ret;
+
+ if (!buffer->access->query_block)
+ return -ENOSYS;
+
+ if (copy_from_user(&block, user_block, sizeof(block)))
+ return -EFAULT;
+
+ ret = buffer->access->query_block(buffer, &block);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(user_block, &block, sizeof(block)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int iio_buffer_dequeue_block(struct iio_dev *indio_dev,
+ struct iio_buffer *buffer,
+ struct iio_buffer_block __user *user_block,
+ bool non_blocking)
+{
+ struct iio_buffer_block block;
+ int ret;
+
+ if (!buffer->access->dequeue_block)
+ return -ENOSYS;
+
+ do {
+ if (!iio_buffer_data_available(buffer)) {
+ if (non_blocking)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(buffer->pollq,
+ iio_buffer_data_available(buffer) ||
+ indio_dev->info == NULL);
+ if (ret)
+ return ret;
+ if (indio_dev->info == NULL)
+ return -ENODEV;
+ }
+
+ ret = buffer->access->dequeue_block(buffer, &block);
+ if (ret == -EAGAIN && non_blocking)
+ ret = 0;
+ } while (ret);
+
+ if (ret)
+ return ret;
+
+ if (copy_to_user(user_block, &block, sizeof(block)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int iio_buffer_enqueue_block(struct iio_buffer *buffer,
+ struct iio_buffer_block __user *user_block)
+{
+ struct iio_buffer_block block;
+
+ if (!buffer->access->enqueue_block)
+ return -ENOSYS;
+
+ if (copy_from_user(&block, user_block, sizeof(block)))
+ return -EFAULT;
+
+ return buffer->access->enqueue_block(buffer, &block);
+}
+
+static int iio_buffer_alloc_blocks(struct iio_buffer *buffer,
+ struct iio_buffer_block_alloc_req __user *user_req)
+{
+ struct iio_buffer_block_alloc_req req;
+ int ret;
+
+ if (!buffer->access->alloc_blocks)
+ return -ENOSYS;
+
+ if (copy_from_user(&req, user_req, sizeof(req)))
+ return -EFAULT;
+
+ ret = buffer->access->alloc_blocks(buffer, &req);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(user_req, &req, sizeof(req)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
+{
+ bool non_blocking = filep->f_flags & O_NONBLOCK;
+ struct iio_dev_buffer_pair *ib = filep->private_data;
+ struct iio_dev *indio_dev = ib->indio_dev;
+ struct iio_buffer *buffer = ib->buffer;
+
+ if (!buffer || !buffer->access)
+ return -ENODEV;
+
+ switch (cmd) {
+ case IIO_BUFFER_BLOCK_ALLOC_IOCTL:
+ return iio_buffer_alloc_blocks(buffer,
+ (struct iio_buffer_block_alloc_req __user *)arg);
+ case IIO_BUFFER_BLOCK_FREE_IOCTL:
+ iio_buffer_free_blocks(buffer);
+ return 0;
+ case IIO_BUFFER_BLOCK_QUERY_IOCTL:
+ return iio_buffer_query_block(buffer,
+ (struct iio_buffer_block __user *)arg);
+ case IIO_BUFFER_BLOCK_ENQUEUE_IOCTL:
+ return iio_buffer_enqueue_block(buffer,
+ (struct iio_buffer_block __user *)arg);
+ case IIO_BUFFER_BLOCK_DEQUEUE_IOCTL:
+ return iio_buffer_dequeue_block(indio_dev, buffer,
+ (struct iio_buffer_block __user *)arg, non_blocking);
+ }
+ return -EINVAL;
+}
+
+static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma)
+{
+ struct iio_dev_buffer_pair *ib = filep->private_data;
+ struct iio_buffer *buffer = ib->buffer;
+
+ if (!buffer->access || !buffer->access->mmap)
+ return -ENODEV;
+
+ if (!(vma->vm_flags & VM_SHARED))
+ return -EINVAL;
+
+ if (!(vma->vm_flags & VM_READ))
+ return -EINVAL;
+
+ return buffer->access->mmap(buffer, vma);
+}
+
/**
* iio_device_attach_buffer - Attach a buffer to a IIO device
* @indio_dev: The device the buffer should be attached to
diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
index ff15c61bf319..6564bdcdac66 100644
--- a/include/linux/iio/buffer-dma.h
+++ b/include/linux/iio/buffer-dma.h
@@ -17,11 +17,6 @@ struct iio_dma_buffer_queue;
struct iio_dma_buffer_ops;
struct device;

-struct iio_buffer_block {
- u32 size;
- u32 bytes_used;
-};
-
/**
* enum iio_block_state - State of a struct iio_dma_buffer_block
* @IIO_BLOCK_STATE_DEQUEUED: Block is not queued
diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index 245b32918ae1..1d57dc7ccb4f 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -34,6 +34,18 @@ struct iio_buffer;
* device stops sampling. Calles are balanced with @enable.
* @release: called when the last reference to the buffer is dropped,
* should free all resources allocated by the buffer.
+ * @alloc_blocks: called from userspace via ioctl to allocate blocks
+ * that will be used via the mmap interface.
+ * @free_blocks: called from userspace via ioctl to free all blocks
+ * allocated for this buffer.
+ * @enqueue_block: called from userspace via ioctl to queue this block
+ * to this buffer. Requires a valid block id.
+ * @dequeue_block: called from userspace via ioctl to dequeue this block
+ * from this buffer. Requires a valid block id.
+ * @query_block: called from userspace via ioctl to query the attributes
+ * of this block. Requires a valid block id.
+ * @mmap: mmap hook for this buffer. Userspace mmap() calls will
+ * get routed to this.
* @modes: Supported operating modes by this buffer type
* @flags: A bitmask combination of INDIO_BUFFER_FLAG_*
*
@@ -60,6 +72,17 @@ struct iio_buffer_access_funcs {

void (*release)(struct iio_buffer *buffer);

+ int (*alloc_blocks)(struct iio_buffer *buffer,
+ struct iio_buffer_block_alloc_req *req);
+ int (*free_blocks)(struct iio_buffer *buffer);
+ int (*enqueue_block)(struct iio_buffer *buffer,
+ struct iio_buffer_block *block);
+ int (*dequeue_block)(struct iio_buffer *buffer,
+ struct iio_buffer_block *block);
+ int (*query_block)(struct iio_buffer *buffer,
+ struct iio_buffer_block *block);
+ int (*mmap)(struct iio_buffer *buffer, struct vm_area_struct *vma);
+
unsigned int modes;
unsigned int flags;
};
diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
index 13939032b3f6..70ad3aea01ea 100644
--- a/include/uapi/linux/iio/buffer.h
+++ b/include/uapi/linux/iio/buffer.h
@@ -5,6 +5,32 @@
#ifndef _UAPI_IIO_BUFFER_H_
#define _UAPI_IIO_BUFFER_H_

+struct iio_buffer_block_alloc_req {
+ __u32 type;
+ __u32 size;
+ __u32 count;
+ __u32 id;
+};
+
+#define IIO_BUFFER_BLOCK_FLAG_TIMESTAMP_VALID (1 << 0)
+
+struct iio_buffer_block {
+ __u32 id;
+ __u32 size;
+ __u32 bytes_used;
+ __u32 type;
+ __u32 flags;
+ union {
+ __u32 offset;
+ } data;
+ __u64 timestamp;
+};
+
#define IIO_BUFFER_GET_FD_IOCTL _IOWR('i', 0x91, int)
+#define IIO_BUFFER_BLOCK_ALLOC_IOCTL _IOWR('i', 0x92, struct iio_buffer_block_alloc_req)
+#define IIO_BUFFER_BLOCK_FREE_IOCTL _IO('i', 0x93)
+#define IIO_BUFFER_BLOCK_QUERY_IOCTL _IOWR('i', 0x93, struct iio_buffer_block)
+#define IIO_BUFFER_BLOCK_ENQUEUE_IOCTL _IOWR('i', 0x94, struct iio_buffer_block)
+#define IIO_BUFFER_BLOCK_DEQUEUE_IOCTL _IOWR('i', 0x95, struct iio_buffer_block)

#endif /* _UAPI_IIO_BUFFER_H_ */
--
2.17.1

2021-02-12 10:14:43

by Alexandru Ardelean

[permalink] [raw]
Subject: [PATCH v2 2/3] iio: buffer-dma: Add mmap support

From: Lars-Peter Clausen <[email protected]>

Add support for the new mmap interface to IIO DMA buffer. This interface
allows to directly map the backing memory of a block to userspace. This is
especially advantageous for high-speed devices where the extra copy from
kernel space to userspace of the data incurs a significant overhead.

In addition this interface allows more fine grained control over how many
blocks are allocated and their size.

Signed-off-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Alexandru Ardelean <[email protected]>
---
drivers/iio/buffer/industrialio-buffer-dma.c | 314 ++++++++++++++++--
.../buffer/industrialio-buffer-dmaengine.c | 22 +-
drivers/iio/industrialio-buffer.c | 1 +
include/linux/iio/buffer-dma.h | 20 +-
4 files changed, 321 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c
index d348af8b9705..befb0a3d2def 100644
--- a/drivers/iio/buffer/industrialio-buffer-dma.c
+++ b/drivers/iio/buffer/industrialio-buffer-dma.c
@@ -90,6 +90,9 @@
* callback is called from within the custom callback.
*/

+static unsigned int iio_dma_buffer_max_block_size = SZ_16M;
+module_param_named(max_block_size, iio_dma_buffer_max_block_size, uint, 0644);
+
static void iio_buffer_block_release(struct kref *kref)
{
struct iio_dma_buffer_block *block = container_of(kref,
@@ -97,7 +100,7 @@ static void iio_buffer_block_release(struct kref *kref)

WARN_ON(block->state != IIO_BLOCK_STATE_DEAD);

- dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->size),
+ dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->block.size),
block->vaddr, block->phys_addr);

iio_buffer_put(&block->queue->buffer);
@@ -178,7 +181,7 @@ static struct iio_dma_buffer_block *iio_dma_buffer_alloc_block(
return NULL;
}

- block->size = size;
+ block->block.size = size;
block->state = IIO_BLOCK_STATE_DEQUEUED;
block->queue = queue;
INIT_LIST_HEAD(&block->head);
@@ -243,7 +246,7 @@ void iio_dma_buffer_block_list_abort(struct iio_dma_buffer_queue *queue,
spin_lock_irqsave(&queue->list_lock, flags);
list_for_each_entry_safe(block, _block, list, head) {
list_del(&block->head);
- block->bytes_used = 0;
+ block->block.bytes_used = 0;
_iio_dma_buffer_block_done(block);
iio_buffer_block_put_atomic(block);
}
@@ -296,6 +299,10 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)

mutex_lock(&queue->lock);

+ /* If in mmap mode dont do anything */
+ if (queue->num_blocks)
+ goto out_unlock;
+
/* Allocations are page aligned */
if (PAGE_ALIGN(queue->fileio.block_size) == PAGE_ALIGN(size))
try_reuse = true;
@@ -330,7 +337,7 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
iio_buffer_block_put(block);
block = NULL;
} else {
- block->size = size;
+ block->block.size = size;
}
} else {
block = NULL;
@@ -345,6 +352,8 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
queue->fileio.blocks[i] = block;
}

+ block->block.id = i;
+
block->state = IIO_BLOCK_STATE_QUEUED;
list_add_tail(&block->head, &queue->incoming);
}
@@ -356,6 +365,30 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
}
EXPORT_SYMBOL_GPL(iio_dma_buffer_request_update);

+static void iio_dma_buffer_fileio_free(struct iio_dma_buffer_queue *queue)
+{
+ unsigned int i;
+
+ spin_lock_irq(&queue->list_lock);
+ for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
+ if (!queue->fileio.blocks[i])
+ continue;
+ queue->fileio.blocks[i]->state = IIO_BLOCK_STATE_DEAD;
+ }
+ INIT_LIST_HEAD(&queue->outgoing);
+ spin_unlock_irq(&queue->list_lock);
+
+ INIT_LIST_HEAD(&queue->incoming);
+
+ for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
+ if (!queue->fileio.blocks[i])
+ continue;
+ iio_buffer_block_put(queue->fileio.blocks[i]);
+ queue->fileio.blocks[i] = NULL;
+ }
+ queue->fileio.active_block = NULL;
+}
+
static void iio_dma_buffer_submit_block(struct iio_dma_buffer_queue *queue,
struct iio_dma_buffer_block *block)
{
@@ -404,6 +437,7 @@ int iio_dma_buffer_enable(struct iio_buffer *buffer,
struct iio_dma_buffer_block *block, *_block;

mutex_lock(&queue->lock);
+ queue->fileio.enabled = !queue->num_blocks;
queue->active = true;
list_for_each_entry_safe(block, _block, &queue->incoming, head) {
list_del(&block->head);
@@ -429,6 +463,7 @@ int iio_dma_buffer_disable(struct iio_buffer *buffer,
struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);

mutex_lock(&queue->lock);
+ queue->fileio.enabled = false;
queue->active = false;

if (queue->ops && queue->ops->abort)
@@ -490,6 +525,11 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,

mutex_lock(&queue->lock);

+ if (!queue->fileio.enabled) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
if (!queue->fileio.active_block) {
block = iio_dma_buffer_dequeue(queue);
if (block == NULL) {
@@ -503,8 +543,8 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,
}

n = rounddown(n, buffer->bytes_per_datum);
- if (n > block->bytes_used - queue->fileio.pos)
- n = block->bytes_used - queue->fileio.pos;
+ if (n > block->block.bytes_used - queue->fileio.pos)
+ n = block->block.bytes_used - queue->fileio.pos;

if (copy_to_user(user_buffer, block->vaddr + queue->fileio.pos, n)) {
ret = -EFAULT;
@@ -513,7 +553,7 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,

queue->fileio.pos += n;

- if (queue->fileio.pos == block->bytes_used) {
+ if (queue->fileio.pos == block->block.bytes_used) {
queue->fileio.active_block = NULL;
iio_dma_buffer_enqueue(queue, block);
}
@@ -549,11 +589,11 @@ size_t iio_dma_buffer_data_available(struct iio_buffer *buf)

mutex_lock(&queue->lock);
if (queue->fileio.active_block)
- data_available += queue->fileio.active_block->size;
+ data_available += queue->fileio.active_block->block.size;

spin_lock_irq(&queue->list_lock);
list_for_each_entry(block, &queue->outgoing, head)
- data_available += block->size;
+ data_available += block->block.size;
spin_unlock_irq(&queue->list_lock);
mutex_unlock(&queue->lock);

@@ -561,6 +601,241 @@ size_t iio_dma_buffer_data_available(struct iio_buffer *buf)
}
EXPORT_SYMBOL_GPL(iio_dma_buffer_data_available);

+int iio_dma_buffer_alloc_blocks(struct iio_buffer *buffer,
+ struct iio_buffer_block_alloc_req *req)
+{
+ struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
+ struct iio_dma_buffer_block **blocks;
+ unsigned int num_blocks;
+ unsigned int i;
+ int ret = 0;
+
+ mutex_lock(&queue->lock);
+
+ /*
+ * If the buffer is enabled and in fileio mode new blocks can't be
+ * allocated.
+ */
+ if (queue->fileio.enabled) {
+ ret = -EBUSY;
+ goto err_unlock;
+ }
+
+ /* Free memory that might be in use for fileio mode */
+ iio_dma_buffer_fileio_free(queue);
+
+ /* 64 blocks ought to be enough for anybody ;) */
+ if (req->count > 64 - queue->num_blocks)
+ req->count = 64 - queue->num_blocks;
+ if (req->size > iio_dma_buffer_max_block_size)
+ req->size = iio_dma_buffer_max_block_size;
+
+ req->id = queue->num_blocks;
+
+ if (req->count == 0 || req->size == 0) {
+ ret = 0;
+ goto err_unlock;
+ }
+
+ num_blocks = req->count + queue->num_blocks;
+
+ blocks = krealloc(queue->blocks, sizeof(*blocks) * num_blocks,
+ GFP_KERNEL);
+ if (!blocks) {
+ ret = -ENOMEM;
+ goto err_unlock;
+ }
+
+ for (i = queue->num_blocks; i < num_blocks; i++) {
+ blocks[i] = iio_dma_buffer_alloc_block(queue, req->size);
+ if (!blocks[i])
+ break;
+ blocks[i]->block.id = i;
+ blocks[i]->block.data.offset = queue->max_offset;
+ queue->max_offset += PAGE_ALIGN(req->size);
+ }
+
+ req->count = i - queue->num_blocks;
+ queue->num_blocks = i;
+ queue->blocks = blocks;
+
+err_unlock:
+ mutex_unlock(&queue->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_alloc_blocks);
+
+int iio_dma_buffer_free_blocks(struct iio_buffer *buffer)
+{
+ struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
+ unsigned int i;
+
+ mutex_lock(&queue->lock);
+
+ spin_lock_irq(&queue->list_lock);
+ INIT_LIST_HEAD(&queue->incoming);
+ INIT_LIST_HEAD(&queue->outgoing);
+
+ for (i = 0; i < queue->num_blocks; i++)
+ queue->blocks[i]->state = IIO_BLOCK_STATE_DEAD;
+ spin_unlock_irq(&queue->list_lock);
+
+ for (i = 0; i < queue->num_blocks; i++)
+ iio_buffer_block_put(queue->blocks[i]);
+
+ kfree(queue->blocks);
+ queue->blocks = NULL;
+ queue->num_blocks = 0;
+ queue->max_offset = 0;
+
+ mutex_unlock(&queue->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_free_blocks);
+
+int iio_dma_buffer_query_block(struct iio_buffer *buffer,
+ struct iio_buffer_block *block)
+{
+ struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
+ int ret = 0;
+
+ mutex_lock(&queue->lock);
+
+ if (block->id >= queue->num_blocks) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ *block = queue->blocks[block->id]->block;
+
+out_unlock:
+ mutex_unlock(&queue->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_query_block);
+
+int iio_dma_buffer_enqueue_block(struct iio_buffer *buffer,
+ struct iio_buffer_block *block)
+{
+ struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
+ struct iio_dma_buffer_block *dma_block;
+ int ret = 0;
+
+ mutex_lock(&queue->lock);
+
+ if (block->id >= queue->num_blocks) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ dma_block = queue->blocks[block->id];
+ dma_block->block.bytes_used = block->bytes_used;
+ dma_block->block.flags = block->flags;
+
+ switch (dma_block->state) {
+ case IIO_BLOCK_STATE_DONE:
+ list_del_init(&dma_block->head);
+ break;
+ case IIO_BLOCK_STATE_QUEUED:
+ /* Nothing to do */
+ goto out_unlock;
+ case IIO_BLOCK_STATE_DEQUEUED:
+ break;
+ default:
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
+ iio_dma_buffer_enqueue(queue, dma_block);
+
+out_unlock:
+ mutex_unlock(&queue->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_enqueue_block);
+
+int iio_dma_buffer_dequeue_block(struct iio_buffer *buffer,
+ struct iio_buffer_block *block)
+{
+ struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
+ struct iio_dma_buffer_block *dma_block;
+ int ret = 0;
+
+ mutex_lock(&queue->lock);
+
+ dma_block = iio_dma_buffer_dequeue(queue);
+ if (!dma_block) {
+ ret = -EAGAIN;
+ goto out_unlock;
+ }
+
+ *block = dma_block->block;
+
+out_unlock:
+ mutex_unlock(&queue->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_dequeue_block);
+
+static void iio_dma_buffer_mmap_open(struct vm_area_struct *area)
+{
+ struct iio_dma_buffer_block *block = area->vm_private_data;
+
+ iio_buffer_block_get(block);
+}
+
+static void iio_dma_buffer_mmap_close(struct vm_area_struct *area)
+{
+ struct iio_dma_buffer_block *block = area->vm_private_data;
+
+ iio_buffer_block_put(block);
+}
+
+static const struct vm_operations_struct iio_dma_buffer_vm_ops = {
+ .open = iio_dma_buffer_mmap_open,
+ .close = iio_dma_buffer_mmap_close,
+};
+
+int iio_dma_buffer_mmap(struct iio_buffer *buffer, struct vm_area_struct *vma)
+{
+ struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
+ struct iio_dma_buffer_block *block = NULL;
+ size_t vm_offset;
+ unsigned int i;
+
+ vm_offset = vma->vm_pgoff << PAGE_SHIFT;
+
+ for (i = 0; i < queue->num_blocks; i++) {
+ if (queue->blocks[i]->block.data.offset == vm_offset) {
+ block = queue->blocks[i];
+ break;
+ }
+ }
+
+ if (block == NULL)
+ return -EINVAL;
+
+ if (PAGE_ALIGN(block->block.size) < vma->vm_end - vma->vm_start)
+ return -EINVAL;
+
+ vma->vm_pgoff = 0;
+
+ vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_ops = &iio_dma_buffer_vm_ops;
+ vma->vm_private_data = block;
+
+ vma->vm_ops->open(vma);
+
+ return dma_mmap_coherent(queue->dev, vma, block->vaddr,
+ block->phys_addr, vma->vm_end - vma->vm_start);
+}
+EXPORT_SYMBOL_GPL(iio_dma_buffer_mmap);
+
/**
* iio_dma_buffer_set_bytes_per_datum() - DMA buffer set_bytes_per_datum callback
* @buffer: Buffer to set the bytes-per-datum for
@@ -635,28 +910,9 @@ EXPORT_SYMBOL_GPL(iio_dma_buffer_init);
*/
void iio_dma_buffer_exit(struct iio_dma_buffer_queue *queue)
{
- unsigned int i;
-
mutex_lock(&queue->lock);

- spin_lock_irq(&queue->list_lock);
- for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
- if (!queue->fileio.blocks[i])
- continue;
- queue->fileio.blocks[i]->state = IIO_BLOCK_STATE_DEAD;
- }
- INIT_LIST_HEAD(&queue->outgoing);
- spin_unlock_irq(&queue->list_lock);
-
- INIT_LIST_HEAD(&queue->incoming);
-
- for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
- if (!queue->fileio.blocks[i])
- continue;
- iio_buffer_block_put(queue->fileio.blocks[i]);
- queue->fileio.blocks[i] = NULL;
- }
- queue->fileio.active_block = NULL;
+ iio_dma_buffer_fileio_free(queue);
queue->ops = NULL;

mutex_unlock(&queue->lock);
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 8ecdbae83414..bb022922ec23 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -54,7 +54,7 @@ static void iio_dmaengine_buffer_block_done(void *data,
spin_lock_irqsave(&block->queue->list_lock, flags);
list_del(&block->head);
spin_unlock_irqrestore(&block->queue->list_lock, flags);
- block->bytes_used -= result->residue;
+ block->block.bytes_used -= result->residue;
iio_dma_buffer_block_done(block);
}

@@ -66,12 +66,17 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
struct dma_async_tx_descriptor *desc;
dma_cookie_t cookie;

- block->bytes_used = min(block->size, dmaengine_buffer->max_size);
- block->bytes_used = rounddown(block->bytes_used,
- dmaengine_buffer->align);
+ block->block.bytes_used = min(block->block.size,
+ dmaengine_buffer->max_size);
+ block->block.bytes_used = rounddown(block->block.bytes_used,
+ dmaengine_buffer->align);
+ if (block->block.bytes_used == 0) {
+ iio_dma_buffer_block_done(block);
+ return 0;
+ }

desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
- block->phys_addr, block->bytes_used, DMA_DEV_TO_MEM,
+ block->phys_addr, block->block.bytes_used, DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT);
if (!desc)
return -ENOMEM;
@@ -120,6 +125,13 @@ static const struct iio_buffer_access_funcs iio_dmaengine_buffer_ops = {
.data_available = iio_dma_buffer_data_available,
.release = iio_dmaengine_buffer_release,

+ .alloc_blocks = iio_dma_buffer_alloc_blocks,
+ .free_blocks = iio_dma_buffer_free_blocks,
+ .query_block = iio_dma_buffer_query_block,
+ .enqueue_block = iio_dma_buffer_enqueue_block,
+ .dequeue_block = iio_dma_buffer_dequeue_block,
+ .mmap = iio_dma_buffer_mmap,
+
.modes = INDIO_BUFFER_HARDWARE,
.flags = INDIO_BUFFER_FLAG_FIXED_WATERMARK,
};
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 50228df0b09f..a0d1ad86022f 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -19,6 +19,7 @@
#include <linux/mm.h>
#include <linux/poll.h>
#include <linux/sched/signal.h>
+#include <linux/mm.h>

#include <linux/iio/iio.h>
#include <linux/iio/iio-opaque.h>
diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
index 6564bdcdac66..315a8d750986 100644
--- a/include/linux/iio/buffer-dma.h
+++ b/include/linux/iio/buffer-dma.h
@@ -47,7 +47,7 @@ enum iio_block_state {
struct iio_dma_buffer_block {
/* May only be accessed by the owner of the block */
struct list_head head;
- size_t bytes_used;
+ struct iio_buffer_block block;

/*
* Set during allocation, constant thereafter. May be accessed read-only
@@ -55,7 +55,6 @@ struct iio_dma_buffer_block {
*/
void *vaddr;
dma_addr_t phys_addr;
- size_t size;
struct iio_dma_buffer_queue *queue;

/* Must not be accessed outside the core. */
@@ -73,12 +72,14 @@ struct iio_dma_buffer_block {
* @active_block: Block being used in read()
* @pos: Read offset in the active block
* @block_size: Size of each block
+ * @enabled: Whether the buffer is operating in fileio mode
*/
struct iio_dma_buffer_queue_fileio {
struct iio_dma_buffer_block *blocks[2];
struct iio_dma_buffer_block *active_block;
size_t pos;
size_t block_size;
+ bool enabled;
};

/**
@@ -109,6 +110,10 @@ struct iio_dma_buffer_queue {

bool active;

+ unsigned int num_blocks;
+ struct iio_dma_buffer_block **blocks;
+ unsigned int max_offset;
+
struct iio_dma_buffer_queue_fileio fileio;
};

@@ -143,4 +148,15 @@ int iio_dma_buffer_init(struct iio_dma_buffer_queue *queue,
void iio_dma_buffer_exit(struct iio_dma_buffer_queue *queue);
void iio_dma_buffer_release(struct iio_dma_buffer_queue *queue);

+int iio_dma_buffer_alloc_blocks(struct iio_buffer *buffer,
+ struct iio_buffer_block_alloc_req *req);
+int iio_dma_buffer_free_blocks(struct iio_buffer *buffer);
+int iio_dma_buffer_query_block(struct iio_buffer *buffer,
+ struct iio_buffer_block *block);
+int iio_dma_buffer_enqueue_block(struct iio_buffer *buffer,
+ struct iio_buffer_block *block);
+int iio_dma_buffer_dequeue_block(struct iio_buffer *buffer,
+ struct iio_buffer_block *block);
+int iio_dma_buffer_mmap(struct iio_buffer *buffer, struct vm_area_struct *vma);
+
#endif
--
2.17.1

2021-02-12 10:27:15

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] iio: core: Add mmap interface infrastructure

On Fri, Feb 12, 2021 at 12:12 PM Alexandru Ardelean
<[email protected]> wrote:
>
> From: Lars-Peter Clausen <[email protected]>
>
> Add the necessary infrastructure to the IIO core to support an mmap based
> interface to access the capture data.
>
> The advantage of the mmap based interface compared to the read() based
> interface is that it avoids an extra copy of the data between kernel and
> userspace. This is particular useful for high-speed devices which produce
> several megabytes or even gigabytes of data per second.
>
> The data for the mmap interface is managed at the granularity of so called
> blocks. A block is a contiguous region of memory (at the moment both
> physically and virtually contiguous). 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
> data-rate of a few megabytes is not feasible.
>
> This of course leads to a slightly increased latency. For this reason an
> application can choose the size of the blocks as well as how many blocks it
> allocates. E.g. two blocks 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.
>
> A block can either be owned by kernel space or userspace. When owned by
> userspace it save to access the data in the block and process it. When
> owned by kernel space the block can be in one of 3 states.
>
> It can be in the incoming queue where all blocks submitted from userspace
> are placed and are waiting to be processed by the kernel driver.
>
> It can be currently being processed by the kernel driver, this means it is
> actively placing capturing data in it (usually using DMA).
>
> Or it can be in the outgoing queue where all blocks that have been
> processed by the kernel are placed. Userspace can dequeue the blocks as
> necessary.
>
> As part of the interface 5 new IOCTLs to manage the blocks and exchange
> them between userspace and kernelspace. The IOCTLs can be accessed through
> a open file descriptor to a IIO device.
>
> IIO_BUFFER_BLOCK_ALLOC_IOCTL(struct iio_buffer_block_alloc_req *):
> Allocates new blocks. Can be called multiple times if necessary. A newly
> allocated block is initially owned by userspace.
>
> IIO_BUFFER_BLOCK_FREE_IOCTL(void):
> Frees all previously allocated blocks. If the backing memory of a block is
> still in use by a kernel driver (i.e. active DMA transfer) it will be
> freed once the kernel driver has released it.
>
> IIO_BUFFER_BLOCK_QUERY_IOCTL(struct iio_buffer_block *):
> Queries information about a block. The id of the block about which
> information is to be queried needs to be set by userspace.
>
> IIO_BUFFER_BLOCK_ENQUEUE_IOCTL(struct iio_buffer_block *):
> Places a block on the incoming queue. This transfers ownership of the
> block from userspace to kernelspace. Userspace must populate the id field
> of the block to indicate which block to enqueue.
>
> IIO_BUFFER_BLOCK_DEQUEUE_IOCTL(struct iio_buffer_block *):
> Removes the first block from the outgoing queue. This transfers ownership
> of the block from kernelspace to userspace. Kernelspace will populate all
> fields of the block. If the queue is empty and the file descriptor is set
> to blocking the IOCTL will block until a new block is available on the
> outgoing queue.
>
> 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 IIO device
> file descriptor. Each block has a unique offset assigned to it which should
> be passed to the mmap interface. E.g.
>
> mmap(0, block.size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> block.offset);
>
> A typical workflow for the new interface is:
>
> BLOCK_ALLOC
>
> foreach block
> BLOCK_QUERY block
> mmap block.data.offset
> BLOCK_ENQUEUE block
>
> enable buffer
>
> while !done
> BLOCK_DEQUEUE block
> process data
> BLOCK_ENQUEUE block
>
> disable buffer
>
> BLOCK_FREE
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> Signed-off-by: Alexandru Ardelean <[email protected]>
> ---
> drivers/iio/industrialio-buffer.c | 157 ++++++++++++++++++++++++++++++
> include/linux/iio/buffer-dma.h | 5 -
> include/linux/iio/buffer_impl.h | 23 +++++
> include/uapi/linux/iio/buffer.h | 26 +++++
> 4 files changed, 206 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 3aa6702a5811..50228df0b09f 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -16,6 +16,7 @@
> #include <linux/fs.h>
> #include <linux/cdev.h>
> #include <linux/slab.h>
> +#include <linux/mm.h>
> #include <linux/poll.h>
> #include <linux/sched/signal.h>
>
> @@ -1370,6 +1371,12 @@ static void iio_buffer_unregister_legacy_sysfs_groups(struct iio_dev *indio_dev)
> kfree(iio_dev_opaque->legacy_scan_el_group.attrs);
> }
>
> +static void iio_buffer_free_blocks(struct iio_buffer *buffer)
> +{
> + if (buffer->access->free_blocks)
> + buffer->access->free_blocks(buffer);
> +}
> +
> static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)
> {
> struct iio_dev_buffer_pair *ib = filep->private_data;
> @@ -1378,18 +1385,24 @@ static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)
>
> wake_up(&buffer->pollq);
> clear_bit(IIO_BUSY_BIT_POS, &buffer->flags);
> + iio_buffer_free_blocks(buffer);
> iio_device_put(indio_dev);
> kfree(ib);
>
> return 0;
> }
>
> +static long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
> +static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma);
> +
> static const struct file_operations iio_buffer_chrdev_fileops = {
> .owner = THIS_MODULE,
> .llseek = noop_llseek,
> .read = iio_buffer_read,
> .poll = iio_buffer_poll,
> + .unlocked_ioctl = iio_buffer_ioctl,
> .compat_ioctl = compat_ptr_ioctl,
> + .mmap = iio_buffer_mmap,
> .release = iio_buffer_chrdev_release,
> };
>
> @@ -1762,6 +1775,150 @@ void iio_buffer_put(struct iio_buffer *buffer)
> }
> EXPORT_SYMBOL_GPL(iio_buffer_put);
>
> +static int iio_buffer_query_block(struct iio_buffer *buffer,
> + struct iio_buffer_block __user *user_block)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + if (!buffer->access->query_block)
> + return -ENOSYS;
> +
> + if (copy_from_user(&block, user_block, sizeof(block)))
> + return -EFAULT;
> +
> + ret = buffer->access->query_block(buffer, &block);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user(user_block, &block, sizeof(block)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int iio_buffer_dequeue_block(struct iio_dev *indio_dev,
> + struct iio_buffer *buffer,
> + struct iio_buffer_block __user *user_block,
> + bool non_blocking)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + if (!buffer->access->dequeue_block)
> + return -ENOSYS;
> +
> + do {
> + if (!iio_buffer_data_available(buffer)) {
> + if (non_blocking)
> + return -EAGAIN;
> +
> + ret = wait_event_interruptible(buffer->pollq,
> + iio_buffer_data_available(buffer) ||
> + indio_dev->info == NULL);
> + if (ret)
> + return ret;
> + if (indio_dev->info == NULL)
> + return -ENODEV;
> + }
> +
> + ret = buffer->access->dequeue_block(buffer, &block);
> + if (ret == -EAGAIN && non_blocking)
> + ret = 0;
> + } while (ret);
> +
> + if (ret)
> + return ret;
> +
> + if (copy_to_user(user_block, &block, sizeof(block)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int iio_buffer_enqueue_block(struct iio_buffer *buffer,
> + struct iio_buffer_block __user *user_block)
> +{
> + struct iio_buffer_block block;
> +
> + if (!buffer->access->enqueue_block)
> + return -ENOSYS;
> +
> + if (copy_from_user(&block, user_block, sizeof(block)))
> + return -EFAULT;
> +
> + return buffer->access->enqueue_block(buffer, &block);
> +}
> +
> +static int iio_buffer_alloc_blocks(struct iio_buffer *buffer,
> + struct iio_buffer_block_alloc_req __user *user_req)
> +{
> + struct iio_buffer_block_alloc_req req;
> + int ret;
> +
> + if (!buffer->access->alloc_blocks)
> + return -ENOSYS;
> +
> + if (copy_from_user(&req, user_req, sizeof(req)))
> + return -EFAULT;
> +
> + ret = buffer->access->alloc_blocks(buffer, &req);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user(user_req, &req, sizeof(req)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> +{
> + bool non_blocking = filep->f_flags & O_NONBLOCK;
> + struct iio_dev_buffer_pair *ib = filep->private_data;
> + struct iio_dev *indio_dev = ib->indio_dev;
> + struct iio_buffer *buffer = ib->buffer;
> +
> + if (!buffer || !buffer->access)
> + return -ENODEV;
> +
> + switch (cmd) {
> + case IIO_BUFFER_BLOCK_ALLOC_IOCTL:
> + return iio_buffer_alloc_blocks(buffer,
> + (struct iio_buffer_block_alloc_req __user *)arg);
> + case IIO_BUFFER_BLOCK_FREE_IOCTL:
> + iio_buffer_free_blocks(buffer);
> + return 0;
> + case IIO_BUFFER_BLOCK_QUERY_IOCTL:
> + return iio_buffer_query_block(buffer,
> + (struct iio_buffer_block __user *)arg);
> + case IIO_BUFFER_BLOCK_ENQUEUE_IOCTL:
> + return iio_buffer_enqueue_block(buffer,
> + (struct iio_buffer_block __user *)arg);
> + case IIO_BUFFER_BLOCK_DEQUEUE_IOCTL:
> + return iio_buffer_dequeue_block(indio_dev, buffer,
> + (struct iio_buffer_block __user *)arg, non_blocking);
> + }
> + return -EINVAL;
> +}
> +
> +static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma)
> +{
> + struct iio_dev_buffer_pair *ib = filep->private_data;
> + struct iio_buffer *buffer = ib->buffer;
> +
> + if (!buffer->access || !buffer->access->mmap)
> + return -ENODEV;
> +
> + if (!(vma->vm_flags & VM_SHARED))
> + return -EINVAL;
> +
> + if (!(vma->vm_flags & VM_READ))
> + return -EINVAL;
> +
> + return buffer->access->mmap(buffer, vma);
> +}
> +
> /**
> * iio_device_attach_buffer - Attach a buffer to a IIO device
> * @indio_dev: The device the buffer should be attached to
> diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
> index ff15c61bf319..6564bdcdac66 100644
> --- a/include/linux/iio/buffer-dma.h
> +++ b/include/linux/iio/buffer-dma.h
> @@ -17,11 +17,6 @@ struct iio_dma_buffer_queue;
> struct iio_dma_buffer_ops;
> struct device;
>
> -struct iio_buffer_block {
> - u32 size;
> - u32 bytes_used;
> -};
> -
> /**
> * enum iio_block_state - State of a struct iio_dma_buffer_block
> * @IIO_BLOCK_STATE_DEQUEUED: Block is not queued
> diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
> index 245b32918ae1..1d57dc7ccb4f 100644
> --- a/include/linux/iio/buffer_impl.h
> +++ b/include/linux/iio/buffer_impl.h
> @@ -34,6 +34,18 @@ struct iio_buffer;
> * device stops sampling. Calles are balanced with @enable.
> * @release: called when the last reference to the buffer is dropped,
> * should free all resources allocated by the buffer.
> + * @alloc_blocks: called from userspace via ioctl to allocate blocks
> + * that will be used via the mmap interface.
> + * @free_blocks: called from userspace via ioctl to free all blocks
> + * allocated for this buffer.
> + * @enqueue_block: called from userspace via ioctl to queue this block
> + * to this buffer. Requires a valid block id.
> + * @dequeue_block: called from userspace via ioctl to dequeue this block
> + * from this buffer. Requires a valid block id.
> + * @query_block: called from userspace via ioctl to query the attributes
> + * of this block. Requires a valid block id.
> + * @mmap: mmap hook for this buffer. Userspace mmap() calls will
> + * get routed to this.
> * @modes: Supported operating modes by this buffer type
> * @flags: A bitmask combination of INDIO_BUFFER_FLAG_*
> *
> @@ -60,6 +72,17 @@ struct iio_buffer_access_funcs {
>
> void (*release)(struct iio_buffer *buffer);
>
> + int (*alloc_blocks)(struct iio_buffer *buffer,
> + struct iio_buffer_block_alloc_req *req);
> + int (*free_blocks)(struct iio_buffer *buffer);
> + int (*enqueue_block)(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> + int (*dequeue_block)(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> + int (*query_block)(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> + int (*mmap)(struct iio_buffer *buffer, struct vm_area_struct *vma);
> +
> unsigned int modes;
> unsigned int flags;
> };
> diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
> index 13939032b3f6..70ad3aea01ea 100644
> --- a/include/uapi/linux/iio/buffer.h
> +++ b/include/uapi/linux/iio/buffer.h
> @@ -5,6 +5,32 @@
> #ifndef _UAPI_IIO_BUFFER_H_
> #define _UAPI_IIO_BUFFER_H_
>
> +struct iio_buffer_block_alloc_req {
> + __u32 type;
> + __u32 size;
> + __u32 count;
> + __u32 id;
> +};
> +
> +#define IIO_BUFFER_BLOCK_FLAG_TIMESTAMP_VALID (1 << 0)

@Lars
On a recent round of review I did, I noticed that this flag exists and
isn't set/used anywhere yet.
The only issue with this is that IIO_BUFFER_BLOCK_FLAG_CYCLIC is now at BIT(1).

This doesn't look used anywhere, neither in libiio.
Which would mean maybe making IIO_BUFFER_BLOCK_FLAG_CYCLIC BIT(0)



> +
> +struct iio_buffer_block {
> + __u32 id;
> + __u32 size;
> + __u32 bytes_used;
> + __u32 type;
> + __u32 flags;
> + union {
> + __u32 offset;
> + } data;
> + __u64 timestamp;
> +};
> +
> #define IIO_BUFFER_GET_FD_IOCTL _IOWR('i', 0x91, int)
> +#define IIO_BUFFER_BLOCK_ALLOC_IOCTL _IOWR('i', 0x92, struct iio_buffer_block_alloc_req)
> +#define IIO_BUFFER_BLOCK_FREE_IOCTL _IO('i', 0x93)
> +#define IIO_BUFFER_BLOCK_QUERY_IOCTL _IOWR('i', 0x93, struct iio_buffer_block)
> +#define IIO_BUFFER_BLOCK_ENQUEUE_IOCTL _IOWR('i', 0x94, struct iio_buffer_block)
> +#define IIO_BUFFER_BLOCK_DEQUEUE_IOCTL _IOWR('i', 0x95, struct iio_buffer_block)
>
> #endif /* _UAPI_IIO_BUFFER_H_ */
> --
> 2.17.1
>

2021-02-14 15:29:06

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] iio: core: Add mmap interface infrastructure

On Fri, 12 Feb 2021 12:11:41 +0200
Alexandru Ardelean <[email protected]> wrote:

> From: Lars-Peter Clausen <[email protected]>
>
> Add the necessary infrastructure to the IIO core to support an mmap based
> interface to access the capture data.
>
> The advantage of the mmap based interface compared to the read() based
> interface is that it avoids an extra copy of the data between kernel and
> userspace. This is particular useful for high-speed devices which produce
> several megabytes or even gigabytes of data per second.
>
> The data for the mmap interface is managed at the granularity of so called
> blocks. A block is a contiguous region of memory (at the moment both
> physically and virtually contiguous). 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
> data-rate of a few megabytes is not feasible.
>
> This of course leads to a slightly increased latency. For this reason an
> application can choose the size of the blocks as well as how many blocks it
> allocates. E.g. two blocks 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.
>
> A block can either be owned by kernel space or userspace. When owned by
> userspace it save to access the data in the block and process it. When
> owned by kernel space the block can be in one of 3 states.
>
> It can be in the incoming queue where all blocks submitted from userspace
> are placed and are waiting to be processed by the kernel driver.
>
> It can be currently being processed by the kernel driver, this means it is
> actively placing capturing data in it (usually using DMA).
>
> Or it can be in the outgoing queue where all blocks that have been
> processed by the kernel are placed. Userspace can dequeue the blocks as
> necessary.
>
> As part of the interface 5 new IOCTLs to manage the blocks and exchange
> them between userspace and kernelspace. The IOCTLs can be accessed through
> a open file descriptor to a IIO device.
>
> IIO_BUFFER_BLOCK_ALLOC_IOCTL(struct iio_buffer_block_alloc_req *):
> Allocates new blocks. Can be called multiple times if necessary. A newly
> allocated block is initially owned by userspace.
>
> IIO_BUFFER_BLOCK_FREE_IOCTL(void):
> Frees all previously allocated blocks. If the backing memory of a block is
> still in use by a kernel driver (i.e. active DMA transfer) it will be
> freed once the kernel driver has released it.
>
> IIO_BUFFER_BLOCK_QUERY_IOCTL(struct iio_buffer_block *):
> Queries information about a block. The id of the block about which
> information is to be queried needs to be set by userspace.
>
> IIO_BUFFER_BLOCK_ENQUEUE_IOCTL(struct iio_buffer_block *):
> Places a block on the incoming queue. This transfers ownership of the
> block from userspace to kernelspace. Userspace must populate the id field
> of the block to indicate which block to enqueue.
>
> IIO_BUFFER_BLOCK_DEQUEUE_IOCTL(struct iio_buffer_block *):
> Removes the first block from the outgoing queue. This transfers ownership
> of the block from kernelspace to userspace. Kernelspace will populate all
> fields of the block. If the queue is empty and the file descriptor is set
> to blocking the IOCTL will block until a new block is available on the
> outgoing queue.
>
> 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 IIO device
> file descriptor. Each block has a unique offset assigned to it which should
> be passed to the mmap interface. E.g.
>
> mmap(0, block.size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> block.offset);
>
> A typical workflow for the new interface is:
>
> BLOCK_ALLOC
>
> foreach block
> BLOCK_QUERY block
> mmap block.data.offset
> BLOCK_ENQUEUE block
>
> enable buffer
>
> while !done
> BLOCK_DEQUEUE block
> process data
> BLOCK_ENQUEUE block
>
> disable buffer
>
> BLOCK_FREE
Rather feels like some of this info should be in the formal docs somewhere rather
than just burried in this patch description.

Also good to have a bit more in the way of docs in the uapi header.


>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> Signed-off-by: Alexandru Ardelean <[email protected]>
Few trivial things inline, but otherwise seems like straightforwards
wrappers.

Jonathan

> ---
> drivers/iio/industrialio-buffer.c | 157 ++++++++++++++++++++++++++++++
> include/linux/iio/buffer-dma.h | 5 -
> include/linux/iio/buffer_impl.h | 23 +++++
> include/uapi/linux/iio/buffer.h | 26 +++++
> 4 files changed, 206 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 3aa6702a5811..50228df0b09f 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -16,6 +16,7 @@
> #include <linux/fs.h>
> #include <linux/cdev.h>
> #include <linux/slab.h>
> +#include <linux/mm.h>
> #include <linux/poll.h>
> #include <linux/sched/signal.h>
>
> @@ -1370,6 +1371,12 @@ static void iio_buffer_unregister_legacy_sysfs_groups(struct iio_dev *indio_dev)
> kfree(iio_dev_opaque->legacy_scan_el_group.attrs);
> }
>
> +static void iio_buffer_free_blocks(struct iio_buffer *buffer)
> +{
> + if (buffer->access->free_blocks)
> + buffer->access->free_blocks(buffer);
> +}
> +
> static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)
> {
> struct iio_dev_buffer_pair *ib = filep->private_data;
> @@ -1378,18 +1385,24 @@ static int iio_buffer_chrdev_release(struct inode *inode, struct file *filep)
>
> wake_up(&buffer->pollq);
> clear_bit(IIO_BUSY_BIT_POS, &buffer->flags);
> + iio_buffer_free_blocks(buffer);
> iio_device_put(indio_dev);
> kfree(ib);
>
> return 0;
> }
>
> +static long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
> +static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma);
> +
> static const struct file_operations iio_buffer_chrdev_fileops = {
> .owner = THIS_MODULE,
> .llseek = noop_llseek,
> .read = iio_buffer_read,
> .poll = iio_buffer_poll,
> + .unlocked_ioctl = iio_buffer_ioctl,
> .compat_ioctl = compat_ptr_ioctl,

Why do we have an existing compat_ioctl here?
Seems odd without the unlocked_ioctl. I see introduced in the multi buffer
series whereas I think it should in this one.

It has a test against being there without the unlocked_ioctl though so
no great disaster.

> + .mmap = iio_buffer_mmap,
> .release = iio_buffer_chrdev_release,
> };
>
> @@ -1762,6 +1775,150 @@ void iio_buffer_put(struct iio_buffer *buffer)
> }
> EXPORT_SYMBOL_GPL(iio_buffer_put);
>
> +static int iio_buffer_query_block(struct iio_buffer *buffer,
> + struct iio_buffer_block __user *user_block)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + if (!buffer->access->query_block)
> + return -ENOSYS;
> +
> + if (copy_from_user(&block, user_block, sizeof(block)))
> + return -EFAULT;
> +
> + ret = buffer->access->query_block(buffer, &block);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user(user_block, &block, sizeof(block)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int iio_buffer_dequeue_block(struct iio_dev *indio_dev,
> + struct iio_buffer *buffer,
> + struct iio_buffer_block __user *user_block,
> + bool non_blocking)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + if (!buffer->access->dequeue_block)
> + return -ENOSYS;
> +
> + do {
> + if (!iio_buffer_data_available(buffer)) {
> + if (non_blocking)
> + return -EAGAIN;
> +
> + ret = wait_event_interruptible(buffer->pollq,
> + iio_buffer_data_available(buffer) ||
> + indio_dev->info == NULL);
> + if (ret)
> + return ret;
> + if (indio_dev->info == NULL)
> + return -ENODEV;
> + }
> +
> + ret = buffer->access->dequeue_block(buffer, &block);
> + if (ret == -EAGAIN && non_blocking)
> + ret = 0;
> + } while (ret);
> +
> + if (ret)
> + return ret;
> +
> + if (copy_to_user(user_block, &block, sizeof(block)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int iio_buffer_enqueue_block(struct iio_buffer *buffer,
> + struct iio_buffer_block __user *user_block)
> +{
> + struct iio_buffer_block block;
> +
> + if (!buffer->access->enqueue_block)
> + return -ENOSYS;
> +
> + if (copy_from_user(&block, user_block, sizeof(block)))
> + return -EFAULT;
> +
> + return buffer->access->enqueue_block(buffer, &block);
> +}
> +
> +static int iio_buffer_alloc_blocks(struct iio_buffer *buffer,
> + struct iio_buffer_block_alloc_req __user *user_req)
> +{
> + struct iio_buffer_block_alloc_req req;
> + int ret;
> +
> + if (!buffer->access->alloc_blocks)
> + return -ENOSYS;
> +
> + if (copy_from_user(&req, user_req, sizeof(req)))
> + return -EFAULT;
> +
> + ret = buffer->access->alloc_blocks(buffer, &req);
> + if (ret)
> + return ret;
> +
> + if (copy_to_user(user_req, &req, sizeof(req)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> +{
> + bool non_blocking = filep->f_flags & O_NONBLOCK;
> + struct iio_dev_buffer_pair *ib = filep->private_data;
> + struct iio_dev *indio_dev = ib->indio_dev;
> + struct iio_buffer *buffer = ib->buffer;
> +
> + if (!buffer || !buffer->access)
> + return -ENODEV;
> +
> + switch (cmd) {
> + case IIO_BUFFER_BLOCK_ALLOC_IOCTL:
> + return iio_buffer_alloc_blocks(buffer,
> + (struct iio_buffer_block_alloc_req __user *)arg);
> + case IIO_BUFFER_BLOCK_FREE_IOCTL:
> + iio_buffer_free_blocks(buffer);
> + return 0;
> + case IIO_BUFFER_BLOCK_QUERY_IOCTL:
> + return iio_buffer_query_block(buffer,
> + (struct iio_buffer_block __user *)arg);
> + case IIO_BUFFER_BLOCK_ENQUEUE_IOCTL:
> + return iio_buffer_enqueue_block(buffer,
> + (struct iio_buffer_block __user *)arg);
> + case IIO_BUFFER_BLOCK_DEQUEUE_IOCTL:
> + return iio_buffer_dequeue_block(indio_dev, buffer,
> + (struct iio_buffer_block __user *)arg, non_blocking);
> + }
> + return -EINVAL;
> +}
> +
> +static int iio_buffer_mmap(struct file *filep, struct vm_area_struct *vma)
> +{
> + struct iio_dev_buffer_pair *ib = filep->private_data;
> + struct iio_buffer *buffer = ib->buffer;
> +
> + if (!buffer->access || !buffer->access->mmap)
> + return -ENODEV;
> +
> + if (!(vma->vm_flags & VM_SHARED))
> + return -EINVAL;
> +
> + if (!(vma->vm_flags & VM_READ))
> + return -EINVAL;
> +
> + return buffer->access->mmap(buffer, vma);
> +}
> +
> /**
> * iio_device_attach_buffer - Attach a buffer to a IIO device
> * @indio_dev: The device the buffer should be attached to
> diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
> index ff15c61bf319..6564bdcdac66 100644
> --- a/include/linux/iio/buffer-dma.h
> +++ b/include/linux/iio/buffer-dma.h
> @@ -17,11 +17,6 @@ struct iio_dma_buffer_queue;
> struct iio_dma_buffer_ops;
> struct device;
>
> -struct iio_buffer_block {
> - u32 size;
> - u32 bytes_used;
> -};
> -
> /**
> * enum iio_block_state - State of a struct iio_dma_buffer_block
> * @IIO_BLOCK_STATE_DEQUEUED: Block is not queued
> diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
> index 245b32918ae1..1d57dc7ccb4f 100644
> --- a/include/linux/iio/buffer_impl.h
> +++ b/include/linux/iio/buffer_impl.h
> @@ -34,6 +34,18 @@ struct iio_buffer;
> * device stops sampling. Calles are balanced with @enable.
> * @release: called when the last reference to the buffer is dropped,
> * should free all resources allocated by the buffer.
> + * @alloc_blocks: called from userspace via ioctl to allocate blocks
> + * that will be used via the mmap interface.
> + * @free_blocks: called from userspace via ioctl to free all blocks
> + * allocated for this buffer.
> + * @enqueue_block: called from userspace via ioctl to queue this block
> + * to this buffer. Requires a valid block id.
> + * @dequeue_block: called from userspace via ioctl to dequeue this block
> + * from this buffer. Requires a valid block id.
> + * @query_block: called from userspace via ioctl to query the attributes
> + * of this block. Requires a valid block id.
> + * @mmap: mmap hook for this buffer. Userspace mmap() calls will
> + * get routed to this.
> * @modes: Supported operating modes by this buffer type
> * @flags: A bitmask combination of INDIO_BUFFER_FLAG_*
> *
> @@ -60,6 +72,17 @@ struct iio_buffer_access_funcs {
>
> void (*release)(struct iio_buffer *buffer);
>
> + int (*alloc_blocks)(struct iio_buffer *buffer,
> + struct iio_buffer_block_alloc_req *req);
> + int (*free_blocks)(struct iio_buffer *buffer);
> + int (*enqueue_block)(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> + int (*dequeue_block)(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> + int (*query_block)(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> + int (*mmap)(struct iio_buffer *buffer, struct vm_area_struct *vma);
> +
> unsigned int modes;
> unsigned int flags;
> };
> diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
> index 13939032b3f6..70ad3aea01ea 100644
> --- a/include/uapi/linux/iio/buffer.h
> +++ b/include/uapi/linux/iio/buffer.h
> @@ -5,6 +5,32 @@
> #ifndef _UAPI_IIO_BUFFER_H_
> #define _UAPI_IIO_BUFFER_H_
>
> +struct iio_buffer_block_alloc_req {
> + __u32 type;
> + __u32 size;
> + __u32 count;
> + __u32 id;
> +};
> +
> +#define IIO_BUFFER_BLOCK_FLAG_TIMESTAMP_VALID (1 << 0)
> +

Add some docs for the elements of these structures.

> +struct iio_buffer_block {
> + __u32 id;
> + __u32 size;
> + __u32 bytes_used;
> + __u32 type;
> + __u32 flags;
> + union {
> + __u32 offset;
> + } data;
> + __u64 timestamp;
> +};
> +
> #define IIO_BUFFER_GET_FD_IOCTL _IOWR('i', 0x91, int)
> +#define IIO_BUFFER_BLOCK_ALLOC_IOCTL _IOWR('i', 0x92, struct iio_buffer_block_alloc_req)
> +#define IIO_BUFFER_BLOCK_FREE_IOCTL _IO('i', 0x93)
> +#define IIO_BUFFER_BLOCK_QUERY_IOCTL _IOWR('i', 0x93, struct iio_buffer_block)
> +#define IIO_BUFFER_BLOCK_ENQUEUE_IOCTL _IOWR('i', 0x94, struct iio_buffer_block)
> +#define IIO_BUFFER_BLOCK_DEQUEUE_IOCTL _IOWR('i', 0x95, struct iio_buffer_block)
>
> #endif /* _UAPI_IIO_BUFFER_H_ */

2021-02-14 15:29:17

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] iio: core: Add mmap interface infrastructure

On Fri, 12 Feb 2021 12:21:37 +0200
Alexandru Ardelean <[email protected]> wrote:

> On Fri, Feb 12, 2021 at 12:12 PM Alexandru Ardelean
> <[email protected]> wrote:
> >
> > From: Lars-Peter Clausen <[email protected]>
> >
> > Add the necessary infrastructure to the IIO core to support an mmap based
> > interface to access the capture data.
> >
> > The advantage of the mmap based interface compared to the read() based
> > interface is that it avoids an extra copy of the data between kernel and
> > userspace. This is particular useful for high-speed devices which produce
> > several megabytes or even gigabytes of data per second.
> >
> > The data for the mmap interface is managed at the granularity of so called
> > blocks. A block is a contiguous region of memory (at the moment both
> > physically and virtually contiguous). 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
> > data-rate of a few megabytes is not feasible.
> >
> > This of course leads to a slightly increased latency. For this reason an
> > application can choose the size of the blocks as well as how many blocks it
> > allocates. E.g. two blocks 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.
> >
> > A block can either be owned by kernel space or userspace. When owned by
> > userspace it save to access the data in the block and process it. When
> > owned by kernel space the block can be in one of 3 states.
> >
> > It can be in the incoming queue where all blocks submitted from userspace
> > are placed and are waiting to be processed by the kernel driver.
> >
> > It can be currently being processed by the kernel driver, this means it is
> > actively placing capturing data in it (usually using DMA).
> >
> > Or it can be in the outgoing queue where all blocks that have been
> > processed by the kernel are placed. Userspace can dequeue the blocks as
> > necessary.
> >
> > As part of the interface 5 new IOCTLs to manage the blocks and exchange
> > them between userspace and kernelspace. The IOCTLs can be accessed through
> > a open file descriptor to a IIO device.
> >
> > IIO_BUFFER_BLOCK_ALLOC_IOCTL(struct iio_buffer_block_alloc_req *):
> > Allocates new blocks. Can be called multiple times if necessary. A newly
> > allocated block is initially owned by userspace.
> >
> > IIO_BUFFER_BLOCK_FREE_IOCTL(void):
> > Frees all previously allocated blocks. If the backing memory of a block is
> > still in use by a kernel driver (i.e. active DMA transfer) it will be
> > freed once the kernel driver has released it.
> >
> > IIO_BUFFER_BLOCK_QUERY_IOCTL(struct iio_buffer_block *):
> > Queries information about a block. The id of the block about which
> > information is to be queried needs to be set by userspace.
> >
> > IIO_BUFFER_BLOCK_ENQUEUE_IOCTL(struct iio_buffer_block *):
> > Places a block on the incoming queue. This transfers ownership of the
> > block from userspace to kernelspace. Userspace must populate the id field
> > of the block to indicate which block to enqueue.
> >
> > IIO_BUFFER_BLOCK_DEQUEUE_IOCTL(struct iio_buffer_block *):
> > Removes the first block from the outgoing queue. This transfers ownership
> > of the block from kernelspace to userspace. Kernelspace will populate all
> > fields of the block. If the queue is empty and the file descriptor is set
> > to blocking the IOCTL will block until a new block is available on the
> > outgoing queue.
> >
> > 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 IIO device
> > file descriptor. Each block has a unique offset assigned to it which should
> > be passed to the mmap interface. E.g.
> >
> > mmap(0, block.size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> > block.offset);
> >
> > A typical workflow for the new interface is:
> >
> > BLOCK_ALLOC
> >
> > foreach block
> > BLOCK_QUERY block
> > mmap block.data.offset
> > BLOCK_ENQUEUE block
> >
> > enable buffer
> >
> > while !done
> > BLOCK_DEQUEUE block
> > process data
> > BLOCK_ENQUEUE block
> >
> > disable buffer
> >
> > BLOCK_FREE
> >
> > Signed-off-by: Lars-Peter Clausen <[email protected]>
> > Signed-off-by: Alexandru Ardelean <[email protected]>
...


> > diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h
> > index 13939032b3f6..70ad3aea01ea 100644
> > --- a/include/uapi/linux/iio/buffer.h
> > +++ b/include/uapi/linux/iio/buffer.h
> > @@ -5,6 +5,32 @@
> > #ifndef _UAPI_IIO_BUFFER_H_
> > #define _UAPI_IIO_BUFFER_H_
> >
> > +struct iio_buffer_block_alloc_req {
> > + __u32 type;
> > + __u32 size;
> > + __u32 count;
> > + __u32 id;
> > +};
> > +
> > +#define IIO_BUFFER_BLOCK_FLAG_TIMESTAMP_VALID (1 << 0)
>
> @Lars
> On a recent round of review I did, I noticed that this flag exists and
> isn't set/used anywhere yet.
> The only issue with this is that IIO_BUFFER_BLOCK_FLAG_CYCLIC is now at BIT(1).
>
> This doesn't look used anywhere, neither in libiio.
> Which would mean maybe making IIO_BUFFER_BLOCK_FLAG_CYCLIC BIT(0)

It's fine to burn a bit if it makes life a bit easier for you in updating
libiio. Jut add it as 'reserved'.

Jonathan

>
>
>
> > +
> > +struct iio_buffer_block {
> > + __u32 id;
> > + __u32 size;
> > + __u32 bytes_used;
> > + __u32 type;
> > + __u32 flags;
> > + union {
> > + __u32 offset;
> > + } data;
> > + __u64 timestamp;
> > +};
> > +
> > #define IIO_BUFFER_GET_FD_IOCTL _IOWR('i', 0x91, int)
> > +#define IIO_BUFFER_BLOCK_ALLOC_IOCTL _IOWR('i', 0x92, struct iio_buffer_block_alloc_req)
> > +#define IIO_BUFFER_BLOCK_FREE_IOCTL _IO('i', 0x93)
> > +#define IIO_BUFFER_BLOCK_QUERY_IOCTL _IOWR('i', 0x93, struct iio_buffer_block)
> > +#define IIO_BUFFER_BLOCK_ENQUEUE_IOCTL _IOWR('i', 0x94, struct iio_buffer_block)
> > +#define IIO_BUFFER_BLOCK_DEQUEUE_IOCTL _IOWR('i', 0x95, struct iio_buffer_block)
> >
> > #endif /* _UAPI_IIO_BUFFER_H_ */
> > --
> > 2.17.1
> >

2021-02-14 16:00:19

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support

On Fri, 12 Feb 2021 12:11:43 +0200
Alexandru Ardelean <[email protected]> wrote:

> Following a recent update to the IIO buffer infrastructure, this change
> adds a basic example on how to access an IIO buffer via the new mmap()
> interface.
>
> The ioctl() for the high-speed mode needs to be enabled right from the
> start, before setting any parameters via sysfs (length, enable, etc), to
> make sure that the mmap mode is used and not the fileio mode.
>
> Signed-off-by: Alexandru Ardelean <[email protected]>

Just one small question on error handling. Otherwise this looks fine to me.

thanks,

Jonathan

> ---
> tools/iio/iio_generic_buffer.c | 183 +++++++++++++++++++++++++++++++--
> 1 file changed, 177 insertions(+), 6 deletions(-)
>
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index fdd08514d556..675a7e6047e0 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -31,6 +31,7 @@
> #include <stdbool.h>
> #include <signal.h>
> #include <sys/ioctl.h>
> +#include <sys/mman.h>
> #include <linux/iio/buffer.h>
> #include "iio_utils.h"
>
> @@ -239,6 +240,132 @@ static int enable_disable_all_channels(char *dev_dir_name, int buffer_idx, int e
> return 0;
> }
>
> +struct mmap_block {
> + struct iio_buffer_block block;
> + void *addr;
> +};
> +
> +static struct mmap_block *enable_high_speed(int buf_fd, unsigned int block_size,
> + int nblocks)
> +{
> + struct iio_buffer_block_alloc_req req = { 0 };
> + struct mmap_block *mmaps = NULL;
> + int mmaps_cnt = 0;
> + int i, ret;
> +
> + /**
> + * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
> + * If using just BLOCK_ALLOC it's distinguish between ENOSYS
> + * and other error types.
> + */
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> + if (ret < 0) {
> + errno = ENOSYS;
> + return NULL;
> + }
> +
> + /* for now, this */
> + req.id = 0;
> + req.type = 0;
> + req.size = block_size;
> + req.count = nblocks;
> +
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, &req);
> + if (ret < 0)
> + return NULL;
> +
> + if (req.count == 0) {
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + if (req.count < nblocks) {
> + fprintf(stderr, "Requested %d blocks, got %d\n",
> + nblocks, req.count);
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + mmaps = calloc(req.count, sizeof(*mmaps));
> + if (!mmaps) {
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + for (i = 0; i < req.count; i++) {
> + mmaps[i].block.id = i;
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL, &mmaps[i].block);
> + if (ret < 0)
> + goto error;
> +
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, &mmaps[i].block);
> + if (ret < 0)
> + goto error;

> +
> + mmaps[i].addr = mmap(0, mmaps[i].block.size,
> + PROT_READ | PROT_WRITE, MAP_SHARED,
> + buf_fd, mmaps[i].block.data.offset);
> +
> + if (mmaps[i].addr == MAP_FAILED)
> + goto error;
> +
> + mmaps_cnt++;
> + }
> +
> + return mmaps;
> +
> +error:
> + for (i = 0; i < mmaps_cnt; i++)
> + munmap(mmaps[i].addr, mmaps[i].block.size);
> + free(mmaps);

No free of the blocks? We have unmapped them but I'd imagine we'd also
need to free them from the driver side.

> + return NULL;
> +}
> +
> +static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
> + struct mmap_block *mmaps, unsigned int mmaps_cnt)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + /**
> + * This is where some buffer-pool management can do wonders,
> + * but for the sake of this sample-code, we're just going to
> + * copy the data and re-enqueue it back
> + */
> + memset(&block, 0, sizeof(block));
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, &block);
> + if (ret < 0)
> + return ret;
> +
> + /* check for weird conditions */
> + if (block.bytes_used > block_size) {
> + fprintf(stderr,
> + "Got a bigger block (%u) than expected (%u)\n",
> + block.bytes_used, block_size);
> + return -EFBIG;
> + }
> +
> + if (block.bytes_used < block_size) {
> + /**
> + * This can be normal, with some real-world data
> + * terminating abruptly. But log it.
> + */
> + fprintf(stderr,
> + "Got a smaller block (%u) than expected (%u)\n",
> + block.bytes_used, block_size);
> + }
> +
> + /* memcpy() the data, we lose some more performance here :p */
> + memcpy(data, mmaps[block.id].addr, block.bytes_used);
> +
> + /* and re-queue this back */
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, &mmaps[block.id].block);
> + if (ret < 0)
> + return ret;
> +
> + return block.bytes_used;
> +}
> +
> static void print_usage(void)
> {
> fprintf(stderr, "Usage: generic_buffer [options]...\n"
> @@ -249,6 +376,7 @@ static void print_usage(void)
> " -c <n> Do n conversions, or loop forever if n < 0\n"
> " -e Disable wait for event (new data)\n"
> " -g Use trigger-less mode\n"
> + " -h Use high-speed buffer access\n"
> " -l <n> Set buffer length to n samples\n"
> " --device-name -n <name>\n"
> " --device-num -N <num>\n"
> @@ -356,9 +484,15 @@ int main(int argc, char **argv)
>
> struct iio_channel_info *channels = NULL;
>
> + static bool use_high_speed = false;
> + unsigned int block_size;
> + int nblocks = 16; /* default */
> + int mmaps_cnt = 0;
> + struct mmap_block *mmaps = NULL;
> +
> register_cleanup();
>
> - while ((c = getopt_long(argc, argv, "aAb:c:egl:n:N:t:T:w:?", longopts,
> + while ((c = getopt_long(argc, argv, "aAb:c:eghl:n:N:t:T:w:?", longopts,
> NULL)) != -1) {
> switch (c) {
> case 'a':
> @@ -396,6 +530,9 @@ int main(int argc, char **argv)
> case 'g':
> notrigger = 1;
> break;
> + case 'h':
> + use_high_speed = true;
> + break;
> case 'l':
> errno = 0;
> buf_len = strtoul(optarg, &dummy, 10);
> @@ -661,6 +798,29 @@ int main(int argc, char **argv)
> goto error;
> }
>
> + scan_size = size_from_channelarray(channels, num_channels);
> + block_size = scan_size * buf_len;
> + /**
> + * Need to enable high-speed before configuring length/enable.
> + * Otherwise, the DMA buffer will work in fileio mode,
> + * and mmap won't work.
> + */
> + if (use_high_speed) {
> + /**
> + * The block_size for one block is the same as 'data', but it
> + * doesn't need to be the same size. It is easier for the sake
> + * of this example.
> + */
> + mmaps = enable_high_speed(buf_fd, block_size, nblocks);
> + if (!mmaps) {
> + fprintf(stderr, "Could not enable high-speed mode\n");
> + ret = -errno;
> + goto error;
> + }
> + mmaps_cnt = nblocks;
> + printf("Using high-speed mode\n");
> + }
> +
> /* Setup ring buffer parameters */
> ret = write_sysfs_int("length", buf_dir_name, buf_len);
> if (ret < 0)
> @@ -675,8 +835,7 @@ int main(int argc, char **argv)
> goto error;
> }
>
> - scan_size = size_from_channelarray(channels, num_channels);
> - data = malloc(scan_size * buf_len);
> + data = malloc(block_size);
> if (!data) {
> ret = -ENOMEM;
> goto error;
> @@ -719,7 +878,13 @@ int main(int argc, char **argv)
> toread = 64;
> }
>
> - read_size = read(buf_fd, data, toread * scan_size);
> + if (use_high_speed) {
> + read_size = read_high_speed(buf_fd, data, block_size,
> + mmaps, mmaps_cnt);
> + } else {
> + read_size = read(buf_fd, data, toread * scan_size);
> + }
> +
> if (read_size < 0) {
> if (errno == EAGAIN) {
> fprintf(stderr, "nothing available\n");
> @@ -738,8 +903,14 @@ int main(int argc, char **argv)
>
> if (fd >= 0 && close(fd) == -1)
> perror("Failed to close character device");
> - if (buf_fd >= 0 && close(buf_fd) == -1)
> - perror("Failed to close buffer");
> + for (i = 0; i < mmaps_cnt; i++)
> + munmap(mmaps[i].addr, mmaps[i].block.size);
> + free(mmaps);
> + if (buf_fd >= 0) {
> + ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> + if (close(buf_fd) == -1)
> + perror("Failed to close buffer");
> + }
> free(buffer_access);
> free(data);
> free(buf_dir_name);

2021-02-14 16:13:09

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] iio: buffer-dma: Add mmap support

On Fri, 12 Feb 2021 12:11:42 +0200
Alexandru Ardelean <[email protected]> wrote:

> From: Lars-Peter Clausen <[email protected]>
>
> Add support for the new mmap interface to IIO DMA buffer. This interface
> allows to directly map the backing memory of a block to userspace. This is
> especially advantageous for high-speed devices where the extra copy from
> kernel space to userspace of the data incurs a significant overhead.
>
> In addition this interface allows more fine grained control over how many
> blocks are allocated and their size.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> Signed-off-by: Alexandru Ardelean <[email protected]>

There are a few changes in here that could have been prequel noop patches
and made this easier to review.

factoring out iio_dma_buffer_fileio_free() for example

A few questions inline.

I'll admit I haven't followed every detail of what this is doing, but
in general it looks sensible.

Jonathan


> ---
> drivers/iio/buffer/industrialio-buffer-dma.c | 314 ++++++++++++++++--
> .../buffer/industrialio-buffer-dmaengine.c | 22 +-
> drivers/iio/industrialio-buffer.c | 1 +
> include/linux/iio/buffer-dma.h | 20 +-
> 4 files changed, 321 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c
> index d348af8b9705..befb0a3d2def 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dma.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dma.c
> @@ -90,6 +90,9 @@
> * callback is called from within the custom callback.
> */
>
> +static unsigned int iio_dma_buffer_max_block_size = SZ_16M;
> +module_param_named(max_block_size, iio_dma_buffer_max_block_size, uint, 0644);
> +
> static void iio_buffer_block_release(struct kref *kref)
> {
> struct iio_dma_buffer_block *block = container_of(kref,
> @@ -97,7 +100,7 @@ static void iio_buffer_block_release(struct kref *kref)
>
> WARN_ON(block->state != IIO_BLOCK_STATE_DEAD);
>
> - dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->size),
> + dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->block.size),
> block->vaddr, block->phys_addr);
>
> iio_buffer_put(&block->queue->buffer);
> @@ -178,7 +181,7 @@ static struct iio_dma_buffer_block *iio_dma_buffer_alloc_block(
> return NULL;
> }
>
> - block->size = size;
> + block->block.size = size;
> block->state = IIO_BLOCK_STATE_DEQUEUED;
> block->queue = queue;
> INIT_LIST_HEAD(&block->head);
> @@ -243,7 +246,7 @@ void iio_dma_buffer_block_list_abort(struct iio_dma_buffer_queue *queue,
> spin_lock_irqsave(&queue->list_lock, flags);
> list_for_each_entry_safe(block, _block, list, head) {
> list_del(&block->head);
> - block->bytes_used = 0;
> + block->block.bytes_used = 0;
> _iio_dma_buffer_block_done(block);
> iio_buffer_block_put_atomic(block);
> }
> @@ -296,6 +299,10 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
>
> mutex_lock(&queue->lock);
>
> + /* If in mmap mode dont do anything */
> + if (queue->num_blocks)
> + goto out_unlock;
> +
> /* Allocations are page aligned */
> if (PAGE_ALIGN(queue->fileio.block_size) == PAGE_ALIGN(size))
> try_reuse = true;
> @@ -330,7 +337,7 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
> iio_buffer_block_put(block);
> block = NULL;
> } else {
> - block->size = size;
> + block->block.size = size;
> }
> } else {
> block = NULL;
> @@ -345,6 +352,8 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
> queue->fileio.blocks[i] = block;
> }
>
> + block->block.id = i;
> +
> block->state = IIO_BLOCK_STATE_QUEUED;
> list_add_tail(&block->head, &queue->incoming);
> }
> @@ -356,6 +365,30 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer)
> }
> EXPORT_SYMBOL_GPL(iio_dma_buffer_request_update);
>
> +static void iio_dma_buffer_fileio_free(struct iio_dma_buffer_queue *queue)
> +{
> + unsigned int i;
> +
> + spin_lock_irq(&queue->list_lock);
> + for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
> + if (!queue->fileio.blocks[i])
> + continue;
> + queue->fileio.blocks[i]->state = IIO_BLOCK_STATE_DEAD;
> + }
> + INIT_LIST_HEAD(&queue->outgoing);
> + spin_unlock_irq(&queue->list_lock);
> +
> + INIT_LIST_HEAD(&queue->incoming);
> +
> + for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
> + if (!queue->fileio.blocks[i])
> + continue;
> + iio_buffer_block_put(queue->fileio.blocks[i]);
> + queue->fileio.blocks[i] = NULL;
> + }
> + queue->fileio.active_block = NULL;
> +}
> +
> static void iio_dma_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> struct iio_dma_buffer_block *block)
> {
> @@ -404,6 +437,7 @@ int iio_dma_buffer_enable(struct iio_buffer *buffer,
> struct iio_dma_buffer_block *block, *_block;
>
> mutex_lock(&queue->lock);
> + queue->fileio.enabled = !queue->num_blocks;
> queue->active = true;
> list_for_each_entry_safe(block, _block, &queue->incoming, head) {
> list_del(&block->head);
> @@ -429,6 +463,7 @@ int iio_dma_buffer_disable(struct iio_buffer *buffer,
> struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
>
> mutex_lock(&queue->lock);
> + queue->fileio.enabled = false;
> queue->active = false;
>
> if (queue->ops && queue->ops->abort)
> @@ -490,6 +525,11 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,
>
> mutex_lock(&queue->lock);
>
> + if (!queue->fileio.enabled) {
> + ret = -EBUSY;
> + goto out_unlock;
> + }
> +
> if (!queue->fileio.active_block) {
> block = iio_dma_buffer_dequeue(queue);
> if (block == NULL) {
> @@ -503,8 +543,8 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,
> }
>
> n = rounddown(n, buffer->bytes_per_datum);
> - if (n > block->bytes_used - queue->fileio.pos)
> - n = block->bytes_used - queue->fileio.pos;
> + if (n > block->block.bytes_used - queue->fileio.pos)
> + n = block->block.bytes_used - queue->fileio.pos;
>
> if (copy_to_user(user_buffer, block->vaddr + queue->fileio.pos, n)) {
> ret = -EFAULT;
> @@ -513,7 +553,7 @@ int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,
>
> queue->fileio.pos += n;
>
> - if (queue->fileio.pos == block->bytes_used) {
> + if (queue->fileio.pos == block->block.bytes_used) {
> queue->fileio.active_block = NULL;
> iio_dma_buffer_enqueue(queue, block);
> }
> @@ -549,11 +589,11 @@ size_t iio_dma_buffer_data_available(struct iio_buffer *buf)
>
> mutex_lock(&queue->lock);
> if (queue->fileio.active_block)
> - data_available += queue->fileio.active_block->size;
> + data_available += queue->fileio.active_block->block.size;
>
> spin_lock_irq(&queue->list_lock);
> list_for_each_entry(block, &queue->outgoing, head)
> - data_available += block->size;
> + data_available += block->block.size;
> spin_unlock_irq(&queue->list_lock);
> mutex_unlock(&queue->lock);
>
> @@ -561,6 +601,241 @@ size_t iio_dma_buffer_data_available(struct iio_buffer *buf)
> }
> EXPORT_SYMBOL_GPL(iio_dma_buffer_data_available);
>
> +int iio_dma_buffer_alloc_blocks(struct iio_buffer *buffer,
> + struct iio_buffer_block_alloc_req *req)
> +{
> + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> + struct iio_dma_buffer_block **blocks;
> + unsigned int num_blocks;
> + unsigned int i;
> + int ret = 0;
> +
> + mutex_lock(&queue->lock);
> +
> + /*
> + * If the buffer is enabled and in fileio mode new blocks can't be
> + * allocated.
> + */
> + if (queue->fileio.enabled) {
> + ret = -EBUSY;
> + goto err_unlock;
> + }
> +
> + /* Free memory that might be in use for fileio mode */
> + iio_dma_buffer_fileio_free(queue);
> +
> + /* 64 blocks ought to be enough for anybody ;) */
> + if (req->count > 64 - queue->num_blocks)
> + req->count = 64 - queue->num_blocks;
> + if (req->size > iio_dma_buffer_max_block_size)
> + req->size = iio_dma_buffer_max_block_size;
> +
> + req->id = queue->num_blocks;
> +
> + if (req->count == 0 || req->size == 0) {
> + ret = 0;
> + goto err_unlock;
> + }
> +
> + num_blocks = req->count + queue->num_blocks;
> +
> + blocks = krealloc(queue->blocks, sizeof(*blocks) * num_blocks,
> + GFP_KERNEL);
> + if (!blocks) {
> + ret = -ENOMEM;
> + goto err_unlock;
> + }
> +
> + for (i = queue->num_blocks; i < num_blocks; i++) {
> + blocks[i] = iio_dma_buffer_alloc_block(queue, req->size);
> + if (!blocks[i])

Is this an error path? ret isn't set but I can't see why iio_dma_buffer_alloc_block()
would fail in a 'good' way.

> + break;
> + blocks[i]->block.id = i;
> + blocks[i]->block.data.offset = queue->max_offset;
> + queue->max_offset += PAGE_ALIGN(req->size);
> + }
> +
> + req->count = i - queue->num_blocks;
> + queue->num_blocks = i;
> + queue->blocks = blocks;
> +
> +err_unlock:
> + mutex_unlock(&queue->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_alloc_blocks);
> +
> +int iio_dma_buffer_free_blocks(struct iio_buffer *buffer)
> +{
> + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> + unsigned int i;
> +
> + mutex_lock(&queue->lock);
> +
> + spin_lock_irq(&queue->list_lock);
> + INIT_LIST_HEAD(&queue->incoming);
> + INIT_LIST_HEAD(&queue->outgoing);
> +
> + for (i = 0; i < queue->num_blocks; i++)
> + queue->blocks[i]->state = IIO_BLOCK_STATE_DEAD;
> + spin_unlock_irq(&queue->list_lock);
> +
> + for (i = 0; i < queue->num_blocks; i++)
> + iio_buffer_block_put(queue->blocks[i]);
> +
> + kfree(queue->blocks);
> + queue->blocks = NULL;
> + queue->num_blocks = 0;
> + queue->max_offset = 0;
> +
> + mutex_unlock(&queue->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_free_blocks);
> +
> +int iio_dma_buffer_query_block(struct iio_buffer *buffer,
> + struct iio_buffer_block *block)
> +{
> + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> + int ret = 0;
> +
> + mutex_lock(&queue->lock);
> +
> + if (block->id >= queue->num_blocks) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + *block = queue->blocks[block->id]->block;
> +
> +out_unlock:
> + mutex_unlock(&queue->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_query_block);
> +
> +int iio_dma_buffer_enqueue_block(struct iio_buffer *buffer,
> + struct iio_buffer_block *block)
> +{
> + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> + struct iio_dma_buffer_block *dma_block;
> + int ret = 0;
> +
> + mutex_lock(&queue->lock);
> +
> + if (block->id >= queue->num_blocks) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + dma_block = queue->blocks[block->id];
> + dma_block->block.bytes_used = block->bytes_used;
> + dma_block->block.flags = block->flags;
> +
> + switch (dma_block->state) {
> + case IIO_BLOCK_STATE_DONE:
> + list_del_init(&dma_block->head);
> + break;
> + case IIO_BLOCK_STATE_QUEUED:
> + /* Nothing to do */
> + goto out_unlock;
> + case IIO_BLOCK_STATE_DEQUEUED:
> + break;
> + default:
> + ret = -EBUSY;
> + goto out_unlock;
> + }
> +
> + iio_dma_buffer_enqueue(queue, dma_block);
> +
> +out_unlock:
> + mutex_unlock(&queue->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_enqueue_block);
> +
> +int iio_dma_buffer_dequeue_block(struct iio_buffer *buffer,
> + struct iio_buffer_block *block)
> +{
> + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> + struct iio_dma_buffer_block *dma_block;
> + int ret = 0;
> +
> + mutex_lock(&queue->lock);
> +
> + dma_block = iio_dma_buffer_dequeue(queue);
> + if (!dma_block) {
> + ret = -EAGAIN;
> + goto out_unlock;
> + }
> +
> + *block = dma_block->block;
> +
> +out_unlock:
> + mutex_unlock(&queue->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_dequeue_block);
> +
> +static void iio_dma_buffer_mmap_open(struct vm_area_struct *area)
> +{
> + struct iio_dma_buffer_block *block = area->vm_private_data;
> +
> + iio_buffer_block_get(block);
> +}
> +
> +static void iio_dma_buffer_mmap_close(struct vm_area_struct *area)
> +{
> + struct iio_dma_buffer_block *block = area->vm_private_data;
> +
> + iio_buffer_block_put(block);
> +}
> +
> +static const struct vm_operations_struct iio_dma_buffer_vm_ops = {
> + .open = iio_dma_buffer_mmap_open,
> + .close = iio_dma_buffer_mmap_close,
> +};
> +
> +int iio_dma_buffer_mmap(struct iio_buffer *buffer, struct vm_area_struct *vma)
> +{
> + struct iio_dma_buffer_queue *queue = iio_buffer_to_queue(buffer);
> + struct iio_dma_buffer_block *block = NULL;
> + size_t vm_offset;
> + unsigned int i;
> +
> + vm_offset = vma->vm_pgoff << PAGE_SHIFT;
> +
> + for (i = 0; i < queue->num_blocks; i++) {
I wonder if we should be using a better search structure here than linear

search. I guess mostly we are dealing with a small number of blocks though
so this should be fine.

> + if (queue->blocks[i]->block.data.offset == vm_offset) {
> + block = queue->blocks[i];
> + break;
> + }
> + }
> +
> + if (block == NULL)
> + return -EINVAL;
> +
> + if (PAGE_ALIGN(block->block.size) < vma->vm_end - vma->vm_start)
> + return -EINVAL;
> +
> + vma->vm_pgoff = 0;
> +
> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> + vma->vm_ops = &iio_dma_buffer_vm_ops;
> + vma->vm_private_data = block;
> +
> + vma->vm_ops->open(vma);
> +
> + return dma_mmap_coherent(queue->dev, vma, block->vaddr,
> + block->phys_addr, vma->vm_end - vma->vm_start);
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_mmap);
> +
> /**
> * iio_dma_buffer_set_bytes_per_datum() - DMA buffer set_bytes_per_datum callback
> * @buffer: Buffer to set the bytes-per-datum for
> @@ -635,28 +910,9 @@ EXPORT_SYMBOL_GPL(iio_dma_buffer_init);
> */
> void iio_dma_buffer_exit(struct iio_dma_buffer_queue *queue)
> {
> - unsigned int i;
> -
> mutex_lock(&queue->lock);
>
> - spin_lock_irq(&queue->list_lock);
> - for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
> - if (!queue->fileio.blocks[i])
> - continue;
> - queue->fileio.blocks[i]->state = IIO_BLOCK_STATE_DEAD;
> - }
> - INIT_LIST_HEAD(&queue->outgoing);
> - spin_unlock_irq(&queue->list_lock);
> -
> - INIT_LIST_HEAD(&queue->incoming);
> -
> - for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) {
> - if (!queue->fileio.blocks[i])
> - continue;
> - iio_buffer_block_put(queue->fileio.blocks[i]);
> - queue->fileio.blocks[i] = NULL;
> - }
> - queue->fileio.active_block = NULL;
> + iio_dma_buffer_fileio_free(queue);
> queue->ops = NULL;
>
> mutex_unlock(&queue->lock);
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index 8ecdbae83414..bb022922ec23 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -54,7 +54,7 @@ static void iio_dmaengine_buffer_block_done(void *data,
> spin_lock_irqsave(&block->queue->list_lock, flags);
> list_del(&block->head);
> spin_unlock_irqrestore(&block->queue->list_lock, flags);
> - block->bytes_used -= result->residue;
> + block->block.bytes_used -= result->residue;
> iio_dma_buffer_block_done(block);
> }
>
> @@ -66,12 +66,17 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> struct dma_async_tx_descriptor *desc;
> dma_cookie_t cookie;
>
> - block->bytes_used = min(block->size, dmaengine_buffer->max_size);
> - block->bytes_used = rounddown(block->bytes_used,
> - dmaengine_buffer->align);
> + block->block.bytes_used = min(block->block.size,
> + dmaengine_buffer->max_size);
> + block->block.bytes_used = rounddown(block->block.bytes_used,
> + dmaengine_buffer->align);
> + if (block->block.bytes_used == 0) {
> + iio_dma_buffer_block_done(block);
> + return 0;
> + }
>
> desc = dmaengine_prep_slave_single(dmaengine_buffer->chan,
> - block->phys_addr, block->bytes_used, DMA_DEV_TO_MEM,
> + block->phys_addr, block->block.bytes_used, DMA_DEV_TO_MEM,
> DMA_PREP_INTERRUPT);
> if (!desc)
> return -ENOMEM;
> @@ -120,6 +125,13 @@ static const struct iio_buffer_access_funcs iio_dmaengine_buffer_ops = {
> .data_available = iio_dma_buffer_data_available,
> .release = iio_dmaengine_buffer_release,
>
> + .alloc_blocks = iio_dma_buffer_alloc_blocks,
> + .free_blocks = iio_dma_buffer_free_blocks,
> + .query_block = iio_dma_buffer_query_block,
> + .enqueue_block = iio_dma_buffer_enqueue_block,
> + .dequeue_block = iio_dma_buffer_dequeue_block,
> + .mmap = iio_dma_buffer_mmap,
> +
> .modes = INDIO_BUFFER_HARDWARE,
> .flags = INDIO_BUFFER_FLAG_FIXED_WATERMARK,
> };
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 50228df0b09f..a0d1ad86022f 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -19,6 +19,7 @@
> #include <linux/mm.h>
> #include <linux/poll.h>
> #include <linux/sched/signal.h>
> +#include <linux/mm.h>

Double include - guessing you rebase issues.

>
> #include <linux/iio/iio.h>
> #include <linux/iio/iio-opaque.h>
> diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h
> index 6564bdcdac66..315a8d750986 100644
> --- a/include/linux/iio/buffer-dma.h
> +++ b/include/linux/iio/buffer-dma.h
> @@ -47,7 +47,7 @@ enum iio_block_state {
> struct iio_dma_buffer_block {
> /* May only be accessed by the owner of the block */
> struct list_head head;
> - size_t bytes_used;
> + struct iio_buffer_block block;

Docs update

>
> /*
> * Set during allocation, constant thereafter. May be accessed read-only
> @@ -55,7 +55,6 @@ struct iio_dma_buffer_block {
> */
> void *vaddr;
> dma_addr_t phys_addr;
> - size_t size;

Structure has docs that need updating. However, size is documented
in the wrong place (order wise) so not easily missed!

> struct iio_dma_buffer_queue *queue;
>
> /* Must not be accessed outside the core. */
> @@ -73,12 +72,14 @@ struct iio_dma_buffer_block {
> * @active_block: Block being used in read()
> * @pos: Read offset in the active block
> * @block_size: Size of each block
> + * @enabled: Whether the buffer is operating in fileio mode
> */
> struct iio_dma_buffer_queue_fileio {
> struct iio_dma_buffer_block *blocks[2];
> struct iio_dma_buffer_block *active_block;
> size_t pos;
> size_t block_size;
> + bool enabled;
> };
>
> /**
> @@ -109,6 +110,10 @@ struct iio_dma_buffer_queue {
>
> bool active;
>
> + unsigned int num_blocks;
> + struct iio_dma_buffer_block **blocks;
> + unsigned int max_offset;
> +
> struct iio_dma_buffer_queue_fileio fileio;
> };
>
> @@ -143,4 +148,15 @@ int iio_dma_buffer_init(struct iio_dma_buffer_queue *queue,
> void iio_dma_buffer_exit(struct iio_dma_buffer_queue *queue);
> void iio_dma_buffer_release(struct iio_dma_buffer_queue *queue);
>
> +int iio_dma_buffer_alloc_blocks(struct iio_buffer *buffer,
> + struct iio_buffer_block_alloc_req *req);
> +int iio_dma_buffer_free_blocks(struct iio_buffer *buffer);
> +int iio_dma_buffer_query_block(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> +int iio_dma_buffer_enqueue_block(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> +int iio_dma_buffer_dequeue_block(struct iio_buffer *buffer,
> + struct iio_buffer_block *block);
> +int iio_dma_buffer_mmap(struct iio_buffer *buffer, struct vm_area_struct *vma);
> +
> #endif

2021-02-15 11:41:25

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support

On Sun, Feb 14, 2021 at 5:58 PM Jonathan Cameron <[email protected]> wrote:
>
> On Fri, 12 Feb 2021 12:11:43 +0200
> Alexandru Ardelean <[email protected]> wrote:
>
> > Following a recent update to the IIO buffer infrastructure, this change
> > adds a basic example on how to access an IIO buffer via the new mmap()
> > interface.
> >
> > The ioctl() for the high-speed mode needs to be enabled right from the
> > start, before setting any parameters via sysfs (length, enable, etc), to
> > make sure that the mmap mode is used and not the fileio mode.
> >
> > Signed-off-by: Alexandru Ardelean <[email protected]>
>
> Just one small question on error handling. Otherwise this looks fine to me.
>
> thanks,
>
> Jonathan
>
> > ---
> > tools/iio/iio_generic_buffer.c | 183 +++++++++++++++++++++++++++++++--
> > 1 file changed, 177 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> > index fdd08514d556..675a7e6047e0 100644
> > --- a/tools/iio/iio_generic_buffer.c
> > +++ b/tools/iio/iio_generic_buffer.c
> > @@ -31,6 +31,7 @@
> > #include <stdbool.h>
> > #include <signal.h>
> > #include <sys/ioctl.h>
> > +#include <sys/mman.h>
> > #include <linux/iio/buffer.h>
> > #include "iio_utils.h"
> >
> > @@ -239,6 +240,132 @@ static int enable_disable_all_channels(char *dev_dir_name, int buffer_idx, int e
> > return 0;
> > }
> >
> > +struct mmap_block {
> > + struct iio_buffer_block block;
> > + void *addr;
> > +};
> > +
> > +static struct mmap_block *enable_high_speed(int buf_fd, unsigned int block_size,
> > + int nblocks)
> > +{
> > + struct iio_buffer_block_alloc_req req = { 0 };
> > + struct mmap_block *mmaps = NULL;
> > + int mmaps_cnt = 0;
> > + int i, ret;
> > +
> > + /**
> > + * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
> > + * If using just BLOCK_ALLOC it's distinguish between ENOSYS
> > + * and other error types.
> > + */
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> > + if (ret < 0) {
> > + errno = ENOSYS;
> > + return NULL;
> > + }
> > +
> > + /* for now, this */
> > + req.id = 0;
> > + req.type = 0;
> > + req.size = block_size;
> > + req.count = nblocks;
> > +
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, &req);
> > + if (ret < 0)
> > + return NULL;
> > +
> > + if (req.count == 0) {
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + if (req.count < nblocks) {
> > + fprintf(stderr, "Requested %d blocks, got %d\n",
> > + nblocks, req.count);
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + mmaps = calloc(req.count, sizeof(*mmaps));
> > + if (!mmaps) {
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + for (i = 0; i < req.count; i++) {
> > + mmaps[i].block.id = i;
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL, &mmaps[i].block);
> > + if (ret < 0)
> > + goto error;
> > +
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, &mmaps[i].block);
> > + if (ret < 0)
> > + goto error;
>
> > +
> > + mmaps[i].addr = mmap(0, mmaps[i].block.size,
> > + PROT_READ | PROT_WRITE, MAP_SHARED,
> > + buf_fd, mmaps[i].block.data.offset);
> > +
> > + if (mmaps[i].addr == MAP_FAILED)
> > + goto error;
> > +
> > + mmaps_cnt++;
> > + }
> > +
> > + return mmaps;
> > +
> > +error:
> > + for (i = 0; i < mmaps_cnt; i++)
> > + munmap(mmaps[i].addr, mmaps[i].block.size);
> > + free(mmaps);
>
> No free of the blocks? We have unmapped them but I'd imagine we'd also
> need to free them from the driver side.

Good catch.

>
> > + return NULL;
> > +}
> > +
> > +static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
> > + struct mmap_block *mmaps, unsigned int mmaps_cnt)
> > +{
> > + struct iio_buffer_block block;
> > + int ret;
> > +
> > + /**
> > + * This is where some buffer-pool management can do wonders,
> > + * but for the sake of this sample-code, we're just going to
> > + * copy the data and re-enqueue it back
> > + */
> > + memset(&block, 0, sizeof(block));
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, &block);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* check for weird conditions */
> > + if (block.bytes_used > block_size) {
> > + fprintf(stderr,
> > + "Got a bigger block (%u) than expected (%u)\n",
> > + block.bytes_used, block_size);
> > + return -EFBIG;
> > + }
> > +
> > + if (block.bytes_used < block_size) {
> > + /**
> > + * This can be normal, with some real-world data
> > + * terminating abruptly. But log it.
> > + */
> > + fprintf(stderr,
> > + "Got a smaller block (%u) than expected (%u)\n",
> > + block.bytes_used, block_size);
> > + }
> > +
> > + /* memcpy() the data, we lose some more performance here :p */
> > + memcpy(data, mmaps[block.id].addr, block.bytes_used);
> > +
> > + /* and re-queue this back */
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, &mmaps[block.id].block);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return block.bytes_used;
> > +}
> > +
> > static void print_usage(void)
> > {
> > fprintf(stderr, "Usage: generic_buffer [options]...\n"
> > @@ -249,6 +376,7 @@ static void print_usage(void)
> > " -c <n> Do n conversions, or loop forever if n < 0\n"
> > " -e Disable wait for event (new data)\n"
> > " -g Use trigger-less mode\n"
> > + " -h Use high-speed buffer access\n"
> > " -l <n> Set buffer length to n samples\n"
> > " --device-name -n <name>\n"
> > " --device-num -N <num>\n"
> > @@ -356,9 +484,15 @@ int main(int argc, char **argv)
> >
> > struct iio_channel_info *channels = NULL;
> >
> > + static bool use_high_speed = false;
> > + unsigned int block_size;
> > + int nblocks = 16; /* default */
> > + int mmaps_cnt = 0;
> > + struct mmap_block *mmaps = NULL;
> > +
> > register_cleanup();
> >
> > - while ((c = getopt_long(argc, argv, "aAb:c:egl:n:N:t:T:w:?", longopts,
> > + while ((c = getopt_long(argc, argv, "aAb:c:eghl:n:N:t:T:w:?", longopts,
> > NULL)) != -1) {
> > switch (c) {
> > case 'a':
> > @@ -396,6 +530,9 @@ int main(int argc, char **argv)
> > case 'g':
> > notrigger = 1;
> > break;
> > + case 'h':
> > + use_high_speed = true;
> > + break;
> > case 'l':
> > errno = 0;
> > buf_len = strtoul(optarg, &dummy, 10);
> > @@ -661,6 +798,29 @@ int main(int argc, char **argv)
> > goto error;
> > }
> >
> > + scan_size = size_from_channelarray(channels, num_channels);
> > + block_size = scan_size * buf_len;
> > + /**
> > + * Need to enable high-speed before configuring length/enable.
> > + * Otherwise, the DMA buffer will work in fileio mode,
> > + * and mmap won't work.
> > + */
> > + if (use_high_speed) {
> > + /**
> > + * The block_size for one block is the same as 'data', but it
> > + * doesn't need to be the same size. It is easier for the sake
> > + * of this example.
> > + */
> > + mmaps = enable_high_speed(buf_fd, block_size, nblocks);
> > + if (!mmaps) {
> > + fprintf(stderr, "Could not enable high-speed mode\n");
> > + ret = -errno;
> > + goto error;
> > + }
> > + mmaps_cnt = nblocks;
> > + printf("Using high-speed mode\n");
> > + }
> > +
> > /* Setup ring buffer parameters */
> > ret = write_sysfs_int("length", buf_dir_name, buf_len);
> > if (ret < 0)
> > @@ -675,8 +835,7 @@ int main(int argc, char **argv)
> > goto error;
> > }
> >
> > - scan_size = size_from_channelarray(channels, num_channels);
> > - data = malloc(scan_size * buf_len);
> > + data = malloc(block_size);
> > if (!data) {
> > ret = -ENOMEM;
> > goto error;
> > @@ -719,7 +878,13 @@ int main(int argc, char **argv)
> > toread = 64;
> > }
> >
> > - read_size = read(buf_fd, data, toread * scan_size);
> > + if (use_high_speed) {
> > + read_size = read_high_speed(buf_fd, data, block_size,
> > + mmaps, mmaps_cnt);
> > + } else {
> > + read_size = read(buf_fd, data, toread * scan_size);
> > + }
> > +
> > if (read_size < 0) {
> > if (errno == EAGAIN) {
> > fprintf(stderr, "nothing available\n");
> > @@ -738,8 +903,14 @@ int main(int argc, char **argv)
> >
> > if (fd >= 0 && close(fd) == -1)
> > perror("Failed to close character device");
> > - if (buf_fd >= 0 && close(buf_fd) == -1)
> > - perror("Failed to close buffer");
> > + for (i = 0; i < mmaps_cnt; i++)
> > + munmap(mmaps[i].addr, mmaps[i].block.size);
> > + free(mmaps);
> > + if (buf_fd >= 0) {
> > + ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> > + if (close(buf_fd) == -1)
> > + perror("Failed to close buffer");
> > + }
> > free(buffer_access);
> > free(data);
> > free(buf_dir_name);
>