On 05/24/2018 10:32 AM, Daniel Vetter wrote:
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.
My argument here is that, If I understand Noralf, this is intended to be
an API exported outside of drm. In that case we shouldn't replicate the
assumed behaviour of incomplete dma-buf implementations in a new API.
Also the fact that vmwgfx currently isn't using the fbdev helpers isn't
a good argument to design an API so that vmwgfx can _never_ use the
fbdev helpers. The reason we aren't using them is that the kms
implementation was so old that we didn't implement the necessary helper
callbacks...
Also, I might be misunderstanding the code a bit, but I doubt that
vmwgfx is the only hardware with pinning restrictions on the
framebuffer? I was under the assumption that most discrete hardware
required the framebuffer to be pinned in VRAM?
So the important question is, Is this a set of helpers for shared-memory
GEM drivers to implement fbdev? Then I wouldn't bother, If it's intended
to become an API for clients outside of DRM, then I would have to insist
on the API being changed to reflect that.
/Thomas
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
