Hi Gerd
Am 17.09.19 um 10:34 schrieb Gerd Hoffmann:
On Fri, Sep 13, 2019 at 03:05:34PM +0200, Thomas Zimmermann wrote:
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup);
To me, this function looks like an internal helper that should rather remain internal.
Well, I'm moving that to a helper exactly to avoid drm gem ttm helpers messing with ttm internals. To not them initialize vm_flags for example, and to avoid exporting ttm_bo_vm_ops. Also to make sure ttm bo vma's are initialized the same way no matter which code path was taken to mmap the object.
It may not be worth blocking on this, so
Acked-by: Thomas Zimmermann tzimmermann@suse.de
But I still think it's not a good interface because it exposes internal details.
Please consider another idea: how about splitting off the ttm_bo_get() and vma-flags setup of ttm_fbdev_mmap() into a separate function, like this:
void ttm_bo_mmap_refed(vma, bo) { ttm_bo_get(bo) ttm_bo_mmap_vma_setup(vma); } EXPORT_SYMBOL(ttm_bo_mmap_refed)
int ttm_fbdev_mmap(vma, bo) { if (vma->vm_pgoff != 0) return -EACCES; ttm_bo_mmap_refed(vma, bo); return 0; }
That would allow to keep _vma_setup() an internal function.
ttm_fbdev_mmap() sounds like it is only for fbdev and the only user is amdgpu. Can it be moved out of ttm entirely?
Best regards Thomas
As mentioned in my reply to patch 5, maybe re-using ttm_fbdev_mmap() could help.
No, the check in that function prevents that from working.
cheers, Gerd
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel