From 1c5aafa6eee2d5712f774676d407e5ab6dae9a1b Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 16 Apr 2013 14:14:52 +0200 Subject: drm/gem: Split drm_gem_mmap() into object search and object mapping The drm_gem_mmap() function first finds the GEM object to be mapped based on the fake mmap offset and then maps the object. Split the object mapping code into a standalone drm_gem_mmap_obj() function that can be used to implement dma-buf mmap() operations. Signed-off-by: Laurent Pinchart Reviewed-by: Rob Clark --- drivers/gpu/drm/drm_gem.c | 83 +++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 31 deletions(-) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index cf919e36e8ae..43217138816d 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -644,6 +644,55 @@ void drm_gem_vm_close(struct vm_area_struct *vma) } EXPORT_SYMBOL(drm_gem_vm_close); +/** + * drm_gem_mmap_obj - memory map a GEM object + * @obj: the GEM object to map + * @obj_size: the object size to be mapped, in bytes + * @vma: VMA for the area to be mapped + * + * Set up the VMA to prepare mapping of the GEM object using the gem_vm_ops + * provided by the driver. Depending on their requirements, drivers can either + * provide a fault handler in their gem_vm_ops (in which case any accesses to + * the object will be trapped, to perform migration, GTT binding, surface + * register allocation, or performance monitoring), or mmap the buffer memory + * synchronously after calling drm_gem_mmap_obj. + * + * This function is mainly intended to implement the DMABUF mmap operation, when + * the GEM object is not looked up based on its fake offset. To implement the + * DRM mmap operation, drivers should use the drm_gem_mmap() function. + * + * Return 0 or success or -EINVAL if the object size is smaller than the VMA + * size, or if no gem_vm_ops are provided. + */ +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; + + /* Check for valid size. */ + if (obj_size < vma->vm_end - vma->vm_start) + return -EINVAL; + + if (!dev->driver->gem_vm_ops) + return -EINVAL; + + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; + vma->vm_ops = dev->driver->gem_vm_ops; + vma->vm_private_data = obj; + vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); + + /* Take a ref for this mapping of the object, so that the fault + * handler can dereference the mmap offset's pointer to the object. + * This reference is cleaned up by the corresponding vm_close + * (which should happen whether the vma was created by this call, or + * by a vm_open due to mremap or partial unmap or whatever). + */ + drm_gem_object_reference(obj); + + drm_vm_open_locked(dev, vma); + return 0; +} +EXPORT_SYMBOL(drm_gem_mmap_obj); /** * drm_gem_mmap - memory map routine for GEM objects @@ -653,11 +702,9 @@ EXPORT_SYMBOL(drm_gem_vm_close); * If a driver supports GEM object mapping, mmap calls on the DRM file * descriptor will end up here. * - * If we find the object based on the offset passed in (vma->vm_pgoff will + * Look up the GEM object based on the offset passed in (vma->vm_pgoff will * contain the fake offset we created when the GTT map ioctl was called on - * the object), we set up the driver fault handler so that any accesses - * to the object can be trapped, to perform migration, GTT binding, surface - * register allocation, or performance monitoring. + * the object) and map it with a call to drm_gem_mmap_obj(). */ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) { @@ -665,7 +712,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_device *dev = priv->minor->dev; struct drm_gem_mm *mm = dev->mm_private; struct drm_local_map *map = NULL; - struct drm_gem_object *obj; struct drm_hash_item *hash; int ret = 0; @@ -686,32 +732,7 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) goto out_unlock; } - /* Check for valid size. */ - if (map->size < vma->vm_end - vma->vm_start) { - ret = -EINVAL; - goto out_unlock; - } - - obj = map->handle; - if (!obj->dev->driver->gem_vm_ops) { - ret = -EINVAL; - goto out_unlock; - } - - vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; - vma->vm_ops = obj->dev->driver->gem_vm_ops; - vma->vm_private_data = map->handle; - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); - - /* Take a ref for this mapping of the object, so that the fault - * handler can dereference the mmap offset's pointer to the object. - * This reference is cleaned up by the corresponding vm_close - * (which should happen whether the vma was created by this call, or - * by a vm_open due to mremap or partial unmap or whatever). - */ - drm_gem_object_reference(obj); - - drm_vm_open_locked(dev, vma); + ret = drm_gem_mmap_obj(map->handle, map->size, vma); out_unlock: mutex_unlock(&dev->struct_mutex); -- cgit v1.2.3 From 4368dd846d067ed7d11281050e7676ae614d6053 Mon Sep 17 00:00:00 2001 From: YoungJun Cho Date: Thu, 27 Jun 2013 08:39:58 +0900 Subject: drm/gem: add mutex lock when using drm_gem_mmap_obj The drm_gem_mmap_obj() has to be protected with dev->struct_mutex, but some caller functions do not. So it adds mutex lock to missing callers and adds assertion to check whether drm_gem_mmap_obj() is called with mutex lock or not. Signed-off-by: YoungJun Cho Signed-off-by: Seung-Woo Kim Signed-off-by: Kyungmin Park Reviewed-by: Maarten Lankhorst Reviewed-by: Laurent Pinchart Reviewed-by: Rob Clark Reviewed-by: Maarten Lankhorst Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 43217138816d..34c0be70f178 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -661,6 +661,8 @@ EXPORT_SYMBOL(drm_gem_vm_close); * the GEM object is not looked up based on its fake offset. To implement the * DRM mmap operation, drivers should use the drm_gem_mmap() function. * + * NOTE: This function has to be protected with dev->struct_mutex + * * Return 0 or success or -EINVAL if the object size is smaller than the VMA * size, or if no gem_vm_ops are provided. */ @@ -669,6 +671,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, { struct drm_device *dev = obj->dev; + lockdep_assert_held(&dev->struct_mutex); + /* Check for valid size. */ if (obj_size < vma->vm_end - vma->vm_start) return -EINVAL; -- cgit v1.2.3 From 2e07fb229396f99fc173d8612f0f83ea9de0341b Mon Sep 17 00:00:00 2001 From: YoungJun Cho Date: Thu, 27 Jun 2013 08:58:33 +0900 Subject: drm/gem: fix not to assign error value to gem name If idr_alloc() is failed, obj->name can be error value. Also it cleans up duplicated flink processing code. This regression has been introduced in commit 2e928815c1886fe628ed54623aa98d0889cf5509 Author: Tejun Heo Date: Wed Feb 27 17:04:08 2013 -0800 drm: convert to idr_alloc() Signed-off-by: YoungJun Cho Signed-off-by: Seung-Woo Kim Signed-off-by: Kyungmin Park Cc: stable@vger.kernel.org Reviewed-by: Chris Wilson Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 34c0be70f178..bcedaf7b73b9 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -453,25 +453,21 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, spin_lock(&dev->object_name_lock); if (!obj->name) { ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT); - obj->name = ret; - args->name = (uint64_t) obj->name; - spin_unlock(&dev->object_name_lock); - idr_preload_end(); - if (ret < 0) goto err; - ret = 0; + + obj->name = ret; /* Allocate a reference for the name table. */ drm_gem_object_reference(obj); - } else { - args->name = (uint64_t) obj->name; - spin_unlock(&dev->object_name_lock); - idr_preload_end(); - ret = 0; } + args->name = (uint64_t) obj->name; + ret = 0; + err: + spin_unlock(&dev->object_name_lock); + idr_preload_end(); drm_gem_object_unreference_unlocked(obj); return ret; } -- cgit v1.2.3 From 77ef8bbc87be7ad10b410247efc6d0f10676b401 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 1 Jul 2013 20:32:58 +0200 Subject: drm: make drm_mm_init() return void There is no reason to return "int" as this function never fails. Furthermore, several drivers (ast, sis) already depend on this. Signed-off-by: David Herrmann Reviewed-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index bcedaf7b73b9..603f256152ef 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -108,12 +108,8 @@ drm_gem_init(struct drm_device *dev) return -ENOMEM; } - if (drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START, - DRM_FILE_PAGE_OFFSET_SIZE)) { - drm_ht_remove(&mm->offset_hash); - kfree(mm); - return -ENOMEM; - } + drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START, + DRM_FILE_PAGE_OFFSET_SIZE); return 0; } -- cgit v1.2.3