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

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 11, 2014 at 04:22:00PM +0900, Inki Dae wrote:
> On 2014년 09월 11일 15:22, Daniel Vetter wrote:
> > 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.
> 
> Ah, right. that is critical issue. I shouldn't had used
> drm_vm_open_locked. Sorry for this.
> 
> > 
> > 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.
> 
> Sorry for blocking you. I will fix it soon.

No need to rush really, but if you fix this please cc me so that I can
throw the header cleanup patch on top.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel