diff mbox series

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

Message ID 20190913122908.784-5-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm: rework mmap() workflow | expand

Commit Message

Gerd Hoffmann Sept. 13, 2019, 12:29 p.m. UTC
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 <kraxel@redhat.com>
---
 include/drm/ttm/ttm_bo_api.h    |  8 ++++++
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 22 deletions(-)

Comments

Thomas Zimmermann Sept. 13, 2019, 12:56 p.m. UTC | #1
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 <kraxel@redhat.com>
> ---
>  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);
> +
>  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);
Just double-checking:  ttm_bo_mmap_vma_setup() will set VM_DONTDUMP in
vm_flags. Is that OK?

Best regards
Thomas

>  	return 0;
>  }
>  EXPORT_SYMBOL(ttm_fbdev_mmap);
>
Thomas Zimmermann Sept. 13, 2019, 1:05 p.m. UTC | #2
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 <kraxel@redhat.com>
> ---
>  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);
>
Gerd Hoffmann Sept. 17, 2019, 8:26 a.m. UTC | #3
On Fri, Sep 13, 2019 at 02:56:09PM +0200, Thomas Zimmermann wrote:
> 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 <kraxel@redhat.com>
> > ---
> >  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);
> > +
> >  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);
> Just double-checking:  ttm_bo_mmap_vma_setup() will set VM_DONTDUMP in
> vm_flags. Is that OK?

Should be ok, according to daniel vetter this most likely dates back to
the days where drivers exposed hardware registers mmap()able object.

But doing this as separate patch is probably a good idea ...

cheers,
  Gerd
Gerd Hoffmann Sept. 17, 2019, 8:34 a.m. UTC | #4
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
Thomas Zimmermann Sept. 17, 2019, 9:38 a.m. UTC | #5
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
>
Gerd Hoffmann Sept. 17, 2019, 9:54 a.m. UTC | #6
> 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)

ttm_bo_mmap_refed and ttm_bo_mmap_vma_setup are almost identical ...

But, yes, moving the ttm_bo_get call to ttm_bo_mmap_vma_setup probably
makes sense and hides this little detail to the outside.

> ttm_fbdev_mmap() sounds like it is only for fbdev and the only user is
> amdgpu. Can it be moved out of ttm entirely?

Exporting ttm_bo_mmap_vma_setup() allows to do that ;)

cheers,
  Gerd
diff mbox series

Patch

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);
+
 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);