[RFC 3/4] Documentation: move dma-buf documentation to rst
Sumit Semwal
sumit.semwal at linaro.org
Thu Aug 11 12:49:17 UTC 2016
Hi Markus,
On 11 August 2016 at 17:28, Markus Heiser <markus.heiser at darmarit.de> wrote:
> Hi Sumit,
>
> I haven't compiled your patch yet, just my 2cent about the
> reStructuredText (reST) ASCII markup ...
>
Thanks very much for your detailed review comments - highly appreciated!
> Here are some handy links about reST and the Sphinx markup constructs,
> we have not yet added to the documentation (sorry):
>
> * reST primer: http://www.sphinx-doc.org/en/stable/rest.html
> * reST quickref: http://docutils.sourceforge.net/docs/user/rst/quickref.html
> * Sphinx markup constructs: http://www.sphinx-doc.org/en/stable/markup/index.html
> * Sphinx domains: http://www.sphinx-doc.org/en/stable/domains.html
> * Sphinx cross references: http://www.sphinx-doc.org/en/stable/markup/inline.html#cross-referencing-arbitrary-locations
>
> Am 11.08.2016 um 12:47 schrieb Sumit Semwal <sumit.semwal at linaro.org>:
>
>> Branch out dma-buf related documentation into its own rst file to allow
>> adding it to the sphinx documentation generated.
>>
>> While at it, move dma-buf-sharing.txt into rst as the dma-buf guide too.
>>
>> Signed-off-by: Sumit Semwal <sumit.semwal at linaro.org>
> ...
>> +dma-buf operations for device dma only
>> +======================================
>> +
>> +The dma_buf buffer sharing API usage contains the following steps:
>> +
>> +1. Exporter announces that it wishes to export a buffer
>> +2. Userspace gets the file descriptor associated with the exported buffer, and
>> + passes it around to potential buffer-users based on use case
>> +3. Each buffer-user 'connects' itself to the buffer
>> +4. When needed, buffer-user requests access to the buffer from exporter
>> +5. When finished with its use, the buffer-user notifies end-of-DMA to exporter
>> +6. when buffer-user is done using this buffer completely, it 'disconnects'
>> + itself from the buffer.
>> +
>> +
>> +1. Exporter's announcement of buffer export
>> +-------------------------------------------
>
> I can't recommend numbering the (sub-) sections explicit, even if you here
> wanted to numerate the steps. Most often section numbers are controlled by
> the subordinate toctree directive and this might not fit to the step
> numbers.
>
> * http://www.sphinx-doc.org/en/stable/markup/toctree.html?highlight=toc#the-toc-tree
>
>> +
>> + The buffer exporter announces its wish to export a buffer. In this, it
>> + connects its own private buffer data, provides implementation for operations
>> + that can be performed on the exported :c:type:`dma_buf`, and flags for the file
>> + associated with this buffer. All these fields are filled in struct
>> + :c:type:`dma_buf_export_info`, defined via the DEFINE_DMA_BUF_EXPORT_INFO macro.
>> +
>
> In restructuredText whitespace are markups.
>
> Do not indent if you don't want to create a (indented) block. I recommend to drop
> the two spaces in front of the paragraphs.
>
> IMHO you have to decide if you like to use sectioning (drop the to spaces)
> or stay with the two spaces and use an enumeration list. If you wan't to stay
> with the enumeration write:
>
> |1. Exporter's announcement of buffer export
> |
> | The buffer exporter announces its wish to export a buffer. In this, it
> | connects its own private buffer ..
> | ...
>
>
Understood; will correct.
>> + Interface:
>> + :c:type:`DEFINE_DMA_BUF_EXPORT_INFO(exp_info) <DEFINE_DMA_BUF_EXPORT_INFO>`
>
> If haven't tested your patch, but I guess this will result in a Warning.
>
> the markup ":c:type" is a reference to a typedef description
>
> * http://www.sphinx-doc.org/en/stable/domains.html#role-c:type
>
> If the description is given, you can use the short form
>
> :c:type:`DEFINE_DMA_BUF_EXPORT_INFO`
>
> but I think, this is a function, not a typedef.
>
I did test it before sending out the RFC, and didn't get any warnings.
This, btw, is a macro, so I guess that's why no warning.
While on this topic, in my experimentation, if I just use the short
form, the resultant text doesn't include the "(exp_info)" part as
shown above.
>> +
>> + :c:func:`struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)<dma_buf_export>`
>
> short form to refer
>
> :c:func:`dma_buf_export()`
>
When I last tried the short form, it only gave me a link to the full
definition, and I wanted to have the arguments also shown here, so
that the explanation below was easier. Though I think I tried without
the (), so will try again and let you know.
>> +
>> + If this succeeds, dma_buf_export allocates a dma_buf structure, and
>> + returns a pointer to the same. It also associates an anonymous file with this
>> + buffer, so it can be exported. On failure to allocate the dma_buf object,
>> + it returns NULL.
>> +
>> + 'exp_name' in struct dma_buf_export_info is the name of exporter - to
>> + facilitate information while debugging. It is set to KBUILD_MODNAME by
>
> If you want to render a constant in a monospace font you can use the
> inline markup ``KBUILD_MODNAME``, but if you want.
>
Thanks, will do.
>> + default, so exporters don't have to provide a specific name, if they don't
>> + wish to.
>> +
>> + DEFINE_DMA_BUF_EXPORT_INFO macro defines the struct dma_buf_export_info,
>> + zeroes it out and pre-populates exp_name in it.
>> +
>> +2. Userspace gets a handle to pass around to potential buffer-users
>> +-------------------------------------------------------------------
>> +
>> + Userspace entity requests for a file-descriptor (fd) which is a handle to the
>> + anonymous file associated with the buffer. It can then share the fd with other
>> + drivers and/or processes.
>> +
>> + Interface:
>> + :c:func:`int dma_buf_fd(struct dma_buf *dmabuf, int flags) <dma_buf_fd>`
>> +
>> + This API installs an fd for the anonymous file associated with this buffer;
>> + returns either 'fd', or error.
>
> I recommend to markup source code objects uniform with ``fd``.
>
Ok.
>> +
>> +3. Each buffer-user 'connects' itself to the buffer
>> +---------------------------------------------------
>> +
>> + Each buffer-user now gets a reference to the buffer, using the fd passed to
>> + it.
>> +
>> + Interface:
>> + :c:func:`struct dma_buf *dma_buf_get(int fd) <dma_buf_get>`
>> +
>> + This API will return a reference to the dma_buf, and increment refcount for
>> + it.
>> +
>> + After this, the buffer-user needs to attach its device with the buffer, which
>> + helps the exporter to know of device buffer constraints.
>> +
>> + Interface:
>> + :c:func:`struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, struct device *dev) <dma_buf_attach>`
>> +
>> + This API returns reference to an attachment structure, which is then used
>> + for scatterlist operations. It will optionally call the 'attach' dma_buf
>> + operation, if provided by the exporter.
>> +
>> + The dma-buf sharing framework does the bookkeeping bits related to managing
>> + the list of all attachments to a buffer.
>> +
>> +.. note:: Until this stage, the buffer-exporter has the option to choose not to
>> + actually allocate the backing storage for this buffer, but wait for the
>> + first buffer-user to request use of buffer for allocation.
>
> Use newlines ... which are markups in reST ;)
>
> .. note::
>
> Until this stage, the buffer-exporter has the option to choose not to
> actually allocate the backing storage for this buffer, but wait for the
> first buffer-user to request use of buffer for allocation.
>
>
Ok :), will correct.
>> +6. when buffer-user is done using this buffer, it 'disconnects' itself from the buffer.
>> +---------------------------------------------------------------------------------------
>> +
>> + After the buffer-user has no more interest in using this buffer, it should
>> + disconnect itself from the buffer:
>> +
>> + * it first detaches itself from the buffer.
>> +
>> + Interface:
>> + :c:func:`void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *dmabuf_attach) <dma_buf_detach>`
>> +
>> + This API removes the attachment from the list in dmabuf, and optionally calls
>> + dma_buf->ops->detach(), if provided by exporter, for any housekeeping bits.
>> +
>> + * Then, the buffer-user returns the buffer reference to exporter.
>> +
>> + Interface:
>> + :c:func:`void dma_buf_put(struct dma_buf *dmabuf) <dma_buf_put>`
>> +
>> + This API then reduces the refcount for this buffer.
>> +
>> + If, as a result of this call, the refcount becomes 0, the 'release' file
>> + operation related to this fd is called. It calls the dmabuf->ops->release()
>> + operation in turn, and frees the memory allocated for dmabuf when exported.
>> +
>> +NOTES:
>> +------
>> +
>
> I can't recommend to use colons in titles.
>
Ok, will remove them.
>> +* **Importance of attach-detach and {map,unmap}_dma_buf operation pairs**
>> +
>> + The attach-detach calls allow the exporter to figure out backing-storage
>
> only 2 spaces are needed here ... whitespaces are markups ;)
>
Right, got the message;)
>> + constraints for the currently-interested devices. This allows preferential
>> + allocation, and/or migration of pages across different types of storage
>> + available, if possible.
>> +
>> + Bracketing of DMA access with {map,unmap}_dma_buf operations is essential
>> + to allow just-in-time backing of storage, and migration mid-way through a
>> + use-case.
>
>> +
>> +* **Migration of backing storage if needed**
>> +
>
> I can't recommend this style, using a list and a first bold line to emulate
> something what looks like a subsection .. use subsections or leave the bold line.
>
Yes, that's right; will correct.
>
>
>> + If after
>> +
>> + * at least one map_dma_buf has happened,
>> + * and the backing storage has been allocated for this buffer,
>> +
>> + another new buffer-user intends to attach itself to this buffer, it might
>> + be allowed, if possible for the exporter.
>> +
>> + In case it is allowed by the exporter:
>> +
>> + * if the new buffer-user has stricter 'backing-storage constraints', and the
>> + exporter can handle these constraints, the exporter can just stall on the
>> + map_dma_buf until all outstanding access is completed (as signalled by
>> + unmap_dma_buf).
>> +
>> + * Once all users have finished accessing and have unmapped this buffer, the
>> + exporter could potentially move the buffer to the stricter backing-storage,
>> + and then allow further {map,unmap}_dma_buf operations from any buffer-user
>> + from the migrated backing-storage.
>> +
>> + * If the exporter cannot fulfill the backing-storage constraints of the new
>> + buffer-user device as requested, dma_buf_attach() would return an error to
>> + denote non-compatibility of the new buffer-sharing request with the current
>> + buffer.
>> +
>> +
>> + If the exporter chooses not to allow an attach() operation once a
>> + map_dma_buf() API has been called, it simply returns an error.
>> +
>> +Kernel cpu access to a dma-buf buffer object
>> +============================================
>> +
>> +The motivation to allow cpu access from the kernel to a dma-buf object from the
>> +importers side are:
>> +
>> +* fallback operations, e.g. if the devices is connected to a usb bus and the
>> + kernel needs to shuffle the data around first before sending it away.
>> +* full transparency for existing users on the importer side, i.e. userspace
>> + should not notice the difference between a normal object from that subsystem
>> + and an imported one backed by a dma-buf. This is really important for drm
>> + opengl drivers that expect to still use all the existing upload/download
>> + paths.
>
> I is recommended to separate blocks (in this case the list item blocks) with
> a newline. E.g.
>
> * first lorem
> ipsum
>
> * second lorem
> ipsum
>
> If you have only one-liners, it is OK to write
>
> * first
> * second
>
Understood, good tip for me.
>> +
>> +Access to a dma_buf from the kernel context involves three steps:
>> +
>> +1. Prepare access, which invalidate any necessary caches and make the object
>> + available for cpu access.
>> +2. Access the object page-by-page with the dma_buf map apis
>> +3. Finish access, which will flush any necessary cpu caches and free reserved
>> + resources.
>> +:`void dma_buf_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction dir) <dma_buf_end_cpu_access>`
>> +
>> +
>> +Direct Userspace Access/mmap Support
>> +====================================
>> +
>> +Being able to mmap an export dma-buf buffer object has 2 main use-cases:
>> +* CPU fallback processing in a pipeline and
>> +* supporting existing mmap interfaces in importers.
>> +
>
> Insert a newline in front of the list.
>
Ok.
>> +1. CPU fallback processing in a pipeline
>> +----------------------------------------
>> +
>> + In many processing pipelines it is sometimes required that the cpu can access
>> + the data in a dma-buf (e.g. for thumbnail creation, snapshots, ...). To avoid
>> + the need to handle this specially in userspace frameworks for buffer sharing
>> + it's ideal if the dma_buf fd itself can be used to access the backing storage
>> + from userspace using mmap.
>> +
>> + Furthermore Android's ION framework already supports this (and is otherwise
>> + rather similar to dma-buf from a userspace consumer side with using fds as
>> + handles, too). So it's beneficial to support this in a similar fashion on
>> + dma-buf to have a good transition path for existing Android userspace.
>> +
>> + No special interfaces, userspace simply calls mmap on the dma-buf fd, making
>> + sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
>> + used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
>> + -EAGAIN or -EINTR, in which case it must be restarted.
>> +
>> + Some systems might need some sort of cache coherency management e.g. when
>> + CPU and GPU domains are being accessed through dma-buf at the same time. To
>> + circumvent this problem there are begin/end coherency markers, that forward
>> + directly to existing dma-buf device drivers vfunc hooks. Userspace can make
>> + use of those markers through the DMA_BUF_IOCTL_SYNC ioctl. The sequence
>> + would be used like following:
>> +
>> + * mmap dma-buf fd
>> + * for each drawing/upload cycle in CPU
>> +
>> + 1. SYNC_START ioctl,
>> + 2. read/write to mmap area
>> + 3. SYNC_END ioctl.
>> +
>> + This can be repeated as often as you want (with the new data being
>> + consumed by the GPU or say scanout device)
>> + * munmap once you don't need the buffer any more
>> +
>
> see above, use newline to separate last list item from the one before
>
Ok.
>> + For correctness and optimal performance, it is always required to use
>> + SYNC_START and SYNC_END before and after, respectively, when accessing the
>> + mapped address. Userspace cannot rely on coherent access, even when there
>> + are systems where it just works without calling these ioctls.
>> +
>>
>> +Introduction
>> +------------
>> +
>> +The dma-buf subsystem provides the framework for sharing buffers for
>> +hardware (DMA) access across multiple device drivers and subsystems, and
>> +for synchronizing asynchronous hardware access.
>> +
>> +This is used, for example, by drm "prime" multi-GPU support, but is of
>> +course not limited to GPU use cases.
>> +
>> +The three main components of this are:
>> +
>> +* dma-buf_: represents an sg_table, and is exposed to userspace as a file
>> + descriptor to allow passing between devices,
>> +
>> +* fence_: which provides a mechanism to signal when one device has finished
>> + access, and
>> +
>> +* reservation_: manages the shared or exclusive fence(s) associated with the
>> + buffer.
>> +
>> +Please refer to :ref:`DMA buffer sharing guide <dma-buf-guide>` for more details.
>
> alternative you can use the short form :ref:`dma-buf-guide`, depends on the
> context from which you refer and the targets title.
>
Will try that out.
> But over all, I wan't say: thanks for your work :)
>
Thanks to you too, appreciate you taking time to read through this whole doc! :)
> -- Markus --
>
>
BR,
Sumit.
--
Thanks and regards,
Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
More information about the dri-devel
mailing list