Add mmap callback to drm_object
Gerd Hoffmann (8): drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to &drm_gem_object_funcs.mmap drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to &drm_gem_object_funcs.mmap() drm/vram: drop verify_access drm/vram: drop DRM_VRAM_MM_FILE_OPERATIONS
include/drm/drm_gem.h | 14 ++++ include/drm/drm_gem_shmem_helper.h | 30 +------ include/drm/drm_gem_ttm_helper.h | 2 + include/drm/drm_gem_vram_helper.h | 25 ------ include/drm/drm_vram_mm_helper.h | 82 +++++++++++++++++++ include/drm/ttm/ttm_bo_api.h | 8 ++ drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/drm_gem.c | 27 ++++-- drivers/gpu/drm/drm_gem_shmem_helper.c | 26 ++---- drivers/gpu/drm/drm_gem_ttm_helper.c | 19 +++++ drivers/gpu/drm/drm_gem_vram_helper.c | 56 +------------ drivers/gpu/drm/drm_prime.c | 9 ++ .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++----- drivers/gpu/drm/v3d/v3d_bo.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_object.c | 2 +- 25 files changed, 200 insertions(+), 186 deletions(-) create mode 100644 include/drm/drm_vram_mm_helper.h
drm_gem_object_funcs->vm_ops alone can't handle everything which needs to be done for mmap(), tweaking vm_flags for example. So add a new mmap() callback to drm_gem_object_funcs where this code can go to.
Note that the vm_ops field is not used in case the mmap callback is presnt, it is expected that the callback sets vma->vm_ops instead.
drm_gem_mmap_obj() will use the new callback for object specific mmap setup. With this in place the need for driver-speific fops->mmap callbacks goes away, drm_gem_mmap can be hooked instead.
drm_gem_prime_mmap() will use the new callback too to just mmap gem objects directly instead of jumping though loops to make drm_gem_object_lookup() and fops->mmap work.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem.h | 14 ++++++++++++++ drivers/gpu/drm/drm_gem.c | 27 ++++++++++++++++++--------- drivers/gpu/drm/drm_prime.c | 9 +++++++++ 3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 6aaba14f5972..e71f75a2ab57 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -150,6 +150,20 @@ struct drm_gem_object_funcs { */ void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
+ /** + * @mmap: + * + * Handle mmap() of the gem object, setup vma accordingly. + * + * This callback is optional. + * + * The callback is used by by both drm_gem_mmap_obj() and + * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not + * used, the @mmap callback must set vma->vm_ops instead. + * + */ + int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); + /** * @vm_ops: * diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6854f5867d51..56f42e0f2584 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1099,22 +1099,31 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, struct vm_area_struct *vma) { struct drm_device *dev = obj->dev; + int ret;
/* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL;
- if (obj->funcs && obj->funcs->vm_ops) - vma->vm_ops = obj->funcs->vm_ops; - else if (dev->driver->gem_vm_ops) - vma->vm_ops = dev->driver->gem_vm_ops; - else - return -EINVAL; + if (obj->funcs && obj->funcs->mmap) { + ret = obj->funcs->mmap(obj, vma); + if (ret) + return ret; + WARN_ON(!(vma->vm_flags & VM_DONTEXPAND)); + } else { + if (obj->funcs && obj->funcs->vm_ops) + vma->vm_ops = obj->funcs->vm_ops; + else if (dev->driver->gem_vm_ops) + vma->vm_ops = dev->driver->gem_vm_ops; + else + return -EINVAL; + + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + }
- vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); - vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
/* Take a ref for this mapping of the object, so that the fault * handler can dereference the mmap offset's pointer to the object. diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0a2316e0e812..0814211b0f3f 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -713,6 +713,15 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) struct file *fil; int ret;
+ if (obj->funcs && obj->funcs->mmap) { + ret = obj->funcs->mmap(obj, vma); + if (ret) + return ret; + vma->vm_private_data = obj; + drm_gem_object_get(obj); + return 0; + } + priv = kzalloc(sizeof(*priv), GFP_KERNEL); fil = kzalloc(sizeof(*fil), GFP_KERNEL); if (!priv || !fil) {
On Fri, Sep 13, 2019 at 02:29:01PM +0200, Gerd Hoffmann wrote:
drm_gem_object_funcs->vm_ops alone can't handle everything which needs to be done for mmap(), tweaking vm_flags for example. So add a new mmap() callback to drm_gem_object_funcs where this code can go to.
Note that the vm_ops field is not used in case the mmap callback is presnt, it is expected that the callback sets vma->vm_ops instead.
drm_gem_mmap_obj() will use the new callback for object specific mmap setup. With this in place the need for driver-speific fops->mmap callbacks goes away, drm_gem_mmap can be hooked instead.
drm_gem_prime_mmap() will use the new callback too to just mmap gem objects directly instead of jumping though loops to make drm_gem_object_lookup() and fops->mmap work.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem.h | 14 ++++++++++++++ drivers/gpu/drm/drm_gem.c | 27 ++++++++++++++++++--------- drivers/gpu/drm/drm_prime.c | 9 +++++++++ 3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 6aaba14f5972..e71f75a2ab57 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -150,6 +150,20 @@ struct drm_gem_object_funcs { */ void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
- /**
* @mmap:** Handle mmap() of the gem object, setup vma accordingly.** This callback is optional.** The callback is used by by both drm_gem_mmap_obj() and* drm_gem_prime_mmap(). When @mmap is present @vm_ops is not* used, the @mmap callback must set vma->vm_ops instead.**/- int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
- /**
- @vm_ops:
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6854f5867d51..56f42e0f2584 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1099,22 +1099,31 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, struct vm_area_struct *vma) { struct drm_device *dev = obj->dev;
int ret;
/* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL;
- if (obj->funcs && obj->funcs->vm_ops)
vma->vm_ops = obj->funcs->vm_ops;- else if (dev->driver->gem_vm_ops)
vma->vm_ops = dev->driver->gem_vm_ops;- else
return -EINVAL;
- if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);if (ret)return ret;WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
Yeah I think checking for VM_DONTEXPAND is even better than checking for just VM_SPECIAL.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
- } else {
if (obj->funcs && obj->funcs->vm_ops)vma->vm_ops = obj->funcs->vm_ops;else if (dev->driver->gem_vm_ops)vma->vm_ops = dev->driver->gem_vm_ops;elsereturn -EINVAL;vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);- }
vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; vma->vm_private_data = obj;
vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
/* Take a ref for this mapping of the object, so that the fault
- handler can dereference the mmap offset's pointer to the object.
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0a2316e0e812..0814211b0f3f 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -713,6 +713,15 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) struct file *fil; int ret;
- if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);if (ret)return ret;vma->vm_private_data = obj;drm_gem_object_get(obj);return 0;- }
- priv = kzalloc(sizeof(*priv), GFP_KERNEL); fil = kzalloc(sizeof(*fil), GFP_KERNEL); if (!priv || !fil) {
-- 2.18.1
Switch gem shmem helper to the new mmap() workflow, from &gem_driver.fops.mmap to &drm_gem_object_funcs.mmap.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_shmem_helper.h | 6 ++---- drivers/gpu/drm/drm_gem_shmem_helper.c | 26 ++++++++----------------- drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +- drivers/gpu/drm/v3d/v3d_bo.c | 2 +- drivers/gpu/drm/virtio/virtgpu_object.c | 2 +- 5 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index 01f514521687..d89f2116c8ab 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -111,7 +111,7 @@ struct drm_gem_shmem_object { .poll = drm_poll,\ .read = drm_read,\ .llseek = noop_llseek,\ - .mmap = drm_gem_shmem_mmap, \ + .mmap = drm_gem_mmap, \ }
struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size); @@ -143,9 +143,7 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv, int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args);
-int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma); - -extern const struct vm_operations_struct drm_gem_shmem_vm_ops; +int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent, const struct drm_gem_object *obj); diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index f5918707672f..a104140154bb 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -32,7 +32,7 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, - .vm_ops = &drm_gem_shmem_vm_ops, + .mmap = drm_gem_shmem_mmap, };
/** @@ -505,39 +505,30 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma) drm_gem_vm_close(vma); }
-const struct vm_operations_struct drm_gem_shmem_vm_ops = { +static const struct vm_operations_struct drm_gem_shmem_vm_ops = { .fault = drm_gem_shmem_fault, .open = drm_gem_shmem_vm_open, .close = drm_gem_shmem_vm_close, }; -EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
/** * drm_gem_shmem_mmap - Memory-map a shmem GEM object - * @filp: File object + * @obj: gem object * @vma: VMA for the area to be mapped * * This function implements an augmented version of the GEM DRM file mmap * operation for shmem objects. Drivers which employ the shmem helpers should - * use this function as their &file_operations.mmap handler in the DRM device file's - * file_operations structure. - * - * Instead of directly referencing this function, drivers should use the - * DEFINE_DRM_GEM_SHMEM_FOPS() macro. + * use this function as their &drm_gem_object_funcs.mmap handler. * * Returns: * 0 on success or a negative error code on failure. */ -int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma) +int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) { struct drm_gem_shmem_object *shmem; int ret;
- ret = drm_gem_mmap(filp, vma); - if (ret) - return ret; - - shmem = to_drm_gem_shmem_obj(vma->vm_private_data); + shmem = to_drm_gem_shmem_obj(obj);
ret = drm_gem_shmem_get_pages(shmem); if (ret) { @@ -545,9 +536,8 @@ int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma) return ret; }
- /* VM_PFNMAP was set by drm_gem_mmap() */ - vma->vm_flags &= ~VM_PFNMAP; - vma->vm_flags |= VM_MIXEDMAP; + vma->vm_flags |= (VM_MIXEDMAP|VM_DONTEXPAND); + vma->vm_ops = &drm_gem_shmem_vm_ops;
/* Remove the fake offset */ vma->vm_pgoff -= drm_vma_node_start(&shmem->base.vma_node); diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index acb07fe06580..deca0c30bbd4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -112,7 +112,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, - .vm_ops = &drm_gem_shmem_vm_ops, + .mmap = drm_gem_shmem_mmap, };
/** diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c index a22b75a3a533..edd299ab53d8 100644 --- a/drivers/gpu/drm/v3d/v3d_bo.c +++ b/drivers/gpu/drm/v3d/v3d_bo.c @@ -58,7 +58,7 @@ static const struct drm_gem_object_funcs v3d_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, - .vm_ops = &drm_gem_shmem_vm_ops, + .mmap = drm_gem_shmem_mmap, };
/* gem_create_object function for allocating a BO struct and doing diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 69a3d310ff70..017a9e0fc3bb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -86,7 +86,7 @@ static const struct drm_gem_object_funcs virtio_gpu_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap, - .vm_ops = &drm_gem_shmem_vm_ops, + .mmap = &drm_gem_shmem_mmap, };
struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
On 13/09/2019 13:29, Gerd Hoffmann wrote:
Switch gem shmem helper to the new mmap() workflow, from &gem_driver.fops.mmap to &drm_gem_object_funcs.mmap.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem_shmem_helper.h | 6 ++---- drivers/gpu/drm/drm_gem_shmem_helper.c | 26 ++++++++----------------- drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +- drivers/gpu/drm/v3d/v3d_bo.c | 2 +- drivers/gpu/drm/virtio/virtgpu_object.c | 2 +- 5 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index 01f514521687..d89f2116c8ab 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -111,7 +111,7 @@ struct drm_gem_shmem_object { .poll = drm_poll,\ .read = drm_read,\ .llseek = noop_llseek,\
.mmap = drm_gem_shmem_mmap, \
.mmap = drm_gem_mmap, \struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size); @@ -143,9 +143,7 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv, int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args);
-int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma);
-extern const struct vm_operations_struct drm_gem_shmem_vm_ops; +int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent, const struct drm_gem_object *obj); diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index f5918707672f..a104140154bb 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -32,7 +32,7 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap,
- .vm_ops = &drm_gem_shmem_vm_ops,
- .mmap = drm_gem_shmem_mmap,
};
/** @@ -505,39 +505,30 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma) drm_gem_vm_close(vma); }
-const struct vm_operations_struct drm_gem_shmem_vm_ops = { +static const struct vm_operations_struct drm_gem_shmem_vm_ops = { .fault = drm_gem_shmem_fault, .open = drm_gem_shmem_vm_open, .close = drm_gem_shmem_vm_close, }; -EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
/**
- drm_gem_shmem_mmap - Memory-map a shmem GEM object
- @filp: File object
- @obj: gem object
- @vma: VMA for the area to be mapped
- This function implements an augmented version of the GEM DRM file mmap
- operation for shmem objects. Drivers which employ the shmem helpers should
- use this function as their &file_operations.mmap handler in the DRM device file's
- file_operations structure.
- Instead of directly referencing this function, drivers should use the
- DEFINE_DRM_GEM_SHMEM_FOPS() macro.
*/
- use this function as their &drm_gem_object_funcs.mmap handler.
- Returns:
- 0 on success or a negative error code on failure.
-int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma) +int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) { struct drm_gem_shmem_object *shmem; int ret;
- ret = drm_gem_mmap(filp, vma);
- if (ret)
return ret;- shmem = to_drm_gem_shmem_obj(vma->vm_private_data);
shmem = to_drm_gem_shmem_obj(obj);
ret = drm_gem_shmem_get_pages(shmem); if (ret) {
@@ -545,9 +536,8 @@ int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma) return ret; }
- /* VM_PFNMAP was set by drm_gem_mmap() */
- vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= (VM_MIXEDMAP|VM_DONTEXPAND);
I'm finding this a bit hard to follow - but I think here we've lost VM_IO and VM_DONTDUMP which used to be set by drm_gem_mmap().
Also it looks like nothing is fiddling vma->vm_page_prot anymore.
Steve
vma->vm_ops = &drm_gem_shmem_vm_ops;
/* Remove the fake offset */ vma->vm_pgoff -= drm_vma_node_start(&shmem->base.vma_node);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index acb07fe06580..deca0c30bbd4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -112,7 +112,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap,
- .vm_ops = &drm_gem_shmem_vm_ops,
- .mmap = drm_gem_shmem_mmap,
};
/** diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c index a22b75a3a533..edd299ab53d8 100644 --- a/drivers/gpu/drm/v3d/v3d_bo.c +++ b/drivers/gpu/drm/v3d/v3d_bo.c @@ -58,7 +58,7 @@ static const struct drm_gem_object_funcs v3d_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap,
- .vm_ops = &drm_gem_shmem_vm_ops,
- .mmap = drm_gem_shmem_mmap,
};
/* gem_create_object function for allocating a BO struct and doing diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 69a3d310ff70..017a9e0fc3bb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -86,7 +86,7 @@ static const struct drm_gem_object_funcs virtio_gpu_gem_funcs = { .get_sg_table = drm_gem_shmem_get_sg_table, .vmap = drm_gem_shmem_vmap, .vunmap = drm_gem_shmem_vunmap,
- .vm_ops = &drm_gem_shmem_vm_ops,
- .mmap = &drm_gem_shmem_mmap,
};
struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
Hi,
- /* VM_PFNMAP was set by drm_gem_mmap() */
- vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= (VM_MIXEDMAP|VM_DONTEXPAND);
I'm finding this a bit hard to follow - but I think here we've lost VM_IO and VM_DONTDUMP which used to be set by drm_gem_mmap().
Yep. Intentional, but I think I better split that off to a separate patch with a commit message explaining things.
Also it looks like nothing is fiddling vma->vm_page_prot anymore.
Oops, that one was not intentional. Will fix.
cheers, Gerd
DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +- 6 files changed, 5 insertions(+), 31 deletions(-)
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index d89f2116c8ab..6748379a0b44 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -88,32 +88,6 @@ struct drm_gem_shmem_object { #define to_drm_gem_shmem_obj(obj) \ container_of(obj, struct drm_gem_shmem_object, base)
-/** - * DEFINE_DRM_GEM_SHMEM_FOPS() - Macro to generate file operations for shmem drivers - * @name: name for the generated structure - * - * This macro autogenerates a suitable &struct file_operations for shmem based - * drivers, which can be assigned to &drm_driver.fops. Note that this structure - * cannot be shared between drivers, because it contains a reference to the - * current module using THIS_MODULE. - * - * Note that the declaration is already marked as static - if you need a - * non-static version of this you're probably doing it wrong and will break the - * THIS_MODULE reference by accident. - */ -#define DEFINE_DRM_GEM_SHMEM_FOPS(name) \ - static const struct file_operations name = {\ - .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 = noop_llseek,\ - .mmap = drm_gem_mmap, \ - } - struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size); void drm_gem_shmem_free_object(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c index 89d9e6fdeb8c..7d08d067e1a4 100644 --- a/drivers/gpu/drm/cirrus/cirrus.c +++ b/drivers/gpu/drm/cirrus/cirrus.c @@ -510,7 +510,7 @@ static void cirrus_mode_config_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
-DEFINE_DRM_GEM_SHMEM_FOPS(cirrus_fops); +DEFINE_DRM_GEM_FOPS(cirrus_fops);
static struct drm_driver cirrus_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index d74442d71048..a78ce21594d7 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -470,7 +470,7 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), };
-DEFINE_DRM_GEM_SHMEM_FOPS(panfrost_drm_driver_fops); +DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops);
/* * Panfrost driver version: diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c index 03d0e2df6774..94fb1f593564 100644 --- a/drivers/gpu/drm/tiny/gm12u320.c +++ b/drivers/gpu/drm/tiny/gm12u320.c @@ -649,7 +649,7 @@ static void gm12u320_driver_release(struct drm_device *dev) kfree(gm12u320); }
-DEFINE_DRM_GEM_SHMEM_FOPS(gm12u320_fops); +DEFINE_DRM_GEM_FOPS(gm12u320_fops);
static struct drm_driver gm12u320_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 3506ae2723ae..03e4fbe1b92b 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -169,7 +169,7 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file) kfree(v3d_priv); }
-DEFINE_DRM_GEM_SHMEM_FOPS(v3d_drm_fops); +DEFINE_DRM_GEM_FOPS(v3d_drm_fops);
/* DRM_AUTH is required on SUBMIT_CL for now, while we don't have GMP * protection between clients. Note that render nodes would be be diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c index a5cb58754f7d..8dee698c90ff 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.c +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c @@ -184,7 +184,7 @@ MODULE_AUTHOR("Dave Airlie airlied@redhat.com"); MODULE_AUTHOR("Gerd Hoffmann kraxel@redhat.com"); MODULE_AUTHOR("Alon Levy");
-DEFINE_DRM_GEM_SHMEM_FOPS(virtio_gpu_driver_fops); +DEFINE_DRM_GEM_FOPS(virtio_gpu_driver_fops);
static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_RENDER | DRIVER_ATOMIC,
On Fri, Sep 13, 2019 at 7:29 AM Gerd Hoffmann kraxel@redhat.com wrote:
Version? Pretty sure this is not v1.
DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +- 6 files changed, 5 insertions(+), 31 deletions(-)
Acked-by: Rob Herring robh@kernel.org
On Mon, Sep 16, 2019 at 05:07:14PM -0500, Rob Herring wrote:
On Fri, Sep 13, 2019 at 7:29 AM Gerd Hoffmann kraxel@redhat.com wrote:
Version? Pretty sure this is not v1.
Yep, was posted as part of a longer series before.
Splitted the long series into multiple smaller ones by cherry-picking patches into new branches, which re-started the numbering.
Sorry for the confusion, Gerd
Factor out ttm vma setup to a new function. Reduces code duplication a bit and allows to implement &drm_gem_object_funcs.mmap in gem ttm helpers.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/ttm/ttm_bo_api.h | 8 ++++++ drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 43c4929a2171..88c652f49602 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -734,6 +734,14 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo); int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev);
+/** + * ttm_bo_mmap_vma_setup - initialize vma for ttm bo mmap + * + * @bo: The buffer object. + * @vma: vma as input from the mmap method. + */ +void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma); + void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot);
void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 4aa007edffb0..7c0e85c10e0e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -426,6 +426,29 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, return bo; }
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{ + vma->vm_ops = &ttm_bo_vm_ops; + + /* + * Note: We're transferring the bo reference to + * vma->vm_private_data here. + */ + + vma->vm_private_data = bo; + + /* + * We'd like to use VM_PFNMAP on shared mappings, where + * (vma->vm_flags & VM_SHARED) != 0, for performance reasons, + * but for some reason VM_PFNMAP + x86 PAT + write-combine is very + * bad for performance. Until that has been sorted out, use + * VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719 + */ + vma->vm_flags |= VM_MIXEDMAP; + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; +} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup); + int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev) { @@ -449,24 +472,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, if (unlikely(ret != 0)) goto out_unref;
- vma->vm_ops = &ttm_bo_vm_ops; - - /* - * Note: We're transferring the bo reference to - * vma->vm_private_data here. - */ - - vma->vm_private_data = bo; - - /* - * We'd like to use VM_PFNMAP on shared mappings, where - * (vma->vm_flags & VM_SHARED) != 0, for performance reasons, - * but for some reason VM_PFNMAP + x86 PAT + write-combine is very - * bad for performance. Until that has been sorted out, use - * VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719 - */ - vma->vm_flags |= VM_MIXEDMAP; - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; + ttm_bo_mmap_vma_setup(bo, vma); return 0; out_unref: ttm_bo_put(bo); @@ -481,10 +487,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
ttm_bo_get(bo);
- vma->vm_ops = &ttm_bo_vm_ops; - vma->vm_private_data = bo; - vma->vm_flags |= VM_MIXEDMAP; - vma->vm_flags |= VM_IO | VM_DONTEXPAND; + ttm_bo_mmap_vma_setup(bo, vma); return 0; } EXPORT_SYMBOL(ttm_fbdev_mmap);
Hi
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Factor out ttm vma setup to a new function. Reduces code duplication a bit and allows to implement &drm_gem_object_funcs.mmap in gem ttm helpers.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/ttm/ttm_bo_api.h | 8 ++++++ drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 43c4929a2171..88c652f49602 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -734,6 +734,14 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo); int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev);
+/**
- ttm_bo_mmap_vma_setup - initialize vma for ttm bo mmap
- @bo: The buffer object.
- @vma: vma as input from the mmap method.
- */
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma);
void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot);
void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 4aa007edffb0..7c0e85c10e0e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -426,6 +426,29 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, return bo; }
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup);
int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev) { @@ -449,24 +472,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, if (unlikely(ret != 0)) goto out_unref;
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
- ttm_bo_mmap_vma_setup(bo, vma); return 0;
out_unref: ttm_bo_put(bo); @@ -481,10 +487,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
ttm_bo_get(bo);
- vma->vm_ops = &ttm_bo_vm_ops;
- vma->vm_private_data = bo;
- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND;
- ttm_bo_mmap_vma_setup(bo, vma);
Just double-checking: ttm_bo_mmap_vma_setup() will set VM_DONTDUMP in vm_flags. Is that OK?
Best regards Thomas
return 0; } EXPORT_SYMBOL(ttm_fbdev_mmap);
On Fri, Sep 13, 2019 at 02:56:09PM +0200, Thomas Zimmermann wrote:
Hi
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Factor out ttm vma setup to a new function. Reduces code duplication a bit and allows to implement &drm_gem_object_funcs.mmap in gem ttm helpers.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/ttm/ttm_bo_api.h | 8 ++++++ drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 43c4929a2171..88c652f49602 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -734,6 +734,14 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo); int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev);
+/**
- ttm_bo_mmap_vma_setup - initialize vma for ttm bo mmap
- @bo: The buffer object.
- @vma: vma as input from the mmap method.
- */
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma);
void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot);
void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 4aa007edffb0..7c0e85c10e0e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -426,6 +426,29 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, return bo; }
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup);
int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev) { @@ -449,24 +472,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, if (unlikely(ret != 0)) goto out_unref;
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
- ttm_bo_mmap_vma_setup(bo, vma); return 0;
out_unref: ttm_bo_put(bo); @@ -481,10 +487,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
ttm_bo_get(bo);
- vma->vm_ops = &ttm_bo_vm_ops;
- vma->vm_private_data = bo;
- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND;
- ttm_bo_mmap_vma_setup(bo, vma);
Just double-checking: ttm_bo_mmap_vma_setup() will set VM_DONTDUMP in vm_flags. Is that OK?
Should be ok, according to daniel vetter this most likely dates back to the days where drivers exposed hardware registers mmap()able object.
But doing this as separate patch is probably a good idea ...
cheers, Gerd
Hi
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Factor out ttm vma setup to a new function. Reduces code duplication a bit and allows to implement &drm_gem_object_funcs.mmap in gem ttm helpers.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/ttm/ttm_bo_api.h | 8 ++++++ drivers/gpu/drm/ttm/ttm_bo_vm.c | 47 ++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 43c4929a2171..88c652f49602 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -734,6 +734,14 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo); int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev);
+/**
- ttm_bo_mmap_vma_setup - initialize vma for ttm bo mmap
- @bo: The buffer object.
- @vma: vma as input from the mmap method.
- */
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma);
void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot);
void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 4aa007edffb0..7c0e85c10e0e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -426,6 +426,29 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, return bo; }
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup);
To me, this function looks like an internal helper that should rather remain internal. As mentioned in my reply to patch 5, maybe re-using ttm_fbdev_mmap() could help.
Best regards Thomas
int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev) { @@ -449,24 +472,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, if (unlikely(ret != 0)) goto out_unref;
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
- ttm_bo_mmap_vma_setup(bo, vma); return 0;
out_unref: ttm_bo_put(bo); @@ -481,10 +487,7 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
ttm_bo_get(bo);
- vma->vm_ops = &ttm_bo_vm_ops;
- vma->vm_private_data = bo;
- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND;
- ttm_bo_mmap_vma_setup(bo, vma); return 0;
} EXPORT_SYMBOL(ttm_fbdev_mmap);
On Fri, Sep 13, 2019 at 03:05:34PM +0200, Thomas Zimmermann wrote:
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup);
To me, this function looks like an internal helper that should rather remain internal.
Well, I'm moving that to a helper exactly to avoid drm gem ttm helpers messing with ttm internals. To not them initialize vm_flags for example, and to avoid exporting ttm_bo_vm_ops. Also to make sure ttm bo vma's are initialized the same way no matter which code path was taken to mmap the object.
As mentioned in my reply to patch 5, maybe re-using ttm_fbdev_mmap() could help.
No, the check in that function prevents that from working.
cheers, Gerd
Hi Gerd
Am 17.09.19 um 10:34 schrieb Gerd Hoffmann:
On Fri, Sep 13, 2019 at 03:05:34PM +0200, Thomas Zimmermann wrote:
+void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma) +{
- vma->vm_ops = &ttm_bo_vm_ops;
- /*
* Note: We're transferring the bo reference to* vma->vm_private_data here.*/- vma->vm_private_data = bo;
- /*
* We'd like to use VM_PFNMAP on shared mappings, where* (vma->vm_flags & VM_SHARED) != 0, for performance reasons,* but for some reason VM_PFNMAP + x86 PAT + write-combine is very* bad for performance. Until that has been sorted out, use* VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719*/- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+} +EXPORT_SYMBOL(ttm_bo_mmap_vma_setup);
To me, this function looks like an internal helper that should rather remain internal.
Well, I'm moving that to a helper exactly to avoid drm gem ttm helpers messing with ttm internals. To not them initialize vm_flags for example, and to avoid exporting ttm_bo_vm_ops. Also to make sure ttm bo vma's are initialized the same way no matter which code path was taken to mmap the object.
It may not be worth blocking on this, so
Acked-by: Thomas Zimmermann tzimmermann@suse.de
But I still think it's not a good interface because it exposes internal details.
Please consider another idea: how about splitting off the ttm_bo_get() and vma-flags setup of ttm_fbdev_mmap() into a separate function, like this:
void ttm_bo_mmap_refed(vma, bo) { ttm_bo_get(bo) ttm_bo_mmap_vma_setup(vma); } EXPORT_SYMBOL(ttm_bo_mmap_refed)
int ttm_fbdev_mmap(vma, bo) { if (vma->vm_pgoff != 0) return -EACCES; ttm_bo_mmap_refed(vma, bo); return 0; }
That would allow to keep _vma_setup() an internal function.
ttm_fbdev_mmap() sounds like it is only for fbdev and the only user is amdgpu. Can it be moved out of ttm entirely?
Best regards Thomas
As mentioned in my reply to patch 5, maybe re-using ttm_fbdev_mmap() could help.
No, the check in that function prevents that from working.
cheers, Gerd
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
It may not be worth blocking on this, so
Acked-by: Thomas Zimmermann tzimmermann@suse.de
But I still think it's not a good interface because it exposes internal details.
Please consider another idea: how about splitting off the ttm_bo_get() and vma-flags setup of ttm_fbdev_mmap() into a separate function, like this:
void ttm_bo_mmap_refed(vma, bo) { ttm_bo_get(bo) ttm_bo_mmap_vma_setup(vma); } EXPORT_SYMBOL(ttm_bo_mmap_refed)
ttm_bo_mmap_refed and ttm_bo_mmap_vma_setup are almost identical ...
But, yes, moving the ttm_bo_get call to ttm_bo_mmap_vma_setup probably makes sense and hides this little detail to the outside.
ttm_fbdev_mmap() sounds like it is only for fbdev and the only user is amdgpu. Can it be moved out of ttm entirely?
Exporting ttm_bo_mmap_vma_setup() allows to do that ;)
cheers, Gerd
Add helper function to mmap ttm bo's using &drm_gem_object_funcs.mmap().
Note that with this code path access verification is done by drm_gem_mmap() (which calls drm_vma_node_is_allowed(()). The &ttm_bo_driver.verify_access() callback is is not used.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_ttm_helper.h | 2 ++ drivers/gpu/drm/drm_gem_ttm_helper.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+)
diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h index 6268f89c5a48..118cef76f84f 100644 --- a/include/drm/drm_gem_ttm_helper.h +++ b/include/drm/drm_gem_ttm_helper.h @@ -15,5 +15,7 @@
void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent, const struct drm_gem_object *gem); +int drm_gem_ttm_mmap(struct drm_gem_object *gem, + struct vm_area_struct *vma);
#endif diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c index 9a4bafcf20df..34ce6cf78b35 100644 --- a/drivers/gpu/drm/drm_gem_ttm_helper.c +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c @@ -52,5 +52,24 @@ void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent, } EXPORT_SYMBOL(drm_gem_ttm_print_info);
+/** + * drm_gem_ttm_mmap() - mmap &ttm_buffer_object + * @gem: GEM object. + * @vma: vm area. + * + * This function can be used as &drm_gem_object_funcs.mmap + * callback. + */ +int drm_gem_ttm_mmap(struct drm_gem_object *gem, + struct vm_area_struct *vma) +{ + struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem); + + ttm_bo_get(bo); + ttm_bo_mmap_vma_setup(bo, vma); + return 0; +} +EXPORT_SYMBOL(drm_gem_ttm_mmap); + MODULE_DESCRIPTION("DRM gem ttm helpers"); MODULE_LICENSE("GPL");
Hi
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Add helper function to mmap ttm bo's using &drm_gem_object_funcs.mmap().
Note that with this code path access verification is done by drm_gem_mmap() (which calls drm_vma_node_is_allowed(()). The &ttm_bo_driver.verify_access() callback is is not used.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem_ttm_helper.h | 2 ++ drivers/gpu/drm/drm_gem_ttm_helper.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+)
diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h index 6268f89c5a48..118cef76f84f 100644 --- a/include/drm/drm_gem_ttm_helper.h +++ b/include/drm/drm_gem_ttm_helper.h @@ -15,5 +15,7 @@
void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent, const struct drm_gem_object *gem); +int drm_gem_ttm_mmap(struct drm_gem_object *gem,
struct vm_area_struct *vma);#endif diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c index 9a4bafcf20df..34ce6cf78b35 100644 --- a/drivers/gpu/drm/drm_gem_ttm_helper.c +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c @@ -52,5 +52,24 @@ void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent, } EXPORT_SYMBOL(drm_gem_ttm_print_info);
+/**
- drm_gem_ttm_mmap() - mmap &ttm_buffer_object
- @gem: GEM object.
- @vma: vm area.
- This function can be used as &drm_gem_object_funcs.mmap
- callback.
- */
+int drm_gem_ttm_mmap(struct drm_gem_object *gem,
struct vm_area_struct *vma)+{
- struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
- ttm_bo_get(bo);
- ttm_bo_mmap_vma_setup(bo, vma);
- return 0;
+}
This function looks like ttm_fbdev_mmap(). Can you use that instead?
Best regards Thomas
+EXPORT_SYMBOL(drm_gem_ttm_mmap);
MODULE_DESCRIPTION("DRM gem ttm helpers"); MODULE_LICENSE("GPL");
Wire up the new drm_gem_ttm_mmap() helper function, use generic drm_gem_mmap for &fops.mmap and delete dead drm_vram_mm_file_operations_mmap().
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_vram_helper.h | 9 +------ drivers/gpu/drm/drm_gem_vram_helper.c | 34 +-------------------------- 2 files changed, 2 insertions(+), 41 deletions(-)
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 9aaef4f8c327..9d5526650291 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -180,13 +180,6 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm( struct drm_device *dev, uint64_t vram_base, size_t vram_size); void drm_vram_helper_release_mm(struct drm_device *dev);
-/* - * Helpers for &struct file_operations - */ - -int drm_vram_mm_file_operations_mmap( - struct file *filp, struct vm_area_struct *vma); - /** * define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \ &struct file_operations @@ -200,7 +193,7 @@ int drm_vram_mm_file_operations_mmap( .poll = drm_poll, \ .unlocked_ioctl = drm_ioctl, \ .compat_ioctl = drm_compat_ioctl, \ - .mmap = drm_vram_mm_file_operations_mmap, \ + .mmap = drm_gem_mmap, \ .open = drm_open, \ .release = drm_release \
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 7bee80c6b6e8..e100b97ea6e3 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -681,6 +681,7 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { .unpin = drm_gem_vram_object_unpin, .vmap = drm_gem_vram_object_vmap, .vunmap = drm_gem_vram_object_vunmap, + .mmap = drm_gem_ttm_mmap, .print_info = drm_gem_ttm_print_info, };
@@ -915,12 +916,6 @@ static void drm_vram_mm_cleanup(struct drm_vram_mm *vmm) ttm_bo_device_release(&vmm->bdev); }
-static int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma, - struct drm_vram_mm *vmm) -{ - return ttm_bo_mmap(filp, vma, &vmm->bdev); -} - /* * Helpers for integration with struct drm_device */ @@ -976,30 +971,3 @@ void drm_vram_helper_release_mm(struct drm_device *dev) dev->vram_mm = NULL; } EXPORT_SYMBOL(drm_vram_helper_release_mm); - -/* - * Helpers for &struct file_operations - */ - -/** - * drm_vram_mm_file_operations_mmap() - \ - Implements &struct file_operations.mmap() - * @filp: the mapping's file structure - * @vma: the mapping's memory area - * - * Returns: - * 0 on success, or - * a negative error code otherwise. - */ -int drm_vram_mm_file_operations_mmap( - struct file *filp, struct vm_area_struct *vma) -{ - struct drm_file *file_priv = filp->private_data; - struct drm_device *dev = file_priv->minor->dev; - - if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized")) - return -EINVAL; - - return drm_vram_mm_mmap(filp, vma, dev->vram_mm); -} -EXPORT_SYMBOL(drm_vram_mm_file_operations_mmap);
Hi
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Wire up the new drm_gem_ttm_mmap() helper function, use generic drm_gem_mmap for &fops.mmap and delete dead drm_vram_mm_file_operations_mmap().
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Reviewed-by: Thomas Zimmermann tzimmermann@suse.de
include/drm/drm_gem_vram_helper.h | 9 +------ drivers/gpu/drm/drm_gem_vram_helper.c | 34 +-------------------------- 2 files changed, 2 insertions(+), 41 deletions(-)
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 9aaef4f8c327..9d5526650291 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -180,13 +180,6 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm( struct drm_device *dev, uint64_t vram_base, size_t vram_size); void drm_vram_helper_release_mm(struct drm_device *dev);
-/*
- Helpers for &struct file_operations
- */
-int drm_vram_mm_file_operations_mmap(
- struct file *filp, struct vm_area_struct *vma);
/**
- define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \
&struct file_operations @@ -200,7 +193,7 @@ int drm_vram_mm_file_operations_mmap( .poll = drm_poll, \ .unlocked_ioctl = drm_ioctl, \ .compat_ioctl = drm_compat_ioctl, \
- .mmap = drm_vram_mm_file_operations_mmap, \
- .mmap = drm_gem_mmap, \ .open = drm_open, \ .release = drm_release \
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 7bee80c6b6e8..e100b97ea6e3 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -681,6 +681,7 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { .unpin = drm_gem_vram_object_unpin, .vmap = drm_gem_vram_object_vmap, .vunmap = drm_gem_vram_object_vunmap,
- .mmap = drm_gem_ttm_mmap, .print_info = drm_gem_ttm_print_info,
};
@@ -915,12 +916,6 @@ static void drm_vram_mm_cleanup(struct drm_vram_mm *vmm) ttm_bo_device_release(&vmm->bdev); }
-static int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma,
struct drm_vram_mm *vmm)-{
- return ttm_bo_mmap(filp, vma, &vmm->bdev);
-}
/*
- Helpers for integration with struct drm_device
*/ @@ -976,30 +971,3 @@ void drm_vram_helper_release_mm(struct drm_device *dev) dev->vram_mm = NULL; } EXPORT_SYMBOL(drm_vram_helper_release_mm);
-/*
- Helpers for &struct file_operations
- */
-/**
- drm_vram_mm_file_operations_mmap() - \
- Implements &struct file_operations.mmap()
- @filp: the mapping's file structure
- @vma: the mapping's memory area
- Returns:
- 0 on success, or
- a negative error code otherwise.
- */
-int drm_vram_mm_file_operations_mmap(
- struct file *filp, struct vm_area_struct *vma)
-{
- struct drm_file *file_priv = filp->private_data;
- struct drm_device *dev = file_priv->minor->dev;
- if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized"))
return -EINVAL;- return drm_vram_mm_mmap(filp, vma, dev->vram_mm);
-} -EXPORT_SYMBOL(drm_vram_mm_file_operations_mmap);
Not needed any more.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/drm_gem_vram_helper.c | 22 ---------------------- 1 file changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index e100b97ea6e3..42ee80414273 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -469,13 +469,6 @@ static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo, *pl = gbo->placement; }
-static int drm_gem_vram_bo_driver_verify_access(struct drm_gem_vram_object *gbo, - struct file *filp) -{ - return drm_vma_node_verify_access(&gbo->bo.base.vma_node, - filp->private_data); -} - static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo, bool evict, struct ttm_mem_reg *new_mem) @@ -767,20 +760,6 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo, drm_gem_vram_bo_driver_evict_flags(gbo, placement); }
-static int bo_driver_verify_access(struct ttm_buffer_object *bo, - struct file *filp) -{ - struct drm_gem_vram_object *gbo; - - /* TTM may pass BOs that are not GEM VRAM BOs. */ - if (!drm_is_gem_vram(bo)) - return -EINVAL; - - gbo = drm_gem_vram_of_bo(bo); - - return drm_gem_vram_bo_driver_verify_access(gbo, filp); -} - static void bo_driver_move_notify(struct ttm_buffer_object *bo, bool evict, struct ttm_mem_reg *new_mem) @@ -837,7 +816,6 @@ static struct ttm_bo_driver bo_driver = { .init_mem_type = bo_driver_init_mem_type, .eviction_valuable = ttm_bo_eviction_valuable, .evict_flags = bo_driver_evict_flags, - .verify_access = bo_driver_verify_access, .move_notify = bo_driver_move_notify, .io_mem_reserve = bo_driver_io_mem_reserve, .io_mem_free = bo_driver_io_mem_free,
Hi
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Not needed any more.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Reviewed-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/drm_gem_vram_helper.c | 22 ---------------------- 1 file changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index e100b97ea6e3..42ee80414273 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -469,13 +469,6 @@ static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo, *pl = gbo->placement; }
-static int drm_gem_vram_bo_driver_verify_access(struct drm_gem_vram_object *gbo,
struct file *filp)-{
- return drm_vma_node_verify_access(&gbo->bo.base.vma_node,
filp->private_data);-}
static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo, bool evict, struct ttm_mem_reg *new_mem) @@ -767,20 +760,6 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo, drm_gem_vram_bo_driver_evict_flags(gbo, placement); }
-static int bo_driver_verify_access(struct ttm_buffer_object *bo,
struct file *filp)-{
- struct drm_gem_vram_object *gbo;
- /* TTM may pass BOs that are not GEM VRAM BOs. */
- if (!drm_is_gem_vram(bo))
return -EINVAL;- gbo = drm_gem_vram_of_bo(bo);
- return drm_gem_vram_bo_driver_verify_access(gbo, filp);
-}
static void bo_driver_move_notify(struct ttm_buffer_object *bo, bool evict, struct ttm_mem_reg *new_mem) @@ -837,7 +816,6 @@ static struct ttm_bo_driver bo_driver = { .init_mem_type = bo_driver_init_mem_type, .eviction_valuable = ttm_bo_eviction_valuable, .evict_flags = bo_driver_evict_flags,
- .verify_access = bo_driver_verify_access, .move_notify = bo_driver_move_notify, .io_mem_reserve = bo_driver_io_mem_reserve, .io_mem_free = bo_driver_io_mem_free,
Not needed any more because we don't have vram specific fops any more. DEFINE_DRM_GEM_FOPS() can be used instead.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- include/drm/drm_gem_vram_helper.h | 18 ---- include/drm/drm_vram_mm_helper.h | 82 +++++++++++++++++++ drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- 7 files changed, 87 insertions(+), 38 deletions(-) create mode 100644 include/drm/drm_vram_mm_helper.h
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 9d5526650291..3503ff784803 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -180,22 +180,4 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm( struct drm_device *dev, uint64_t vram_base, size_t vram_size); void drm_vram_helper_release_mm(struct drm_device *dev);
-/** - * define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \ - &struct file_operations - * - * Drivers that use VRAM MM can use this macro to initialize - * &struct file_operations with default functions. - */ -#define DRM_VRAM_MM_FILE_OPERATIONS \ - .llseek = no_llseek, \ - .read = drm_read, \ - .poll = drm_poll, \ - .unlocked_ioctl = drm_ioctl, \ - .compat_ioctl = drm_compat_ioctl, \ - .mmap = drm_gem_mmap, \ - .open = drm_open, \ - .release = drm_release \ - - #endif diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h new file mode 100644 index 000000000000..a47b49adba62 --- /dev/null +++ b/include/drm/drm_vram_mm_helper.h @@ -0,0 +1,82 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef DRM_VRAM_MM_HELPER_H +#define DRM_VRAM_MM_HELPER_H + +#include <drm/drm_file.h> +#include <drm/drm_ioctl.h> +#include <drm/ttm/ttm_bo_driver.h> + +struct drm_device; + +/** + * struct drm_vram_mm_funcs - Callback functions for &struct drm_vram_mm + * @evict_flags: Provides an implementation for struct \ + &ttm_bo_driver.evict_flags + * @move_notify: Provides an implementation for + * struct &ttm_bo_driver.move_notify + * + * These callback function integrate VRAM MM with TTM buffer objects. New + * functions can be added if necessary. + */ +struct drm_vram_mm_funcs { + void (*evict_flags)(struct ttm_buffer_object *bo, + struct ttm_placement *placement); + void (*move_notify)(struct ttm_buffer_object *bo, bool evict, + struct ttm_mem_reg *new_mem); +}; + +/** + * struct drm_vram_mm - An instance of VRAM MM + * @vram_base: Base address of the managed video memory + * @vram_size: Size of the managed video memory in bytes + * @bdev: The TTM BO device. + * @funcs: TTM BO functions + * + * The fields &struct drm_vram_mm.vram_base and + * &struct drm_vram_mm.vrm_size are managed by VRAM MM, but are + * available for public read access. Use the field + * &struct drm_vram_mm.bdev to access the TTM BO device. + */ +struct drm_vram_mm { + uint64_t vram_base; + size_t vram_size; + + struct ttm_bo_device bdev; + + const struct drm_vram_mm_funcs *funcs; +}; + +/** + * drm_vram_mm_of_bdev() - \ + Returns the container of type &struct ttm_bo_device for field bdev. + * @bdev: the TTM BO device + * + * Returns: + * The containing instance of &struct drm_vram_mm + */ +static inline struct drm_vram_mm *drm_vram_mm_of_bdev( + struct ttm_bo_device *bdev) +{ + return container_of(bdev, struct drm_vram_mm, bdev); +} + +int drm_vram_mm_debugfs_init(struct drm_minor *minor); +int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev, + uint64_t vram_base, size_t vram_size, + const struct drm_vram_mm_funcs *funcs); +void drm_vram_mm_cleanup(struct drm_vram_mm *vmm); + +int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma, + struct drm_vram_mm *vmm); + +/* + * Helpers for integration with struct drm_device + */ + +struct drm_vram_mm *drm_vram_helper_alloc_mm( + struct drm_device *dev, uint64_t vram_base, size_t vram_size, + const struct drm_vram_mm_funcs *funcs); +void drm_vram_helper_release_mm(struct drm_device *dev); + +#endif diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index e0e8770462bc..1f17794b0890 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -200,10 +200,7 @@ static struct pci_driver ast_pci_driver = { .driver.pm = &ast_pm_ops, };
-static const struct file_operations ast_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(ast_fops);
static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM, diff --git a/drivers/gpu/drm/bochs/bochs_drv.c b/drivers/gpu/drm/bochs/bochs_drv.c index 3b9b0d9bbc14..10460878414e 100644 --- a/drivers/gpu/drm/bochs/bochs_drv.c +++ b/drivers/gpu/drm/bochs/bochs_drv.c @@ -58,10 +58,7 @@ static int bochs_load(struct drm_device *dev) return ret; }
-static const struct file_operations bochs_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(bochs_fops);
static struct drm_driver bochs_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index f5b35fdef6f3..b6fdac91e502 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -26,10 +26,7 @@ #include "hibmc_drm_drv.h" #include "hibmc_drm_regs.h"
-static const struct file_operations hibmc_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(hibmc_fops);
static irqreturn_t hibmc_drm_interrupt(int irq, void *arg) { diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 4f9df3b93598..397f8b0a9af8 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -58,10 +58,7 @@ static void mga_pci_remove(struct pci_dev *pdev) drm_put_dev(dev); }
-static const struct file_operations mgag200_driver_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
static struct drm_driver driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET, diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 862db495d111..0c37032c8b65 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -189,10 +189,7 @@ static struct pci_driver vbox_pci_driver = { #endif };
-static const struct file_operations vbox_fops = { - .owner = THIS_MODULE, - DRM_VRAM_MM_FILE_OPERATIONS -}; +DEFINE_DRM_GEM_FOPS(vbox_fops);
static struct drm_driver driver = { .driver_features =
Am 13.09.19 um 14:29 schrieb Gerd Hoffmann:
Not needed any more because we don't have vram specific fops any more. DEFINE_DRM_GEM_FOPS() can be used instead.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
include/drm/drm_gem_vram_helper.h | 18 ---- include/drm/drm_vram_mm_helper.h | 82 +++++++++++++++++++ drivers/gpu/drm/ast/ast_drv.c | 5 +- drivers/gpu/drm/bochs/bochs_drv.c | 5 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +- 7 files changed, 87 insertions(+), 38 deletions(-) create mode 100644 include/drm/drm_vram_mm_helper.h
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 9d5526650291..3503ff784803 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -180,22 +180,4 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm( struct drm_device *dev, uint64_t vram_base, size_t vram_size); void drm_vram_helper_release_mm(struct drm_device *dev);
-/**
- define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \
- &struct file_operations
- Drivers that use VRAM MM can use this macro to initialize
- &struct file_operations with default functions.
- */
-#define DRM_VRAM_MM_FILE_OPERATIONS \
- .llseek = no_llseek, \
- .read = drm_read, \
- .poll = drm_poll, \
- .unlocked_ioctl = drm_ioctl, \
- .compat_ioctl = drm_compat_ioctl, \
- .mmap = drm_gem_mmap, \
- .open = drm_open, \
- .release = drm_release \
#endif diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h new file mode 100644
Please rebase onto the latest drm-tip. This entire file has been removed in a recent patch.
With this change applied:
Reviewed-by: Thomas Zimmermann tzimmermann@suse.de
index 000000000000..a47b49adba62 --- /dev/null +++ b/include/drm/drm_vram_mm_helper.h @@ -0,0 +1,82 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef DRM_VRAM_MM_HELPER_H +#define DRM_VRAM_MM_HELPER_H
+#include <drm/drm_file.h> +#include <drm/drm_ioctl.h> +#include <drm/ttm/ttm_bo_driver.h>
+struct drm_device;
+/**
- struct drm_vram_mm_funcs - Callback functions for &struct drm_vram_mm
- @evict_flags: Provides an implementation for struct \
- &ttm_bo_driver.evict_flags
- @move_notify: Provides an implementation for
struct &ttm_bo_driver.move_notify
- These callback function integrate VRAM MM with TTM buffer objects. New
- functions can be added if necessary.
- */
+struct drm_vram_mm_funcs {
- void (*evict_flags)(struct ttm_buffer_object *bo,
struct ttm_placement *placement);- void (*move_notify)(struct ttm_buffer_object *bo, bool evict,
struct ttm_mem_reg *new_mem);+};
+/**
- struct drm_vram_mm - An instance of VRAM MM
- @vram_base: Base address of the managed video memory
- @vram_size: Size of the managed video memory in bytes
- @bdev: The TTM BO device.
- @funcs: TTM BO functions
- The fields &struct drm_vram_mm.vram_base and
- &struct drm_vram_mm.vrm_size are managed by VRAM MM, but are
- available for public read access. Use the field
- &struct drm_vram_mm.bdev to access the TTM BO device.
- */
+struct drm_vram_mm {
- uint64_t vram_base;
- size_t vram_size;
- struct ttm_bo_device bdev;
- const struct drm_vram_mm_funcs *funcs;
+};
+/**
- drm_vram_mm_of_bdev() - \
- Returns the container of type &struct ttm_bo_device for field bdev.
- @bdev: the TTM BO device
- Returns:
- The containing instance of &struct drm_vram_mm
- */
+static inline struct drm_vram_mm *drm_vram_mm_of_bdev(
- struct ttm_bo_device *bdev)
+{
- return container_of(bdev, struct drm_vram_mm, bdev);
+}
+int drm_vram_mm_debugfs_init(struct drm_minor *minor); +int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
uint64_t vram_base, size_t vram_size,const struct drm_vram_mm_funcs *funcs);+void drm_vram_mm_cleanup(struct drm_vram_mm *vmm);
+int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma,
struct drm_vram_mm *vmm);+/*
- Helpers for integration with struct drm_device
- */
+struct drm_vram_mm *drm_vram_helper_alloc_mm(
- struct drm_device *dev, uint64_t vram_base, size_t vram_size,
- const struct drm_vram_mm_funcs *funcs);
+void drm_vram_helper_release_mm(struct drm_device *dev);
+#endif diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index e0e8770462bc..1f17794b0890 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -200,10 +200,7 @@ static struct pci_driver ast_pci_driver = { .driver.pm = &ast_pm_ops, };
-static const struct file_operations ast_fops = {
- .owner = THIS_MODULE,
- DRM_VRAM_MM_FILE_OPERATIONS
-}; +DEFINE_DRM_GEM_FOPS(ast_fops);
static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM, diff --git a/drivers/gpu/drm/bochs/bochs_drv.c b/drivers/gpu/drm/bochs/bochs_drv.c index 3b9b0d9bbc14..10460878414e 100644 --- a/drivers/gpu/drm/bochs/bochs_drv.c +++ b/drivers/gpu/drm/bochs/bochs_drv.c @@ -58,10 +58,7 @@ static int bochs_load(struct drm_device *dev) return ret; }
-static const struct file_operations bochs_fops = {
- .owner = THIS_MODULE,
- DRM_VRAM_MM_FILE_OPERATIONS
-}; +DEFINE_DRM_GEM_FOPS(bochs_fops);
static struct drm_driver bochs_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index f5b35fdef6f3..b6fdac91e502 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -26,10 +26,7 @@ #include "hibmc_drm_drv.h" #include "hibmc_drm_regs.h"
-static const struct file_operations hibmc_fops = {
- .owner = THIS_MODULE,
- DRM_VRAM_MM_FILE_OPERATIONS
-}; +DEFINE_DRM_GEM_FOPS(hibmc_fops);
static irqreturn_t hibmc_drm_interrupt(int irq, void *arg) { diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 4f9df3b93598..397f8b0a9af8 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -58,10 +58,7 @@ static void mga_pci_remove(struct pci_dev *pdev) drm_put_dev(dev); }
-static const struct file_operations mgag200_driver_fops = {
- .owner = THIS_MODULE,
- DRM_VRAM_MM_FILE_OPERATIONS
-}; +DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
static struct drm_driver driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET, diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 862db495d111..0c37032c8b65 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -189,10 +189,7 @@ static struct pci_driver vbox_pci_driver = { #endif };
-static const struct file_operations vbox_fops = {
- .owner = THIS_MODULE,
- DRM_VRAM_MM_FILE_OPERATIONS
-}; +DEFINE_DRM_GEM_FOPS(vbox_fops);
static struct drm_driver driver = { .driver_features =
include/drm/drm_vram_mm_helper.h | 82 +++++++++++++++++++
diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h new file mode 100644
Please rebase onto the latest drm-tip. This entire file has been removed in a recent patch.
I did rebase already, then re-added the file by mistake. Didn't pay enough attention while solving the conflict. Fixed now.
cheers, Gerd
On Fri, Sep 13, 2019 at 02:29:00PM +0200, Gerd Hoffmann wrote:
Add mmap callback to drm_object
Oops, hit sent to early. This should have been this:
Add mmap callback to struct drm_gem_object_funcs, which is supposed to handle the vma setup. It will be used by both normal fops->mmap (via drm_gem_mmap_obj()) and prime mmap (via drm_gem_prime_mmap()).
For starters the shmem and vram helpers are switched over to the new workflow, to show things in action for review.
cheers, Gerd
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Gerd Hoffmann -
Rob Herring -
Steven Price -
Thomas Zimmermann