On Tue, Nov 12, 2019 at 10:49:21AM +0100, Thomas Zimmermann wrote:
Hi
Am 12.11.19 um 10:32 schrieb Daniel Vetter:
On Tue, Nov 12, 2019 at 10:26:44AM +0100, Thomas Zimmermann wrote:
Hi
Am 08.11.19 um 17:27 schrieb Daniel Vetter:
On Fri, Oct 25, 2019 at 09:30:42AM +0200, Daniel Vetter wrote:
On Thu, Oct 24, 2019 at 02:18:59PM -0500, Rob Herring wrote:
Commit c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs") introduced a GEM object mmap() hook which is expected to subtract the fake offset from vm_pgoff. However, for mmap() on dmabufs, there is not a fake offset.
To fix this, let's always call mmap() object callback with an offset of 0, and leave it up to drm_gem_mmap_obj() to remove the fake offset.
TTM still needs the fake offset, so we have to add it back until that's fixed.
Fixes: c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs") Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Rob Herring robh@kernel.org
v2:
- Move subtracting the fake offset out of mmap() obj callbacks.
I've tested shmem, but not ttm. Hopefully, I understood what's needed for TTM.
So unfortunately I'm already having regrets on this. We might even have broken some of the ttm drivers here.
Trouble is, if you need to shoot down userspace ptes of a bo (because it's getting moved or whatever), then we do that with unmap_mapping_range. Which means each bo needs to be mapping with a unique (offset, adress_space), or it won't work. By remapping all the bo to 0 we've broken this. We've also had this broken this for a while for the simplistic dma-buf mmap, since without any further action we'll reuse the address space of the dma-buf inode, not of the drm_device.
Strangely both etnaviv and msm have a comment to that effect - grep for unmap_mapping_range. But neither actually uses it.
Not exactly sure what's the best course of action here now.
Also adding Thomas Zimmermann, who's worked on all the vram helpers.
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 ...
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().
cheers, Gerd