[PATCH 1/2] drm/gem-fb-helper: Cleanup docs

[Daniel Vetter]

On Sat, Aug 19, 2017 at 04:34:44PM +0200, Noralf Trønnes wrote:
> Make the docs read a little better.
> 
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>

A few nits on your nits, with those addressed:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index d54a083..9a36d44 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -27,18 +27,20 @@
>   * DOC: overview
>   *
>   * This library provides helpers for drivers that don't subclass
> - * &drm_framebuffer and and use &drm_gem_object for their backing storage.
> + * &drm_framebuffer and use &drm_gem_object for their backing storage.
>   *
>   * Drivers without additional needs to validate framebuffers can simply use
> - * drm_gem_fb_create() and everything is wired up automatically. But all
> - * parts can be used individually.
> + * drm_gem_fb_create() and everything is wired up automatically. Other drivers
> + * can use all parts independently.
>   */
>  
>  /**
>   * drm_gem_fb_get_obj() - Get GEM object for framebuffer
> - * @fb: The framebuffer
> + * @fb: framebuffer
>   * @plane: Which plane
>   *
> + * No reference is taken on the GEM object.

Maybe "No additional reference beyond the one that the &drm_frambuffer
already holds is taken ..."? Otherwise people read this and wonder why it
doesn't insta-oops.

> + *
>   * Returns the GEM object for given framebuffer.
>   */
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> @@ -82,7 +84,7 @@ drm_gem_fb_alloc(struct drm_device *dev,
>  
>  /**
>   * drm_gem_fb_destroy - Free GEM backed framebuffer
> - * @fb: DRM framebuffer
> + * @fb: framebuffer
>   *
>   * Frees a GEM backed framebuffer with its backing buffer(s) and the structure
>   * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
> @@ -102,7 +104,7 @@ EXPORT_SYMBOL(drm_gem_fb_destroy);
>  
>  /**
>   * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
> - * @fb: DRM framebuffer
> + * @fb: framebuffer
>   * @file: drm file
>   * @handle: handle created
>   *
> @@ -123,9 +125,9 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle);
>   * drm_gem_fb_create_with_funcs() - helper function for the
>   *                                  &drm_mode_config_funcs.fb_create
>   *                                  callback
> - * @dev: DRM device
> + * @dev: drm device

In docs we generally try to do all uppercase abbreviations, so DRM, not
drm. Yes I sometimes botch that too.

https://dri.freedesktop.org/docs/drm/gpu/introduction.html#style-guidelines

Has a few other drm specific conventions, plus references the overall
kernel-doc style guide.

>   * @file: drm file for the ioctl call
> - * @mode_cmd: metadata from the userspace fb creation request
> + * @mode_cmd: data from the userspace fb creation request

metadata is more accurate, since just "data" imo is more for referencing
the pixel data in the fb's backing storage. At least I've tried to use
"framebuffer metadata" as consistently as possible for this stuff (pixel
format, size, modifiers, ...).

>   * @funcs: vtable to be used for the new framebuffer object
>   *
>   * This can be used to set &drm_framebuffer_funcs for drivers that need the
> @@ -193,9 +195,9 @@ static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
>  
>  /**
>   * drm_gem_fb_create() - &drm_mode_config_funcs.fb_create callback function
> - * @dev: DRM device
> + * @dev: drm device
>   * @file: drm file for the ioctl call
> - * @mode_cmd: metadata from the userspace fb creation request
> + * @mode_cmd: data from the userspace fb creation request
>   *
>   * If your hardware has special alignment or pitch requirements these should be
>   * checked before calling this function. The function does buffer size
> @@ -214,11 +216,11 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create);
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare gem framebuffer
>   * @plane: Which plane
> - * @state: Plane state attach fence to
> + * @state: Plane state the fence will be attached to
>   *
>   * This can be used as the &drm_plane_helper_funcs.prepare_fb hook.
>   *
> - * This function checks if the plane FB has an dma-buf attached, extracts
> + * This function checks if the plane FB has a dma-buf attached, extracts
>   * the exclusive fence and attaches it to plane state for the atomic helper
>   * to wait on.
>   *
> @@ -247,7 +249,7 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
>  
>  /**
>   * drm_gem_fbdev_fb_create - Create a drm_framebuffer for fbdev emulation
> - * @dev: DRM device
> + * @dev: drm device
>   * @sizes: fbdev size description
>   * @pitch_align: optional pitch alignment
>   * @obj: GEM object backing the framebuffer
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch