Re: [PATCH 01/19] drm/doc: Clarify the dumb object interfaces

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 23, 2014 at 01:56:51PM +0100, Laurent Pinchart wrote:
> > > >        <para>
> > > >          Drivers must first validate the requested frame buffer
> > > >          parameters passed
> > > > @@ -1052,6 +998,71 @@ int max_width, max_height;</synopsis>
> > > >  	<function>drm_framebuffer_unregister_private</function>.
> > > >      </sect2>
> > > >      <sect2>
> > > > +      <title>Dumb GEM Objects</title>
> > > > +      <para>
> > > > +	The KMS API doesn't standardize backing storage object creation and
> > > 
> > > Strictly speaking isn't it the DRM API that's responsible for memory
> > > management, not the KMS API ?
> > 
> > The driver's private api is responsible for memory management, but the
> > crucial thing here is that the KMS ioctls don't mandate anything specific
> > (beyong that it needs to use uint32_t for handles).
> 
> Sure, but my point was that I believe memory management is the responsibility 
> of DRM, not KMS. It of course needs to be driver-specific.

Well imo the dumb ioctls are part of kms. DRM itself doesn't have any
memory management interfaces (at least if you ignore all the horror
stories around legacy ums/dri1 drivers). My reasons are:
- If you implement a kms driver you really should implement the dumb
  interfaces. Even when you have almost no memory management like the
  simpledrm driver.
- If you have a driver with memory management and command submission but
  no KMS, there's no reason at all to implement the dumb interfaces.
- ARM people abused dumb buffers for accelaration "because it worked", so
  imo moving it's documentation as far away as possible from the memory
  management section is a feature.

So the dumb stuff is a KMS interface to abstract away the lowest common
denominator between all kms drivers. Actually memory manament needs a real
interface, and is obviously separate.

> > > > +	leaves it to driver-specific ioctls. Furthermore actually creating a
> > > > +	buffer object even for GEM-based drivers is done through a
> > > > +	driver-specific ioctl - GEM only has a common userspace interface for
> > > > +	sharing and destroying objects. While not an issue for full-fledged
> > > > +	graphics stacks that include device-specific userspace components (in
> > > > +	libdrm for instance), this limit makes DRM-based early boot graphics
> > > > +	unnecessarily complex.
> > > > +      </para>
> > > 
> > > This feels a bit out of place, can't we leave the section where it was ?
> > 
> > Imo the justification for why we have the dumb ioctls should be here. And
> > I wanted to mention both that KMS doesn't mandate a particular bo
> > interface like GEM and that on top GEM wouldn't even provide a common
> > allocation function anyway.
> 
> I agree that a justification here is a good idea, but I would just add one 
> paragraph that references the dumb GEM objects section instead of scattering 
> GEM documentation around.

I've pretty much removed all mention of dumb gem objects ;-) There's one
mention of dumb_create left in the GEM/memory management section, but that
one is just an example for the lifetime and reference counting rules. So
not relevant.
> 
> > But besides that I think there's some room for improvement in the GEM
> > section to clarify what is the actual core interfaces, what is more helper
> > library in nature and what in GEM is more just a common design pattern for
> > driver ioctls but not specified in a mandatory way anywhere. E.g. atm all
> > drivers which implement a GEM interface (radeon, i915, ...) have a mostly
> > implicitly synchronizing buffer access interface, but there's nothing in
> > GEM mandating this. Or the usual confusing between TTM directly exposed to
> > userspace and TTM hidden behind a GEM-based ioctl interface.
> 
> I agree, the GEM section should be improved, and TTM documentation would be 
> nice as well. I'm lacking time though (as well as knowledge about TTM).

I have a few ideas, but I think I'll postpone this until I get around to
documenting the i915 GEM code a bit. Having a concrete driver to talk
about should help greatly to separate common concepts from artifacts of
the i915 implementation. I guess that review will also lead to some abi
cleanups to remove i915-ism from core gem.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch