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

[Laurent Pinchart]

Hi Noralf,

On Wednesday, 13 September 2017 16:41:49 EEST Noralf Trønnes wrote:
> Den 13.09.2017 04.44, skrev Laurent Pinchart:
> > On Monday, 11 September 2017 19:37:44 EEST 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>
> >> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> ---
> >> 
> >> Changes:
> >> Addressed Daniel's comments.
> >> 
> >> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 25 +++++++++++++----------
> >> 
> >> 1 file changed, 14 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index d54a083..e2ca002
> >> 100644
> >> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c

[snip]

> >>  /**
> >>   * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
> >> - * @fb: DRM framebuffer
> >> - * @file: drm file
> >> + * @fb: framebuffer
> >> + * @file: DRM file
> > 
> > I wonder why framebuffer doesn't need a DRM while file does :-)
> 
> Yes this is haphazard.
> I think my problem is that I haven't been able to pick up a consistent
> "signal" from the DRM subsystem when it comes to how documentation
> should be written. Code patterns are fairly consistent and looks much
> the same including variable names, but documentation is more or less
> all over the place.

That doesn't surprise me too much, as documentation in DRM is pretty recent, 
and we never agreed to a documentation style.

> So I guess I need to come to grips with this, since this isn't the last
> time I have to write documentation. I have to make myself some rules
> that I can look at next time when all of this is forgotten.

Or, as the first person to address this problem, you could set your own rules 
that everybody else should then follow :-)

> Should description entries be Capitalized?
> My gut feeling is that most DRM docs don't do that, but when humans
> write for humans we do capatalize the start of sentences. So I guess
> that's the natural thing to do.
> 
> Should DRM objects start with DRM in the argument doc entries?
> 'DRM device' is a given since the kernel has many types of devices, but
> should we write 'DRM framebuffer' or 'Framebuffer'?

I would only mention DRM when the description could otherwise be 
misinterpreted, as in DRM device.

> How descriptive should the descriptions be?
> Let's take this example:
> 
> /**
>   * drm_gem_fb_prepare_fb() - Prepare gem framebuffer
>   * @plane: Which plane
>   * @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
>   * the exclusive fence and attaches it to plane state for the atomic helper
>   * to wait on.
> 
> Both the @state description and the body says that the fence will be
> attached to the plane state. Would it be better to just say:
> 
>   * @state: Plane state
> 
> Another way to ask this is:
> Should the documentation be terse or should it be repeating itself?
> 
> Then we have (copied from the cma library):
> 
>   * @plane: Which plane
> 
> Which is probably short for: The plane which we are operating/acting on.
> 
> More possibilities:
> 
>   * @plane: DRM plane
>   * @plane: Plane
>   * @plane: The plane for which a framebuffer is prepared for scanout
> 
> The text for the last one is also available when clicking on the
> &drm_plane_helper_funcs.prepare_fb link, so it's repeating something
> that is one click away.

Writing kerneldoc just for the sake of it is mostly pointless. We should write 
documentation to make it as useful and easy to consume as possible. Keeping 
that in mind,

 * @plane: Plane

isn't very useful. It's clear from the function argument name (and type) that 
it is a pointer to a plane. I would thus advocate for more detailed parameter 
descriptions.

> I always get comments on my documentation, so it's clearly something I
> need to find a way to improve.

I don't think it's specific to your documentation. You're doing a great job, 
for which I'm personally grateful. The fact that we haven't defined a 
documentation style in a similar way as we have defined a coding style is an 
issue of the DRM/KMS community.

> >>   * @handle: handle created
> >>   *
> >>   * Drivers can use this as their &drm_framebuffer_funcs->create_handle

[snip]

-- 
Regards,

Laurent Pinchart