Re: [PATCH 1/3] drm/i915: Add kerneldoc for intel_pipe_update_{start, end}

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Nov 03, 2014 at 03:26:36PM -0200, Paulo Zanoni wrote:
> 2014-11-03 10:33 GMT-02:00 Daniel Vetter <daniel@ffwll.ch>:
> > On Tue, Oct 28, 2014 at 03:10:12PM +0200, Ander Conselvan de Oliveira wrote:
> >> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_sprite.c | 25 +++++++++++++++++++++++++
> >>  1 file changed, 25 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 8b80d68..f9ddedc 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -46,6 +46,22 @@ static int usecs_to_scanlines(const struct drm_display_mode *mode, int usecs)
> >>       return DIV_ROUND_UP(usecs * mode->crtc_clock, 1000 * mode->crtc_htotal);
> >>  }
> >>
> >> +/**
> >> + * intel_pipe_update_start() - start update of a set of display registers
> >> + * @crtc: the crtc of which the registers are going to be updated
> >> + * @start_vbl_count: vblank counter return pointer used for error checking
> >> + *
> >> + * Mark the start of an update to pipe registers that should be updated
> >> + * atomically regarding vblank. If the next vblank will happens within
> >> + * the next 100 us, this function waits until the vblank passes.
> >> + *
> >> + * After a successful call to this function, interrupts will be disabled
> >> + * until a subsequent call to intel_pipe_update_end(). That is done to
> >> + * avoid random delays. The value written to @start_vbl_count should be
> >> + * supplied to intel_pipe_update_end() for error checking.
> >> + *
> >> + * Return: true if the call was successful
> >> + */
> >
> > It's nice that people now go overboard with kerneldoc, but I think we need
> > to strike a good balance. And in general I think documenting static inline
> > functions isn't worth it - they really should be self-explanatory as-is.
> 
> But patch 3 exports these functions and uses them from another file.

Ah, I've missed that, comment retracted ...
> 
> >
> > Documentation is imo only really useful for the bigger stuff, which
> > usually means it's used in a few places all over. So non-static functions.
> 
> The comments he introduced are useful and helped me review patch 3
> without having to look at the function implementation and waste 20
> minutes wondering what it was supposed to do.
> 
> To me, this patch is an improvement to the codebase, so with or
> without the extra '*' chars:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

... and patch merged.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx