Re: [PATCH 2/2] drm/prime: Update docs

[Daniel Vetter <daniel@ffwll.ch>]

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.
> >  	 */
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx