Re: [PATCH v2 3/8] drm/doc: Add GEM/CMA helpers to kerneldoc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 06, 2014 at 04:49:16PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>

Looks really good. Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux