[PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization function & helper class

[Cheng-Hao Yang]

Hi Laurent,

On Fri, Nov 29, 2024 at 3:49 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Thu, Nov 21, 2024 at 05:51:31AM +0000, Harvey Yang wrote:
> > To synchronize CPU access with mmap and hardware access on DMA buffers,
> > using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and
> > a helper class to allow users to sync buffers more easily.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > ---
> >  .../libcamera/internal/dma_buf_allocator.h    | 21 ++++++
> >  src/libcamera/dma_buf_allocator.cpp           | 75 +++++++++++++++++++
> >  2 files changed, 96 insertions(+)
> >
> > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> > index d2a0a0d19..e4073f668 100644
> > --- a/include/libcamera/internal/dma_buf_allocator.h
> > +++ b/include/libcamera/internal/dma_buf_allocator.h
> > @@ -8,6 +8,7 @@
> >  #pragma once
> >
> >  #include <libcamera/base/flags.h>
> > +#include <libcamera/base/shared_fd.h>
> >  #include <libcamera/base/unique_fd.h>
> >
> >  namespace libcamera {
> > @@ -35,6 +36,26 @@ private:
> >       DmaBufAllocatorFlag type_;
> >  };
> >
> > +class DmaSyncer final
> > +{
> > +public:
> > +     enum class SyncType {
> > +             Read = 0,
> > +             Write,
> > +             ReadWrite,
> > +     };
> > +
> > +     explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);
>
> Why is this calss copyable ?

Thanks for the catch.
Uploaded patch `DmaBufAllocator: Make DmaSyncer non-copyable`.
Please take a look.

BR,
Harvey

>
> > +
> > +     ~DmaSyncer();
> > +
> > +private:
> > +     void sync(uint64_t step);
> > +
> > +     SharedFD fd_;
> > +     uint64_t flags_ = 0;
> > +};
> > +
> >  LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> > index be6efb89f..c1c2103d6 100644
> > --- a/src/libcamera/dma_buf_allocator.cpp
> > +++ b/src/libcamera/dma_buf_allocator.cpp
> > @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
> >               return allocFromHeap(name, size);
> >  }
> >
> > +/**
> > + * \class DmaSyncer
> > + * \brief Helper class for dma-buf's synchronization
> > + *
> > + * This class wraps a userspace dma-buf's synchronization process with an
> > + * object's lifetime.
> > + *
> > + * It's used when the user needs to access a dma-buf with CPU, mostly mapped
> > + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and
> > + * ISP.
> > + */
> > +
> > +/**
> > + * \enum DmaSyncer::SyncType
> > + * \brief Read and/or write access via the CPU map
> > + * \var DmaSyncer::Read
> > + * \brief Indicates that the mapped dma-buf will be read by the client via the
> > + * CPU map
> > + * \var DmaSyncer::Write
> > + * \brief Indicates that the mapped dm-buf will be written by the client via the
> > + * CPU map
> > + * \var DmaSyncer::ReadWrite
> > + * \brief Indicates that the mapped dma-buf will be read and written by the
> > + * client via the CPU map
> > + */
> > +
> > +/**
> > + * \fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)
> > + * \brief Construct a DmaSyncer with a dma-buf's fd and the access type
> > + * \param[in] fd The dma-buf's file descriptor to synchronize
> > + * \param[in] type Read and/or write access via the CPU map
> > + */
> > +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)
> > +     : fd_(fd)
> > +{
> > +     switch (type) {
> > +     case SyncType::Read:
> > +             flags_ = DMA_BUF_SYNC_READ;
> > +             break;
> > +     case SyncType::Write:
> > +             flags_ = DMA_BUF_SYNC_WRITE;
> > +             break;
> > +     case SyncType::ReadWrite:
> > +             flags_ = DMA_BUF_SYNC_RW;
> > +             break;
> > +     }
> > +
> > +     sync(DMA_BUF_SYNC_START);
> > +}
> > +
> > +DmaSyncer::~DmaSyncer()
> > +{
> > +     sync(DMA_BUF_SYNC_END);
> > +}
> > +
> > +void DmaSyncer::sync(uint64_t step)
> > +{
> > +     struct dma_buf_sync sync = {
> > +             .flags = flags_ | step
> > +     };
> > +
> > +     int ret;
> > +     do {
> > +             ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);
> > +     } while (ret && (errno == EINTR || errno == EAGAIN));
> > +
> > +     if (ret) {
> > +             ret = errno;
> > +             LOG(DmaBufAllocator, Error)
> > +                     << "Unable to sync dma fd: " << fd_.get()
> > +                     << ", err: " << strerror(ret)
> > +                     << ", flags: " << sync.flags;
> > +     }
> > +}
> > +
> >  } /* namespace libcamera */
>
> --
> Regards,
>
> Laurent Pinchart