On Wed, Nov 13, 2019 at 8:39 AM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote: > > Hi, > > > > > >> VRAM helpers use drm_gem_ttm_mmap(), which wraps ttm_bo_mmap_obj(). > > > > >> These changes should be transparent. > > > > > > > > > > There's still the issue that for dma-buf mmap vs drm mmap you use > > > > > different f_mapping, which means ttm's pte shootdown won't work correctly > > > > > for dma-buf mmaps. But yeah normal operation for ttm/vram helpers should > > > > > be fine. > > > > > > > > VRAM helpers don't support dmabufs. > > > > > > It's not that simple. Even when not supporting dma-buf export and > > > import it is still possible to create dma-bufs, import them into the > > > same driver (which doesn't actually export+import but just grabs a gem > > > object reference instead) and also to mmap them via prime/dma-buf code > > > path ... > > ... but after looking again I think we are all green here. Given that > only self-import works we'll only see vram gem objects in the mmap code > path, which should have everything set up correctly. Same goes for qxl. > > All other ttm drivers still use the old mmap code path, so all green > there too I think. Also I somehow doubt dma-buf mmap vs. drm mmap ends > up using different f_mapping, ttm code has a WARN_ON in ttm_bo_vm_open() > which would fire should that be the case. > > Do imported dma-bufs hit the drm mmap code path in the first place? > Wouldn't mmap be handled by the exporting driver? drm_gem_dmabuf_mmap -> obj->funcs->mmap -> ttm_bo_mmap_obj And I'm not seeing anyone adjusting vm_file->f_mapping anywhere here at all. Note to hit this you need userspace to - handle2fd on a buffer to create a dma-buf fd - call mmap directly on that dma-buf fd > > > Can ttm use the same trick msm uses? Now that ttm bo's are a gem object > > > superclass I think we should be able to switch vma->vm_file to > > > bo->base.filp, simliar to msm_gem_mmap_obj(), probably best done in > > > drm_gem_ttm_mmap(). > > > > bo->base.filp is the shmem inode file, and I'm not too sure how much shmem > > approves of us misappropriating the mapping. For shmem only objects it > > probably doesn't matter (since both gem mmaps and shmem mmaps will point > > at the same underlying struct page), but for vram/ttm/anything else the > > gem mmap might point into iomem, and shmem then might go boom trying to do > > stuff with that. > > Yes, agreeing here after wading through the code ... > > > I think having our own mapping would be the cleanest > > long-term approach ... > > You mean using per drm object (instead of per drm device) address spaces? Yup. But I think that would be quite a pile of work, since we need an anon inode for each gem bo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel