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

[Inki Dae <inki.dae@samsung.com>]

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.

Thanks,
Inki Dae

> 
> Thanks, Daniel
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel