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

Noralf Trønnes
2019-06-19T12:43:20Z

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.
> */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org