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) {
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) {
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);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) {
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);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
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);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/etnavi...
[2] https://elixir.bootlin.com/linux/v5.12/source/drivers/gpu/drm/drm_gem.c#L108...
Regards, Lucas
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);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
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) {
dri-devel@lists.freedesktop.org
-
Lucas Stach -
Thomas Zimmermann