On Thu, Nov 06, 2014 at 04:49:16PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding at nvidia.com>
>
> Most of the functions already have the beginnings of kerneldoc comments
> but are using the wrong opening marker. Use the correct opening marker
> and flesh out the comments so that they can be integrated with the DRM
> DocBook document.
>
> Signed-off-by: Thierry Reding <treding at nvidia.com>
Looks really good. Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> Changes in v2:
> - document return value consistently with other source files
> - provide better documentation and use-cases for helpers
> ---
> Documentation/DocBook/drm.tmpl | 6 +
> drivers/gpu/drm/drm_gem_cma_helper.c | 220
> ++++++++++++++++++++++++++++-------
> include/drm/drm_gem_cma_helper.h | 25 ++--
> 3 files changed, 203 insertions(+), 48 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 6e32ef9c75fd..6a14eab4a3d8 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -963,6 +963,12 @@ struct drm_gem_object * (*gem_prime_import)(struct
> drm_device *dev,
> !Edrivers/gpu/drm/drm_mm.c
> !Iinclude/drm/drm_mm.h
> </sect2>
> + <sect2>
> + <title>CMA Helper Functions Reference</title>
> +!Pdrivers/gpu/drm/drm_gem_cma_helper.c cma helpers
> +!Edrivers/gpu/drm/drm_gem_cma_helper.c
> +!Iinclude/drm/drm_gem_cma_helper.h
> + </sect2>
> </sect1>
>
> <!-- Internals: mode setting -->
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 0316310e2cc4..7f986d7b8e22 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -29,18 +29,31 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_vma_manager.h>
>
> -/*
> +/**
> + * DOC: cma helpers
> + *
> + * The Contiguous Memory Allocator reserves a pool of memory at early boot
> + * that is used to service requests for large blocks of contiguous memory.
> + *
> + * The DRM GEM/CMA helpers use this allocator as a means to provide buffer
> + * objects that are physically contiguous in memory. This is useful for
> + * display drivers that are unable to map scattered buffers via an IOMMU.
> + */
> +
> +/**
> * __drm_gem_cma_create - Create a GEM CMA object without allocating memory
> - * @drm: The drm device
> - * @size: The GEM object size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> *
> - * This function creates and initializes a GEM CMA object of the given size,
> but
> - * doesn't allocate any memory to back the object.
> + * This function creates and initializes a GEM CMA object of the given size,
> + * but doesn't allocate any memory to back the object.
> *
> - * Return a struct drm_gem_cma_object* on success or ERR_PTR values on
> failure.
> + * Returns:
> + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> */
> static struct drm_gem_cma_object *
> -__drm_gem_cma_create(struct drm_device *drm, unsigned int size)
> +__drm_gem_cma_create(struct drm_device *drm, size_t size)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> @@ -69,14 +82,21 @@ error:
> return ERR_PTR(ret);
> }
>
> -/*
> +/**
> * drm_gem_cma_create - allocate an object with the given size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> + *
> + * This function creates a CMA GEM object and allocates a contiguous chunk of
> + * memory as backing store. The backing memory has the writecombine attribute
> + * set.
> *
> - * returns a struct drm_gem_cma_object* on success or ERR_PTR values
> - * on failure.
> + * Returns:
> + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> */
> struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> - unsigned int size)
> + size_t size)
> {
> struct drm_gem_cma_object *cma_obj;
> int ret;
> @@ -104,17 +124,26 @@ error:
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
>
> -/*
> - * drm_gem_cma_create_with_handle - allocate an object with the given
> - * size and create a gem handle on it
> +/**
> + * drm_gem_cma_create_with_handle - allocate an object with the given size
> and
> + * return a GEM handle to it
> + * @file_priv: DRM file-private structure to register the handle for
> + * @drm: DRM device
> + * @size: size of the object to allocate
> + * @handle: return location for the GEM handle
> + *
> + * This function creates a CMA GEM object, allocating a physically contiguous
> + * chunk of memory as backing store. The GEM object is then added to the list
> + * of object associated with the given file and a handle to it is returned.
> *
> - * returns a struct drm_gem_cma_object* on success or ERR_PTR values
> - * on failure.
> + * Returns:
> + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> */
> -static struct drm_gem_cma_object *drm_gem_cma_create_with_handle(
> - struct drm_file *file_priv,
> - struct drm_device *drm, unsigned int size,
> - unsigned int *handle)
> +static struct drm_gem_cma_object *
> +drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> + struct drm_device *drm, size_t size,
> + uint32_t *handle)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> @@ -145,9 +174,14 @@ err_handle_create:
> return ERR_PTR(ret);
> }
>
> -/*
> - * drm_gem_cma_free_object - (struct drm_driver)->gem_free_object callback
> - * function
> +/**
> + * drm_gem_cma_free_object - free resources associated with a CMA GEM object
> + * @gem_obj: GEM object to free
> + *
> + * This function frees the backing memory of the CMA GEM object, cleans up
> the
> + * GEM object state and frees the memory used to store the object itself.
> + * Drivers using the CMA helpers should set this as their DRM driver's
> + * ->gem_free_object() callback.
> */
> void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
> {
> @@ -170,15 +204,23 @@ void drm_gem_cma_free_object(struct drm_gem_object
> *gem_obj)
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
>
> -/*
> - * drm_gem_cma_dumb_create - (struct drm_driver)->dumb_create callback
> - * function
> +/**
> + * drm_gem_cma_dumb_create - create a dumb buffer object
> + * @file_priv: DRM file-private structure to create the dumb buffer for
> + * @drm: DRM device
> + * @args: IOCTL data
> + *
> + * This function computes the pitch of the dumb buffer and rounds it up to an
> + * integer number of bytes per pixel. Drivers for hardware that doesn't have
> + * any additional restrictions on the pitch can directly use this function as
> + * their ->dumb_create() callback.
> *
> - * This aligns the pitch and size arguments to the minimum required. wrap
> - * this into your own function if you need bigger alignment.
> + * Returns:
> + * 0 on success or a negative error code on failure.
> */
> int drm_gem_cma_dumb_create(struct drm_file *file_priv,
> - struct drm_device *dev, struct drm_mode_create_dumb *args)
> + struct drm_device *drm,
> + struct drm_mode_create_dumb *args)
> {
> struct drm_gem_cma_object *cma_obj;
> int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> @@ -189,18 +231,30 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
> if (args->size < args->pitch * args->height)
> args->size = args->pitch * args->height;
>
> - cma_obj = drm_gem_cma_create_with_handle(file_priv, dev,
> - args->size, &args->handle);
> + cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> + &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
>
> -/*
> - * drm_gem_cma_dumb_map_offset - (struct drm_driver)->dumb_map_offset
> callback
> - * function
> +/**
> + * drm_gem_cma_dumb_map_offset - return the fake mmap offset for a CMA GEM
> + * object
> + * @file_priv: DRM file-private structure containing the GEM object
> + * @drm: DRM device
> + * @handle: GEM object handle
> + * @offset: return location for the fake mmap offset
> + *
> + * This function look up an object by its handle and returns the fake mmap
> + * offset associated with it. Drivers using the CMA helpers should set this
> + * as their DRM driver's ->dumb_map_offset() callback.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> */
> int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
> - struct drm_device *drm, uint32_t handle, uint64_t *offset)
> + struct drm_device *drm, u32 handle,
> + u64 *offset)
> {
> struct drm_gem_object *gem_obj;
>
> @@ -208,7 +262,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file
> *file_priv,
>
> gem_obj = drm_gem_object_lookup(drm, file_priv, handle);
> if (!gem_obj) {
> - dev_err(drm->dev, "failed to lookup gem object\n");
> + dev_err(drm->dev, "failed to lookup GEM object\n");
> mutex_unlock(&drm->struct_mutex);
> return -EINVAL;
> }
> @@ -251,8 +305,20 @@ static int drm_gem_cma_mmap_obj(struct
> drm_gem_cma_object *cma_obj,
> return ret;
> }
>
> -/*
> - * drm_gem_cma_mmap - (struct file_operation)->mmap callback function
> +/**
> + * drm_gem_cma_mmap - memory-map a CMA GEM object
> + * @filp: file object
> + * @vma: VMA for the area to be mapped
> + *
> + * This function implements an augmented version of the GEM DRM file mmap
> + * operation for CMA objects: In addition to the usual GEM VMA setup it
> + * immediately faults in the entire object instead of using on-demaind
> + * faulting. Drivers which employ the CMA helpers should use this function
> + * as their ->mmap() handler in the DRM device file's file_operations
> + * structure.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> */
> int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
> {
> @@ -272,7 +338,16 @@ int drm_gem_cma_mmap(struct file *filp, struct
> vm_area_struct *vma)
> EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
>
> #ifdef CONFIG_DEBUG_FS
> -void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct
> seq_file *m)
> +/**
> + * drm_gem_cma_describe - describe a CMA GEM object for debugfs
> + * @cma_obj: CMA GEM object
> + * @m: debugfs file handle
> + *
> + * This function can be used to dump a human-readable representation of the
> + * CMA GEM object into a synthetic file.
> + */
> +void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj,
> + struct seq_file *m)
> {
> struct drm_gem_object *obj = &cma_obj->base;
> struct drm_device *dev = obj->dev;
> @@ -291,7 +366,18 @@ void drm_gem_cma_describe(struct drm_gem_cma_object
> *cma_obj, struct seq_file *m
> EXPORT_SYMBOL_GPL(drm_gem_cma_describe);
> #endif
>
> -/* low-level interface prime helpers */
> +/**
> + * drm_gem_cma_prime_get_sg_table - provide a scatter/gather table of pinned
> + * pages for a CMA GEM object
> + * @obj: GEM object
> + *
> + * This function exports a scatter/gather table suitable for PRIME usage by
> + * calling the standard DMA mapping API. Drivers using the CMA helpers should
> + * set this as their DRM driver's ->gem_prime_get_sg_table() callback.
> + *
> + * Returns:
> + * A pointer to the scatter/gather table of pinned pages or NULL on failure.
> + */
> struct sg_table *drm_gem_cma_prime_get_sg_table(struct drm_gem_object *obj)
> {
> struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(obj);
> @@ -315,6 +401,23 @@ out:
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_get_sg_table);
>
> +/**
> + * drm_gem_cma_prime_import_sg_table - produce a CMA GEM object from another
> + * driver's scatter/gather table of pinned pages
> + * @dev: device to import into
> + * @attach: DMA-BUF attachment
> + * @sgt: scatter/gather table of pinned pages
> + *
> + * This function imports a scatter/gather table exported via DMA-BUF by
> + * another driver. Imported buffers must be physically contiguous in memory
> + * (i.e. the scatter/gather table must contain a single entry). Drivers that
> + * use the CMA helpers should set this as their DRM driver's
> + * ->gem_prime_import_sg_table() callback.
> + *
> + * Returns:
> + * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
> + * error code on failure.
> + */
> struct drm_gem_object *
> drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach,
> @@ -339,6 +442,18 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_import_sg_table);
>
> +/**
> + * drm_gem_cma_prime_mmap - memory-map an exported CMA GEM object
> + * @obj: GEM object
> + * @vma: VMA for the area to be mapped
> + *
> + * This function maps a buffer imported via DRM PRIME into a userspace
> + * process's address space. Drivers that use the CMA helpers should set this
> + * as their DRM driver's ->gem_prime_mmap() callback.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> int drm_gem_cma_prime_mmap(struct drm_gem_object *obj,
> struct vm_area_struct *vma)
> {
> @@ -357,6 +472,20 @@ int drm_gem_cma_prime_mmap(struct drm_gem_object *obj,
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_mmap);
>
> +/**
> + * drm_gem_cma_prime_vmap - map a CMA GEM object into the kernel's virtual
> + * address space
> + * @obj: GEM object
> + *
> + * This function maps a buffer exported via DRM PRIME into the kernel's
> + * virtual address space. Since the CMA buffers are already mapped into the
> + * kernel virtual address space this simply returns the cached virtual
> + * address. Drivers using the CMA helpers should set this as their DRM
> + * driver's ->gem_prime_vmap() callback.
> + *
> + * Returns:
> + * The kernel virtual address of the CMA GEM object's backing store.
> + */
> void *drm_gem_cma_prime_vmap(struct drm_gem_object *obj)
> {
> struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(obj);
> @@ -365,6 +494,17 @@ void *drm_gem_cma_prime_vmap(struct drm_gem_object *obj)
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vmap);
>
> +/**
> + * drm_gem_cma_prime_vunmap - unmap a CMA GEM object from the kernel's
> virtual
> + * address space
> + * @obj: GEM object
> + * @vaddr: kernel virtual address where the CMA GEM object was mapped
> + *
> + * This function removes a buffer exported via DRM PRIME from the kernel's
> + * virtual address space. This is a no-op because CMA buffers cannot be
> + * unmapped from kernel space. Drivers using the CMA helpers should set this
> + * as their DRM driver's ->gem_prime_vunmap() callback.
> + */
> void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
> {
> /* Nothing to do */
> diff --git a/include/drm/drm_gem_cma_helper.h
> b/include/drm/drm_gem_cma_helper.h
> index 2ff35f3de9c5..873d4eb7f125 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -4,6 +4,13 @@
> #include <drm/drmP.h>
> #include <drm/drm_gem.h>
>
> +/**
> + * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
> + * @base: base GEM object
> + * @paddr: physical address of the backing memory
> + * @sgt: scatter/gather table for imported PRIME buffers
> + * @vaddr: kernel virtual address of the backing memory
> + */
> struct drm_gem_cma_object {
> struct drm_gem_object base;
> dma_addr_t paddr;
> @@ -19,23 +26,25 @@ to_drm_gem_cma_obj(struct drm_gem_object *gem_obj)
> return container_of(gem_obj, struct drm_gem_cma_object, base);
> }
>
> -/* free gem object. */
> +/* free GEM object */
> void drm_gem_cma_free_object(struct drm_gem_object *gem_obj);
>
> -/* create memory region for drm framebuffer. */
> +/* create memory region for DRM framebuffer */
> int drm_gem_cma_dumb_create(struct drm_file *file_priv,
> - struct drm_device *drm, struct drm_mode_create_dumb *args);
> + struct drm_device *drm,
> + struct drm_mode_create_dumb *args);
>
> -/* map memory region for drm framebuffer to user space. */
> +/* map memory region for DRM framebuffer to user space */
> int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
> - struct drm_device *drm, uint32_t handle, uint64_t *offset);
> + struct drm_device *drm, u32 handle,
> + u64 *offset);
>
> -/* set vm_flags and we can change the vm attribute to other one at here. */
> +/* set vm_flags and we can change the VM attribute to other one at here */
> int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
>
> -/* allocate physical memory. */
> +/* allocate physical memory */
> struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> - unsigned int size);
> + size_t size);
>
> extern const struct vm_operations_struct drm_gem_cma_vm_ops;
>
> --
> 2.1.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
