On Fri, Sep 15, 2017 at 12:45:14AM +0300, Laurent Pinchart wrote: > 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@xxxxxxxxxxxxxxxx> > > >> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx> > > >> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > >> --- > > >> > > >> 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 :-) Better is to actually switch all cases over. Which yes is a lot more work, but otherwise it won't converge. Since at least me personally I tend to just copy the patterns from the same file ... > > 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. There's unfortunately a certain amount of boilerplate in some of the argument docs. It happens, and as long as the main text explains it all, I think it's ok to have terse doc like this. If we put nothing, kernel-doc will complain (and those warnings are kinda nice to spot outdated docs). Thanks, Daniel > > 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 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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
