Re: [PATCH] drm/etnaviv: Implement mmap as GEM object function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxx>
---
   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: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux