Am 04.03.24 um 14:59 schrieb Paul Cercueil:
[SNIP]
+       dma_to_ram = buffer->direction == IIO_BUFFER_DIRECTION_IN;
+
+       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.
No, I get exactly what I want. If "dma_to_ram", I get
DMA_RESV_USAGE_READ, otherwise I get DMA_RESV_USAGE_WRITE.

Ah, so basically !dma_to_ram means that you have a write access to the buffer?

If you really don't like the macro, I can inline things here.

Yeah, that would still be incorrect use.

The dma__resv_usage_rw() is for retrieving the fences to sync to for read and write operations and should never be used together with dma_fence_resv_add().

Regards,
Christian.


Cheers,
-Paul

Regards,
Christian.

+       dma_resv_unlock(dmabuf->resv);
+
+       cookie = dma_fence_begin_signalling();
+
+       ret = buffer->access->enqueue_dmabuf(buffer, priv->block,
&fence->base,
+                                            priv->sgt,
iio_dmabuf.bytes_used,
+                                            cyclic);
+       if (ret) {
+               /*
+                * DMABUF enqueue failed, but we already added the
fence.
+                * Signal the error through the fence completion
mechanism.
+                */
+               iio_buffer_signal_dmabuf_done(&fence->base, ret);
+       }
+
+       if (buffer->access->unlock_queue)
+               buffer->access->unlock_queue(buffer);
+
+       dma_fence_end_signalling(cookie);
+       dma_buf_put(dmabuf);
+
+       return ret;
+
+err_queue_unlock:
+       if (buffer->access->unlock_queue)
+               buffer->access->unlock_queue(buffer);
+err_resv_unlock:
+       dma_resv_unlock(dmabuf->resv);
+err_fence_put:
+       dma_fence_put(&fence->base);
+err_attachment_put:
+       iio_buffer_dmabuf_put(attach);
+err_dmabuf_put:
+       dma_buf_put(dmabuf);
+
+       return ret;
+}
+
+static void iio_buffer_cleanup(struct work_struct *work)
+{
+       struct iio_dma_fence *fence =
+               container_of(work, struct iio_dma_fence, work);
+       struct iio_dmabuf_priv *priv = fence->priv;
+       struct dma_buf_attachment *attach = priv->attach;
+
+       dma_fence_put(&fence->base);
+       iio_buffer_dmabuf_put(attach);
+}
+
+void iio_buffer_signal_dmabuf_done(struct dma_fence *fence, int
ret)
+{
+       struct iio_dma_fence *iio_fence =
+               container_of(fence, struct iio_dma_fence, base);
+       bool cookie = dma_fence_begin_signalling();
+
+       /*
+        * Get a reference to the fence, so that it's not freed as
soon as
+        * it's signaled.
+        */
+       dma_fence_get(fence);
+
+       fence->error = ret;
+       dma_fence_signal(fence);
+       dma_fence_end_signalling(cookie);
+
+       /*
+        * The fence will be unref'd in iio_buffer_cleanup.
+        * It can't be done here, as the unref functions might try
to lock the
+        * resv object, which can deadlock.
+        */
+       INIT_WORK(&iio_fence->work, iio_buffer_cleanup);
+       schedule_work(&iio_fence->work);
+}
+EXPORT_SYMBOL_GPL(iio_buffer_signal_dmabuf_done);
+
+static long iio_buffer_chrdev_ioctl(struct file *filp,
+                                   unsigned int cmd, unsigned
long arg)
+{
+       struct iio_dev_buffer_pair *ib = filp->private_data;
+       void __user *_arg = (void __user *)arg;
+       bool nonblock = filp->f_flags & O_NONBLOCK;
+
+       switch (cmd) {
+       case IIO_BUFFER_DMABUF_ATTACH_IOCTL:
+               return iio_buffer_attach_dmabuf(ib, _arg,
nonblock);
+       case IIO_BUFFER_DMABUF_DETACH_IOCTL:
+               return iio_buffer_detach_dmabuf(ib, _arg,
nonblock);
+       case IIO_BUFFER_DMABUF_ENQUEUE_IOCTL:
+               return iio_buffer_enqueue_dmabuf(ib, _arg,
nonblock);
+       default:
+               return -EINVAL;
+       }
+}
+
   static const struct file_operations iio_buffer_chrdev_fileops = {
        .owner = THIS_MODULE,
        .llseek = noop_llseek,
        .read = iio_buffer_read,
        .write = iio_buffer_write,
+       .unlocked_ioctl = iio_buffer_chrdev_ioctl,
+       .compat_ioctl = compat_ptr_ioctl,
        .poll = iio_buffer_poll,
        .release = iio_buffer_chrdev_release,
   };
@@ -1953,6 +2414,7 @@ static void iio_buffer_release(struct kref
*ref)
   {
        struct iio_buffer *buffer = container_of(ref, struct
iio_buffer, ref);
+ mutex_destroy(&buffer->dmabufs_mutex);
        buffer->access->release(buffer);
   }
diff --git a/include/linux/iio/buffer_impl.h
b/include/linux/iio/buffer_impl.h
index 89c3fd7c29ca..f4b1147291e5 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -9,8 +9,12 @@
   #include <uapi/linux/iio/buffer.h>
   #include <linux/iio/buffer.h>
+struct dma_buf_attachment;
+struct dma_fence;
   struct iio_dev;
+struct iio_dma_buffer_block;
   struct iio_buffer;
+struct sg_table;
  /**
    * INDIO_BUFFER_FLAG_FIXED_WATERMARK - Watermark level of the
buffer can not be
@@ -39,6 +43,13 @@ 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.
+ * @attach_dmabuf:     called from userspace via ioctl to attach
one external
+ *                     DMABUF.
+ * @detach_dmabuf:     called from userspace via ioctl to detach
one previously
+ *                     attached DMABUF.
+ * @enqueue_dmabuf:    called from userspace via ioctl to queue
this DMABUF
+ *                     object to this buffer. Requires a valid
DMABUF fd, that
+ *                     was previouly attached to this buffer.
    * @modes:           Supported operating modes by this buffer
type
    * @flags:           A bitmask combination of
INDIO_BUFFER_FLAG_*
    *
@@ -68,6 +79,17 @@ struct iio_buffer_access_funcs {
   void (*release)(struct iio_buffer *buffer); + struct iio_dma_buffer_block * (*attach_dmabuf)(struct
iio_buffer *buffer,
+                                                      struct
dma_buf_attachment *attach);
+       void (*detach_dmabuf)(struct iio_buffer *buffer,
+                             struct iio_dma_buffer_block *block);
+       int (*enqueue_dmabuf)(struct iio_buffer *buffer,
+                             struct iio_dma_buffer_block *block,
+                             struct dma_fence *fence, struct
sg_table *sgt,
+                             size_t size, bool cyclic);
+       void (*lock_queue)(struct iio_buffer *buffer);
+       void (*unlock_queue)(struct iio_buffer *buffer);
+
        unsigned int modes;
        unsigned int flags;
   };
@@ -136,6 +158,12 @@ struct iio_buffer {
   /* @ref: Reference count of the buffer. */
        struct kref ref;
+
+       /* @dmabufs: List of DMABUF attachments */
+       struct list_head dmabufs; /* P: dmabufs_mutex */
+
+       /* @dmabufs_mutex: Protects dmabufs */
+       struct mutex dmabufs_mutex;
   };
  /**
@@ -156,9 +184,14 @@ int iio_update_buffers(struct iio_dev
*indio_dev,
    **/
   void iio_buffer_init(struct iio_buffer *buffer);
+void iio_buffer_dmabuf_get(struct dma_buf_attachment *attach);
+void iio_buffer_dmabuf_put(struct dma_buf_attachment *attach);
+
   struct iio_buffer *iio_buffer_get(struct iio_buffer *buffer);
   void iio_buffer_put(struct iio_buffer *buffer);
+void iio_buffer_signal_dmabuf_done(struct dma_fence *fence, int
ret);
+
   #else /* CONFIG_IIO_BUFFER */
  static inline void iio_buffer_get(struct iio_buffer *buffer) {}
diff --git a/include/uapi/linux/iio/buffer.h
b/include/uapi/linux/iio/buffer.h
index 13939032b3f6..c666aa95e532 100644
--- a/include/uapi/linux/iio/buffer.h
+++ b/include/uapi/linux/iio/buffer.h
@@ -5,6 +5,28 @@
   #ifndef _UAPI_IIO_BUFFER_H_
   #define _UAPI_IIO_BUFFER_H_
+#include <linux/types.h>
+
+/* Flags for iio_dmabuf.flags */
+#define IIO_BUFFER_DMABUF_CYCLIC               (1 << 0)
+#define IIO_BUFFER_DMABUF_SUPPORTED_FLAGS      0x00000001
+
+/**
+ * struct iio_dmabuf - Descriptor for a single IIO DMABUF object
+ * @fd:                file descriptor of the DMABUF object
+ * @flags:     one or more IIO_BUFFER_DMABUF_* flags
+ * @bytes_used:        number of bytes used in this DMABUF for
the data transfer.
+ *             Should generally be set to the DMABUF's size.
+ */
+struct iio_dmabuf {
+       __u32 fd;
+       __u32 flags;
+       __u64 bytes_used;
+};
+
   #define IIO_BUFFER_GET_FD_IOCTL                      _IOWR('i',
0x91, int)
+#define IIO_BUFFER_DMABUF_ATTACH_IOCTL         _IOW('i', 0x92,
int)
+#define IIO_BUFFER_DMABUF_DETACH_IOCTL         _IOW('i', 0x93,
int)
+#define IIO_BUFFER_DMABUF_ENQUEUE_IOCTL                _IOW('i',
0x94, struct iio_dmabuf)
  #endif /* _UAPI_IIO_BUFFER_H_ */

Reply via email to