[PATCH] drm/exynos: use a new anon file for exynos gem mmaper

[Daniel Vetter]

On Thu, Sep 11, 2014 at 11:16:53AM +0900, Inki Dae wrote:
> On 2014년 09월 10일 18:01, Daniel Vetter wrote:
> > Ok I've stumbled over the exynos mmap stuff again while cleaning up
> > drm legacy cruft and I just don't get what you're doing and why
> > exactly exynos needs to be special.
> > 
> > _All_ other drm drivers happily get along with the vma offset manger
> > stuff to handle mmaps, but somehow exynos does something really crazy.
> 
> We are also using the vma offset manager stuff. We just added direct
> mapping interface specific to Exynos additionally.
> 
> > 
> > Can you please explain the design justification for this and why
> > switching to the standard gem mmap support isn't possible?
> 
> As I mentioned above, we are using the standard gem mmap support.
> However, the standard gem mmap is required for on-demand paging mostly
> suitable for Desktop. In case of ARM SoC, whole memory region requested
> by userspace would be allocated once the gem creation interface is
> called. In this case, it wouldn't need to map userspace with physical
> page in page fault handler, and the use of the vma offset manager stuff
> would be unnecessary step.

You don't need to do demand paging at all, you can simply put in all the
ptes in one go and then never unbind it. So strictly speaking you don't
need to roll your own mmap, but otoh other drivers (including i915) do
their own special mmap too. And since you now have it you must support it
forever anyway.

Aside: We have patches floating around for i915 to prefault aggressively,
so you're not the only ones who noticed the faulting overhead. ARM SoC
really aren't all that special compared to traditional desktop gpus, so if
you stumble over such issues please raise them on dri-devel so that we
could look into useful generic solutions next time around.

> For the same question, Al Viro did,
> http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html
> 
> Is there any issue I am missing , that could be incurred by Exynos codes?

I've stumbled over it again because you're reusing the drm_vm_open_locked
function, which really should just be an implementation detail of the core
drm/gem mmap support.

If you want to roll your own mmap (and that's ok, i915 has it and ttm also
does it) then imo you should not reuse any of the core mmap code, but
implement your own set of vm_ops. You don't need a faul handler for this
(since it will never fault), and open/close would just grabbing/dropping a
reference of the underlying gem object. Instead of trying to reuse the
same vm_ops you use for normal gem mmaps, which just doesn't make a lot of
sense to me.

If exynos stops using drm_vm_open_locked then I can move it into the new
drm_internal.h header since this function really should be private to
drm.ko.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch