On Thu, May 27, 2021 at 5:38 AM Daniel Vetter <daniel@xxxxxxxx> wrote:
>
> On Tue, May 25, 2021 at 04:17:50PM -0500, Jason Ekstrand wrote:
> > This adds a new "DMA Buffer ioctls" section to the dma-buf docs and adds
> > documentation for DMA_BUF_IOCTL_SYNC.
> >
> > Signed-off-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > Cc: Christian König <christian.koenig@xxxxxxx>
> > Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
>
> We're still missing the doc for the SET_NAME ioctl, but maybe Sumit can be
> motivated to fix that?
>
> > ---
> > Documentation/driver-api/dma-buf.rst | 8 +++++++
> > include/uapi/linux/dma-buf.h | 32 +++++++++++++++++++++++++++-
> > 2 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
> > index 7f37ec30d9fd7..784f84fe50a5e 100644
> > --- a/Documentation/driver-api/dma-buf.rst
> > +++ b/Documentation/driver-api/dma-buf.rst
> > @@ -88,6 +88,9 @@ consider though:
> > - The DMA buffer FD is also pollable, see `Implicit Fence Poll Support`_ below for
> > details.
> >
> > +- The DMA buffer FD also supports a few dma-buf-specific ioctls, see
> > + `DMA Buffer ioctls`_ below for details.
> > +
> > Basic Operation and Device DMA Access
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > @@ -106,6 +109,11 @@ Implicit Fence Poll Support
> > .. kernel-doc:: drivers/dma-buf/dma-buf.c
> > :doc: implicit fence polling
> >
> > +DMA Buffer ioctls
> > +~~~~~~~~~~~~~~~~~
> > +
> > +.. kernel-doc:: include/uapi/linux/dma-buf.h
> > +
> > Kernel Functions and Structures Reference
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
> > index 7f30393b92c3b..1f67ced853b14 100644
> > --- a/include/uapi/linux/dma-buf.h
> > +++ b/include/uapi/linux/dma-buf.h
> > @@ -22,8 +22,38 @@
> >
> > #include <linux/types.h>
> >
> > -/* begin/end dma-buf functions used for userspace mmap. */
> > +/**
> > + * struct dma_buf_sync - Synchronize with CPU access.
> > + *
> > + * When a DMA buffer is accessed from the CPU via mmap, it is not always
> > + * possible to guarantee coherency between the CPU-visible map and underlying
> > + * memory. To manage coherency, DMA_BUF_IOCTL_SYNC must be used to bracket
> > + * any CPU access to give the kernel the chance to shuffle memory around if
> > + * needed.
> > + *
> > + * Prior to accessing the map, the client should call DMA_BUF_IOCTL_SYNC
>
> s/should/must/
>
> > + * with DMA_BUF_SYNC_START and the appropriate read/write flags. Once the
> > + * access is complete, the client should call DMA_BUF_IOCTL_SYNC with
> > + * DMA_BUF_SYNC_END and the same read/write flags.
>
> I think we should make it really clear here that this is _only_ for cache
> coherency, and that furthermore if you want coherency with gpu access you
> either need to use poll() for implicit sync (link to the relevant section)
> or handle explicit sync with sync_file (again link would be awesome).
I've added such a comment. I encourage you to look at v2 which I'll
be sending shortly. I'm not sure how to get the poll() reference to
hyperlink, though.
> > + */
> > struct dma_buf_sync {
> > + /**
> > + * @flags: Set of access flags
> > + *
> > + * - DMA_BUF_SYNC_START: Indicates the start of a map access
>
> Bikeshed, but I think the item list format instead of bullet point list
> looks neater, e.g. DOC: standard plane properties in drm_plane.c.
Yeah, that's better.
> > + * session.
> > + *
> > + * - DMA_BUF_SYNC_END: Indicates the end of a map access session.
> > + *
> > + * - DMA_BUF_SYNC_READ: Indicates that the mapped DMA buffer will
> > + * be read by the client via the CPU map.
> > + *
> > + * - DMA_BUF_SYNC_READ: Indicates that the mapped DMA buffer will
>
> s/READ/WRITE/
Oops.
> > + * be written by the client via the CPU map.
> > + *
> > + * - DMA_BUF_SYNC_RW: An alias for DMA_BUF_SYNC_READ |
> > + * DMA_BUF_SYNC_WRITE.
> > + */
>
> With the nits addressed: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Thanks!
> > __u64 flags;
> > };
> >
> > --
> > 2.31.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
