dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/etnaviv: Implement mmap as GEM object function
@ 2021-06-24  8:58 Thomas Zimmermann
  2021-06-24  9:11 ` Lucas Stach
  2021-07-06 16:34 ` Lucas Stach
  0 siblings, 2 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2021-06-24  8:58 UTC (permalink / raw)
  To: l.stach, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: Thomas Zimmermann, etnaviv, dri-devel

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

The respective etnaviv functions are being removed. The file_operations
structure fops is now being created by the helper macro
DEFINE_DRM_GEM_FOPS().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
 drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
 4 files changed, 7 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index f0a07278ad04..7dcc6392792d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
 	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
 };
 
-static const struct file_operations fops = {
-	.owner              = THIS_MODULE,
-	.open               = drm_open,
-	.release            = drm_release,
-	.unlocked_ioctl     = drm_ioctl,
-	.compat_ioctl       = drm_compat_ioctl,
-	.poll               = drm_poll,
-	.read               = drm_read,
-	.llseek             = no_llseek,
-	.mmap               = etnaviv_gem_mmap,
-};
+DEFINE_DRM_GEM_FOPS(fops);
 
 static const struct drm_driver etnaviv_drm_driver = {
 	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
@@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
-	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
+	.gem_prime_mmap     = drm_gem_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init       = etnaviv_debugfs_init,
 #endif
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 003288ebd896..049ae87de9be 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -47,12 +47,9 @@ struct etnaviv_drm_private {
 int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		struct drm_file *file);
 
-int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
 struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
 int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
-int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
-			   struct vm_area_struct *vma);
 struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
 	struct dma_buf_attachment *attach, struct sg_table *sg);
 int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index b8fa6ed3dd73..8f1b5af47dd6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
 {
 	pgprot_t vm_page_prot;
 
-	vma->vm_flags &= ~VM_PFNMAP;
-	vma->vm_flags |= VM_MIXEDMAP;
+	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
 
 	vm_page_prot = vm_get_page_prot(vma->vm_flags);
 
@@ -154,19 +153,11 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
 	return 0;
 }
 
-int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+static int etnaviv_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
 {
-	struct etnaviv_gem_object *obj;
-	int ret;
-
-	ret = drm_gem_mmap(filp, vma);
-	if (ret) {
-		DBG("mmap failed: %d", ret);
-		return ret;
-	}
+	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 
-	obj = to_etnaviv_bo(vma->vm_private_data);
-	return obj->ops->mmap(obj, vma);
+	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
 }
 
 static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
@@ -567,6 +558,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
 	.unpin = etnaviv_gem_prime_unpin,
 	.get_sg_table = etnaviv_gem_prime_get_sg_table,
 	.vmap = etnaviv_gem_prime_vmap,
+	.mmap = etnaviv_gem_mmap,
 	.vm_ops = &vm_ops,
 };
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index d741b1d735f7..6d8bed9c739d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -34,19 +34,6 @@ int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
 	return 0;
 }
 
-int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
-			   struct vm_area_struct *vma)
-{
-	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
-	int ret;
-
-	ret = drm_gem_mmap_obj(obj, obj->size, vma);
-	if (ret < 0)
-		return ret;
-
-	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
-}
-
 int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
 {
 	if (!obj->import_attach) {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function
  2021-06-24  8:58 [PATCH] drm/etnaviv: Implement mmap as GEM object function Thomas Zimmermann
@ 2021-06-24  9:11 ` Lucas Stach
  2021-06-24 10:47   ` Thomas Zimmermann
  2021-07-06 16:34 ` Lucas Stach
  1 sibling, 1 reply; 7+ messages in thread
From: Lucas Stach @ 2021-06-24  9:11 UTC (permalink / raw)
  To: Thomas Zimmermann, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: etnaviv, dri-devel

Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
> 
> The respective etnaviv functions are being removed. The file_operations
> structure fops is now being created by the helper macro
> DEFINE_DRM_GEM_FOPS().
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
>  4 files changed, 7 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index f0a07278ad04..7dcc6392792d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>  	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>  };
>  
> -static const struct file_operations fops = {
> -	.owner              = THIS_MODULE,
> -	.open               = drm_open,
> -	.release            = drm_release,
> -	.unlocked_ioctl     = drm_ioctl,
> -	.compat_ioctl       = drm_compat_ioctl,
> -	.poll               = drm_poll,
> -	.read               = drm_read,
> -	.llseek             = no_llseek,
> -	.mmap               = etnaviv_gem_mmap,
> -};
> +DEFINE_DRM_GEM_FOPS(fops);
>  
>  static const struct drm_driver etnaviv_drm_driver = {
>  	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
> @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>  	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>  	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>  	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> -	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
> +	.gem_prime_mmap     = drm_gem_prime_mmap,
>  #ifdef CONFIG_DEBUG_FS
>  	.debugfs_init       = etnaviv_debugfs_init,
>  #endif
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 003288ebd896..049ae87de9be 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
>  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>  		struct drm_file *file);
>  
> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>  int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>  struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
>  int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
> -			   struct vm_area_struct *vma);
>  struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  	struct dma_buf_attachment *attach, struct sg_table *sg);
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index b8fa6ed3dd73..8f1b5af47dd6 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  {
>  	pgprot_t vm_page_prot;
>  
> -	vma->vm_flags &= ~VM_PFNMAP;
> -	vma->vm_flags |= VM_MIXEDMAP;
> +	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;

I don't fully understand why this change is needed and the commit log
is silent about it. Excuse my ignorance, but can you please explain the
reasoning behind this change?

Regards,
Lucas

>  
>  	vm_page_prot = vm_get_page_prot(vma->vm_flags);
>  
> @@ -154,19 +153,11 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  	return 0;
>  }
>  
> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> +static int etnaviv_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>  {
> -	struct etnaviv_gem_object *obj;
> -	int ret;
> -
> -	ret = drm_gem_mmap(filp, vma);
> -	if (ret) {
> -		DBG("mmap failed: %d", ret);
> -		return ret;
> -	}
> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  
> -	obj = to_etnaviv_bo(vma->vm_private_data);
> -	return obj->ops->mmap(obj, vma);
> +	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
>  }
>  
>  static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
> @@ -567,6 +558,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>  	.unpin = etnaviv_gem_prime_unpin,
>  	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>  	.vmap = etnaviv_gem_prime_vmap,
> +	.mmap = etnaviv_gem_mmap,
>  	.vm_ops = &vm_ops,
>  };
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index d741b1d735f7..6d8bed9c739d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -34,19 +34,6 @@ int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>  	return 0;
>  }
>  
> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
> -			   struct vm_area_struct *vma)
> -{
> -	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> -	int ret;
> -
> -	ret = drm_gem_mmap_obj(obj, obj->size, vma);
> -	if (ret < 0)
> -		return ret;
> -
> -	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
> -}
> -
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
>  {
>  	if (!obj->import_attach) {



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function
  2021-06-24  9:11 ` Lucas Stach
@ 2021-06-24 10:47   ` Thomas Zimmermann
  2021-06-24 10:50     ` Lucas Stach
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Zimmermann @ 2021-06-24 10:47 UTC (permalink / raw)
  To: Lucas Stach, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: etnaviv, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 7583 bytes --]

Hi

Am 24.06.21 um 11:11 schrieb Lucas Stach:
> Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
>> Moving the driver-specific mmap code into a GEM object function allows
>> for using DRM helpers for various mmap callbacks.
>>
>> The respective etnaviv functions are being removed. The file_operations
>> structure fops is now being created by the helper macro
>> DEFINE_DRM_GEM_FOPS().
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
>>   drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
>>   drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
>>   4 files changed, 7 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> index f0a07278ad04..7dcc6392792d 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>>   	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>>   };
>>   
>> -static const struct file_operations fops = {
>> -	.owner              = THIS_MODULE,
>> -	.open               = drm_open,
>> -	.release            = drm_release,
>> -	.unlocked_ioctl     = drm_ioctl,
>> -	.compat_ioctl       = drm_compat_ioctl,
>> -	.poll               = drm_poll,
>> -	.read               = drm_read,
>> -	.llseek             = no_llseek,
>> -	.mmap               = etnaviv_gem_mmap,
>> -};
>> +DEFINE_DRM_GEM_FOPS(fops);
>>   
>>   static const struct drm_driver etnaviv_drm_driver = {
>>   	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
>> @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>>   	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>   	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>   	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
>> -	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
>> +	.gem_prime_mmap     = drm_gem_prime_mmap,
>>   #ifdef CONFIG_DEBUG_FS
>>   	.debugfs_init       = etnaviv_debugfs_init,
>>   #endif
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>> index 003288ebd896..049ae87de9be 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>> @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
>>   int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>>   		struct drm_file *file);
>>   
>> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>>   int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>>   struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
>>   int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
>> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
>> -			   struct vm_area_struct *vma);
>>   struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>>   	struct dma_buf_attachment *attach, struct sg_table *sg);
>>   int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> index b8fa6ed3dd73..8f1b5af47dd6 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>>   {
>>   	pgprot_t vm_page_prot;
>>   
>> -	vma->vm_flags &= ~VM_PFNMAP;
>> -	vma->vm_flags |= VM_MIXEDMAP;
>> +	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
> 
> I don't fully understand why this change is needed and the commit log
> is silent about it. Excuse my ignorance, but can you please explain the
> reasoning behind this change?

Sure, sorry for being brief.

I worked on cleaning up the deprecated gem_prime_* callbacks in struct 
drm_driver. These are supposed to be GEM object functions. The only 
obsolete gem prime callback in drm_driver is gem_prime_mmap.

Currently drivers
  implement mmap in via callbacks in struct file_operations.mmap, struct 
drm_driver.gem_prime_mmap and struct drm_gem_object_funcs.mmap (and 
sometimes an additional mmap for fbdev emulation). That's way too much 
duplication. The correct place to implement mmap is in struct 
drm_gem_object_funcs.

I'm consolidating DRM mmap code in struct drm_gem_object_funcs.mmap. 
There's even a fixme comment about this. [1] With the cleaned up mmap, 
DRM drivers can switch to DRM helpers and macros for all other instances 
of mmap.

And only a
  handful of drivers if left to convert. For these final drivers (e.g., 
etnaviv) I replace the driver code with the generic helpers and move the 
specific flags and setup into the GEM object's mmap function.

Once finished, all DRM drivers will implement gem_prime_mmap with 
drm_gem_prime_mmap().
  The core code can be further simplified from there and this will allow 
to remove gem_prime_mmap and the occasional fbdev mmap.


Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/latest/source/include/drm/drm_drv.h#L388

> 
> Regards,
> Lucas
> 
>>   
>>   	vm_page_prot = vm_get_page_prot(vma->vm_flags);
>>   
>> @@ -154,19 +153,11 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>>   	return 0;
>>   }
>>   
>> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> +static int etnaviv_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>>   {
>> -	struct etnaviv_gem_object *obj;
>> -	int ret;
>> -
>> -	ret = drm_gem_mmap(filp, vma);
>> -	if (ret) {
>> -		DBG("mmap failed: %d", ret);
>> -		return ret;
>> -	}
>> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>>   
>> -	obj = to_etnaviv_bo(vma->vm_private_data);
>> -	return obj->ops->mmap(obj, vma);
>> +	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
>>   }
>>   
>>   static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
>> @@ -567,6 +558,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>>   	.unpin = etnaviv_gem_prime_unpin,
>>   	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>>   	.vmap = etnaviv_gem_prime_vmap,
>> +	.mmap = etnaviv_gem_mmap,
>>   	.vm_ops = &vm_ops,
>>   };
>>   
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> index d741b1d735f7..6d8bed9c739d 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> @@ -34,19 +34,6 @@ int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>>   	return 0;
>>   }
>>   
>> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
>> -			   struct vm_area_struct *vma)
>> -{
>> -	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>> -	int ret;
>> -
>> -	ret = drm_gem_mmap_obj(obj, obj->size, vma);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
>> -}
>> -
>>   int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
>>   {
>>   	if (!obj->import_attach) {
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function
  2021-06-24 10:47   ` Thomas Zimmermann
@ 2021-06-24 10:50     ` Lucas Stach
  2021-06-24 12:22       ` Thomas Zimmermann
  2021-07-05 11:47       ` Thomas Zimmermann
  0 siblings, 2 replies; 7+ messages in thread
From: Lucas Stach @ 2021-06-24 10:50 UTC (permalink / raw)
  To: Thomas Zimmermann, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: etnaviv, dri-devel

Am Donnerstag, dem 24.06.2021 um 12:47 +0200 schrieb Thomas Zimmermann:
> Hi
> 
> Am 24.06.21 um 11:11 schrieb Lucas Stach:
> > Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
> > > Moving the driver-specific mmap code into a GEM object function allows
> > > for using DRM helpers for various mmap callbacks.
> > > 
> > > The respective etnaviv functions are being removed. The file_operations
> > > structure fops is now being created by the helper macro
> > > DEFINE_DRM_GEM_FOPS().
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > ---
> > >   drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
> > >   drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
> > >   drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
> > >   drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
> > >   4 files changed, 7 insertions(+), 41 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > index f0a07278ad04..7dcc6392792d 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> > >   	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> > >   };
> > >   
> > > -static const struct file_operations fops = {
> > > -	.owner              = THIS_MODULE,
> > > -	.open               = drm_open,
> > > -	.release            = drm_release,
> > > -	.unlocked_ioctl     = drm_ioctl,
> > > -	.compat_ioctl       = drm_compat_ioctl,
> > > -	.poll               = drm_poll,
> > > -	.read               = drm_read,
> > > -	.llseek             = no_llseek,
> > > -	.mmap               = etnaviv_gem_mmap,
> > > -};
> > > +DEFINE_DRM_GEM_FOPS(fops);
> > >   
> > >   static const struct drm_driver etnaviv_drm_driver = {
> > >   	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
> > > @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
> > >   	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > >   	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > >   	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> > > -	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
> > > +	.gem_prime_mmap     = drm_gem_prime_mmap,
> > >   #ifdef CONFIG_DEBUG_FS
> > >   	.debugfs_init       = etnaviv_debugfs_init,
> > >   #endif
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > index 003288ebd896..049ae87de9be 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
> > >   int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> > >   		struct drm_file *file);
> > >   
> > > -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> > >   int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
> > >   struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
> > >   int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
> > > -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
> > > -			   struct vm_area_struct *vma);
> > >   struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
> > >   	struct dma_buf_attachment *attach, struct sg_table *sg);
> > >   int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > index b8fa6ed3dd73..8f1b5af47dd6 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
> > >   {
> > >   	pgprot_t vm_page_prot;
> > >   
> > > -	vma->vm_flags &= ~VM_PFNMAP;
> > > -	vma->vm_flags |= VM_MIXEDMAP;
> > > +	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
> > 
> > I don't fully understand why this change is needed and the commit log
> > is silent about it. Excuse my ignorance, but can you please explain the
> > reasoning behind this change?
> 
> Sure, sorry for being brief.
> 
> I worked on cleaning up the deprecated gem_prime_* callbacks in struct 
> drm_driver. These are supposed to be GEM object functions. The only 
> obsolete gem prime callback in drm_driver is gem_prime_mmap.

Sorry, that's a misunderstanding. I see the justification for the patch
as a whole. I was asking specifically about the hunk above my comment.
Why are the vm_flags changed and how did you come up with this exact
combination of flags?

Regards,
Lucas


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function
  2021-06-24 10:50     ` Lucas Stach
@ 2021-06-24 12:22       ` Thomas Zimmermann
  2021-07-05 11:47       ` Thomas Zimmermann
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2021-06-24 12:22 UTC (permalink / raw)
  To: Lucas Stach, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: etnaviv, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 5762 bytes --]



Am 24.06.21 um 12:50 schrieb Lucas Stach:
> Am Donnerstag, dem 24.06.2021 um 12:47 +0200 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 24.06.21 um 11:11 schrieb Lucas Stach:
>>> Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
>>>> Moving the driver-specific mmap code into a GEM object function allows
>>>> for using DRM helpers for various mmap callbacks.
>>>>
>>>> The respective etnaviv functions are being removed. The file_operations
>>>> structure fops is now being created by the helper macro
>>>> DEFINE_DRM_GEM_FOPS().
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> ---
>>>>    drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
>>>>    drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
>>>>    drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
>>>>    drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
>>>>    4 files changed, 7 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>>> index f0a07278ad04..7dcc6392792d 100644
>>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>>> @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>>>>    	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>>>>    };
>>>>    
>>>> -static const struct file_operations fops = {
>>>> -	.owner              = THIS_MODULE,
>>>> -	.open               = drm_open,
>>>> -	.release            = drm_release,
>>>> -	.unlocked_ioctl     = drm_ioctl,
>>>> -	.compat_ioctl       = drm_compat_ioctl,
>>>> -	.poll               = drm_poll,
>>>> -	.read               = drm_read,
>>>> -	.llseek             = no_llseek,
>>>> -	.mmap               = etnaviv_gem_mmap,
>>>> -};
>>>> +DEFINE_DRM_GEM_FOPS(fops);
>>>>    
>>>>    static const struct drm_driver etnaviv_drm_driver = {
>>>>    	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
>>>> @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>>>>    	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>>>    	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>>>    	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
>>>> -	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
>>>> +	.gem_prime_mmap     = drm_gem_prime_mmap,
>>>>    #ifdef CONFIG_DEBUG_FS
>>>>    	.debugfs_init       = etnaviv_debugfs_init,
>>>>    #endif
>>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>>>> index 003288ebd896..049ae87de9be 100644
>>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>>>> @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
>>>>    int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>>>>    		struct drm_file *file);
>>>>    
>>>> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>>>>    int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>>>>    struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
>>>>    int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
>>>> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
>>>> -			   struct vm_area_struct *vma);
>>>>    struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>>>>    	struct dma_buf_attachment *attach, struct sg_table *sg);
>>>>    int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>>>> index b8fa6ed3dd73..8f1b5af47dd6 100644
>>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>>>> @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>>>>    {
>>>>    	pgprot_t vm_page_prot;
>>>>    
>>>> -	vma->vm_flags &= ~VM_PFNMAP;
>>>> -	vma->vm_flags |= VM_MIXEDMAP;
>>>> +	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
>>>
>>> I don't fully understand why this change is needed and the commit log
>>> is silent about it. Excuse my ignorance, but can you please explain the
>>> reasoning behind this change?
>>
>> Sure, sorry for being brief.
>>
>> I worked on cleaning up the deprecated gem_prime_* callbacks in struct
>> drm_driver. These are supposed to be GEM object functions. The only
>> obsolete gem prime callback in drm_driver is gem_prime_mmap.
> 
> Sorry, that's a misunderstanding. I see the justification for the patch
> as a whole. I was asking specifically about the hunk above my comment.
> Why are the vm_flags changed and how did you come up with this exact
> combination of flags?

I took the existing implementation and looked through it for the current 
combination of flags.

Etnaviv calls etnaviv_gem_prime_mmap(), which in turn calls 
drm_gem_mmap_obj(). [1] Because etnaviv doesn't have a gem object mmap 
callback, drm_gem_mmap_obj() sets some default flags. [2] VM_PFNMAP 
later gets cleared by the current code, so I left it out. And that's 
where these flags come from.

I should add that I don't have the HW to test this change. If you can, 
maybe give it a try.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.12/source/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c#L43

[2] 
https://elixir.bootlin.com/linux/v5.12/source/drivers/gpu/drm/drm_gem.c#L1084

> 
> Regards,
> Lucas
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function
  2021-06-24 10:50     ` Lucas Stach
  2021-06-24 12:22       ` Thomas Zimmermann
@ 2021-07-05 11:47       ` Thomas Zimmermann
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2021-07-05 11:47 UTC (permalink / raw)
  To: Lucas Stach, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: etnaviv, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 5097 bytes --]

Hi,

will you review my patch?

Best regards
Thomas

Am 24.06.21 um 12:50 schrieb Lucas Stach:
> Am Donnerstag, dem 24.06.2021 um 12:47 +0200 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 24.06.21 um 11:11 schrieb Lucas Stach:
>>> Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
>>>> Moving the driver-specific mmap code into a GEM object function allows
>>>> for using DRM helpers for various mmap callbacks.
>>>>
>>>> The respective etnaviv functions are being removed. The file_operations
>>>> structure fops is now being created by the helper macro
>>>> DEFINE_DRM_GEM_FOPS().
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> ---
>>>>    drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
>>>>    drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
>>>>    drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
>>>>    drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
>>>>    4 files changed, 7 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>>> index f0a07278ad04..7dcc6392792d 100644
>>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>>> @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>>>>    	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>>>>    };
>>>>    
>>>> -static const struct file_operations fops = {
>>>> -	.owner              = THIS_MODULE,
>>>> -	.open               = drm_open,
>>>> -	.release            = drm_release,
>>>> -	.unlocked_ioctl     = drm_ioctl,
>>>> -	.compat_ioctl       = drm_compat_ioctl,
>>>> -	.poll               = drm_poll,
>>>> -	.read               = drm_read,
>>>> -	.llseek             = no_llseek,
>>>> -	.mmap               = etnaviv_gem_mmap,
>>>> -};
>>>> +DEFINE_DRM_GEM_FOPS(fops);
>>>>    
>>>>    static const struct drm_driver etnaviv_drm_driver = {
>>>>    	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
>>>> @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>>>>    	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>>>    	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>>>    	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
>>>> -	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
>>>> +	.gem_prime_mmap     = drm_gem_prime_mmap,
>>>>    #ifdef CONFIG_DEBUG_FS
>>>>    	.debugfs_init       = etnaviv_debugfs_init,
>>>>    #endif
>>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>>>> index 003288ebd896..049ae87de9be 100644
>>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>>>> @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
>>>>    int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>>>>    		struct drm_file *file);
>>>>    
>>>> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>>>>    int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>>>>    struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
>>>>    int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
>>>> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
>>>> -			   struct vm_area_struct *vma);
>>>>    struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>>>>    	struct dma_buf_attachment *attach, struct sg_table *sg);
>>>>    int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>>>> index b8fa6ed3dd73..8f1b5af47dd6 100644
>>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>>>> @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>>>>    {
>>>>    	pgprot_t vm_page_prot;
>>>>    
>>>> -	vma->vm_flags &= ~VM_PFNMAP;
>>>> -	vma->vm_flags |= VM_MIXEDMAP;
>>>> +	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
>>>
>>> I don't fully understand why this change is needed and the commit log
>>> is silent about it. Excuse my ignorance, but can you please explain the
>>> reasoning behind this change?
>>
>> Sure, sorry for being brief.
>>
>> I worked on cleaning up the deprecated gem_prime_* callbacks in struct
>> drm_driver. These are supposed to be GEM object functions. The only
>> obsolete gem prime callback in drm_driver is gem_prime_mmap.
> 
> Sorry, that's a misunderstanding. I see the justification for the patch
> as a whole. I was asking specifically about the hunk above my comment.
> Why are the vm_flags changed and how did you come up with this exact
> combination of flags?
> 
> Regards,
> Lucas
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function
  2021-06-24  8:58 [PATCH] drm/etnaviv: Implement mmap as GEM object function Thomas Zimmermann
  2021-06-24  9:11 ` Lucas Stach
@ 2021-07-06 16:34 ` Lucas Stach
  1 sibling, 0 replies; 7+ messages in thread
From: Lucas Stach @ 2021-07-06 16:34 UTC (permalink / raw)
  To: Thomas Zimmermann, linux+etnaviv, christian.gmeiner, airlied, daniel
  Cc: etnaviv, dri-devel

Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
> 
> The respective etnaviv functions are being removed. The file_operations
> structure fops is now being created by the helper macro
> DEFINE_DRM_GEM_FOPS().
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks, I've tested this patch and applied to my etnaviv/next branch.

Regards,
Lucas

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c       | 14 ++------------
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  3 ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 18 +++++-------------
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
>  4 files changed, 7 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index f0a07278ad04..7dcc6392792d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>  	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>  };
>  
> -static const struct file_operations fops = {
> -	.owner              = THIS_MODULE,
> -	.open               = drm_open,
> -	.release            = drm_release,
> -	.unlocked_ioctl     = drm_ioctl,
> -	.compat_ioctl       = drm_compat_ioctl,
> -	.poll               = drm_poll,
> -	.read               = drm_read,
> -	.llseek             = no_llseek,
> -	.mmap               = etnaviv_gem_mmap,
> -};
> +DEFINE_DRM_GEM_FOPS(fops);
>  
>  static const struct drm_driver etnaviv_drm_driver = {
>  	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
> @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>  	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>  	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>  	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> -	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
> +	.gem_prime_mmap     = drm_gem_prime_mmap,
>  #ifdef CONFIG_DEBUG_FS
>  	.debugfs_init       = etnaviv_debugfs_init,
>  #endif
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 003288ebd896..049ae87de9be 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
>  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>  		struct drm_file *file);
>  
> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>  int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>  struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
>  int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map);
> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
> -			   struct vm_area_struct *vma);
>  struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  	struct dma_buf_attachment *attach, struct sg_table *sg);
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index b8fa6ed3dd73..8f1b5af47dd6 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  {
>  	pgprot_t vm_page_prot;
>  
> -	vma->vm_flags &= ~VM_PFNMAP;
> -	vma->vm_flags |= VM_MIXEDMAP;
> +	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
>  
>  	vm_page_prot = vm_get_page_prot(vma->vm_flags);
>  
> @@ -154,19 +153,11 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  	return 0;
>  }
>  
> -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> +static int etnaviv_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>  {
> -	struct etnaviv_gem_object *obj;
> -	int ret;
> -
> -	ret = drm_gem_mmap(filp, vma);
> -	if (ret) {
> -		DBG("mmap failed: %d", ret);
> -		return ret;
> -	}
> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  
> -	obj = to_etnaviv_bo(vma->vm_private_data);
> -	return obj->ops->mmap(obj, vma);
> +	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
>  }
>  
>  static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
> @@ -567,6 +558,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>  	.unpin = etnaviv_gem_prime_unpin,
>  	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>  	.vmap = etnaviv_gem_prime_vmap,
> +	.mmap = etnaviv_gem_mmap,
>  	.vm_ops = &vm_ops,
>  };
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index d741b1d735f7..6d8bed9c739d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -34,19 +34,6 @@ int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>  	return 0;
>  }
>  
> -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
> -			   struct vm_area_struct *vma)
> -{
> -	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> -	int ret;
> -
> -	ret = drm_gem_mmap_obj(obj, obj->size, vma);
> -	if (ret < 0)
> -		return ret;
> -
> -	return etnaviv_obj->ops->mmap(etnaviv_obj, vma);
> -}
> -
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
>  {
>  	if (!obj->import_attach) {



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-06 16:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24  8:58 [PATCH] drm/etnaviv: Implement mmap as GEM object function Thomas Zimmermann
2021-06-24  9:11 ` Lucas Stach
2021-06-24 10:47   ` Thomas Zimmermann
2021-06-24 10:50     ` Lucas Stach
2021-06-24 12:22       ` Thomas Zimmermann
2021-07-05 11:47       ` Thomas Zimmermann
2021-07-06 16:34 ` Lucas Stach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).