| Message ID | 20190618092038.17929-2-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/prime: Shuffle functions. | expand |
On Tue, Jun 18, 2019 at 11:20 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Yes this is a bit a big patch, but since it's essentially a complete > rewrite of all the prime docs I didn't see how to better split it up. > > Changes: > - Consistently point to drm_gem_object_funcs as the preferred hooks, > where applicable. > > - Document all the hooks in &drm_driver that lacked kerneldoc. > > - Completely new overview section, which now also includes the cleaned > up lifetime/reference counting subchapter. I also mentioned the weak > references in there due to the lookup caches. > > - Completely rewritten helper intro section, highlight the > import/export related functionality. > > - Polish for all the functions and more cross references. > > I also sprinkled a bunch of todos all over. > > Most important: 0 code changes in here. The cleanup motivated by > reading and improving all this will follow later on. > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > I won't address right away in subsequent cleanup patches. > > v3: > - Split out the function moving. This patch is now exclusively > documentation changes. > - Typos and nits (Sam). > > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Eric Anholt <eric@anholt.net> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Adding Thomas, Gerd & Noralf. I think you folks have looked most closely at this recently, I'd much appreciate some review on this and the previous patch. Thanks, Daniel > --- > Documentation/gpu/drm-mm.rst | 40 +------ > drivers/gpu/drm/drm_prime.c | 197 +++++++++++++++++++++-------------- > include/drm/drm_drv.h | 104 +++++++++++++++--- > include/drm/drm_gem.h | 18 ++-- > 4 files changed, 226 insertions(+), 133 deletions(-) > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index c8ebd4f66a6a..b664f054c259 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -433,43 +433,11 @@ PRIME is the cross device buffer sharing framework in drm, originally > created for the OPTIMUS range of multi-gpu platforms. To userspace PRIME > buffers are dma-buf based file descriptors. > > -Overview and Driver Interface > ------------------------------ > +Overview and Lifetime Rules > +--------------------------- > > -Similar to GEM global names, PRIME file descriptors are also used to > -share buffer objects across processes. They offer additional security: > -as file descriptors must be explicitly sent over UNIX domain sockets to > -be shared between applications, they can't be guessed like the globally > -unique GEM names. > - > -Drivers that support the PRIME API must set the DRIVER_PRIME bit in the > -struct :c:type:`struct drm_driver <drm_driver>` > -driver_features field, and implement the prime_handle_to_fd and > -prime_fd_to_handle operations. > - > -int (\*prime_handle_to_fd)(struct drm_device \*dev, struct drm_file > -\*file_priv, uint32_t handle, uint32_t flags, int \*prime_fd); int > -(\*prime_fd_to_handle)(struct drm_device \*dev, struct drm_file > -\*file_priv, int prime_fd, uint32_t \*handle); Those two operations > -convert a handle to a PRIME file descriptor and vice versa. Drivers must > -use the kernel dma-buf buffer sharing framework to manage the PRIME file > -descriptors. Similar to the mode setting API PRIME is agnostic to the > -underlying buffer object manager, as long as handles are 32bit unsigned > -integers. > - > -While non-GEM drivers must implement the operations themselves, GEM > -drivers must use the :c:func:`drm_gem_prime_handle_to_fd()` and > -:c:func:`drm_gem_prime_fd_to_handle()` helper functions. Those > -helpers rely on the driver gem_prime_export and gem_prime_import > -operations to create a dma-buf instance from a GEM object (dma-buf > -exporter role) and to create a GEM object from a dma-buf instance > -(dma-buf importer role). > - > -struct dma_buf \* (\*gem_prime_export)(struct drm_device \*dev, > -struct drm_gem_object \*obj, int flags); struct drm_gem_object \* > -(\*gem_prime_import)(struct drm_device \*dev, struct dma_buf > -\*dma_buf); These two operations are mandatory for GEM drivers that > -support PRIME. > +.. kernel-doc:: drivers/gpu/drm/drm_prime.c > + :doc: overview and lifetime rules > > PRIME Helper Functions > ---------------------- > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 68b4de85370c..2234206288fa 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -38,47 +38,53 @@ > > #include "drm_internal.h" > > -/* > - * DMA-BUF/GEM Object references and lifetime overview: > - * > - * On the export the dma_buf holds a reference to the exporting GEM > - * object. It takes this reference in handle_to_fd_ioctl, when it > - * first calls .prime_export and stores the exporting GEM object in > - * the dma_buf priv. This reference needs to be released when the > - * final reference to the &dma_buf itself is dropped and its > - * &dma_buf_ops.release function is called. For GEM-based drivers, > - * the dma_buf should be exported using drm_gem_dmabuf_export() and > - * then released by drm_gem_dmabuf_release(). > - * > - * On the import the importing GEM object holds a reference to the > - * dma_buf (which in turn holds a ref to the exporting GEM object). > - * It takes that reference in the fd_to_handle ioctl. > - * It calls dma_buf_get, creates an attachment to it and stores the > - * attachment in the GEM object. When this attachment is destroyed > - * when the imported object is destroyed, we remove the attachment > - * and drop the reference to the dma_buf. > - * > - * When all the references to the &dma_buf are dropped, i.e. when > - * userspace has closed both handles to the imported GEM object (through the > - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported > - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal references > - * are also gone, then the dma_buf gets destroyed. This can also happen as a > - * part of the clean up procedure in the drm_release() function if userspace > - * fails to properly clean up. Note that both the kernel and userspace (by > - * keeeping the PRIME file descriptors open) can hold references onto a > - * &dma_buf. > - * > - * Thus the chain of references always flows in one direction > - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem > - * > - * Self-importing: if userspace is using PRIME as a replacement for flink > - * then it will get a fd->handle request for a GEM object that it created. > - * Drivers should detect this situation and return back the gem object > - * from the dma-buf private. Prime will do this automatically for drivers that > - * use the drm_gem_prime_{import,export} helpers. > - * > - * GEM struct &dma_buf_ops symbols are now exported. They can be resued by > - * drivers which implement GEM interface. > +/** > + * DOC: overview and lifetime rules > + * > + * Similar to GEM global names, PRIME file descriptors are also used to share > + * buffer objects across processes. They offer additional security: as file > + * descriptors must be explicitly sent over UNIX domain sockets to be shared > + * between applications, they can't be guessed like the globally unique GEM > + * names. > + * > + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the > + * &drm_driver.driver_features field, and implement the > + * &drm_driver.prime_handle_to_fd and &drm_driver.prime_fd_to_handle operations. > + * GEM based drivers must use drm_gem_prime_handle_to_fd() and > + * drm_gem_prime_fd_to_handle() to implement these. For GEM based drivers the > + * actual driver interfaces is provided through the &drm_gem_object_funcs.export > + * and &drm_driver.gem_prime_import hooks. > + * > + * &dma_buf_ops implementations for GEM drivers are all individually exported > + * for drivers which need to overwrite or reimplement some of them. > + * > + * Reference Counting for GEM Drivers > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + * > + * On the export the &dma_buf holds a reference to the exported buffer object, > + * usually a &drm_gem_object. It takes this reference in the PRIME_HANDLE_TO_FD > + * IOCTL, when it first calls &drm_gem_object_funcs.export > + * and stores the exporting GEM object in the &dma_buf.priv field. This > + * reference needs to be released when the final reference to the &dma_buf > + * itself is dropped and its &dma_buf_ops.release function is called. For > + * GEM-based drivers, the &dma_buf should be exported using > + * drm_gem_dmabuf_export() and then released by drm_gem_dmabuf_release(). > + * > + * Thus the chain of references always flows in one direction, avoiding loops: > + * importing GEM object -> dma-buf -> exported GEM bo. A further complication > + * are the lookup caches for import and export. These are required to guarantee > + * that any given object will always have only one uniqe userspace handle. This > + * is required to allow userspace to detect duplicated imports, since some GEM > + * drivers do fail command submissions if a given buffer object is listed more > + * than once. These import and export caches in &drm_prime_file_private only > + * retain a weak reference, which is cleaned up when the corresponding object is > + * released. > + * > + * Self-importing: If userspace is using PRIME as a replacement for flink then > + * it will get a fd->handle request for a GEM object that it created. Drivers > + * should detect this situation and return back the underlying object from the > + * dma-buf private. For GEM based drivers this is handled in > + * drm_gem_prime_import() already. > */ > > struct drm_prime_member { > @@ -220,7 +226,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) > } > > /** > - * drm_gem_dmabuf_export - dma_buf export implementation for GEM > + * drm_gem_dmabuf_export - &dma_buf export implementation for GEM > * @dev: parent device for the exported dmabuf > * @exp_info: the export information used by dma_buf_export() > * > @@ -248,11 +254,11 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, > EXPORT_SYMBOL(drm_gem_dmabuf_export); > > /** > - * drm_gem_dmabuf_release - dma_buf release implementation for GEM > + * drm_gem_dmabuf_release - &dma_buf release implementation for GEM > * @dma_buf: buffer to be released > * > * Generic release function for dma_bufs exported as PRIME buffers. GEM drivers > - * must use this in their dma_buf ops structure as the release callback. > + * must use this in their &dma_buf_ops structure as the release callback. > * drm_gem_dmabuf_release() should be used in conjunction with > * drm_gem_dmabuf_export(). > */ > @@ -278,7 +284,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release); > * This is the PRIME import function which must be used mandatorily by GEM > * drivers to ensure correct lifetime management of the underlying GEM object. > * The actual importing of GEM object from the dma-buf is done through the > - * gem_import_export driver callback. > + * &drm_driver.gem_import_export driver callback. > + * > + * Returns 0 on success or a negative error code on failure. > */ > int drm_gem_prime_fd_to_handle(struct drm_device *dev, > struct drm_file *file_priv, int prime_fd, > @@ -412,7 +420,7 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, > * This is the PRIME export function which must be used mandatorily by GEM > * drivers to ensure correct lifetime management of the underlying GEM object. > * The actual exporting from GEM object to a dma-buf is done through the > - * gem_prime_export driver callback. > + * &drm_driver.gem_prime_export driver callback. > */ > int drm_gem_prime_handle_to_fd(struct drm_device *dev, > struct drm_file *file_priv, uint32_t handle, > @@ -523,23 +531,39 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > /** > * DOC: PRIME Helpers > * > - * Drivers can implement @gem_prime_export and @gem_prime_import in terms of > - * simpler APIs by using the helper functions @drm_gem_prime_export and > - * @drm_gem_prime_import. These functions implement dma-buf support in terms of > - * six lower-level driver callbacks: > + * Drivers can implement &drm_gem_object_funcs.export and > + * &drm_driver.gem_prime_import in terms of simpler APIs by using the helper > + * functions drm_gem_prime_export() and drm_gem_prime_import(). These functions > + * implement dma-buf support in terms of some lower-level helpers, which are > + * again exported for drivers to use individually: > + * > + * Exporting buffers > + * ~~~~~~~~~~~~~~~~~ > + * > + * Optional pinning of buffers is handled at dma-buf attach and detach time in > + * drm_gem_map_attach() and drm_gem_map_detach(). Backing storage itself is > + * handled by drm_gem_map_dma_buf() and drm_gem_unmap_dma_buf(), which relies on > + * &drm_gem_object_funcs.get_sg_table. > + * > + * For kernel-internal access there's drm_gem_dmabuf_vmap() and > + * drm_gem_dmabuf_vunmap(). Userspace mmap support is provided by > + * drm_gem_dmabuf_mmap(). > + * > + * Note that these export helpers can only be used if the underlying backing > + * storage is fully coherent and either permanently pinned, or it is safe to pin > + * it indefinitely. > * > - * Export callbacks: > + * FIXME: The underlying helper functions are named rather inconsistently. > * > - * * @gem_prime_pin (optional): prepare a GEM object for exporting > - * * @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages > - * * @gem_prime_vmap: vmap a buffer exported by your driver > - * * @gem_prime_vunmap: vunmap a buffer exported by your driver > - * * @gem_prime_mmap (optional): mmap a buffer exported by your driver > + * Exporting buffers > + * ~~~~~~~~~~~~~~~~~ > * > - * Import callback: > + * Importing dma-bufs using drm_gem_prime_import() relies on > + * &drm_driver.gem_prime_import_sg_table. > * > - * * @gem_prime_import_sg_table (import): produce a GEM object from another > - * driver's scatter/gather table > + * Note that similarly to the export helpers this permanently pins the > + * underlying backing storage. Which is ok for scanout, but is not the best > + * option for sharing lots of buffers for rendering. > */ > > /** > @@ -547,8 +571,9 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > * @dma_buf: buffer to attach device to > * @attach: buffer attachment data > * > - * Calls &drm_driver.gem_prime_pin for device specific handling. This can be > - * used as the &dma_buf_ops.attach callback. > + * Calls &drm_gem_object_funcs.pin for device specific handling. This can be > + * used as the &dma_buf_ops.attach callback. Must be used together with > + * drm_gem_map_detach(). > * > * Returns 0 on success, negative error code on failure. > */ > @@ -566,8 +591,9 @@ EXPORT_SYMBOL(drm_gem_map_attach); > * @dma_buf: buffer to detach from > * @attach: attachment to be detached > * > - * Cleans up &dma_buf_attachment. This can be used as the &dma_buf_ops.detach > - * callback. > + * Calls &drm_gem_object_funcs.pin for device specific handling. Cleans up > + * &dma_buf_attachment from drm_gem_map_attach(). This can be used as the > + * &dma_buf_ops.detach callback. > */ > void drm_gem_map_detach(struct dma_buf *dma_buf, > struct dma_buf_attachment *attach) > @@ -583,13 +609,13 @@ EXPORT_SYMBOL(drm_gem_map_detach); > * @attach: attachment whose scatterlist is to be returned > * @dir: direction of DMA transfer > * > - * Calls &drm_driver.gem_prime_get_sg_table and then maps the scatterlist. This > - * can be used as the &dma_buf_ops.map_dma_buf callback. > + * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This > + * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > + * with drm_gem_unmap_dma_buf(). > * > - * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR > + * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR > * on error. May return -EINTR if it is interrupted by a signal. > */ > - > struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, > enum dma_data_direction dir) > { > @@ -642,9 +668,9 @@ EXPORT_SYMBOL(drm_gem_unmap_dma_buf); > * @dma_buf: buffer to be mapped > * > * Sets up a kernel virtual mapping. This can be used as the &dma_buf_ops.vmap > - * callback. > + * callback. Calls into &drm_gem_object_funcs.vmap for device specific handling. > * > - * Returns the kernel virtual address. > + * Returns the kernel virtual address or NULL on failure. > */ > void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf) > { > @@ -665,7 +691,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap); > * @vaddr: the virtual address of the buffer > * > * Releases a kernel virtual mapping. This can be used as the > - * &dma_buf_ops.vunmap callback. > + * &dma_buf_ops.vunmap callback. Calls into &drm_gem_object_funcs.vunmap for device specific handling. > */ > void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) > { > @@ -727,7 +753,11 @@ EXPORT_SYMBOL(drm_gem_prime_mmap); > * @vma: virtual address range > * > * Provides memory mapping for the buffer. This can be used as the > - * &dma_buf_ops.mmap callback. > + * &dma_buf_ops.mmap callback. It just forwards to &drm_driver.gem_prime_mmap, > + * which should be set to drm_gem_prime_mmap(). > + * > + * FIXME: There's really no point to this wrapper, drivers which need anything > + * else but drm_gem_prime_mmap can roll their own &dma_buf_ops.mmap callback. > * > * Returns 0 on success or a negative error code on failure. > */ > @@ -763,6 +793,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { > * This helper creates an sg table object from a set of pages > * the driver is responsible for mapping the pages into the > * importers address space for use with dma_buf itself. > + * > + * This is useful for implementing &drm_gem_object_funcs.get_sg_table. > */ > struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages) > { > @@ -793,7 +825,7 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg); > * @obj: GEM object to export > * @flags: flags like DRM_CLOEXEC and DRM_RDWR > * > - * This is the implementation of the gem_prime_export functions for GEM drivers > + * This is the implementation of the &drm_gem_object_funcs.export functions for GEM drivers > * using the PRIME helpers. > */ > struct dma_buf *drm_gem_prime_export(struct drm_device *dev, > @@ -823,9 +855,13 @@ EXPORT_SYMBOL(drm_gem_prime_export); > * @dma_buf: dma-buf object to import > * @attach_dev: struct device to dma_buf attach > * > - * This is the core of drm_gem_prime_import. It's designed to be called by > - * drivers who want to use a different device structure than dev->dev for > - * attaching via dma_buf. > + * This is the core of drm_gem_prime_import(). It's designed to be called by > + * drivers who want to use a different device structure than &drm_device.dev for > + * attaching via dma_buf. This function calls > + * &drm_driver.gem_prime_import_sg_table internally. > + * > + * Drivers must arrange to call drm_prime_gem_destroy() from their > + * &drm_gem_object_funcs.free hook when using this function. > */ > struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, > struct dma_buf *dma_buf, > @@ -889,7 +925,11 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); > * @dma_buf: dma-buf object to import > * > * This is the implementation of the gem_prime_import functions for GEM drivers > - * using the PRIME helpers. > + * using the PRIME helpers. Drivers can use this as their > + * &drm_driver.gem_prime_import implementation. > + * > + * Drivers must arrange to call drm_prime_gem_destroy() from their > + * &drm_gem_object_funcs.free hook when using this function. > */ > struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, > struct dma_buf *dma_buf) > @@ -907,6 +947,9 @@ EXPORT_SYMBOL(drm_gem_prime_import); > * > * Exports an sg table into an array of pages and addresses. This is currently > * required by the TTM driver in order to do correct fault handling. > + * > + * Drivers can use this in their &drm_driver.gem_prime_import_sg_table > + * implementation. > */ > int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, > dma_addr_t *addrs, int max_entries) > @@ -947,7 +990,7 @@ EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); > * @sg: the sg-table which was pinned at import time > * > * This is the cleanup functions which GEM drivers need to call when they use > - * @drm_gem_prime_import to import dma-bufs. > + * drm_gem_prime_import() or drm_gem_prime_import_dev() to import dma-bufs. > */ > void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) > { > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 5c4fc0ddc863..bbb3a6ff4418 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -505,21 +505,25 @@ struct drm_driver { > * @gem_free_object: deconstructor for drm_gem_objects > * > * This is deprecated and should not be used by new drivers. Use > - * @gem_free_object_unlocked instead. > + * &drm_gem_object_funcs.free instead. > */ > void (*gem_free_object) (struct drm_gem_object *obj); > > /** > * @gem_free_object_unlocked: deconstructor for drm_gem_objects > * > - * This is for drivers which are not encumbered with &drm_device.struct_mutex > - * legacy locking schemes. Use this hook instead of @gem_free_object. > + * This is deprecated and should not be used by new drivers. Use > + * &drm_gem_object_funcs.free instead. > + * Compared to @gem_free_object this is not encumbered with > + * &drm_device.struct_mutex legacy locking schemes. > */ > void (*gem_free_object_unlocked) (struct drm_gem_object *obj); > > /** > * @gem_open_object: > * > + * This callback is deprecated in favour of &drm_gem_object_funcs.open. > + * > * Driver hook called upon gem handle creation > */ > int (*gem_open_object) (struct drm_gem_object *, struct drm_file *); > @@ -527,6 +531,8 @@ struct drm_driver { > /** > * @gem_close_object: > * > + * This callback is deprecated in favour of &drm_gem_object_funcs.close. > + * > * Driver hook called upon gem handle release > */ > void (*gem_close_object) (struct drm_gem_object *, struct drm_file *); > @@ -534,6 +540,9 @@ struct drm_driver { > /** > * @gem_print_info: > * > + * This callback is deprecated in favour of > + * &drm_gem_object_funcs.print_info. > + * > * If driver subclasses struct &drm_gem_object, it can implement this > * optional hook for printing additional driver specific info. > * > @@ -548,56 +557,120 @@ struct drm_driver { > /** > * @gem_create_object: constructor for gem objects > * > - * Hook for allocating the GEM object struct, for use by core > - * helpers. > + * Hook for allocating the GEM object struct, for use by the CMA and > + * SHMEM GEM helpers. > */ > struct drm_gem_object *(*gem_create_object)(struct drm_device *dev, > size_t size); > - > - /* prime: */ > /** > * @prime_handle_to_fd: > * > - * export handle -> fd (see drm_gem_prime_handle_to_fd() helper) > + * Main PRIME export function. Should be implemented with > + * drm_gem_prime_handle_to_fd() for GEM based drivers. > + * > + * For an in-depth discussion see :ref:`PRIME buffer sharing > + * documentation <prime_buffer_sharing>`. > */ > int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv, > uint32_t handle, uint32_t flags, int *prime_fd); > /** > * @prime_fd_to_handle: > * > - * import fd -> handle (see drm_gem_prime_fd_to_handle() helper) > + * Main PRIME import function. Should be implemented with > + * drm_gem_prime_fd_to_handle() for GEM based drivers. > + * > + * For an in-depth discussion see :ref:`PRIME buffer sharing > + * documentation <prime_buffer_sharing>`. > */ > int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv, > int prime_fd, uint32_t *handle); > /** > * @gem_prime_export: > * > - * export GEM -> dmabuf > - * > - * This defaults to drm_gem_prime_export() if not set. > + * Export hook for GEM drivers. Deprecated in favour of > + * &drm_gem_object_funcs.export. > */ > struct dma_buf * (*gem_prime_export)(struct drm_device *dev, > struct drm_gem_object *obj, int flags); > /** > * @gem_prime_import: > * > - * import dmabuf -> GEM > + * Import hook for GEM drivers. > * > * This defaults to drm_gem_prime_import() if not set. > */ > struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, > struct dma_buf *dma_buf); > + > + /** > + * @gem_prime_pin: > + * > + * Deprecated hook in favour of &drm_gem_object_funcs.pin. > + */ > int (*gem_prime_pin)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_unpin: > + * > + * Deprecated hook in favour of &drm_gem_object_funcs.unpin. > + */ > void (*gem_prime_unpin)(struct drm_gem_object *obj); > + > + > + /** > + * @gem_prime_get_sg_table: > + * > + * Deprecated hook in favour of &drm_gem_object_funcs.get_sg_table. > + */ > + struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_res_obj: > + * > + * Optional hook to look up the &reservation_object for an buffer when > + * exporting it. > + * > + * FIXME: This hook is deprecated. Users of this hook should be replaced > + * by setting &drm_gem_object.resv instead. > + */ > struct reservation_object * (*gem_prime_res_obj)( > struct drm_gem_object *obj); > - struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_import_sg_table: > + * > + * Optional hook used by the PRIME helper functions > + * drm_gem_prime_import() respectively drm_gem_prime_import_dev(). > + */ > struct drm_gem_object *(*gem_prime_import_sg_table)( > struct drm_device *dev, > struct dma_buf_attachment *attach, > struct sg_table *sgt); > + /** > + * @gem_prime_vmap: > + * > + * Deprecated vmap hook for GEM drivers. Please use > + * &drm_gem_object_funcs.vmap instead. > + */ > void *(*gem_prime_vmap)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_vunmap: > + * > + * Deprecated vunmap hook for GEM drivers. Please use > + * &drm_gem_object_funcs.vunmap instead. > + */ > void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr); > + > + /** > + * @gem_prime_mmap: > + * > + * mmap hook for GEM drivers, used to implement dma-buf mmap in the > + * PRIME helpers. > + * > + * FIXME: There's way too much duplication going on here, and also moved > + * to &drm_gem_object_funcs. > + */ > int (*gem_prime_mmap)(struct drm_gem_object *obj, > struct vm_area_struct *vma); > > @@ -665,6 +738,9 @@ struct drm_driver { > > /** > * @gem_vm_ops: Driver private ops for this object > + * > + * For GEM driver this is deprecated in favour of > + * &drm_gem_object_funcs.vm_ops. > */ > const struct vm_operations_struct *gem_vm_ops; > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index a9121fe66ea2..9af88238ee5c 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -101,7 +101,7 @@ struct drm_gem_object_funcs { > /** > * @pin: > * > - * Pin backing buffer in memory. > + * Pin backing buffer in memory. Used by the drm_gem_map_attach helper. > * > * This callback is optional. > */ > @@ -110,7 +110,7 @@ struct drm_gem_object_funcs { > /** > * @unpin: > * > - * Unpin backing buffer. > + * Unpin backing buffer. Used by the drm_gem_map_detach() helper. > * > * This callback is optional. > */ > @@ -120,16 +120,21 @@ struct drm_gem_object_funcs { > * @get_sg_table: > * > * Returns a Scatter-Gather table representation of the buffer. > - * Used when exporting a buffer. > + * Used when exporting a buffer by the drm_gem_map_dma_buf() helper. > + * Releasing is done by calling dma_unmap_sg_attrs() and sg_free_table() > + * in drm_gem_unmap_buf(), therefore these helpers and this callback > + * here cannot be used for sg tables pointing at driver private memory > + * ranges. > * > - * This callback is mandatory if buffer export is supported. > + * See also drm_prime_pages_to_sg(). > */ > struct sg_table *(*get_sg_table)(struct drm_gem_object *obj); > > /** > * @vmap: > * > - * Returns a virtual address for the buffer. > + * Returns a virtual address for the buffer. Used by the > + * drm_gem_dmabuf_vmap() helper. > * > * This callback is optional. > */ > @@ -138,7 +143,8 @@ struct drm_gem_object_funcs { > /** > * @vunmap: > * > - * Releases the the address previously returned by @vmap. > + * Releases the the address previously returned by @vmap. Used by the > + * drm_gem_dmabuf_vunmap() helper. > * > * This callback is optional. > */ > -- > 2.20.1 >
Hi, > - Polish for all the functions and more cross references. That is nice, helps figurung how all this works together without wading through the source code (or at least less of it). For that kind of changes it is probably helpful for review to generate the diff with more context (say --unified=10), so it is easier to spot the function where this doc update belongs to. Acked-by: Gerd Hoffmann <kraxel@redhat.com> cheers, Gerd
On 2019/06/18, Daniel Vetter wrote: > Yes this is a bit a big patch, but since it's essentially a complete > rewrite of all the prime docs I didn't see how to better split it up. > > Changes: > - Consistently point to drm_gem_object_funcs as the preferred hooks, > where applicable. > > - Document all the hooks in &drm_driver that lacked kerneldoc. > > - Completely new overview section, which now also includes the cleaned > up lifetime/reference counting subchapter. I also mentioned the weak > references in there due to the lookup caches. > > - Completely rewritten helper intro section, highlight the > import/export related functionality. > > - Polish for all the functions and more cross references. > > I also sprinkled a bunch of todos all over. > > Most important: 0 code changes in here. The cleanup motivated by > reading and improving all this will follow later on. > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > I won't address right away in subsequent cleanup patches. > > v3: > - Split out the function moving. This patch is now exclusively > documentation changes. > - Typos and nits (Sam). > > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Eric Anholt <eric@anholt.net> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- Had a quick read and it looks reasonable. Acked-by: Emil Velikov <emil.velikov@collabora.com> HTH Emil
Den 18.06.2019 11.20, skrev Daniel Vetter: > Yes this is a bit a big patch, but since it's essentially a complete > rewrite of all the prime docs I didn't see how to better split it up. > > Changes: > - Consistently point to drm_gem_object_funcs as the preferred hooks, > where applicable. > > - Document all the hooks in &drm_driver that lacked kerneldoc. > > - Completely new overview section, which now also includes the cleaned > up lifetime/reference counting subchapter. I also mentioned the weak > references in there due to the lookup caches. > > - Completely rewritten helper intro section, highlight the > import/export related functionality. > > - Polish for all the functions and more cross references. > > I also sprinkled a bunch of todos all over. > > Most important: 0 code changes in here. The cleanup motivated by > reading and improving all this will follow later on. > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > I won't address right away in subsequent cleanup patches. > > v3: > - Split out the function moving. This patch is now exclusively > documentation changes. > - Typos and nits (Sam). > > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Eric Anholt <eric@anholt.net> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > Documentation/gpu/drm-mm.rst | 40 +------ > drivers/gpu/drm/drm_prime.c | 197 +++++++++++++++++++++-------------- > include/drm/drm_drv.h | 104 +++++++++++++++--- > include/drm/drm_gem.h | 18 ++-- > 4 files changed, 226 insertions(+), 133 deletions(-) > <snip> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 68b4de85370c..2234206288fa 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -38,47 +38,53 @@ > > #include "drm_internal.h" > > -/* > - * DMA-BUF/GEM Object references and lifetime overview: > - * > - * On the export the dma_buf holds a reference to the exporting GEM > - * object. It takes this reference in handle_to_fd_ioctl, when it > - * first calls .prime_export and stores the exporting GEM object in > - * the dma_buf priv. This reference needs to be released when the > - * final reference to the &dma_buf itself is dropped and its > - * &dma_buf_ops.release function is called. For GEM-based drivers, > - * the dma_buf should be exported using drm_gem_dmabuf_export() and > - * then released by drm_gem_dmabuf_release(). > - * > - * On the import the importing GEM object holds a reference to the > - * dma_buf (which in turn holds a ref to the exporting GEM object). > - * It takes that reference in the fd_to_handle ioctl. > - * It calls dma_buf_get, creates an attachment to it and stores the > - * attachment in the GEM object. When this attachment is destroyed > - * when the imported object is destroyed, we remove the attachment > - * and drop the reference to the dma_buf. > - * > - * When all the references to the &dma_buf are dropped, i.e. when > - * userspace has closed both handles to the imported GEM object (through the > - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported > - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal references > - * are also gone, then the dma_buf gets destroyed. This can also happen as a > - * part of the clean up procedure in the drm_release() function if userspace > - * fails to properly clean up. Note that both the kernel and userspace (by > - * keeeping the PRIME file descriptors open) can hold references onto a > - * &dma_buf. > - * > - * Thus the chain of references always flows in one direction > - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem > - * > - * Self-importing: if userspace is using PRIME as a replacement for flink > - * then it will get a fd->handle request for a GEM object that it created. > - * Drivers should detect this situation and return back the gem object > - * from the dma-buf private. Prime will do this automatically for drivers that > - * use the drm_gem_prime_{import,export} helpers. > - * > - * GEM struct &dma_buf_ops symbols are now exported. They can be resued by > - * drivers which implement GEM interface. > +/** > + * DOC: overview and lifetime rules > + * > + * Similar to GEM global names, PRIME file descriptors are also used to share > + * buffer objects across processes. They offer additional security: as file > + * descriptors must be explicitly sent over UNIX domain sockets to be shared > + * between applications, they can't be guessed like the globally unique GEM > + * names. > + * > + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the > + * &drm_driver.driver_features field, and implement the > + * &drm_driver.prime_handle_to_fd and &drm_driver.prime_fd_to_handle operations. > + * GEM based drivers must use drm_gem_prime_handle_to_fd() and > + * drm_gem_prime_fd_to_handle() to implement these. For GEM based drivers the > + * actual driver interfaces is provided through the &drm_gem_object_funcs.export > + * and &drm_driver.gem_prime_import hooks. > + * > + * &dma_buf_ops implementations for GEM drivers are all individually exported > + * for drivers which need to overwrite or reimplement some of them. > + * > + * Reference Counting for GEM Drivers > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + * > + * On the export the &dma_buf holds a reference to the exported buffer object, > + * usually a &drm_gem_object. It takes this reference in the PRIME_HANDLE_TO_FD > + * IOCTL, when it first calls &drm_gem_object_funcs.export > + * and stores the exporting GEM object in the &dma_buf.priv field. This > + * reference needs to be released when the final reference to the &dma_buf > + * itself is dropped and its &dma_buf_ops.release function is called. For > + * GEM-based drivers, the &dma_buf should be exported using > + * drm_gem_dmabuf_export() and then released by drm_gem_dmabuf_release(). > + * > + * Thus the chain of references always flows in one direction, avoiding loops: > + * importing GEM object -> dma-buf -> exported GEM bo. A further complication > + * are the lookup caches for import and export. These are required to guarantee > + * that any given object will always have only one uniqe userspace handle. This > + * is required to allow userspace to detect duplicated imports, since some GEM > + * drivers do fail command submissions if a given buffer object is listed more > + * than once. These import and export caches in &drm_prime_file_private only > + * retain a weak reference, which is cleaned up when the corresponding object is > + * released. > + * > + * Self-importing: If userspace is using PRIME as a replacement for flink then > + * it will get a fd->handle request for a GEM object that it created. Drivers > + * should detect this situation and return back the underlying object from the > + * dma-buf private. For GEM based drivers this is handled in > + * drm_gem_prime_import() already. > */ > > struct drm_prime_member { > @@ -220,7 +226,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) > } > > /** > - * drm_gem_dmabuf_export - dma_buf export implementation for GEM > + * drm_gem_dmabuf_export - &dma_buf export implementation for GEM > * @dev: parent device for the exported dmabuf > * @exp_info: the export information used by dma_buf_export() > * > @@ -248,11 +254,11 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, > EXPORT_SYMBOL(drm_gem_dmabuf_export); > > /** > - * drm_gem_dmabuf_release - dma_buf release implementation for GEM > + * drm_gem_dmabuf_release - &dma_buf release implementation for GEM > * @dma_buf: buffer to be released > * > * Generic release function for dma_bufs exported as PRIME buffers. GEM drivers > - * must use this in their dma_buf ops structure as the release callback. > + * must use this in their &dma_buf_ops structure as the release callback. > * drm_gem_dmabuf_release() should be used in conjunction with > * drm_gem_dmabuf_export(). > */ > @@ -278,7 +284,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release); > * This is the PRIME import function which must be used mandatorily by GEM > * drivers to ensure correct lifetime management of the underlying GEM object. > * The actual importing of GEM object from the dma-buf is done through the > - * gem_import_export driver callback. > + * &drm_driver.gem_import_export driver callback. s/gem_import_export/gem_prime_import/ > + * > + * Returns 0 on success or a negative error code on failure. > */ > int drm_gem_prime_fd_to_handle(struct drm_device *dev, > struct drm_file *file_priv, int prime_fd, > @@ -412,7 +420,7 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, > * This is the PRIME export function which must be used mandatorily by GEM > * drivers to ensure correct lifetime management of the underlying GEM object. > * The actual exporting from GEM object to a dma-buf is done through the > - * gem_prime_export driver callback. > + * &drm_driver.gem_prime_export driver callback. > */ > int drm_gem_prime_handle_to_fd(struct drm_device *dev, > struct drm_file *file_priv, uint32_t handle, > @@ -523,23 +531,39 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > /** > * DOC: PRIME Helpers > * > - * Drivers can implement @gem_prime_export and @gem_prime_import in terms of > - * simpler APIs by using the helper functions @drm_gem_prime_export and > - * @drm_gem_prime_import. These functions implement dma-buf support in terms of > - * six lower-level driver callbacks: > + * Drivers can implement &drm_gem_object_funcs.export and > + * &drm_driver.gem_prime_import in terms of simpler APIs by using the helper > + * functions drm_gem_prime_export() and drm_gem_prime_import(). These functions Maybe you want to mention that these functions are the default so drivers don't have to set them. Double space before: These functions > + * implement dma-buf support in terms of some lower-level helpers, which are > + * again exported for drivers to use individually: > + * > + * Exporting buffers > + * ~~~~~~~~~~~~~~~~~ > + * > + * Optional pinning of buffers is handled at dma-buf attach and detach time in > + * drm_gem_map_attach() and drm_gem_map_detach(). Backing storage itself is > + * handled by drm_gem_map_dma_buf() and drm_gem_unmap_dma_buf(), which relies on > + * &drm_gem_object_funcs.get_sg_table. > + * > + * For kernel-internal access there's drm_gem_dmabuf_vmap() and > + * drm_gem_dmabuf_vunmap(). Userspace mmap support is provided by > + * drm_gem_dmabuf_mmap(). > + * > + * Note that these export helpers can only be used if the underlying backing > + * storage is fully coherent and either permanently pinned, or it is safe to pin > + * it indefinitely. > * > - * Export callbacks: > + * FIXME: The underlying helper functions are named rather inconsistently. > * > - * * @gem_prime_pin (optional): prepare a GEM object for exporting > - * * @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages > - * * @gem_prime_vmap: vmap a buffer exported by your driver > - * * @gem_prime_vunmap: vunmap a buffer exported by your driver > - * * @gem_prime_mmap (optional): mmap a buffer exported by your driver > + * Exporting buffers > + * ~~~~~~~~~~~~~~~~~ > * > - * Import callback: > + * Importing dma-bufs using drm_gem_prime_import() relies on > + * &drm_driver.gem_prime_import_sg_table. > * > - * * @gem_prime_import_sg_table (import): produce a GEM object from another > - * driver's scatter/gather table > + * Note that similarly to the export helpers this permanently pins the > + * underlying backing storage. Which is ok for scanout, but is not the best > + * option for sharing lots of buffers for rendering. > */ > > /** > @@ -547,8 +571,9 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > * @dma_buf: buffer to attach device to > * @attach: buffer attachment data > * > - * Calls &drm_driver.gem_prime_pin for device specific handling. This can be > - * used as the &dma_buf_ops.attach callback. > + * Calls &drm_gem_object_funcs.pin for device specific handling. This can be > + * used as the &dma_buf_ops.attach callback. Must be used together with > + * drm_gem_map_detach(). > * > * Returns 0 on success, negative error code on failure. > */ > @@ -566,8 +591,9 @@ EXPORT_SYMBOL(drm_gem_map_attach); > * @dma_buf: buffer to detach from > * @attach: attachment to be detached > * > - * Cleans up &dma_buf_attachment. This can be used as the &dma_buf_ops.detach > - * callback. > + * Calls &drm_gem_object_funcs.pin for device specific handling. Cleans up > + * &dma_buf_attachment from drm_gem_map_attach(). This can be used as the > + * &dma_buf_ops.detach callback. > */ > void drm_gem_map_detach(struct dma_buf *dma_buf, > struct dma_buf_attachment *attach) > @@ -583,13 +609,13 @@ EXPORT_SYMBOL(drm_gem_map_detach); > * @attach: attachment whose scatterlist is to be returned > * @dir: direction of DMA transfer > * > - * Calls &drm_driver.gem_prime_get_sg_table and then maps the scatterlist. This > - * can be used as the &dma_buf_ops.map_dma_buf callback. > + * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This > + * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > + * with drm_gem_unmap_dma_buf(). > * > - * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR > + * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR > * on error. May return -EINTR if it is interrupted by a signal. > */ > - > struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, > enum dma_data_direction dir) > { > @@ -642,9 +668,9 @@ EXPORT_SYMBOL(drm_gem_unmap_dma_buf); > * @dma_buf: buffer to be mapped > * > * Sets up a kernel virtual mapping. This can be used as the &dma_buf_ops.vmap > - * callback. > + * callback. Calls into &drm_gem_object_funcs.vmap for device specific handling. > * > - * Returns the kernel virtual address. > + * Returns the kernel virtual address or NULL on failure. > */ > void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf) > { > @@ -665,7 +691,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap); > * @vaddr: the virtual address of the buffer > * > * Releases a kernel virtual mapping. This can be used as the > - * &dma_buf_ops.vunmap callback. > + * &dma_buf_ops.vunmap callback. Calls into &drm_gem_object_funcs.vunmap for device specific handling. > */ > void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) > { > @@ -727,7 +753,11 @@ EXPORT_SYMBOL(drm_gem_prime_mmap); > * @vma: virtual address range > * > * Provides memory mapping for the buffer. This can be used as the > - * &dma_buf_ops.mmap callback. > + * &dma_buf_ops.mmap callback. It just forwards to &drm_driver.gem_prime_mmap, > + * which should be set to drm_gem_prime_mmap(). > + * > + * FIXME: There's really no point to this wrapper, drivers which need anything > + * else but drm_gem_prime_mmap can roll their own &dma_buf_ops.mmap callback. > * > * Returns 0 on success or a negative error code on failure. > */ > @@ -763,6 +793,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { > * This helper creates an sg table object from a set of pages > * the driver is responsible for mapping the pages into the > * importers address space for use with dma_buf itself. > + * > + * This is useful for implementing &drm_gem_object_funcs.get_sg_table. > */ > struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages) > { > @@ -793,7 +825,7 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg); > * @obj: GEM object to export > * @flags: flags like DRM_CLOEXEC and DRM_RDWR > * > - * This is the implementation of the gem_prime_export functions for GEM drivers > + * This is the implementation of the &drm_gem_object_funcs.export functions for GEM drivers > * using the PRIME helpers. > */ > struct dma_buf *drm_gem_prime_export(struct drm_device *dev, > @@ -823,9 +855,13 @@ EXPORT_SYMBOL(drm_gem_prime_export); > * @dma_buf: dma-buf object to import > * @attach_dev: struct device to dma_buf attach > * > - * This is the core of drm_gem_prime_import. It's designed to be called by > - * drivers who want to use a different device structure than dev->dev for > - * attaching via dma_buf. > + * This is the core of drm_gem_prime_import(). It's designed to be called by > + * drivers who want to use a different device structure than &drm_device.dev for > + * attaching via dma_buf. This function calls > + * &drm_driver.gem_prime_import_sg_table internally. > + * > + * Drivers must arrange to call drm_prime_gem_destroy() from their > + * &drm_gem_object_funcs.free hook when using this function. > */ > struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, > struct dma_buf *dma_buf, > @@ -889,7 +925,11 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); > * @dma_buf: dma-buf object to import > * > * This is the implementation of the gem_prime_import functions for GEM drivers > - * using the PRIME helpers. > + * using the PRIME helpers. Drivers can use this as their > + * &drm_driver.gem_prime_import implementation. > + * > + * Drivers must arrange to call drm_prime_gem_destroy() from their > + * &drm_gem_object_funcs.free hook when using this function. > */ > struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, > struct dma_buf *dma_buf) > @@ -907,6 +947,9 @@ EXPORT_SYMBOL(drm_gem_prime_import); > * > * Exports an sg table into an array of pages and addresses. This is currently > * required by the TTM driver in order to do correct fault handling. > + * > + * Drivers can use this in their &drm_driver.gem_prime_import_sg_table > + * implementation. > */ > int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, > dma_addr_t *addrs, int max_entries) > @@ -947,7 +990,7 @@ EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); > * @sg: the sg-table which was pinned at import time > * > * This is the cleanup functions which GEM drivers need to call when they use > - * @drm_gem_prime_import to import dma-bufs. > + * drm_gem_prime_import() or drm_gem_prime_import_dev() to import dma-bufs. > */ > void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) > { > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 5c4fc0ddc863..bbb3a6ff4418 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -505,21 +505,25 @@ struct drm_driver { > * @gem_free_object: deconstructor for drm_gem_objects > * > * This is deprecated and should not be used by new drivers. Use > - * @gem_free_object_unlocked instead. > + * &drm_gem_object_funcs.free instead. > */ > void (*gem_free_object) (struct drm_gem_object *obj); > > /** > * @gem_free_object_unlocked: deconstructor for drm_gem_objects > * > - * This is for drivers which are not encumbered with &drm_device.struct_mutex > - * legacy locking schemes. Use this hook instead of @gem_free_object. > + * This is deprecated and should not be used by new drivers. Use > + * &drm_gem_object_funcs.free instead. > + * Compared to @gem_free_object this is not encumbered with > + * &drm_device.struct_mutex legacy locking schemes. > */ > void (*gem_free_object_unlocked) (struct drm_gem_object *obj); > > /** > * @gem_open_object: > * > + * This callback is deprecated in favour of &drm_gem_object_funcs.open. > + * > * Driver hook called upon gem handle creation > */ > int (*gem_open_object) (struct drm_gem_object *, struct drm_file *); > @@ -527,6 +531,8 @@ struct drm_driver { > /** > * @gem_close_object: > * > + * This callback is deprecated in favour of &drm_gem_object_funcs.close. > + * > * Driver hook called upon gem handle release > */ > void (*gem_close_object) (struct drm_gem_object *, struct drm_file *); > @@ -534,6 +540,9 @@ struct drm_driver { > /** > * @gem_print_info: > * > + * This callback is deprecated in favour of > + * &drm_gem_object_funcs.print_info. > + * > * If driver subclasses struct &drm_gem_object, it can implement this > * optional hook for printing additional driver specific info. > * > @@ -548,56 +557,120 @@ struct drm_driver { > /** > * @gem_create_object: constructor for gem objects > * > - * Hook for allocating the GEM object struct, for use by core > - * helpers. > + * Hook for allocating the GEM object struct, for use by the CMA and > + * SHMEM GEM helpers. > */ > struct drm_gem_object *(*gem_create_object)(struct drm_device *dev, > size_t size); > - > - /* prime: */ > /** > * @prime_handle_to_fd: > * > - * export handle -> fd (see drm_gem_prime_handle_to_fd() helper) > + * Main PRIME export function. Should be implemented with > + * drm_gem_prime_handle_to_fd() for GEM based drivers. > + * > + * For an in-depth discussion see :ref:`PRIME buffer sharing > + * documentation <prime_buffer_sharing>`. > */ > int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv, > uint32_t handle, uint32_t flags, int *prime_fd); > /** > * @prime_fd_to_handle: > * > - * import fd -> handle (see drm_gem_prime_fd_to_handle() helper) > + * Main PRIME import function. Should be implemented with > + * drm_gem_prime_fd_to_handle() for GEM based drivers. > + * > + * For an in-depth discussion see :ref:`PRIME buffer sharing > + * documentation <prime_buffer_sharing>`. > */ > int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv, > int prime_fd, uint32_t *handle); > /** > * @gem_prime_export: > * > - * export GEM -> dmabuf > - * > - * This defaults to drm_gem_prime_export() if not set. > + * Export hook for GEM drivers. Deprecated in favour of > + * &drm_gem_object_funcs.export. > */ > struct dma_buf * (*gem_prime_export)(struct drm_device *dev, > struct drm_gem_object *obj, int flags); > /** > * @gem_prime_import: > * > - * import dmabuf -> GEM > + * Import hook for GEM drivers. > * > * This defaults to drm_gem_prime_import() if not set. > */ > struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, > struct dma_buf *dma_buf); > + > + /** > + * @gem_prime_pin: > + * > + * Deprecated hook in favour of &drm_gem_object_funcs.pin. > + */ > int (*gem_prime_pin)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_unpin: > + * > + * Deprecated hook in favour of &drm_gem_object_funcs.unpin. > + */ > void (*gem_prime_unpin)(struct drm_gem_object *obj); > + > + > + /** > + * @gem_prime_get_sg_table: > + * > + * Deprecated hook in favour of &drm_gem_object_funcs.get_sg_table. > + */ > + struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_res_obj: > + * > + * Optional hook to look up the &reservation_object for an buffer when > + * exporting it. > + * > + * FIXME: This hook is deprecated. Users of this hook should be replaced > + * by setting &drm_gem_object.resv instead. > + */ > struct reservation_object * (*gem_prime_res_obj)( > struct drm_gem_object *obj); > - struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_import_sg_table: > + * > + * Optional hook used by the PRIME helper functions > + * drm_gem_prime_import() respectively drm_gem_prime_import_dev(). > + */ > struct drm_gem_object *(*gem_prime_import_sg_table)( > struct drm_device *dev, > struct dma_buf_attachment *attach, > struct sg_table *sgt); > + /** > + * @gem_prime_vmap: > + * > + * Deprecated vmap hook for GEM drivers. Please use > + * &drm_gem_object_funcs.vmap instead. > + */ > void *(*gem_prime_vmap)(struct drm_gem_object *obj); > + > + /** > + * @gem_prime_vunmap: > + * > + * Deprecated vunmap hook for GEM drivers. Please use > + * &drm_gem_object_funcs.vunmap instead. > + */ > void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr); > + > + /** > + * @gem_prime_mmap: > + * > + * mmap hook for GEM drivers, used to implement dma-buf mmap in the > + * PRIME helpers. > + * > + * FIXME: There's way too much duplication going on here, and also moved > + * to &drm_gem_object_funcs. > + */ > int (*gem_prime_mmap)(struct drm_gem_object *obj, > struct vm_area_struct *vma); > > @@ -665,6 +738,9 @@ struct drm_driver { > > /** > * @gem_vm_ops: Driver private ops for this object > + * > + * For GEM driver this is deprecated in favour of s/driver/drivers/ > + * &drm_gem_object_funcs.vm_ops. > */ > const struct vm_operations_struct *gem_vm_ops; > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index a9121fe66ea2..9af88238ee5c 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -101,7 +101,7 @@ struct drm_gem_object_funcs { > /** > * @pin: > * > - * Pin backing buffer in memory. > + * Pin backing buffer in memory. Used by the drm_gem_map_attach helper. s/drm_gem_map_attach/drm_gem_map_attach()/ Looks sane: Acked-by: Noralf Trønnes <noralf@tronnes.org> > * > * This callback is optional. > */ > @@ -110,7 +110,7 @@ struct drm_gem_object_funcs { > /** > * @unpin: > * > - * Unpin backing buffer. > + * Unpin backing buffer. Used by the drm_gem_map_detach() helper. > * > * This callback is optional. > */ > @@ -120,16 +120,21 @@ struct drm_gem_object_funcs { > * @get_sg_table: > * > * Returns a Scatter-Gather table representation of the buffer. > - * Used when exporting a buffer. > + * Used when exporting a buffer by the drm_gem_map_dma_buf() helper. > + * Releasing is done by calling dma_unmap_sg_attrs() and sg_free_table() > + * in drm_gem_unmap_buf(), therefore these helpers and this callback > + * here cannot be used for sg tables pointing at driver private memory > + * ranges. > * > - * This callback is mandatory if buffer export is supported. > + * See also drm_prime_pages_to_sg(). > */ > struct sg_table *(*get_sg_table)(struct drm_gem_object *obj); > > /** > * @vmap: > * > - * Returns a virtual address for the buffer. > + * Returns a virtual address for the buffer. Used by the > + * drm_gem_dmabuf_vmap() helper. > * > * This callback is optional. > */ > @@ -138,7 +143,8 @@ struct drm_gem_object_funcs { > /** > * @vunmap: > * > - * Releases the the address previously returned by @vmap. > + * Releases the the address previously returned by @vmap. Used by the > + * drm_gem_dmabuf_vunmap() helper. > * > * This callback is optional. > */ >
On Wed, Jun 19, 2019 at 02:43:20PM +0200, Noralf Trønnes wrote: > > > Den 18.06.2019 11.20, skrev Daniel Vetter: > > Yes this is a bit a big patch, but since it's essentially a complete > > rewrite of all the prime docs I didn't see how to better split it up. > > > > Changes: > > - Consistently point to drm_gem_object_funcs as the preferred hooks, > > where applicable. > > > > - Document all the hooks in &drm_driver that lacked kerneldoc. > > > > - Completely new overview section, which now also includes the cleaned > > up lifetime/reference counting subchapter. I also mentioned the weak > > references in there due to the lookup caches. > > > > - Completely rewritten helper intro section, highlight the > > import/export related functionality. > > > > - Polish for all the functions and more cross references. > > > > I also sprinkled a bunch of todos all over. > > > > Most important: 0 code changes in here. The cleanup motivated by > > reading and improving all this will follow later on. > > > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > > I won't address right away in subsequent cleanup patches. > > > > v3: > > - Split out the function moving. This patch is now exclusively > > documentation changes. > > - Typos and nits (Sam). > > > > Cc: Sam Ravnborg <sam@ravnborg.org> > > Cc: Eric Anholt <eric@anholt.net> > > Cc: Emil Velikov <emil.l.velikov@gmail.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > Documentation/gpu/drm-mm.rst | 40 +------ > > drivers/gpu/drm/drm_prime.c | 197 +++++++++++++++++++++-------------- > > include/drm/drm_drv.h | 104 +++++++++++++++--- > > include/drm/drm_gem.h | 18 ++-- > > 4 files changed, 226 insertions(+), 133 deletions(-) > > > > <snip> > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index 68b4de85370c..2234206288fa 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -38,47 +38,53 @@ > > > > #include "drm_internal.h" > > > > -/* > > - * DMA-BUF/GEM Object references and lifetime overview: > > - * > > - * On the export the dma_buf holds a reference to the exporting GEM > > - * object. It takes this reference in handle_to_fd_ioctl, when it > > - * first calls .prime_export and stores the exporting GEM object in > > - * the dma_buf priv. This reference needs to be released when the > > - * final reference to the &dma_buf itself is dropped and its > > - * &dma_buf_ops.release function is called. For GEM-based drivers, > > - * the dma_buf should be exported using drm_gem_dmabuf_export() and > > - * then released by drm_gem_dmabuf_release(). > > - * > > - * On the import the importing GEM object holds a reference to the > > - * dma_buf (which in turn holds a ref to the exporting GEM object). > > - * It takes that reference in the fd_to_handle ioctl. > > - * It calls dma_buf_get, creates an attachment to it and stores the > > - * attachment in the GEM object. When this attachment is destroyed > > - * when the imported object is destroyed, we remove the attachment > > - * and drop the reference to the dma_buf. > > - * > > - * When all the references to the &dma_buf are dropped, i.e. when > > - * userspace has closed both handles to the imported GEM object (through the > > - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported > > - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal references > > - * are also gone, then the dma_buf gets destroyed. This can also happen as a > > - * part of the clean up procedure in the drm_release() function if userspace > > - * fails to properly clean up. Note that both the kernel and userspace (by > > - * keeeping the PRIME file descriptors open) can hold references onto a > > - * &dma_buf. > > - * > > - * Thus the chain of references always flows in one direction > > - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem > > - * > > - * Self-importing: if userspace is using PRIME as a replacement for flink > > - * then it will get a fd->handle request for a GEM object that it created. > > - * Drivers should detect this situation and return back the gem object > > - * from the dma-buf private. Prime will do this automatically for drivers that > > - * use the drm_gem_prime_{import,export} helpers. > > - * > > - * GEM struct &dma_buf_ops symbols are now exported. They can be resued by > > - * drivers which implement GEM interface. > > +/** > > + * DOC: overview and lifetime rules > > + * > > + * Similar to GEM global names, PRIME file descriptors are also used to share > > + * buffer objects across processes. They offer additional security: as file > > + * descriptors must be explicitly sent over UNIX domain sockets to be shared > > + * between applications, they can't be guessed like the globally unique GEM > > + * names. > > + * > > + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the > > + * &drm_driver.driver_features field, and implement the > > + * &drm_driver.prime_handle_to_fd and &drm_driver.prime_fd_to_handle operations. > > + * GEM based drivers must use drm_gem_prime_handle_to_fd() and > > + * drm_gem_prime_fd_to_handle() to implement these. For GEM based drivers the > > + * actual driver interfaces is provided through the &drm_gem_object_funcs.export > > + * and &drm_driver.gem_prime_import hooks. > > + * > > + * &dma_buf_ops implementations for GEM drivers are all individually exported > > + * for drivers which need to overwrite or reimplement some of them. > > + * > > + * Reference Counting for GEM Drivers > > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > + * > > + * On the export the &dma_buf holds a reference to the exported buffer object, > > + * usually a &drm_gem_object. It takes this reference in the PRIME_HANDLE_TO_FD > > + * IOCTL, when it first calls &drm_gem_object_funcs.export > > + * and stores the exporting GEM object in the &dma_buf.priv field. This > > + * reference needs to be released when the final reference to the &dma_buf > > + * itself is dropped and its &dma_buf_ops.release function is called. For > > + * GEM-based drivers, the &dma_buf should be exported using > > + * drm_gem_dmabuf_export() and then released by drm_gem_dmabuf_release(). > > + * > > + * Thus the chain of references always flows in one direction, avoiding loops: > > + * importing GEM object -> dma-buf -> exported GEM bo. A further complication > > + * are the lookup caches for import and export. These are required to guarantee > > + * that any given object will always have only one uniqe userspace handle. This > > + * is required to allow userspace to detect duplicated imports, since some GEM > > + * drivers do fail command submissions if a given buffer object is listed more > > + * than once. These import and export caches in &drm_prime_file_private only > > + * retain a weak reference, which is cleaned up when the corresponding object is > > + * released. > > + * > > + * Self-importing: If userspace is using PRIME as a replacement for flink then > > + * it will get a fd->handle request for a GEM object that it created. Drivers > > + * should detect this situation and return back the underlying object from the > > + * dma-buf private. For GEM based drivers this is handled in > > + * drm_gem_prime_import() already. > > */ > > > > struct drm_prime_member { > > @@ -220,7 +226,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) > > } > > > > /** > > - * drm_gem_dmabuf_export - dma_buf export implementation for GEM > > + * drm_gem_dmabuf_export - &dma_buf export implementation for GEM > > * @dev: parent device for the exported dmabuf > > * @exp_info: the export information used by dma_buf_export() > > * > > @@ -248,11 +254,11 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, > > EXPORT_SYMBOL(drm_gem_dmabuf_export); > > > > /** > > - * drm_gem_dmabuf_release - dma_buf release implementation for GEM > > + * drm_gem_dmabuf_release - &dma_buf release implementation for GEM > > * @dma_buf: buffer to be released > > * > > * Generic release function for dma_bufs exported as PRIME buffers. GEM drivers > > - * must use this in their dma_buf ops structure as the release callback. > > + * must use this in their &dma_buf_ops structure as the release callback. > > * drm_gem_dmabuf_release() should be used in conjunction with > > * drm_gem_dmabuf_export(). > > */ > > @@ -278,7 +284,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release); > > * This is the PRIME import function which must be used mandatorily by GEM > > * drivers to ensure correct lifetime management of the underlying GEM object. > > * The actual importing of GEM object from the dma-buf is done through the > > - * gem_import_export driver callback. > > + * &drm_driver.gem_import_export driver callback. > > s/gem_import_export/gem_prime_import/ > > > + * > > + * Returns 0 on success or a negative error code on failure. > > */ > > int drm_gem_prime_fd_to_handle(struct drm_device *dev, > > struct drm_file *file_priv, int prime_fd, > > @@ -412,7 +420,7 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, > > * This is the PRIME export function which must be used mandatorily by GEM > > * drivers to ensure correct lifetime management of the underlying GEM object. > > * The actual exporting from GEM object to a dma-buf is done through the > > - * gem_prime_export driver callback. > > + * &drm_driver.gem_prime_export driver callback. > > */ > > int drm_gem_prime_handle_to_fd(struct drm_device *dev, > > struct drm_file *file_priv, uint32_t handle, > > @@ -523,23 +531,39 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > > /** > > * DOC: PRIME Helpers > > * > > - * Drivers can implement @gem_prime_export and @gem_prime_import in terms of > > - * simpler APIs by using the helper functions @drm_gem_prime_export and > > - * @drm_gem_prime_import. These functions implement dma-buf support in terms of > > - * six lower-level driver callbacks: > > + * Drivers can implement &drm_gem_object_funcs.export and > > + * &drm_driver.gem_prime_import in terms of simpler APIs by using the helper > > + * functions drm_gem_prime_export() and drm_gem_prime_import(). These functions > > Maybe you want to mention that these functions are the default so > drivers don't have to set them. It's already mentioned in the kerneldoc for the callback structures in headers, but worth repeating. To not upset the flow too much I put that into the individual function comments - here I couldn't come up with a good way to integrate that bit of information. I'll take all your other feedback and resend, thanks a lot for taking a look. -Daniel > > Double space before: These functions > > > + * implement dma-buf support in terms of some lower-level helpers, which are > > + * again exported for drivers to use individually: > > + * > > + * Exporting buffers > > + * ~~~~~~~~~~~~~~~~~ > > + * > > + * Optional pinning of buffers is handled at dma-buf attach and detach time in > > + * drm_gem_map_attach() and drm_gem_map_detach(). Backing storage itself is > > + * handled by drm_gem_map_dma_buf() and drm_gem_unmap_dma_buf(), which relies on > > + * &drm_gem_object_funcs.get_sg_table. > > + * > > + * For kernel-internal access there's drm_gem_dmabuf_vmap() and > > + * drm_gem_dmabuf_vunmap(). Userspace mmap support is provided by > > + * drm_gem_dmabuf_mmap(). > > + * > > + * Note that these export helpers can only be used if the underlying backing > > + * storage is fully coherent and either permanently pinned, or it is safe to pin > > + * it indefinitely. > > * > > - * Export callbacks: > > + * FIXME: The underlying helper functions are named rather inconsistently. > > * > > - * * @gem_prime_pin (optional): prepare a GEM object for exporting > > - * * @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages > > - * * @gem_prime_vmap: vmap a buffer exported by your driver > > - * * @gem_prime_vunmap: vunmap a buffer exported by your driver > > - * * @gem_prime_mmap (optional): mmap a buffer exported by your driver > > + * Exporting buffers > > + * ~~~~~~~~~~~~~~~~~ > > * > > - * Import callback: > > + * Importing dma-bufs using drm_gem_prime_import() relies on > > + * &drm_driver.gem_prime_import_sg_table. > > * > > - * * @gem_prime_import_sg_table (import): produce a GEM object from another > > - * driver's scatter/gather table > > + * Note that similarly to the export helpers this permanently pins the > > + * underlying backing storage. Which is ok for scanout, but is not the best > > + * option for sharing lots of buffers for rendering. > > */ > > > > /** > > @@ -547,8 +571,9 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > > * @dma_buf: buffer to attach device to > > * @attach: buffer attachment data > > * > > - * Calls &drm_driver.gem_prime_pin for device specific handling. This can be > > - * used as the &dma_buf_ops.attach callback. > > + * Calls &drm_gem_object_funcs.pin for device specific handling. This can be > > + * used as the &dma_buf_ops.attach callback. Must be used together with > > + * drm_gem_map_detach(). > > * > > * Returns 0 on success, negative error code on failure. > > */ > > @@ -566,8 +591,9 @@ EXPORT_SYMBOL(drm_gem_map_attach); > > * @dma_buf: buffer to detach from > > * @attach: attachment to be detached > > * > > - * Cleans up &dma_buf_attachment. This can be used as the &dma_buf_ops.detach > > - * callback. > > + * Calls &drm_gem_object_funcs.pin for device specific handling. Cleans up > > + * &dma_buf_attachment from drm_gem_map_attach(). This can be used as the > > + * &dma_buf_ops.detach callback. > > */ > > void drm_gem_map_detach(struct dma_buf *dma_buf, > > struct dma_buf_attachment *attach) > > @@ -583,13 +609,13 @@ EXPORT_SYMBOL(drm_gem_map_detach); > > * @attach: attachment whose scatterlist is to be returned > > * @dir: direction of DMA transfer > > * > > - * Calls &drm_driver.gem_prime_get_sg_table and then maps the scatterlist. This > > - * can be used as the &dma_buf_ops.map_dma_buf callback. > > + * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This > > + * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together > > + * with drm_gem_unmap_dma_buf(). > > * > > - * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR > > + * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR > > * on error. May return -EINTR if it is interrupted by a signal. > > */ > > - > > struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, > > enum dma_data_direction dir) > > { > > @@ -642,9 +668,9 @@ EXPORT_SYMBOL(drm_gem_unmap_dma_buf); > > * @dma_buf: buffer to be mapped > > * > > * Sets up a kernel virtual mapping. This can be used as the &dma_buf_ops.vmap > > - * callback. > > + * callback. Calls into &drm_gem_object_funcs.vmap for device specific handling. > > * > > - * Returns the kernel virtual address. > > + * Returns the kernel virtual address or NULL on failure. > > */ > > void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf) > > { > > @@ -665,7 +691,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap); > > * @vaddr: the virtual address of the buffer > > * > > * Releases a kernel virtual mapping. This can be used as the > > - * &dma_buf_ops.vunmap callback. > > + * &dma_buf_ops.vunmap callback. Calls into &drm_gem_object_funcs.vunmap for device specific handling. > > */ > > void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) > > { > > @@ -727,7 +753,11 @@ EXPORT_SYMBOL(drm_gem_prime_mmap); > > * @vma: virtual address range > > * > > * Provides memory mapping for the buffer. This can be used as the > > - * &dma_buf_ops.mmap callback. > > + * &dma_buf_ops.mmap callback. It just forwards to &drm_driver.gem_prime_mmap, > > + * which should be set to drm_gem_prime_mmap(). > > + * > > + * FIXME: There's really no point to this wrapper, drivers which need anything > > + * else but drm_gem_prime_mmap can roll their own &dma_buf_ops.mmap callback. > > * > > * Returns 0 on success or a negative error code on failure. > > */ > > @@ -763,6 +793,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { > > * This helper creates an sg table object from a set of pages > > * the driver is responsible for mapping the pages into the > > * importers address space for use with dma_buf itself. > > + * > > + * This is useful for implementing &drm_gem_object_funcs.get_sg_table. > > */ > > struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages) > > { > > @@ -793,7 +825,7 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg); > > * @obj: GEM object to export > > * @flags: flags like DRM_CLOEXEC and DRM_RDWR > > * > > - * This is the implementation of the gem_prime_export functions for GEM drivers > > + * This is the implementation of the &drm_gem_object_funcs.export functions for GEM drivers > > * using the PRIME helpers. > > */ > > struct dma_buf *drm_gem_prime_export(struct drm_device *dev, > > @@ -823,9 +855,13 @@ EXPORT_SYMBOL(drm_gem_prime_export); > > * @dma_buf: dma-buf object to import > > * @attach_dev: struct device to dma_buf attach > > * > > - * This is the core of drm_gem_prime_import. It's designed to be called by > > - * drivers who want to use a different device structure than dev->dev for > > - * attaching via dma_buf. > > + * This is the core of drm_gem_prime_import(). It's designed to be called by > > + * drivers who want to use a different device structure than &drm_device.dev for > > + * attaching via dma_buf. This function calls > > + * &drm_driver.gem_prime_import_sg_table internally. > > + * > > + * Drivers must arrange to call drm_prime_gem_destroy() from their > > + * &drm_gem_object_funcs.free hook when using this function. > > */ > > struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, > > struct dma_buf *dma_buf, > > @@ -889,7 +925,11 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); > > * @dma_buf: dma-buf object to import > > * > > * This is the implementation of the gem_prime_import functions for GEM drivers > > - * using the PRIME helpers. > > + * using the PRIME helpers. Drivers can use this as their > > + * &drm_driver.gem_prime_import implementation. > > + * > > + * Drivers must arrange to call drm_prime_gem_destroy() from their > > + * &drm_gem_object_funcs.free hook when using this function. > > */ > > struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, > > struct dma_buf *dma_buf) > > @@ -907,6 +947,9 @@ EXPORT_SYMBOL(drm_gem_prime_import); > > * > > * Exports an sg table into an array of pages and addresses. This is currently > > * required by the TTM driver in order to do correct fault handling. > > + * > > + * Drivers can use this in their &drm_driver.gem_prime_import_sg_table > > + * implementation. > > */ > > int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, > > dma_addr_t *addrs, int max_entries) > > @@ -947,7 +990,7 @@ EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); > > * @sg: the sg-table which was pinned at import time > > * > > * This is the cleanup functions which GEM drivers need to call when they use > > - * @drm_gem_prime_import to import dma-bufs. > > + * drm_gem_prime_import() or drm_gem_prime_import_dev() to import dma-bufs. > > */ > > void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) > > { > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > > index 5c4fc0ddc863..bbb3a6ff4418 100644 > > --- a/include/drm/drm_drv.h > > +++ b/include/drm/drm_drv.h > > @@ -505,21 +505,25 @@ struct drm_driver { > > * @gem_free_object: deconstructor for drm_gem_objects > > * > > * This is deprecated and should not be used by new drivers. Use > > - * @gem_free_object_unlocked instead. > > + * &drm_gem_object_funcs.free instead. > > */ > > void (*gem_free_object) (struct drm_gem_object *obj); > > > > /** > > * @gem_free_object_unlocked: deconstructor for drm_gem_objects > > * > > - * This is for drivers which are not encumbered with &drm_device.struct_mutex > > - * legacy locking schemes. Use this hook instead of @gem_free_object. > > + * This is deprecated and should not be used by new drivers. Use > > + * &drm_gem_object_funcs.free instead. > > + * Compared to @gem_free_object this is not encumbered with > > + * &drm_device.struct_mutex legacy locking schemes. > > */ > > void (*gem_free_object_unlocked) (struct drm_gem_object *obj); > > > > /** > > * @gem_open_object: > > * > > + * This callback is deprecated in favour of &drm_gem_object_funcs.open. > > + * > > * Driver hook called upon gem handle creation > > */ > > int (*gem_open_object) (struct drm_gem_object *, struct drm_file *); > > @@ -527,6 +531,8 @@ struct drm_driver { > > /** > > * @gem_close_object: > > * > > + * This callback is deprecated in favour of &drm_gem_object_funcs.close. > > + * > > * Driver hook called upon gem handle release > > */ > > void (*gem_close_object) (struct drm_gem_object *, struct drm_file *); > > @@ -534,6 +540,9 @@ struct drm_driver { > > /** > > * @gem_print_info: > > * > > + * This callback is deprecated in favour of > > + * &drm_gem_object_funcs.print_info. > > + * > > * If driver subclasses struct &drm_gem_object, it can implement this > > * optional hook for printing additional driver specific info. > > * > > @@ -548,56 +557,120 @@ struct drm_driver { > > /** > > * @gem_create_object: constructor for gem objects > > * > > - * Hook for allocating the GEM object struct, for use by core > > - * helpers. > > + * Hook for allocating the GEM object struct, for use by the CMA and > > + * SHMEM GEM helpers. > > */ > > struct drm_gem_object *(*gem_create_object)(struct drm_device *dev, > > size_t size); > > - > > - /* prime: */ > > /** > > * @prime_handle_to_fd: > > * > > - * export handle -> fd (see drm_gem_prime_handle_to_fd() helper) > > + * Main PRIME export function. Should be implemented with > > + * drm_gem_prime_handle_to_fd() for GEM based drivers. > > + * > > + * For an in-depth discussion see :ref:`PRIME buffer sharing > > + * documentation <prime_buffer_sharing>`. > > */ > > int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv, > > uint32_t handle, uint32_t flags, int *prime_fd); > > /** > > * @prime_fd_to_handle: > > * > > - * import fd -> handle (see drm_gem_prime_fd_to_handle() helper) > > + * Main PRIME import function. Should be implemented with > > + * drm_gem_prime_fd_to_handle() for GEM based drivers. > > + * > > + * For an in-depth discussion see :ref:`PRIME buffer sharing > > + * documentation <prime_buffer_sharing>`. > > */ > > int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv, > > int prime_fd, uint32_t *handle); > > /** > > * @gem_prime_export: > > * > > - * export GEM -> dmabuf > > - * > > - * This defaults to drm_gem_prime_export() if not set. > > + * Export hook for GEM drivers. Deprecated in favour of > > + * &drm_gem_object_funcs.export. > > */ > > struct dma_buf * (*gem_prime_export)(struct drm_device *dev, > > struct drm_gem_object *obj, int flags); > > /** > > * @gem_prime_import: > > * > > - * import dmabuf -> GEM > > + * Import hook for GEM drivers. > > * > > * This defaults to drm_gem_prime_import() if not set. > > */ > > struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, > > struct dma_buf *dma_buf); > > + > > + /** > > + * @gem_prime_pin: > > + * > > + * Deprecated hook in favour of &drm_gem_object_funcs.pin. > > + */ > > int (*gem_prime_pin)(struct drm_gem_object *obj); > > + > > + /** > > + * @gem_prime_unpin: > > + * > > + * Deprecated hook in favour of &drm_gem_object_funcs.unpin. > > + */ > > void (*gem_prime_unpin)(struct drm_gem_object *obj); > > + > > + > > + /** > > + * @gem_prime_get_sg_table: > > + * > > + * Deprecated hook in favour of &drm_gem_object_funcs.get_sg_table. > > + */ > > + struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); > > + > > + /** > > + * @gem_prime_res_obj: > > + * > > + * Optional hook to look up the &reservation_object for an buffer when > > + * exporting it. > > + * > > + * FIXME: This hook is deprecated. Users of this hook should be replaced > > + * by setting &drm_gem_object.resv instead. > > + */ > > struct reservation_object * (*gem_prime_res_obj)( > > struct drm_gem_object *obj); > > - struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); > > + > > + /** > > + * @gem_prime_import_sg_table: > > + * > > + * Optional hook used by the PRIME helper functions > > + * drm_gem_prime_import() respectively drm_gem_prime_import_dev(). > > + */ > > struct drm_gem_object *(*gem_prime_import_sg_table)( > > struct drm_device *dev, > > struct dma_buf_attachment *attach, > > struct sg_table *sgt); > > + /** > > + * @gem_prime_vmap: > > + * > > + * Deprecated vmap hook for GEM drivers. Please use > > + * &drm_gem_object_funcs.vmap instead. > > + */ > > void *(*gem_prime_vmap)(struct drm_gem_object *obj); > > + > > + /** > > + * @gem_prime_vunmap: > > + * > > + * Deprecated vunmap hook for GEM drivers. Please use > > + * &drm_gem_object_funcs.vunmap instead. > > + */ > > void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr); > > + > > + /** > > + * @gem_prime_mmap: > > + * > > + * mmap hook for GEM drivers, used to implement dma-buf mmap in the > > + * PRIME helpers. > > + * > > + * FIXME: There's way too much duplication going on here, and also moved > > + * to &drm_gem_object_funcs. > > + */ > > int (*gem_prime_mmap)(struct drm_gem_object *obj, > > struct vm_area_struct *vma); > > > > @@ -665,6 +738,9 @@ struct drm_driver { > > > > /** > > * @gem_vm_ops: Driver private ops for this object > > + * > > + * For GEM driver this is deprecated in favour of > > s/driver/drivers/ > > > + * &drm_gem_object_funcs.vm_ops. > > */ > > const struct vm_operations_struct *gem_vm_ops; > > > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > > index a9121fe66ea2..9af88238ee5c 100644 > > --- a/include/drm/drm_gem.h > > +++ b/include/drm/drm_gem.h > > @@ -101,7 +101,7 @@ struct drm_gem_object_funcs { > > /** > > * @pin: > > * > > - * Pin backing buffer in memory. > > + * Pin backing buffer in memory. Used by the drm_gem_map_attach helper. > > s/drm_gem_map_attach/drm_gem_map_attach()/ > > Looks sane: > > Acked-by: Noralf Trønnes <noralf@tronnes.org> > > > * > > * This callback is optional. > > */ > > @@ -110,7 +110,7 @@ struct drm_gem_object_funcs { > > /** > > * @unpin: > > * > > - * Unpin backing buffer. > > + * Unpin backing buffer. Used by the drm_gem_map_detach() helper. > > * > > * This callback is optional. > > */ > > @@ -120,16 +120,21 @@ struct drm_gem_object_funcs { > > * @get_sg_table: > > * > > * Returns a Scatter-Gather table representation of the buffer. > > - * Used when exporting a buffer. > > + * Used when exporting a buffer by the drm_gem_map_dma_buf() helper. > > + * Releasing is done by calling dma_unmap_sg_attrs() and sg_free_table() > > + * in drm_gem_unmap_buf(), therefore these helpers and this callback > > + * here cannot be used for sg tables pointing at driver private memory > > + * ranges. > > * > > - * This callback is mandatory if buffer export is supported. > > + * See also drm_prime_pages_to_sg(). > > */ > > struct sg_table *(*get_sg_table)(struct drm_gem_object *obj); > > > > /** > > * @vmap: > > * > > - * Returns a virtual address for the buffer. > > + * Returns a virtual address for the buffer. Used by the > > + * drm_gem_dmabuf_vmap() helper. > > * > > * This callback is optional. > > */ > > @@ -138,7 +143,8 @@ struct drm_gem_object_funcs { > > /** > > * @vunmap: > > * > > - * Releases the the address previously returned by @vmap. > > + * Releases the the address previously returned by @vmap. Used by the > > + * drm_gem_dmabuf_vunmap() helper. > > * > > * This callback is optional. > > */ > >
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index c8ebd4f66a6a..b664f054c259 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -433,43 +433,11 @@ PRIME is the cross device buffer sharing framework in drm, originally created for the OPTIMUS range of multi-gpu platforms. To userspace PRIME buffers are dma-buf based file descriptors. -Overview and Driver Interface ------------------------------ +Overview and Lifetime Rules +--------------------------- -Similar to GEM global names, PRIME file descriptors are also used to -share buffer objects across processes. They offer additional security: -as file descriptors must be explicitly sent over UNIX domain sockets to -be shared between applications, they can't be guessed like the globally -unique GEM names. - -Drivers that support the PRIME API must set the DRIVER_PRIME bit in the -struct :c:type:`struct drm_driver <drm_driver>` -driver_features field, and implement the prime_handle_to_fd and -prime_fd_to_handle operations. - -int (\*prime_handle_to_fd)(struct drm_device \*dev, struct drm_file -\*file_priv, uint32_t handle, uint32_t flags, int \*prime_fd); int -(\*prime_fd_to_handle)(struct drm_device \*dev, struct drm_file -\*file_priv, int prime_fd, uint32_t \*handle); Those two operations -convert a handle to a PRIME file descriptor and vice versa. Drivers must -use the kernel dma-buf buffer sharing framework to manage the PRIME file -descriptors. Similar to the mode setting API PRIME is agnostic to the -underlying buffer object manager, as long as handles are 32bit unsigned -integers. - -While non-GEM drivers must implement the operations themselves, GEM -drivers must use the :c:func:`drm_gem_prime_handle_to_fd()` and -:c:func:`drm_gem_prime_fd_to_handle()` helper functions. Those -helpers rely on the driver gem_prime_export and gem_prime_import -operations to create a dma-buf instance from a GEM object (dma-buf -exporter role) and to create a GEM object from a dma-buf instance -(dma-buf importer role). - -struct dma_buf \* (\*gem_prime_export)(struct drm_device \*dev, -struct drm_gem_object \*obj, int flags); struct drm_gem_object \* -(\*gem_prime_import)(struct drm_device \*dev, struct dma_buf -\*dma_buf); These two operations are mandatory for GEM drivers that -support PRIME. +.. kernel-doc:: drivers/gpu/drm/drm_prime.c + :doc: overview and lifetime rules PRIME Helper Functions ---------------------- diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 68b4de85370c..2234206288fa 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -38,47 +38,53 @@ #include "drm_internal.h" -/* - * DMA-BUF/GEM Object references and lifetime overview: - * - * On the export the dma_buf holds a reference to the exporting GEM - * object. It takes this reference in handle_to_fd_ioctl, when it - * first calls .prime_export and stores the exporting GEM object in - * the dma_buf priv. This reference needs to be released when the - * final reference to the &dma_buf itself is dropped and its - * &dma_buf_ops.release function is called. For GEM-based drivers, - * the dma_buf should be exported using drm_gem_dmabuf_export() and - * then released by drm_gem_dmabuf_release(). - * - * On the import the importing GEM object holds a reference to the - * dma_buf (which in turn holds a ref to the exporting GEM object). - * It takes that reference in the fd_to_handle ioctl. - * It calls dma_buf_get, creates an attachment to it and stores the - * attachment in the GEM object. When this attachment is destroyed - * when the imported object is destroyed, we remove the attachment - * and drop the reference to the dma_buf. - * - * When all the references to the &dma_buf are dropped, i.e. when - * userspace has closed both handles to the imported GEM object (through the - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal references - * are also gone, then the dma_buf gets destroyed. This can also happen as a - * part of the clean up procedure in the drm_release() function if userspace - * fails to properly clean up. Note that both the kernel and userspace (by - * keeeping the PRIME file descriptors open) can hold references onto a - * &dma_buf. - * - * Thus the chain of references always flows in one direction - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem - * - * Self-importing: if userspace is using PRIME as a replacement for flink - * then it will get a fd->handle request for a GEM object that it created. - * Drivers should detect this situation and return back the gem object - * from the dma-buf private. Prime will do this automatically for drivers that - * use the drm_gem_prime_{import,export} helpers. - * - * GEM struct &dma_buf_ops symbols are now exported. They can be resued by - * drivers which implement GEM interface. +/** + * DOC: overview and lifetime rules + * + * Similar to GEM global names, PRIME file descriptors are also used to share + * buffer objects across processes. They offer additional security: as file + * descriptors must be explicitly sent over UNIX domain sockets to be shared + * between applications, they can't be guessed like the globally unique GEM + * names. + * + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the + * &drm_driver.driver_features field, and implement the + * &drm_driver.prime_handle_to_fd and &drm_driver.prime_fd_to_handle operations. + * GEM based drivers must use drm_gem_prime_handle_to_fd() and + * drm_gem_prime_fd_to_handle() to implement these. For GEM based drivers the + * actual driver interfaces is provided through the &drm_gem_object_funcs.export + * and &drm_driver.gem_prime_import hooks. + * + * &dma_buf_ops implementations for GEM drivers are all individually exported + * for drivers which need to overwrite or reimplement some of them. + * + * Reference Counting for GEM Drivers + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * On the export the &dma_buf holds a reference to the exported buffer object, + * usually a &drm_gem_object. It takes this reference in the PRIME_HANDLE_TO_FD + * IOCTL, when it first calls &drm_gem_object_funcs.export + * and stores the exporting GEM object in the &dma_buf.priv field. This + * reference needs to be released when the final reference to the &dma_buf + * itself is dropped and its &dma_buf_ops.release function is called. For + * GEM-based drivers, the &dma_buf should be exported using + * drm_gem_dmabuf_export() and then released by drm_gem_dmabuf_release(). + * + * Thus the chain of references always flows in one direction, avoiding loops: + * importing GEM object -> dma-buf -> exported GEM bo. A further complication + * are the lookup caches for import and export. These are required to guarantee + * that any given object will always have only one uniqe userspace handle. This + * is required to allow userspace to detect duplicated imports, since some GEM + * drivers do fail command submissions if a given buffer object is listed more + * than once. These import and export caches in &drm_prime_file_private only + * retain a weak reference, which is cleaned up when the corresponding object is + * released. + * + * Self-importing: If userspace is using PRIME as a replacement for flink then + * it will get a fd->handle request for a GEM object that it created. Drivers + * should detect this situation and return back the underlying object from the + * dma-buf private. For GEM based drivers this is handled in + * drm_gem_prime_import() already. */ struct drm_prime_member { @@ -220,7 +226,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv) } /** - * drm_gem_dmabuf_export - dma_buf export implementation for GEM + * drm_gem_dmabuf_export - &dma_buf export implementation for GEM * @dev: parent device for the exported dmabuf * @exp_info: the export information used by dma_buf_export() * @@ -248,11 +254,11 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, EXPORT_SYMBOL(drm_gem_dmabuf_export); /** - * drm_gem_dmabuf_release - dma_buf release implementation for GEM + * drm_gem_dmabuf_release - &dma_buf release implementation for GEM * @dma_buf: buffer to be released * * Generic release function for dma_bufs exported as PRIME buffers. GEM drivers - * must use this in their dma_buf ops structure as the release callback. + * must use this in their &dma_buf_ops structure as the release callback. * drm_gem_dmabuf_release() should be used in conjunction with * drm_gem_dmabuf_export(). */ @@ -278,7 +284,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release); * This is the PRIME import function which must be used mandatorily by GEM * drivers to ensure correct lifetime management of the underlying GEM object. * The actual importing of GEM object from the dma-buf is done through the - * gem_import_export driver callback. + * &drm_driver.gem_import_export driver callback. + * + * Returns 0 on success or a negative error code on failure. */ int drm_gem_prime_fd_to_handle(struct drm_device *dev, struct drm_file *file_priv, int prime_fd, @@ -412,7 +420,7 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, * This is the PRIME export function which must be used mandatorily by GEM * drivers to ensure correct lifetime management of the underlying GEM object. * The actual exporting from GEM object to a dma-buf is done through the - * gem_prime_export driver callback. + * &drm_driver.gem_prime_export driver callback. */ int drm_gem_prime_handle_to_fd(struct drm_device *dev, struct drm_file *file_priv, uint32_t handle, @@ -523,23 +531,39 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, /** * DOC: PRIME Helpers * - * Drivers can implement @gem_prime_export and @gem_prime_import in terms of - * simpler APIs by using the helper functions @drm_gem_prime_export and - * @drm_gem_prime_import. These functions implement dma-buf support in terms of - * six lower-level driver callbacks: + * Drivers can implement &drm_gem_object_funcs.export and + * &drm_driver.gem_prime_import in terms of simpler APIs by using the helper + * functions drm_gem_prime_export() and drm_gem_prime_import(). These functions + * implement dma-buf support in terms of some lower-level helpers, which are + * again exported for drivers to use individually: + * + * Exporting buffers + * ~~~~~~~~~~~~~~~~~ + * + * Optional pinning of buffers is handled at dma-buf attach and detach time in + * drm_gem_map_attach() and drm_gem_map_detach(). Backing storage itself is + * handled by drm_gem_map_dma_buf() and drm_gem_unmap_dma_buf(), which relies on + * &drm_gem_object_funcs.get_sg_table. + * + * For kernel-internal access there's drm_gem_dmabuf_vmap() and + * drm_gem_dmabuf_vunmap(). Userspace mmap support is provided by + * drm_gem_dmabuf_mmap(). + * + * Note that these export helpers can only be used if the underlying backing + * storage is fully coherent and either permanently pinned, or it is safe to pin + * it indefinitely. * - * Export callbacks: + * FIXME: The underlying helper functions are named rather inconsistently. * - * * @gem_prime_pin (optional): prepare a GEM object for exporting - * * @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages - * * @gem_prime_vmap: vmap a buffer exported by your driver - * * @gem_prime_vunmap: vunmap a buffer exported by your driver - * * @gem_prime_mmap (optional): mmap a buffer exported by your driver + * Exporting buffers + * ~~~~~~~~~~~~~~~~~ * - * Import callback: + * Importing dma-bufs using drm_gem_prime_import() relies on + * &drm_driver.gem_prime_import_sg_table. * - * * @gem_prime_import_sg_table (import): produce a GEM object from another - * driver's scatter/gather table + * Note that similarly to the export helpers this permanently pins the + * underlying backing storage. Which is ok for scanout, but is not the best + * option for sharing lots of buffers for rendering. */ /** @@ -547,8 +571,9 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, * @dma_buf: buffer to attach device to * @attach: buffer attachment data * - * Calls &drm_driver.gem_prime_pin for device specific handling. This can be - * used as the &dma_buf_ops.attach callback. + * Calls &drm_gem_object_funcs.pin for device specific handling. This can be + * used as the &dma_buf_ops.attach callback. Must be used together with + * drm_gem_map_detach(). * * Returns 0 on success, negative error code on failure. */ @@ -566,8 +591,9 @@ EXPORT_SYMBOL(drm_gem_map_attach); * @dma_buf: buffer to detach from * @attach: attachment to be detached * - * Cleans up &dma_buf_attachment. This can be used as the &dma_buf_ops.detach - * callback. + * Calls &drm_gem_object_funcs.pin for device specific handling. Cleans up + * &dma_buf_attachment from drm_gem_map_attach(). This can be used as the + * &dma_buf_ops.detach callback. */ void drm_gem_map_detach(struct dma_buf *dma_buf, struct dma_buf_attachment *attach) @@ -583,13 +609,13 @@ EXPORT_SYMBOL(drm_gem_map_detach); * @attach: attachment whose scatterlist is to be returned * @dir: direction of DMA transfer * - * Calls &drm_driver.gem_prime_get_sg_table and then maps the scatterlist. This - * can be used as the &dma_buf_ops.map_dma_buf callback. + * Calls &drm_gem_object_funcs.get_sg_table and then maps the scatterlist. This + * can be used as the &dma_buf_ops.map_dma_buf callback. Should be used together + * with drm_gem_unmap_dma_buf(). * - * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR + * Returns:sg_table containing the scatterlist to be returned; returns ERR_PTR * on error. May return -EINTR if it is interrupted by a signal. */ - struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, enum dma_data_direction dir) { @@ -642,9 +668,9 @@ EXPORT_SYMBOL(drm_gem_unmap_dma_buf); * @dma_buf: buffer to be mapped * * Sets up a kernel virtual mapping. This can be used as the &dma_buf_ops.vmap - * callback. + * callback. Calls into &drm_gem_object_funcs.vmap for device specific handling. * - * Returns the kernel virtual address. + * Returns the kernel virtual address or NULL on failure. */ void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf) { @@ -665,7 +691,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap); * @vaddr: the virtual address of the buffer * * Releases a kernel virtual mapping. This can be used as the - * &dma_buf_ops.vunmap callback. + * &dma_buf_ops.vunmap callback. Calls into &drm_gem_object_funcs.vunmap for device specific handling. */ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr) { @@ -727,7 +753,11 @@ EXPORT_SYMBOL(drm_gem_prime_mmap); * @vma: virtual address range * * Provides memory mapping for the buffer. This can be used as the - * &dma_buf_ops.mmap callback. + * &dma_buf_ops.mmap callback. It just forwards to &drm_driver.gem_prime_mmap, + * which should be set to drm_gem_prime_mmap(). + * + * FIXME: There's really no point to this wrapper, drivers which need anything + * else but drm_gem_prime_mmap can roll their own &dma_buf_ops.mmap callback. * * Returns 0 on success or a negative error code on failure. */ @@ -763,6 +793,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { * This helper creates an sg table object from a set of pages * the driver is responsible for mapping the pages into the * importers address space for use with dma_buf itself. + * + * This is useful for implementing &drm_gem_object_funcs.get_sg_table. */ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages) { @@ -793,7 +825,7 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg); * @obj: GEM object to export * @flags: flags like DRM_CLOEXEC and DRM_RDWR * - * This is the implementation of the gem_prime_export functions for GEM drivers + * This is the implementation of the &drm_gem_object_funcs.export functions for GEM drivers * using the PRIME helpers. */ struct dma_buf *drm_gem_prime_export(struct drm_device *dev, @@ -823,9 +855,13 @@ EXPORT_SYMBOL(drm_gem_prime_export); * @dma_buf: dma-buf object to import * @attach_dev: struct device to dma_buf attach * - * This is the core of drm_gem_prime_import. It's designed to be called by - * drivers who want to use a different device structure than dev->dev for - * attaching via dma_buf. + * This is the core of drm_gem_prime_import(). It's designed to be called by + * drivers who want to use a different device structure than &drm_device.dev for + * attaching via dma_buf. This function calls + * &drm_driver.gem_prime_import_sg_table internally. + * + * Drivers must arrange to call drm_prime_gem_destroy() from their + * &drm_gem_object_funcs.free hook when using this function. */ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, struct dma_buf *dma_buf, @@ -889,7 +925,11 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); * @dma_buf: dma-buf object to import * * This is the implementation of the gem_prime_import functions for GEM drivers - * using the PRIME helpers. + * using the PRIME helpers. Drivers can use this as their + * &drm_driver.gem_prime_import implementation. + * + * Drivers must arrange to call drm_prime_gem_destroy() from their + * &drm_gem_object_funcs.free hook when using this function. */ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf) @@ -907,6 +947,9 @@ EXPORT_SYMBOL(drm_gem_prime_import); * * Exports an sg table into an array of pages and addresses. This is currently * required by the TTM driver in order to do correct fault handling. + * + * Drivers can use this in their &drm_driver.gem_prime_import_sg_table + * implementation. */ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_entries) @@ -947,7 +990,7 @@ EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays); * @sg: the sg-table which was pinned at import time * * This is the cleanup functions which GEM drivers need to call when they use - * @drm_gem_prime_import to import dma-bufs. + * drm_gem_prime_import() or drm_gem_prime_import_dev() to import dma-bufs. */ void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) { diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 5c4fc0ddc863..bbb3a6ff4418 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -505,21 +505,25 @@ struct drm_driver { * @gem_free_object: deconstructor for drm_gem_objects * * This is deprecated and should not be used by new drivers. Use - * @gem_free_object_unlocked instead. + * &drm_gem_object_funcs.free instead. */ void (*gem_free_object) (struct drm_gem_object *obj); /** * @gem_free_object_unlocked: deconstructor for drm_gem_objects * - * This is for drivers which are not encumbered with &drm_device.struct_mutex - * legacy locking schemes. Use this hook instead of @gem_free_object. + * This is deprecated and should not be used by new drivers. Use + * &drm_gem_object_funcs.free instead. + * Compared to @gem_free_object this is not encumbered with + * &drm_device.struct_mutex legacy locking schemes. */ void (*gem_free_object_unlocked) (struct drm_gem_object *obj); /** * @gem_open_object: * + * This callback is deprecated in favour of &drm_gem_object_funcs.open. + * * Driver hook called upon gem handle creation */ int (*gem_open_object) (struct drm_gem_object *, struct drm_file *); @@ -527,6 +531,8 @@ struct drm_driver { /** * @gem_close_object: * + * This callback is deprecated in favour of &drm_gem_object_funcs.close. + * * Driver hook called upon gem handle release */ void (*gem_close_object) (struct drm_gem_object *, struct drm_file *); @@ -534,6 +540,9 @@ struct drm_driver { /** * @gem_print_info: * + * This callback is deprecated in favour of + * &drm_gem_object_funcs.print_info. + * * If driver subclasses struct &drm_gem_object, it can implement this * optional hook for printing additional driver specific info. * @@ -548,56 +557,120 @@ struct drm_driver { /** * @gem_create_object: constructor for gem objects * - * Hook for allocating the GEM object struct, for use by core - * helpers. + * Hook for allocating the GEM object struct, for use by the CMA and + * SHMEM GEM helpers. */ struct drm_gem_object *(*gem_create_object)(struct drm_device *dev, size_t size); - - /* prime: */ /** * @prime_handle_to_fd: * - * export handle -> fd (see drm_gem_prime_handle_to_fd() helper) + * Main PRIME export function. Should be implemented with + * drm_gem_prime_handle_to_fd() for GEM based drivers. + * + * For an in-depth discussion see :ref:`PRIME buffer sharing + * documentation <prime_buffer_sharing>`. */ int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv, uint32_t handle, uint32_t flags, int *prime_fd); /** * @prime_fd_to_handle: * - * import fd -> handle (see drm_gem_prime_fd_to_handle() helper) + * Main PRIME import function. Should be implemented with + * drm_gem_prime_fd_to_handle() for GEM based drivers. + * + * For an in-depth discussion see :ref:`PRIME buffer sharing + * documentation <prime_buffer_sharing>`. */ int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv, int prime_fd, uint32_t *handle); /** * @gem_prime_export: * - * export GEM -> dmabuf - * - * This defaults to drm_gem_prime_export() if not set. + * Export hook for GEM drivers. Deprecated in favour of + * &drm_gem_object_funcs.export. */ struct dma_buf * (*gem_prime_export)(struct drm_device *dev, struct drm_gem_object *obj, int flags); /** * @gem_prime_import: * - * import dmabuf -> GEM + * Import hook for GEM drivers. * * This defaults to drm_gem_prime_import() if not set. */ struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev, struct dma_buf *dma_buf); + + /** + * @gem_prime_pin: + * + * Deprecated hook in favour of &drm_gem_object_funcs.pin. + */ int (*gem_prime_pin)(struct drm_gem_object *obj); + + /** + * @gem_prime_unpin: + * + * Deprecated hook in favour of &drm_gem_object_funcs.unpin. + */ void (*gem_prime_unpin)(struct drm_gem_object *obj); + + + /** + * @gem_prime_get_sg_table: + * + * Deprecated hook in favour of &drm_gem_object_funcs.get_sg_table. + */ + struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); + + /** + * @gem_prime_res_obj: + * + * Optional hook to look up the &reservation_object for an buffer when + * exporting it. + * + * FIXME: This hook is deprecated. Users of this hook should be replaced + * by setting &drm_gem_object.resv instead. + */ struct reservation_object * (*gem_prime_res_obj)( struct drm_gem_object *obj); - struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj); + + /** + * @gem_prime_import_sg_table: + * + * Optional hook used by the PRIME helper functions + * drm_gem_prime_import() respectively drm_gem_prime_import_dev(). + */ struct drm_gem_object *(*gem_prime_import_sg_table)( struct drm_device *dev, struct dma_buf_attachment *attach, struct sg_table *sgt); + /** + * @gem_prime_vmap: + * + * Deprecated vmap hook for GEM drivers. Please use + * &drm_gem_object_funcs.vmap instead. + */ void *(*gem_prime_vmap)(struct drm_gem_object *obj); + + /** + * @gem_prime_vunmap: + * + * Deprecated vunmap hook for GEM drivers. Please use + * &drm_gem_object_funcs.vunmap instead. + */ void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr); + + /** + * @gem_prime_mmap: + * + * mmap hook for GEM drivers, used to implement dma-buf mmap in the + * PRIME helpers. + * + * FIXME: There's way too much duplication going on here, and also moved + * to &drm_gem_object_funcs. + */ int (*gem_prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); @@ -665,6 +738,9 @@ struct drm_driver { /** * @gem_vm_ops: Driver private ops for this object + * + * For GEM driver this is deprecated in favour of + * &drm_gem_object_funcs.vm_ops. */ const struct vm_operations_struct *gem_vm_ops; diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index a9121fe66ea2..9af88238ee5c 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -101,7 +101,7 @@ struct drm_gem_object_funcs { /** * @pin: * - * Pin backing buffer in memory. + * Pin backing buffer in memory. Used by the drm_gem_map_attach helper. * * This callback is optional. */ @@ -110,7 +110,7 @@ struct drm_gem_object_funcs { /** * @unpin: * - * Unpin backing buffer. + * Unpin backing buffer. Used by the drm_gem_map_detach() helper. * * This callback is optional. */ @@ -120,16 +120,21 @@ struct drm_gem_object_funcs { * @get_sg_table: * * Returns a Scatter-Gather table representation of the buffer. - * Used when exporting a buffer. + * Used when exporting a buffer by the drm_gem_map_dma_buf() helper. + * Releasing is done by calling dma_unmap_sg_attrs() and sg_free_table() + * in drm_gem_unmap_buf(), therefore these helpers and this callback + * here cannot be used for sg tables pointing at driver private memory + * ranges. * - * This callback is mandatory if buffer export is supported. + * See also drm_prime_pages_to_sg(). */ struct sg_table *(*get_sg_table)(struct drm_gem_object *obj); /** * @vmap: * - * Returns a virtual address for the buffer. + * Returns a virtual address for the buffer. Used by the + * drm_gem_dmabuf_vmap() helper. * * This callback is optional. */ @@ -138,7 +143,8 @@ struct drm_gem_object_funcs { /** * @vunmap: * - * Releases the the address previously returned by @vmap. + * Releases the the address previously returned by @vmap. Used by the + * drm_gem_dmabuf_vunmap() helper. * * This callback is optional. */
Yes this is a bit a big patch, but since it's essentially a complete rewrite of all the prime docs I didn't see how to better split it up. Changes: - Consistently point to drm_gem_object_funcs as the preferred hooks, where applicable. - Document all the hooks in &drm_driver that lacked kerneldoc. - Completely new overview section, which now also includes the cleaned up lifetime/reference counting subchapter. I also mentioned the weak references in there due to the lookup caches. - Completely rewritten helper intro section, highlight the import/export related functionality. - Polish for all the functions and more cross references. I also sprinkled a bunch of todos all over. Most important: 0 code changes in here. The cleanup motivated by reading and improving all this will follow later on. v2: Actually update the prime helper docs. Plus add a few FIXMEs that I won't address right away in subsequent cleanup patches. v3: - Split out the function moving. This patch is now exclusively documentation changes. - Typos and nits (Sam). Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Eric Anholt <eric@anholt.net> Cc: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/gpu/drm-mm.rst | 40 +------ drivers/gpu/drm/drm_prime.c | 197 +++++++++++++++++++++-------------- include/drm/drm_drv.h | 104 +++++++++++++++--- include/drm/drm_gem.h | 18 ++-- 4 files changed, 226 insertions(+), 133 deletions(-)