<div dir="ltr"><div dir="ltr">Hi Daniel,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 27 May 2021 at 16:08, Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, May 25, 2021 at 04:17:50PM -0500, Jason Ekstrand wrote:<br>
> This adds a new "DMA Buffer ioctls" section to the dma-buf docs and adds<br>
> documentation for DMA_BUF_IOCTL_SYNC.<br>
> <br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
> Cc: Daniel Vetter <<a href="mailto:daniel.vetter@ffwll.ch" target="_blank">daniel.vetter@ffwll.ch</a>><br>
> Cc: Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>><br>
> Cc: Sumit Semwal <<a href="mailto:sumit.semwal@linaro.org" target="_blank">sumit.semwal@linaro.org</a>><br>
<br>
We're still missing the doc for the SET_NAME ioctl, but maybe Sumit can be<br>
motivated to fix that?<br></blockquote><div><br></div><div>Yes, certainly, I'll cook up a patch and send it soon. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> ---<br>
> Documentation/driver-api/dma-buf.rst | 8 +++++++<br>
> include/uapi/linux/dma-buf.h | 32 +++++++++++++++++++++++++++-<br>
> 2 files changed, 39 insertions(+), 1 deletion(-)<br>
> <br>
> diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst<br>
> index 7f37ec30d9fd7..784f84fe50a5e 100644<br>
> --- a/Documentation/driver-api/dma-buf.rst<br>
> +++ b/Documentation/driver-api/dma-buf.rst<br>
> @@ -88,6 +88,9 @@ consider though:<br>
> - The DMA buffer FD is also pollable, see `Implicit Fence Poll Support`_ below for<br>
> details.<br>
> <br>
> +- The DMA buffer FD also supports a few dma-buf-specific ioctls, see<br>
> + `DMA Buffer ioctls`_ below for details.<br>
> +<br>
> Basic Operation and Device DMA Access<br>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
> <br>
> @@ -106,6 +109,11 @@ Implicit Fence Poll Support<br>
> .. kernel-doc:: drivers/dma-buf/dma-buf.c<br>
> :doc: implicit fence polling<br>
> <br>
> +DMA Buffer ioctls<br>
> +~~~~~~~~~~~~~~~~~<br>
> +<br>
> +.. kernel-doc:: include/uapi/linux/dma-buf.h<br>
> +<br>
> Kernel Functions and Structures Reference<br>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
> <br>
> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h<br>
> index 7f30393b92c3b..1f67ced853b14 100644<br>
> --- a/include/uapi/linux/dma-buf.h<br>
> +++ b/include/uapi/linux/dma-buf.h<br>
> @@ -22,8 +22,38 @@<br>
> <br>
> #include <linux/types.h><br>
> <br>
> -/* begin/end dma-buf functions used for userspace mmap. */<br>
> +/**<br>
> + * struct dma_buf_sync - Synchronize with CPU access.<br>
> + *<br>
> + * When a DMA buffer is accessed from the CPU via mmap, it is not always<br>
> + * possible to guarantee coherency between the CPU-visible map and underlying<br>
> + * memory. To manage coherency, DMA_BUF_IOCTL_SYNC must be used to bracket<br>
> + * any CPU access to give the kernel the chance to shuffle memory around if<br>
> + * needed.<br>
> + *<br>
> + * Prior to accessing the map, the client should call DMA_BUF_IOCTL_SYNC<br>
<br>
s/should/must/<br>
<br>
> + * with DMA_BUF_SYNC_START and the appropriate read/write flags. Once the<br>
> + * access is complete, the client should call DMA_BUF_IOCTL_SYNC with<br>
> + * DMA_BUF_SYNC_END and the same read/write flags.<br>
<br>
I think we should make it really clear here that this is _only_ for cache<br>
coherency, and that furthermore if you want coherency with gpu access you<br>
either need to use poll() for implicit sync (link to the relevant section)<br>
or handle explicit sync with sync_file (again link would be awesome).<br>
<br>
> + */<br>
> struct dma_buf_sync {<br>
> + /**<br>
> + * @flags: Set of access flags<br>
> + *<br>
> + * - DMA_BUF_SYNC_START: Indicates the start of a map access<br>
<br>
Bikeshed, but I think the item list format instead of bullet point list<br>
looks neater, e.g. DOC: standard plane properties in drm_plane.c.<br>
<br>
<br>
> + * session.<br>
> + *<br>
> + * - DMA_BUF_SYNC_END: Indicates the end of a map access session.<br>
> + *<br>
> + * - DMA_BUF_SYNC_READ: Indicates that the mapped DMA buffer will<br>
> + * be read by the client via the CPU map.<br>
> + *<br>
> + * - DMA_BUF_SYNC_READ: Indicates that the mapped DMA buffer will<br>
<br>
s/READ/WRITE/<br>
<br>
> + * be written by the client via the CPU map.<br>
> + *<br>
> + * - DMA_BUF_SYNC_RW: An alias for DMA_BUF_SYNC_READ |<br>
> + * DMA_BUF_SYNC_WRITE.<br>
> + */<br>
<br>
With the nits addressed: Reviewed-by: Daniel Vetter <<a href="mailto:daniel.vetter@ffwll.ch" target="_blank">daniel.vetter@ffwll.ch</a>><br>
<br>
> __u64 flags;<br>
> };<br>
> <br>
> -- <br>
> 2.31.1<br>
> <br>
<br>
-- <br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="http://blog.ffwll.ch" rel="noreferrer" target="_blank">http://blog.ffwll.ch</a><br>
</blockquote></div><div><br></div>Best,<div>Sumit.<br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr">Thanks and regards,<br><br>Sumit Semwal<br>Tech Lead - Android, Vertical Technologies<br>Linaro.org │ Open source software for ARM SoCs</div></div></div></div>