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@xxxxxxxxxxxxxxxx>
> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
