Re: [PATCH v3 01/22] drm: Add GEM backed framebuffer library

[Daniel Vetter <daniel@ffwll.ch>]

On Sat, Aug 19, 2017 at 04:46:30PM +0200, Noralf Trønnes wrote:
> 
> Den 16.08.2017 22.50, skrev Laurent Pinchart:
> > Hi Noralf,
> > 
> > One additional comment.
> > 
> > On Wednesday 16 Aug 2017 23:37:54 Laurent Pinchart wrote:
> > > On Sunday 13 Aug 2017 15:31:44 Noralf Trønnes wrote:
> > > > This library provides helpers for drivers that don't subclass
> > > > drm_framebuffer and are backed by drm_gem_object. The code is
> > > > taken from drm_fb_cma_helper.
> > > > 
> > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > ---
> > > > 
> > > >   Documentation/gpu/drm-kms-helpers.rst        |   9 +
> > > >   drivers/gpu/drm/Makefile                     |   2 +-
> > > >   drivers/gpu/drm/drm_gem_framebuffer_helper.c | 283 +++++++++++++++++++++
> > > >   include/drm/drm_framebuffer.h                |   7 +
> > > >   include/drm/drm_gem_framebuffer_helper.h     |  37 ++++
> > > >   5 files changed, 337 insertions(+), 1 deletion(-)
> > > >   create mode 100644 drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > >   create mode 100644 include/drm/drm_gem_framebuffer_helper.h
> > > [snip]
> > > 
> > > > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > > b/drivers/gpu/drm/drm_gem_framebuffer_helper.c new file mode 100644
> > > > index 0000000..068a630
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > > @@ -0,0 +1,283 @@
> > > [snip]
> > > 
> > > > +/**
> > > > + * DOC: overview
> > > > + *
> > > > + * This library provides helpers for drivers that don't subclass
> > > > + * &drm_framebuffer and and use &drm_gem_object for their backing
> > > > storage.
> > > s/and and/and/
> > > 
> > > > + *
> > > > + * Drivers without additional needs to validate framebuffers can simply
> > > > use
> > > > + * drm_gem_fb_create() and everything is wired up automatically. But all
> > > > + * parts can be used individually.
> > > A sentence should not start by "but". How about "Other drivers can use all
> > > parts independently." ?
> > > 
> > > > + */
> > We now have the GEM CMA helpers, the GEM FB helpers and the FB CMA helper. It
> > starts getting very confusing for driver authors. The overview documentation
> > should explain how they all interact and which helpers a driver can/should use
> > in the different cases.
> 
> Personally I rarely read the documentation, I just read the code, unless
> for complex codepaths, but they seldom have good documentation.
> 
> However documentation that ties things together, like you suggest, I
> also think is valuable. The problem is that it's difficult to write it,
> and that's probably why it's lacking almost everywhere. But I must say
> that Daniel really is persistent in trying to fix this.
> 
> The bottom line for me is that I'm not capable of writing such docs.
> I both lack the knowledge about the drm machinery and the skill to
> write for humans.

Yeah I've practically volunteered myself already to clean up the gem/prime
docs after your cleanups have landed. In case I don't, please ping me
about that. I'll call in requests for review in return :-)
-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