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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
