Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DMABUF API v3 #1035

Merged
merged 8 commits into from
Sep 27, 2024
Merged

DMABUF API v3 #1035

merged 8 commits into from
Sep 27, 2024

Conversation

pcercuei
Copy link
Contributor

Follow-up of #928

These commits will modify how the local-DMABUF interface works, because the interface itself in the kernel changed (as it is not yet upstream).

The local backend will now make use of the "udmabuf" kernel driver, which was added in 5.19, to create DMABUF objects from memfd buffers. These DMABUFs can then be attached to the IIO device and/or other devices from other subsystems, and memory-mapped to access the data.

IIOD has also been modified to support enqueueing DMABUFs to the USB (functionfs) interface, and will support passing the buffers between IIO and USB in a zero-copy fashion.

From a Libiio API standpoint, some changes were introduced:

  • iio_block_enqueue will support partial transfers on RX buffers as well;
  • Setting bytes_used=0 in iio_block_enqueue will default to transferring the full buffer;
  • New API function iio_block_disable_cpu_access, to disable CPU synchronization with the buffers when doing zero-copy;
  • New API function iio_block_get_dmabuf_fd which can be used when doing zero-copy as well.

@rgetz
Copy link
Contributor

rgetz commented Aug 28, 2023

Do we need some CMake and ifdef so that things still build on old kernels (pre 5.19?)

I think that is what is causing the centos 7 fails.

/__w/1/s/local-dmabuf.c: In function 'local_create_dmabuf':
/__w/1/s/local-dmabuf.c:100:56: error: 'MFD_ALLOW_SEALING' undeclared (first use in this function)
  memfd = syscall(__NR_memfd_create, "/libiio-udmabuf", MFD_ALLOW_SEALING);
                                                        ^
/__w/1/s/local-dmabuf.c:100:56: note: each undeclared identifier is reported only once for each function it appears in
/__w/1/s/local-dmabuf.c:106:19: error: 'F_ADD_SEALS' undeclared (first use in this function)
  if (fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK) < 0) {
                   ^
/__w/1/s/local-dmabuf.c:106:32: error: 'F_SEAL_SHRINK' undeclared (first use in this function)
  if (fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK) < 0) {
                                ^
make[2]: *** [CMakeFiles/iio.dir/local-dmabuf.c.o] Error 1
make[1]: *** [CMakeFiles/iio.dir/all] Error 2
make: *** [all] Error 2

@pcercuei pcercuei force-pushed the pcercuei/dev-new-dmabuf-api branch 5 times, most recently from 95260ac to fd24382 Compare August 30, 2023 12:20
@pcercuei pcercuei mentioned this pull request Oct 5, 2023
@pcercuei pcercuei marked this pull request as draft October 10, 2023 12:43
@pcercuei pcercuei force-pushed the pcercuei/dev-new-dmabuf-api branch 2 times, most recently from 6e3d99e to d472f19 Compare January 15, 2024 10:11
@pcercuei
Copy link
Contributor Author

Updated the PR; it now uses dma-heaps instead of udmabuf to create the DMABUF object.

@pcercuei
Copy link
Contributor Author

Rebased and added a fix so that the DMABUFs are correctly detached from USB after #1136.

@pcercuei pcercuei marked this pull request as ready for review January 30, 2024 13:04
@pcercuei
Copy link
Contributor Author

@mhennerich reminder to merge this :)

Note that I cannot update the branch anymore as I'm not in ADI's organization, so I cannot rebase it.

@mhennerich
Copy link
Contributor

Hi Paul,

@nunojsa is in progress testing it on a backported 6.6 Linux kernel with DMABUF.
We want to test it once more before merging it.
...Hopefully done by the end of the week in case testing doesn't exhibit any problems.

-Michael

@pcercuei
Copy link
Contributor Author

Nice! @nunojsa don't hesitate to ping me if you have problems or questions.

@nunojsa
Copy link
Contributor

nunojsa commented Aug 7, 2024

@pcercuei,

I finally got to do some testing with this with an ADI updated kernel. Things look good and working. Just some hick ups on the MMAP interface (I guess you're more focused on testing the new API :)). While I'm eager to remove that code from ADI kernel, we still need to support it for at least one release.

Anyways, first patch can be squashed in the fixup (just put it like this so it's easier for you to see the change) and the second I guess it needs to be in it's own patch.

@pcercuei
Copy link
Contributor Author

pcercuei commented Sep 5, 2024

@nunojsa forgot to answer, sorry. LGTM!

pcercuei and others added 8 commits September 5, 2024 13:31
This function is meant to be used when the data is never accessed
directly, but passed around as DMABUF objects. In that case, keeping the
data in DMA space means that it doesn't have to be synchronized with the
CPU cache, which improves performance, and saves two ioctl calls.

When CPU access is disabled, all API functions to access a block's data
should not be used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
This API function can be used to retrieve a file descriptor of the
underlying DMABUF object. This file descriptor can then be used for
instance to attach the block to a different device driver for zero-copy
applications.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
The previous DMABUF-based API was refused upstream as it had several
problems and was very complex.

A lot of that complexity was lifted by making the IIO core a DMABUF
importer instead of an exporter. This means that the IIO core is no more
responsible for creating the DMABUF objects, and to map them in the DMA
space.

This task is now delegated to the "dma-heap" kernel driver, available
since kernel 5.6, which allows userspace programs to create DMABUF
objects from the system heap.

The DMABUF interface of the IIO core then simply consists in three
IOCTLs: one to attach a DMABUF to the interface, one to detach it, and
one to request a data transfer.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Implement the .disable_cpu_access() callback. This will only work when
the DMABUF interface is used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Implement the .get_dmabuf_fd() callback. This will only work when the
DMABUF interface is used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Add support for passing sample data as DMABUF objects between IIO
devices, and the USB stack.

This mechanism has the benefit that the CPU will never access the data
to copy it from one hardware buffer to another, and therefore results in
a much higher throughput at a much lower CPU usage.

For this new mechanism to work, the DMABUF-based IIO kernel API must be
available and used by the local backend, and the FunctionFS stack must
also support importing DMABUFs. If those conditions are not met, the
standard way of transferring the data will be used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Add an entry in the CMake options table for the WITH_IIOD_USB_DMABUF
toggle.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
We were setting dmabuf_supported to true before calling
IIO_DMABUF_ATTACH_IOCTL that can still fail. Thererefore we would be
wrongly using the DMABUF interface for things like enqueue/dequeue
blocks.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
@cristina-suteu
Copy link
Collaborator

did another round of testing on this, everything seems good to go.
will wait another day before merging, if there are any other issues, please let me know.

@cristina-suteu cristina-suteu merged commit 9f24ce5 into main Sep 27, 2024
21 of 24 checks passed
@cristina-suteu cristina-suteu deleted the pcercuei/dev-new-dmabuf-api branch September 27, 2024 10:24
@pcercuei
Copy link
Contributor Author

🥳

@rgetz
Copy link
Contributor

rgetz commented Sep 27, 2024

I think we missed adding WITH_IIOD_USB_DMABUF to Cmake so someone knows what they are building... (and turning it on for CI/CD).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants