Re: [PATCH 14/17] drm/cma-helpers: Use recommened kerneldoc for struct member refs

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

Thank you for the patch.

On Thursday 29 Dec 2016 21:48:34 Daniel Vetter wrote:
> I just learned that &struct_name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
> 
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
> 
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c  | 24 ++++++++++++------------
>  drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++++++++--------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c index ec081727cd5a..0a0ac77b464b
> 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -48,14 +48,14 @@ struct drm_fbdev_cma {
>   * Provides helper functions for creating a cma (contiguous memory
> allocator)
>   * backed framebuffer.
>   *
> - * drm_fb_cma_create() is used in the &drm_mode_config_funcs ->fb_create
> + * drm_fb_cma_create() is used in the &drm_mode_config_funcs.fb_create
>   * callback function to create a cma backed framebuffer.
>   *
>   * An fbdev framebuffer backed by cma is also available by calling
>   * drm_fbdev_cma_init(). drm_fbdev_cma_fini() tears it down.
> - * If the &drm_framebuffer_funcs ->dirty callback is set, fb_deferred_io
> - * will be set up automatically. dirty() is called by
> - * drm_fb_helper_deferred_io() in process context (struct delayed_work).
> + * If the &drm_framebuffer_funcs.dirty callback is set, fb_deferred_io will
> be
> + * set up automatically. &drm_framebuffer_funcs.dirty is called by
> + * drm_fb_helper_deferred_io() in process context (&struct delayed_work).
>   *
>   * Example fbdev deferred io code::
>   *
> @@ -155,16 +155,16 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct
> drm_device *dev,
> 
>  /**
>   * drm_fb_cma_create_with_funcs() - helper function for the
> - *                                  &drm_mode_config_funcs ->fb_create
> - *                                  callback function
> + *                                  &drm_mode_config_funcs.fb_create
> + *                                  callback
>   * @dev: DRM device
>   * @file_priv: drm file for the ioctl call
>   * @mode_cmd: metadata from the userspace fb creation request
>   * @funcs: vtable to be used for the new framebuffer object
>   *
>   * This can be used to set &drm_framebuffer_funcs for drivers that need the
> - * dirty() callback. Use drm_fb_cma_create() if you don't need to change
> - * &drm_framebuffer_funcs.
> + * &drm_framebuffer_funcs.dirty callback. Use drm_fb_cma_create() if you
> don't
> + * need to change &drm_framebuffer_funcs.
>   */
>  struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device
> *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -221,14 +221,14 @@ struct drm_framebuffer
> *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
> 
>  /**
> - * drm_fb_cma_create() - &drm_mode_config_funcs ->fb_create callback
> function
> + * drm_fb_cma_create() - &drm_mode_config_funcs.fb_create callback function
>   * @dev: DRM device
>   * @file_priv: drm file for the ioctl call
>   * @mode_cmd: metadata from the userspace fb creation request
>   *
>   * If your hardware has special alignment or pitch requirements these 
> should be
>   * checked before calling this function. Use drm_fb_cma_create_with_funcs()
> if
> - * you need to set &drm_framebuffer_funcs ->dirty.
> + * you need to set &drm_framebuffer_funcs.dirty.
>   */
>  struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
>  	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
> @@ -264,7 +264,7 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
>   * @plane: Which plane
>   * @state: Plane state attach fence to
>   *
> - * This should be put into prepare_fb hook of &struct
> drm_plane_helper_funcs .
> + * This should be set as the &struct drm_plane_helper_funcs.prepare_fb
> hook.
>   *
>   * This function checks if the plane FB has an dma-buf attached, extracts
>   * the exclusive fence and attaches it to plane state for the atomic helper
> @@ -491,7 +491,7 @@ static const struct drm_fb_helper_funcs
> drm_fb_cma_helper_funcs = { * @preferred_bpp: Preferred bits per pixel for
> the device
>   * @num_crtc: Number of CRTCs
>   * @max_conn_count: Maximum number of connectors
> - * @funcs: fb helper functions, in particular fb_probe()
> + * @funcs: fb helper functions, in particular a custom dirty() callback

Doesn't this belong to a different patch ?

>   * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
>   */
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 1d6c335584ec..6ec2d8096b2c
> 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -177,7 +177,7 @@ drm_gem_cma_create_with_handle(struct drm_file
> *file_priv,
>   * 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.
> + * &drm_driver.gem_free_object callback.

How about s/DRM driver's // here and below ? It's kind of redundant now that 
you reference drm_driver directly, and some of the functions already don't 
mention "DRM driver's" in their documentation.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>   */
>  void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
>  {
> @@ -207,7 +207,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_free_object);
>   * This aligns the pitch and size arguments to the minimum required. This
> is * an internal helper that can be wrapped by a driver to account for
> hardware * with more specific alignment requirements. It should not be used
> directly - * as the ->dumb_create() callback in a DRM driver.
> + * as the &drm_driver.dumb_create callback in a DRM driver.
>   *
>   * Returns:
>   * 0 on success or a negative error code on failure.
> @@ -240,7 +240,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
>   * 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.
> + * their &drm_driver.dumb_create callback.
>   *
>   * For hardware with additional restrictions, drivers can adjust the fields
> * set up by userspace and pass the IOCTL data along to the
> @@ -274,7 +274,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
>   *
>   * 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.
> + * as their DRM driver's &drm_driver.dumb_map_offset callback.
>   *
>   * Returns:
>   * 0 on success or a negative error code on failure.
> @@ -391,7 +391,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_describe);
>   *
>   * 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. + * set this as their DRM driver's
> &drm_driver.gem_prime_get_sg_table callback. *
>   * Returns:
>   * A pointer to the scatter/gather table of pinned pages or NULL on
> failure. @@ -430,7 +430,7 @@
> EXPORT_SYMBOL_GPL(drm_gem_cma_prime_get_sg_table); * 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.
> + * &drm_driver.gem_prime_import_sg_table callback.
>   *
>   * Returns:
>   * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
> @@ -496,7 +496,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_prime_mmap);
>   * 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.
> + * driver's &drm_driver.gem_prime_vmap callback.
>   *
>   * Returns:
>   * The kernel virtual address of the CMA GEM object's backing store.
> @@ -518,7 +518,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vmap);
>   * 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.
> + * as their DRM driver's &drm_driver.gem_prime_vunmap callback.
>   */
>  void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
>  {

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel