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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Dec 30, 2016 at 04:11:44PM +0200, Laurent Pinchart wrote:
> 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 ?


Yup, that shoudl have been in the previous cma patch. I moved it while
applying.

> 
> >   * 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.

Yeah, good idea. I did that while applying, and in doing so noticed 3 more
cases where we can replace a prose reference with a proper one.

> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for the review, both cma patches applied.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel