Am 04.03.24 um 15:29 schrieb Paul Cercueil:
Le lundi 04 mars 2024 à 14:41 +0100, Christian König a écrit :
Trying to send this once more as text only.
Am 04.03.24 um 14:40 schrieb Christian König:
Am 04.03.24 um 14:28 schrieb Nuno Sá:
On Mon, 2024-03-04 at 13:44 +0100, Christian König wrote:
Am 23.02.24 um 13:14 schrieb Nuno Sa:
From: Paul Cercueil<paul@xxxxxxxxxxxxxxx>
Add the necessary infrastructure to the IIO core to support a
new
optional DMABUF based interface.
With this new interface, DMABUF objects (externally created)
can be
attached to a IIO buffer, and subsequently used for data
transfer.
A userspace application can then use this interface to share
DMABUF
objects between several interfaces, allowing it to transfer
data in a
zero-copy fashion, for instance between IIO and the USB
stack.
The userspace application can also memory-map the DMABUF
objects, and
access the sample data directly. The advantage of doing this
vs. the
read() interface is that it avoids an extra copy of the data
between the
kernel and userspace. This is particularly userful for high-
speed
devices which produce several megabytes or even gigabytes of
data per
second.
As part of the interface, 3 new IOCTLs have been added:
IIO_BUFFER_DMABUF_ATTACH_IOCTL(int fd):
Attach the DMABUF object identified by the given file
descriptor to the
buffer.
IIO_BUFFER_DMABUF_DETACH_IOCTL(int fd):
Detach the DMABUF object identified by the given file
descriptor from
the buffer. Note that closing the IIO buffer's file
descriptor will
automatically detach all previously attached DMABUF
objects.
IIO_BUFFER_DMABUF_ENQUEUE_IOCTL(struct iio_dmabuf *):
Request a data transfer to/from the given DMABUF object.
Its file
descriptor, as well as the transfer size and flags are
provided in the
"iio_dmabuf" structure.
These three IOCTLs have to be performed on the IIO buffer's
file
descriptor, obtained using the IIO_BUFFER_GET_FD_IOCTL()
ioctl.
A few nit picks and one bug below, apart from that looks good
to me.
Hi Christian,
Thanks for looking at it. I'll just add some comment on the bug
below and for
the other stuff I hope Paul is already able to step in and reply
to it. My
assumption (which seems to be wrong) is that the dmabuf bits
should be already
good to go as they should be pretty similar to the USB part of
it.
...
+ if (dma_to_ram) {
+ /*
+ * If we're writing to the DMABUF, make sure
we don't have
+ * readers
+ */
+ retl = dma_resv_wait_timeout(dmabuf->resv,
+
DMA_RESV_USAGE_READ, true,
+ timeout);
+ if (retl == 0)
+ retl = -EBUSY;
+ if (retl < 0) {
+ ret = (int)retl;
+ goto err_resv_unlock;
+ }
+ }
+
+ if (buffer->access->lock_queue)
+ buffer->access->lock_queue(buffer);
+
+ ret = dma_resv_reserve_fences(dmabuf->resv, 1);
+ if (ret)
+ goto err_queue_unlock;
+
+ dma_resv_add_fence(dmabuf->resv, &fence->base,
+ dma_resv_usage_rw(dma_to_ram));
That is incorrect use of the function dma_resv_usage_rw(). That
function
is for retrieving fences and not adding them.
See the function implementation and comments, when you use it
like this
you get exactly what you don't want.
Does that mean that the USB stuff [1] is also wrong?
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/tr
ee/drivers/usb/gadget/function/f_fs.c?h=usb-next#n1669
Yes, that's broken as well. The dma_resv_usage_rw() function is
supposed to be used while retrieving fences.
Ok, I'll fix it there too.
In other words your "if (dma_to_ram) ..." above is incorrect as
well.
That one should look more like this:
/*
* Writes needs to wait for other writes and reads, but readers
only have to wait for writers.
*/
retl = dma_resv_wait_timeout(dmabuf->resv,
dma_resv_usage_rw(dma_to_ram), timeout);
When writing the DMABUF, the USB code (and the IIO code above) will
wait for writers/readers, but it does so through two consecutive calls
to dma_resv_wait_timeout (because I did not know the proper usage - I
thought I had to check both manually).
Yeah, see the documentation on the dma_resv_usage enum. Basically you
have KERNEL>WRITE>READ>BOOKKEEP.
When waiting for READ you automatically wait for WRITE and KERNEL as
well. So no need for two calls to the wait function.
If you have any idea how to improve the documentation feel free to
suggest, it's certainly not obvious how that works :)
Cheers,
Christian.
So this code block should technically be correct; but I'll update this
code nonetheless.
Regards,
Christian.
Cheers,
-Paul
