Re: [PATCH v2 1/8] drm/vmwgfx: Use the common gem mmap instead of the custom code

2023-01-31 Thread "Maaz Mombasawala (VMware)
On 1/30/23 19:35, Zack Rusin wrote:
> From: Zack Rusin 
> 
> Before vmwgfx supported gem it needed to implement the entire mmap logic
> explicitly. With GEM support that's not needed and the generic code
> can be used by simply setting the vm_ops to vmwgfx specific ones on the
> gem object itself.
> 
> Removes a lot of code from vmwgfx without any functional difference.
> 
> Signed-off-by: Zack Rusin 
> Reviewed-by: Thomas Zimmermann 
> Reviewed-by: Martin Krastev 
> ---
>  drivers/gpu/drm/vmwgfx/Makefile  |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   6 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c  |   8 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 110 ---
>  5 files changed, 10 insertions(+), 118 deletions(-)
>  delete mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> 
> diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
> index 2a644f035597..e94479d9cd5b 100644
> --- a/drivers/gpu/drm/vmwgfx/Makefile
> +++ b/drivers/gpu/drm/vmwgfx/Makefile
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>   vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
> - vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
> + vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o \
>   vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
>   vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
>   vmwgfx_surface.o vmwgfx_prime.o vmwgfx_mob.o vmwgfx_shader.o \
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index bd02cb0e6837..e0c2e3748015 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1566,7 +1566,7 @@ static const struct file_operations vmwgfx_driver_fops 
> = {
>   .open = drm_open,
>   .release = drm_release,
>   .unlocked_ioctl = vmw_unlocked_ioctl,
> - .mmap = vmw_mmap,
> + .mmap = drm_gem_mmap,
>   .poll = drm_poll,
>   .read = drm_read,
>  #if defined(CONFIG_COMPAT)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 5acbf5849b27..4dfa5044a9e7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -1053,12 +1053,6 @@ vmw_is_cursor_bypass3_enabled(const struct vmw_private 
> *dev_priv)
>   return (vmw_fifo_caps(dev_priv) & SVGA_FIFO_CAP_CURSOR_BYPASS_3) != 0;
>  }
>  
> -/**
> - * TTM glue - vmwgfx_ttm_glue.c
> - */
> -
> -extern int vmw_mmap(struct file *filp, struct vm_area_struct *vma);
> -
>  /**
>   * TTM buffer object driver - vmwgfx_ttm_buffer.c
>   */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> index ce609e7d758f..ba4ddd9f7a7e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> @@ -103,6 +103,13 @@ static struct sg_table 
> *vmw_gem_object_get_sg_table(struct drm_gem_object *obj)
>   return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages, 
> vmw_tt->dma_ttm.num_pages);
>  }
>  
> +static const struct vm_operations_struct vmw_vm_ops = {
> + .pfn_mkwrite = vmw_bo_vm_mkwrite,
> + .page_mkwrite = vmw_bo_vm_mkwrite,
> + .fault = vmw_bo_vm_fault,
> + .open = ttm_bo_vm_open,
> + .close = ttm_bo_vm_close,
> +};
>  
>  static const struct drm_gem_object_funcs vmw_gem_object_funcs = {
>   .free = vmw_gem_object_free,
> @@ -115,6 +122,7 @@ static const struct drm_gem_object_funcs 
> vmw_gem_object_funcs = {
>   .vmap = drm_gem_ttm_vmap,
>   .vunmap = drm_gem_ttm_vunmap,
>   .mmap = drm_gem_ttm_mmap,
> + .vm_ops = &vmw_vm_ops,
>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> deleted file mode 100644
> index 265f7c48d856..
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> +++ /dev/null
> @@ -1,110 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0 OR MIT
> -/**
> - *
> - * Copyright 2009-2011 VMware, Inc., Palo Alto, CA., USA
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the
> - * "Software"), to deal in the Software without restriction, including
> - * without limitation the rights to use, copy, modify, merge, publish,
> - * distribute, sub license, and/or sell copies of the Software, and to
> - * permit persons to whom the Software is furnished to do so, subject to
> - * the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the
> - * next paragraph) shall be included in all copies or substantial portions
> - * of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMP

Re: [PATCH v2 1/8] drm/vmwgfx: Use the common gem mmap instead of the custom code

2023-01-31 Thread Martin Krastev (VMware)

From: Martin Krastev 


LGTM
Reviewed-by: Martin Krastev 


Regards,
Martin


On 31.01.23 г. 5:35 ч., Zack Rusin wrote:

From: Zack Rusin 

Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be used by simply setting the vm_ops to vmwgfx specific ones on the
gem object itself.

Removes a lot of code from vmwgfx without any functional difference.

Signed-off-by: Zack Rusin 
Reviewed-by: Thomas Zimmermann 
Reviewed-by: Martin Krastev 
---
  drivers/gpu/drm/vmwgfx/Makefile  |   2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   6 --
  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c  |   8 ++
  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 110 ---
  5 files changed, 10 insertions(+), 118 deletions(-)
  delete mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c

diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 2a644f035597..e94479d9cd5b 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -1,7 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
  vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
-   vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
+   vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o \
vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
vmwgfx_surface.o vmwgfx_prime.o vmwgfx_mob.o vmwgfx_shader.o \
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bd02cb0e6837..e0c2e3748015 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1566,7 +1566,7 @@ static const struct file_operations vmwgfx_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = vmw_unlocked_ioctl,
-   .mmap = vmw_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
  #if defined(CONFIG_COMPAT)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 5acbf5849b27..4dfa5044a9e7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1053,12 +1053,6 @@ vmw_is_cursor_bypass3_enabled(const struct vmw_private 
*dev_priv)
return (vmw_fifo_caps(dev_priv) & SVGA_FIFO_CAP_CURSOR_BYPASS_3) != 0;
  }
  
-/**

- * TTM glue - vmwgfx_ttm_glue.c
- */
-
-extern int vmw_mmap(struct file *filp, struct vm_area_struct *vma);
-
  /**
   * TTM buffer object driver - vmwgfx_ttm_buffer.c
   */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index ce609e7d758f..ba4ddd9f7a7e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -103,6 +103,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct 
drm_gem_object *obj)
return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages, 
vmw_tt->dma_ttm.num_pages);
  }
  
+static const struct vm_operations_struct vmw_vm_ops = {

+   .pfn_mkwrite = vmw_bo_vm_mkwrite,
+   .page_mkwrite = vmw_bo_vm_mkwrite,
+   .fault = vmw_bo_vm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+};
  
  static const struct drm_gem_object_funcs vmw_gem_object_funcs = {

.free = vmw_gem_object_free,
@@ -115,6 +122,7 @@ static const struct drm_gem_object_funcs 
vmw_gem_object_funcs = {
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
.mmap = drm_gem_ttm_mmap,
+   .vm_ops = &vmw_vm_ops,
  };
  
  /**

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
deleted file mode 100644
index 265f7c48d856..
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ /dev/null
@@ -1,110 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0 OR MIT
-/**
- *
- * Copyright 2009-2011 VMware, Inc., Palo Alto, CA., USA
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A

[PATCH v2 1/8] drm/vmwgfx: Use the common gem mmap instead of the custom code

2023-01-30 Thread Zack Rusin
From: Zack Rusin 

Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be used by simply setting the vm_ops to vmwgfx specific ones on the
gem object itself.

Removes a lot of code from vmwgfx without any functional difference.

Signed-off-by: Zack Rusin 
Reviewed-by: Thomas Zimmermann 
Reviewed-by: Martin Krastev 
---
 drivers/gpu/drm/vmwgfx/Makefile  |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   6 --
 drivers/gpu/drm/vmwgfx/vmwgfx_gem.c  |   8 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 110 ---
 5 files changed, 10 insertions(+), 118 deletions(-)
 delete mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c

diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 2a644f035597..e94479d9cd5b 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
-   vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
+   vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o \
vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
vmwgfx_surface.o vmwgfx_prime.o vmwgfx_mob.o vmwgfx_shader.o \
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bd02cb0e6837..e0c2e3748015 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1566,7 +1566,7 @@ static const struct file_operations vmwgfx_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = vmw_unlocked_ioctl,
-   .mmap = vmw_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
 #if defined(CONFIG_COMPAT)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 5acbf5849b27..4dfa5044a9e7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1053,12 +1053,6 @@ vmw_is_cursor_bypass3_enabled(const struct vmw_private 
*dev_priv)
return (vmw_fifo_caps(dev_priv) & SVGA_FIFO_CAP_CURSOR_BYPASS_3) != 0;
 }
 
-/**
- * TTM glue - vmwgfx_ttm_glue.c
- */
-
-extern int vmw_mmap(struct file *filp, struct vm_area_struct *vma);
-
 /**
  * TTM buffer object driver - vmwgfx_ttm_buffer.c
  */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index ce609e7d758f..ba4ddd9f7a7e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -103,6 +103,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct 
drm_gem_object *obj)
return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages, 
vmw_tt->dma_ttm.num_pages);
 }
 
+static const struct vm_operations_struct vmw_vm_ops = {
+   .pfn_mkwrite = vmw_bo_vm_mkwrite,
+   .page_mkwrite = vmw_bo_vm_mkwrite,
+   .fault = vmw_bo_vm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+};
 
 static const struct drm_gem_object_funcs vmw_gem_object_funcs = {
.free = vmw_gem_object_free,
@@ -115,6 +122,7 @@ static const struct drm_gem_object_funcs 
vmw_gem_object_funcs = {
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
.mmap = drm_gem_ttm_mmap,
+   .vm_ops = &vmw_vm_ops,
 };
 
 /**
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
deleted file mode 100644
index 265f7c48d856..
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ /dev/null
@@ -1,110 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0 OR MIT
-/**
- *
- * Copyright 2009-2011 VMware, Inc., Palo Alto, CA., USA
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAM