Re: [PATCH 4/8] drm/ttm: factor out ttm_bo_mmap_vma_setup

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.

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