[Intel-gfx] [PATCH 4/9] drm: Begin an API for in-kernel clients

[Daniel Vetter]

On Wed, May 23, 2018 at 11:45:00PM +0200, Thomas Hellstrom wrote:
> Hi, Noralf.
> 
> A couple of issues below:
> 
> On 05/23/2018 04:34 PM, Noralf Trønnes wrote:
> > This the beginning of an API for in-kernel clients.
> > First out is a way to get a framebuffer backed by a dumb buffer.
> > 
> > Only GEM drivers are supported.
> > The original idea of using an exported dma-buf was dropped because it
> > also creates an anonomous file descriptor which doesn't work when the
> > buffer is created from a kernel thread. The easy way out is to use
> > drm_driver.gem_prime_vmap to get the virtual address, which requires a
> > GEM object. This excludes the vmwgfx driver which is the only non-GEM
> > driver apart from the legacy ones. A solution for vmwgfx will have to be
> > worked out later if it wants to support the client API which it probably
> > will when we have a bootsplash client.
> 
> Couldn't you add vmap() and  vunmap() to the dumb buffer API for in-kernel
> use rather than using GEM directly?
> 
> But the main issue is pinning. It looks like the buffers are going to be
> vmapped() for a long time, which requires pinning, and that doesn't work for
> some drivers when they bind the framebuffer to a plane, since that might
> require pinning in another memory region and the vmap would have to be torn
> down. Besides, buffer pinning should really be avoided if possible:
> 
> Since we can't page-fault vmaps, and setting up / tearing down vmaps is
> potentially an expensive operation, could we perhaps have a mapping api that
> allows the driver to cache vmaps?
> 
> vmap()   // Indicates that we want to map a bo
> begin_access() // Returns a virtual address which may vary between calls.
> Allows access. A fast operation. Behind the lines pins / reserves the bo and
> returns a cached vmap if the bo didn't move since last begin_access(), which
> is the typical case.
> end_access() // Disable access. Unpins / unreserves the bo.
> vunmap_cached() //Indicates that the map is no longer needed. The driver can
> release the cached map.
> 
> The idea is that the API client would wrap all bo map accesses with
> begin_access() end_access(), allowing for the bo to be moved in between.

So originally my ideas for the cpu side dma-buf interfaces where all meant
to handle this. But then the first implementations bothered with none of
this, but instead expected that stuff is pinned, and vmap Just Works.

Which yeah doesn't work for vmwgfx and is a pain in a few other cases.

I agree it'd be nice to fix all this, but it's also not a problem that
this patch set here started. And since it's all optional (and vmwgfx isn't
even using the current fb helper code) I think it's reasonable to address
this post-merge (if someone gets around to it ever). What we'd need is is
a fallback for when vmap doesn't exist (for fbdev that probably means a
vmalloc'ed buffer + manual uploads, because fbdev), plus making sure
dma-buf implementations actually implement it.

Wrt tying this to gem hooks: I don't think this should be needed, I
thought we've discussed a way to get at the handle2fd logic without having
to end up with an fd, but directly return the dma-buf instead. Then we
could use the dma-buf vmap interfaces instead of the gem ones.

But getting there means we need to rework all the dma-buf export functions
to move the fd_install into core code, so it's possible to make it
optional. Not difficult work, but I think not necessary for the first
merged version either since fairly auxiliary. Also right now there's no
demand for this, since vmwgfx isn't using the fb helpers (yet). But we do
have a solid plan to remove the gem dependency at least.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch