Hi Am 13.09.19 um 14:29 schrieb Gerd Hoffmann: > Factor out ttm vma setup to a new function. Reduces > code duplication a bit and allows to implement > &drm_gem_object_funcs.mmap in gem ttm helpers. > > Signed-off-by: Gerd Hoffmann <[email protected]> > --- > include/drm/ttm/ttm_bo_api.h | 8 ++++++ > drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++--------------- > 2 files changed, 33 insertions(+), 22 deletions(-) > > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h > index 43c4929a2171..88c652f49602 100644 > --- a/include/drm/ttm/ttm_bo_api.h > +++ b/include/drm/ttm/ttm_bo_api.h > @@ -734,6 +734,14 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct > ttm_buffer_object *bo); > int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, > struct ttm_bo_device *bdev); > > +/** > + * ttm_bo_mmap_vma_setup - initialize vma for ttm bo mmap > + * > + * @bo: The buffer object. > + * @vma: vma as input from the mmap method. > + */ > +void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct > vm_area_struct *vma); > + > void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot); > > void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot); > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 4aa007edffb0..7c0e85c10e0e 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -426,6 +426,29 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct > ttm_bo_device *bdev, > return bo; > } > > +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. As mentioned in my reply to patch 5, maybe re-using
ttm_fbdev_mmap() could help.
Best regards
Thomas
> +
> int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
> struct ttm_bo_device *bdev)
> {
> @@ -449,24 +472,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct
> *vma,
> if (unlikely(ret != 0))
> goto out_unref;
>
> - 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;
> + ttm_bo_mmap_vma_setup(bo, vma);
> return 0;
> out_unref:
> ttm_bo_put(bo);
> @@ -481,10 +487,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct
> ttm_buffer_object *bo)
>
> ttm_bo_get(bo);
>
> - vma->vm_ops = &ttm_bo_vm_ops;
> - vma->vm_private_data = bo;
> - vma->vm_flags |= VM_MIXEDMAP;
> - vma->vm_flags |= VM_IO | VM_DONTEXPAND;
> + ttm_bo_mmap_vma_setup(bo, vma);
> return 0;
> }
> EXPORT_SYMBOL(ttm_fbdev_mmap);
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
signature.asc
Description: OpenPGP digital signature
