| Message ID | 1374583636-14248-3-git-send-email-dh.herrmann@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 23, 2013 at 02:47:14PM +0200, David Herrmann wrote: > Use the new vma manager instead of the old hashtable. Also convert all > drivers to use the new convenience helpers. This drops all the > (map_list.hash.key << PAGE_SHIFT) non-sense. > > Locking and access-management is exactly the same as before with an > additional lock inside of the vma-manager, which strictly wouldn't be > needed for gem. > > v2: > - rebase on drm-next > - init nodes via drm_vma_node_reset() in drm_gem.c > v3: > - fix tegra > v4: > - remove duplicate if (drm_vma_node_has_offset()) checks > - inline now trivial drm_vma_node_offset_addr() calls > > Cc: Inki Dae <inki.dae@samsung.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > --- > drivers/gpu/drm/drm_gem.c | 89 +++++------------------------- > drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++---- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 14 ++--- > drivers/gpu/drm/gma500/gem.c | 15 ++--- > drivers/gpu/drm/i915/i915_gem.c | 10 ++-- > drivers/gpu/drm/omapdrm/omap_gem.c | 28 +++++----- > drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +--------------- > drivers/gpu/drm/udl/udl_gem.c | 13 ++--- > drivers/gpu/host1x/drm/gem.c | 5 +- > include/drm/drmP.h | 7 +-- > include/uapi/drm/drm.h | 2 +- > 11 files changed, 62 insertions(+), 186 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 1ad9e7e..8d3cdf3 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -37,6 +37,7 @@ > #include <linux/shmem_fs.h> > #include <linux/dma-buf.h> > #include <drm/drmP.h> > +#include <drm/drm_vma_manager.h> > > /** @file drm_gem.c > * > @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev) > } > > dev->mm_private = mm; > - > - if (drm_ht_create(&mm->offset_hash, 12)) { > - kfree(mm); > - return -ENOMEM; > - } > - > - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START, > - DRM_FILE_PAGE_OFFSET_SIZE); > + drm_vma_offset_manager_init(&mm->vma_manager, > + DRM_FILE_PAGE_OFFSET_START, > + DRM_FILE_PAGE_OFFSET_SIZE); > > return 0; > } > @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev) > { > struct drm_gem_mm *mm = dev->mm_private; > > - drm_mm_takedown(&mm->offset_manager); > - drm_ht_remove(&mm->offset_hash); > + drm_vma_offset_manager_destroy(&mm->vma_manager); > kfree(mm); > dev->mm_private = NULL; > } > @@ -160,6 +155,7 @@ void drm_gem_private_object_init(struct drm_device *dev, > > kref_init(&obj->refcount); > atomic_set(&obj->handle_count, 0); > + drm_vma_node_reset(&obj->vma_node); We already presume (but it's not really documented) that a gem object is cleared already (or allocated with kzalloc) so feels a bit like overhead. > obj->size = size; > } > EXPORT_SYMBOL(drm_gem_private_object_init); > @@ -302,12 +298,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj) > { > struct drm_device *dev = obj->dev; > struct drm_gem_mm *mm = dev->mm_private; > - struct drm_map_list *list = &obj->map_list; > > - drm_ht_remove_item(&mm->offset_hash, &list->hash); > - drm_mm_put_block(list->file_offset_node); > - kfree(list->map); > - list->map = NULL; > + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node); > } > EXPORT_SYMBOL(drm_gem_free_mmap_offset); > > @@ -327,54 +319,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj) > { > struct drm_device *dev = obj->dev; > struct drm_gem_mm *mm = dev->mm_private; > - struct drm_map_list *list; > - struct drm_local_map *map; > - int ret; > - > - /* Set the object up for mmap'ing */ > - list = &obj->map_list; > - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); > - if (!list->map) > - return -ENOMEM; > - > - map = list->map; > - map->type = _DRM_GEM; > - map->size = obj->size; > - map->handle = obj; > - > - /* Get a DRM GEM mmap offset allocated... */ > - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, > - obj->size / PAGE_SIZE, 0, false); > - > - if (!list->file_offset_node) { > - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); > - ret = -ENOSPC; > - goto out_free_list; > - } > > - list->file_offset_node = drm_mm_get_block(list->file_offset_node, > - obj->size / PAGE_SIZE, 0); > - if (!list->file_offset_node) { > - ret = -ENOMEM; > - goto out_free_list; > - } > - > - list->hash.key = list->file_offset_node->start; > - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); > - if (ret) { > - DRM_ERROR("failed to add to map hash\n"); > - goto out_free_mm; > - } > - > - return 0; > - > -out_free_mm: > - drm_mm_put_block(list->file_offset_node); > -out_free_list: > - kfree(list->map); > - list->map = NULL; > - > - return ret; > + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, > + obj->size / PAGE_SIZE); > } > EXPORT_SYMBOL(drm_gem_create_mmap_offset); > > @@ -703,8 +650,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) > struct drm_file *priv = filp->private_data; > struct drm_device *dev = priv->minor->dev; > struct drm_gem_mm *mm = dev->mm_private; > - struct drm_local_map *map = NULL; > - struct drm_hash_item *hash; > + struct drm_gem_object *obj; > + struct drm_vma_offset_node *node; > int ret = 0; > > if (drm_device_is_unplugged(dev)) > @@ -712,21 +659,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) > > mutex_lock(&dev->struct_mutex); > > - if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) { > + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff); > + if (!node) { > mutex_unlock(&dev->struct_mutex); > return drm_mmap(filp, vma); > } > > - map = drm_hash_entry(hash, struct drm_map_list, hash)->map; > - if (!map || > - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) { > - ret = -EPERM; > - goto out_unlock; > - } > - > - ret = drm_gem_mmap_obj(map->handle, map->size, vma); > + obj = container_of(node, struct drm_gem_object, vma_node); > + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma); Iirc the old code only resulted in a ht hit if the start of the mmap regioni exactly matched the offset. The new code (I think at least, maybe I've misread it a bit) will return an object even if the start isn't aligned. drm_gem_mmap_obj checks already that the object is big enough but we don't check the start any more. I think this should be readded. It's a bit funny that gem wants an exact match but is ok with just mapping the beginning of the object, but I guess that abi is now enshrined ;-) > > -out_unlock: > mutex_unlock(&dev->struct_mutex); > > return ret; > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > index ece72a8..bf09a6dc 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -27,11 +27,7 @@ > #include <drm/drmP.h> > #include <drm/drm.h> > #include <drm/drm_gem_cma_helper.h> > - > -static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) > -{ > - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; > -} > +#include <drm/drm_vma_manager.h> > > /* > * __drm_gem_cma_create - Create a GEM CMA object without allocating memory > @@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) > { > struct drm_gem_cma_object *cma_obj; > > - if (gem_obj->map_list.map) > - drm_gem_free_mmap_offset(gem_obj); > + drm_gem_free_mmap_offset(gem_obj); > > cma_obj = to_drm_gem_cma_obj(gem_obj); > > @@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv, > return -EINVAL; > } > > - *offset = get_gem_mmap_offset(gem_obj); > + *offset = (unsigned int)drm_vma_node_offset_addr(&gem_obj->vma_node); *offset is an u64, same as drm_vma_node_offset_addr. Why do we need a cast here? > > drm_gem_object_unreference(gem_obj); > > @@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m > { > struct drm_gem_object *obj = &cma_obj->base; > struct drm_device *dev = obj->dev; > - uint64_t off = 0; > + uint64_t off; > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > > - if (obj->map_list.map) > - off = (uint64_t)obj->map_list.hash.key; > + off = drm_vma_node_start(&obj->vma_node); > > seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d", > obj->name, obj->refcount.refcount.counter, > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c > index 24c22a8..be32db1 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c > @@ -10,6 +10,7 @@ > */ > > #include <drm/drmP.h> > +#include <drm/drm_vma_manager.h> > > #include <linux/shmem_fs.h> > #include <drm/exynos_drm.h> > @@ -152,8 +153,7 @@ out: > exynos_drm_fini_buf(obj->dev, buf); > exynos_gem_obj->buffer = NULL; > > - if (obj->map_list.map) > - drm_gem_free_mmap_offset(obj); > + drm_gem_free_mmap_offset(obj); > > /* release file pointer to gem object. */ > drm_gem_object_release(obj); > @@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv, > goto unlock; > } > > - if (!obj->map_list.map) { > - ret = drm_gem_create_mmap_offset(obj); > - if (ret) > - goto out; > - } > + ret = drm_gem_create_mmap_offset(obj); > + if (ret) > + goto out; > > - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; > + *offset = drm_vma_node_offset_addr(&obj->vma_node); > DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset); > > out: > diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c > index fe1d332..2f77bea 100644 > --- a/drivers/gpu/drm/gma500/gem.c > +++ b/drivers/gpu/drm/gma500/gem.c > @@ -26,6 +26,7 @@ > #include <drm/drmP.h> > #include <drm/drm.h> > #include <drm/gma_drm.h> > +#include <drm/drm_vma_manager.h> > #include "psb_drv.h" > > int psb_gem_init_object(struct drm_gem_object *obj) > @@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) > struct gtt_range *gtt = container_of(obj, struct gtt_range, gem); > > /* Remove the list map if one is present */ > - if (obj->map_list.map) > - drm_gem_free_mmap_offset(obj); > + drm_gem_free_mmap_offset(obj); > drm_gem_object_release(obj); > > /* This must occur last as it frees up the memory of the GEM object */ > @@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, > /* What validation is needed here ? */ > > /* Make it mmapable */ > - if (!obj->map_list.map) { > - ret = drm_gem_create_mmap_offset(obj); > - if (ret) > - goto out; > - } > - /* GEM should really work out the hash offsets for us */ > - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; > + ret = drm_gem_create_mmap_offset(obj); > + if (ret) > + goto out; > + *offset = drm_vma_node_offset_addr(&obj->vma_node); > out: > drm_gem_object_unreference(obj); > unlock: > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 46bf7e3..53f81b3 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -26,6 +26,7 @@ > */ > > #include <drm/drmP.h> > +#include <drm/drm_vma_manager.h> > #include <drm/i915_drm.h> > #include "i915_drv.h" > #include "i915_trace.h" > @@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) > > if (obj->base.dev->dev_mapping) > unmap_mapping_range(obj->base.dev->dev_mapping, > - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT, > + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node), > obj->base.size, 1); > > obj->fault_mappable = false; > @@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) > struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > int ret; > > - if (obj->base.map_list.map) > + if (drm_vma_node_has_offset(&obj->base.vma_node)) > return 0; > > dev_priv->mm.shrinker_no_lock_stealing = true; > @@ -1517,9 +1518,6 @@ out: > > static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj) > { > - if (!obj->base.map_list.map) > - return; > - > drm_gem_free_mmap_offset(&obj->base); > } > > @@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file, > if (ret) > goto out; > > - *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT; > + *offset = drm_vma_node_offset_addr(&obj->base.vma_node); > > out: > drm_gem_object_unreference(&obj->base); > diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c > index cbcd71e..f90531fc00 100644 > --- a/drivers/gpu/drm/omapdrm/omap_gem.c > +++ b/drivers/gpu/drm/omapdrm/omap_gem.c > @@ -20,6 +20,7 @@ > > #include <linux/spinlock.h> > #include <linux/shmem_fs.h> > +#include <drm/drm_vma_manager.h> > > #include "omap_drv.h" > #include "omap_dmm_tiler.h" > @@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj) > static uint64_t mmap_offset(struct drm_gem_object *obj) > { > struct drm_device *dev = obj->dev; > + int ret; > + size_t size; > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > > - if (!obj->map_list.map) { > - /* Make it mmapable */ > - size_t size = omap_gem_mmap_size(obj); > - int ret = _drm_gem_create_mmap_offset_size(obj, size); > - > - if (ret) { > - dev_err(dev->dev, "could not allocate mmap offset\n"); > - return 0; > - } > + /* Make it mmapable */ > + size = omap_gem_mmap_size(obj); > + ret = _drm_gem_create_mmap_offset_size(obj, size); > + if (ret) { > + dev_err(dev->dev, "could not allocate mmap offset\n"); > + return 0; > } > > - return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT; > + return drm_vma_node_offset_addr(&obj->vma_node); > } > > uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj) > @@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m) > { > struct drm_device *dev = obj->dev; > struct omap_gem_object *omap_obj = to_omap_bo(obj); > - uint64_t off = 0; > + uint64_t off; > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > > - if (obj->map_list.map) > - off = (uint64_t)obj->map_list.hash.key; > + off = drm_vma_node_start(&obj->vma_node); > > seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d", > omap_obj->flags, obj->name, obj->refcount.refcount.counter, > @@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj) > > list_del(&omap_obj->mm_list); > > - if (obj->map_list.map) > - drm_gem_free_mmap_offset(obj); > + drm_gem_free_mmap_offset(obj); > > /* this means the object is still pinned.. which really should > * not happen. I think.. > diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c > index f9eb679..dbb1575 100644 > --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c > +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c > @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) > { > struct drm_device *dev = obj->dev; > struct drm_gem_mm *mm = dev->mm_private; > - struct drm_map_list *list; > - struct drm_local_map *map; > - int ret = 0; > - > - /* Set the object up for mmap'ing */ > - list = &obj->map_list; > - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); > - if (!list->map) > - return -ENOMEM; > - > - map = list->map; > - map->type = _DRM_GEM; > - map->size = size; > - map->handle = obj; > - > - /* Get a DRM GEM mmap offset allocated... */ > - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, > - size / PAGE_SIZE, 0, 0); > - > - if (!list->file_offset_node) { > - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); > - ret = -ENOSPC; > - goto out_free_list; > - } > - > - list->file_offset_node = drm_mm_get_block(list->file_offset_node, > - size / PAGE_SIZE, 0); > - if (!list->file_offset_node) { > - ret = -ENOMEM; > - goto out_free_list; > - } > - > - list->hash.key = list->file_offset_node->start; > - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); > - if (ret) { > - DRM_ERROR("failed to add to map hash\n"); > - goto out_free_mm; > - } > - > - return 0; > - > -out_free_mm: > - drm_mm_put_block(list->file_offset_node); > -out_free_list: > - kfree(list->map); > - list->map = NULL; > > - return ret; > + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, > + size / PAGE_SIZE); > } > diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c > index ef034fa..2a4cb2f 100644 > --- a/drivers/gpu/drm/udl/udl_gem.c > +++ b/drivers/gpu/drm/udl/udl_gem.c > @@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj) > if (obj->pages) > udl_gem_put_pages(obj); > > - if (gem_obj->map_list.map) > - drm_gem_free_mmap_offset(gem_obj); > + drm_gem_free_mmap_offset(gem_obj); > } > > /* the dumb interface doesn't work with the GEM straight MMAP > @@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev, > ret = udl_gem_get_pages(gobj, GFP_KERNEL); > if (ret) > goto out; > - if (!gobj->base.map_list.map) { > - ret = drm_gem_create_mmap_offset(obj); > - if (ret) > - goto out; > - } > + ret = drm_gem_create_mmap_offset(obj); > + if (ret) > + goto out; > > - *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT; > + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node); > > out: > drm_gem_object_unreference(&gobj->base); > diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c > index c5e9a9b..bc323b3 100644 > --- a/drivers/gpu/host1x/drm/gem.c > +++ b/drivers/gpu/host1x/drm/gem.c > @@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo) > > unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo) > { > - return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT; > + return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node); Ok, tegra is pretty funny here about mmap offsets, using an int. Whatever, pre-existing fanciness so not our problem. > } > > struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size) > @@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem) > { > struct tegra_bo *bo = to_tegra_bo(gem); > > - if (gem->map_list.map) > - drm_gem_free_mmap_offset(gem); > + drm_gem_free_mmap_offset(gem); > > drm_gem_object_release(gem); > tegra_bo_destroy(gem->dev, bo); > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 0ab6a09..4b518e0 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -71,6 +71,7 @@ > #include <asm/pgalloc.h> > #include <drm/drm.h> > #include <drm/drm_sarea.h> > +#include <drm/drm_vma_manager.h> > > #include <linux/idr.h> > > @@ -587,7 +588,6 @@ struct drm_map_list { > struct drm_local_map *map; /**< mapping */ > uint64_t user_token; > struct drm_master *master; > - struct drm_mm_node *file_offset_node; /**< fake offset */ > }; > > /** > @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info { > * GEM specific mm private for tracking GEM objects > */ > struct drm_gem_mm { > - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */ > - struct drm_open_hash offset_hash; /**< User token hash table for maps */ > + struct drm_vma_offset_manager vma_manager; > }; > > /** > @@ -644,7 +643,7 @@ struct drm_gem_object { > struct file *filp; > > /* Mapping info for this object */ > - struct drm_map_list map_list; > + struct drm_vma_offset_node vma_node; > > /** > * Size of the object, in bytes. Immutable over the object's > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 238a166..272580c 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -181,7 +181,7 @@ enum drm_map_type { > _DRM_AGP = 3, /**< AGP/GART */ > _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */ > _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */ > - _DRM_GEM = 6, /**< GEM object */ > + _DRM_GEM = 6, /**< GEM object (obsolete) */ > }; > > /** > -- > 1.8.3.3 A few small things, with those addressed this is: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> I guess with the common infrastructure we'll have a bit of room for follow-up cleanups, but that can be done on especially rainy autumn days ;-) Cheers, Daniel
Hi On Wed, Jul 24, 2013 at 5:52 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Jul 23, 2013 at 02:47:14PM +0200, David Herrmann wrote: >> Use the new vma manager instead of the old hashtable. Also convert all >> drivers to use the new convenience helpers. This drops all the >> (map_list.hash.key << PAGE_SHIFT) non-sense. >> >> Locking and access-management is exactly the same as before with an >> additional lock inside of the vma-manager, which strictly wouldn't be >> needed for gem. >> >> v2: >> - rebase on drm-next >> - init nodes via drm_vma_node_reset() in drm_gem.c >> v3: >> - fix tegra >> v4: >> - remove duplicate if (drm_vma_node_has_offset()) checks >> - inline now trivial drm_vma_node_offset_addr() calls >> >> Cc: Inki Dae <inki.dae@samsung.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Rob Clark <robdclark@gmail.com> >> Cc: Dave Airlie <airlied@redhat.com> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> >> Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >> --- >> drivers/gpu/drm/drm_gem.c | 89 +++++------------------------- >> drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++---- >> drivers/gpu/drm/exynos/exynos_drm_gem.c | 14 ++--- >> drivers/gpu/drm/gma500/gem.c | 15 ++--- >> drivers/gpu/drm/i915/i915_gem.c | 10 ++-- >> drivers/gpu/drm/omapdrm/omap_gem.c | 28 +++++----- >> drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +--------------- >> drivers/gpu/drm/udl/udl_gem.c | 13 ++--- >> drivers/gpu/host1x/drm/gem.c | 5 +- >> include/drm/drmP.h | 7 +-- >> include/uapi/drm/drm.h | 2 +- >> 11 files changed, 62 insertions(+), 186 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >> index 1ad9e7e..8d3cdf3 100644 >> --- a/drivers/gpu/drm/drm_gem.c >> +++ b/drivers/gpu/drm/drm_gem.c >> @@ -37,6 +37,7 @@ >> #include <linux/shmem_fs.h> >> #include <linux/dma-buf.h> >> #include <drm/drmP.h> >> +#include <drm/drm_vma_manager.h> >> >> /** @file drm_gem.c >> * >> @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev) >> } >> >> dev->mm_private = mm; >> - >> - if (drm_ht_create(&mm->offset_hash, 12)) { >> - kfree(mm); >> - return -ENOMEM; >> - } >> - >> - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START, >> - DRM_FILE_PAGE_OFFSET_SIZE); >> + drm_vma_offset_manager_init(&mm->vma_manager, >> + DRM_FILE_PAGE_OFFSET_START, >> + DRM_FILE_PAGE_OFFSET_SIZE); >> >> return 0; >> } >> @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev) >> { >> struct drm_gem_mm *mm = dev->mm_private; >> >> - drm_mm_takedown(&mm->offset_manager); >> - drm_ht_remove(&mm->offset_hash); >> + drm_vma_offset_manager_destroy(&mm->vma_manager); >> kfree(mm); >> dev->mm_private = NULL; >> } >> @@ -160,6 +155,7 @@ void drm_gem_private_object_init(struct drm_device *dev, >> >> kref_init(&obj->refcount); >> atomic_set(&obj->handle_count, 0); >> + drm_vma_node_reset(&obj->vma_node); > > We already presume (but it's not really documented) that a gem object is > cleared already (or allocated with kzalloc) so feels a bit like overhead. I am fine with dropping it, if that's what gem already requires from drivers. >> obj->size = size; >> } >> EXPORT_SYMBOL(drm_gem_private_object_init); >> @@ -302,12 +298,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj) >> { >> struct drm_device *dev = obj->dev; >> struct drm_gem_mm *mm = dev->mm_private; >> - struct drm_map_list *list = &obj->map_list; >> >> - drm_ht_remove_item(&mm->offset_hash, &list->hash); >> - drm_mm_put_block(list->file_offset_node); >> - kfree(list->map); >> - list->map = NULL; >> + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node); >> } >> EXPORT_SYMBOL(drm_gem_free_mmap_offset); >> >> @@ -327,54 +319,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj) >> { >> struct drm_device *dev = obj->dev; >> struct drm_gem_mm *mm = dev->mm_private; >> - struct drm_map_list *list; >> - struct drm_local_map *map; >> - int ret; >> - >> - /* Set the object up for mmap'ing */ >> - list = &obj->map_list; >> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); >> - if (!list->map) >> - return -ENOMEM; >> - >> - map = list->map; >> - map->type = _DRM_GEM; >> - map->size = obj->size; >> - map->handle = obj; >> - >> - /* Get a DRM GEM mmap offset allocated... */ >> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, >> - obj->size / PAGE_SIZE, 0, false); >> - >> - if (!list->file_offset_node) { >> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); >> - ret = -ENOSPC; >> - goto out_free_list; >> - } >> >> - list->file_offset_node = drm_mm_get_block(list->file_offset_node, >> - obj->size / PAGE_SIZE, 0); >> - if (!list->file_offset_node) { >> - ret = -ENOMEM; >> - goto out_free_list; >> - } >> - >> - list->hash.key = list->file_offset_node->start; >> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); >> - if (ret) { >> - DRM_ERROR("failed to add to map hash\n"); >> - goto out_free_mm; >> - } >> - >> - return 0; >> - >> -out_free_mm: >> - drm_mm_put_block(list->file_offset_node); >> -out_free_list: >> - kfree(list->map); >> - list->map = NULL; >> - >> - return ret; >> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, >> + obj->size / PAGE_SIZE); >> } >> EXPORT_SYMBOL(drm_gem_create_mmap_offset); >> >> @@ -703,8 +650,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) >> struct drm_file *priv = filp->private_data; >> struct drm_device *dev = priv->minor->dev; >> struct drm_gem_mm *mm = dev->mm_private; >> - struct drm_local_map *map = NULL; >> - struct drm_hash_item *hash; >> + struct drm_gem_object *obj; >> + struct drm_vma_offset_node *node; >> int ret = 0; >> >> if (drm_device_is_unplugged(dev)) >> @@ -712,21 +659,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) >> >> mutex_lock(&dev->struct_mutex); >> >> - if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) { >> + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff); >> + if (!node) { >> mutex_unlock(&dev->struct_mutex); >> return drm_mmap(filp, vma); >> } >> >> - map = drm_hash_entry(hash, struct drm_map_list, hash)->map; >> - if (!map || >> - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) { >> - ret = -EPERM; >> - goto out_unlock; >> - } >> - >> - ret = drm_gem_mmap_obj(map->handle, map->size, vma); >> + obj = container_of(node, struct drm_gem_object, vma_node); >> + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma); > > Iirc the old code only resulted in a ht hit if the start of the mmap > regioni exactly matched the offset. The new code (I think at least, maybe > I've misread it a bit) will return an object even if the start isn't > aligned. drm_gem_mmap_obj checks already that the object is big enough but > we don't check the start any more. I think this should be readded. > > It's a bit funny that gem wants an exact match but is ok with just mapping > the beginning of the object, but I guess that abi is now enshrined ;-) drm_vma_offset_exact_lookup() was meant for exactly that but it fails to actually verify it. I can add a "drm_vma_node_start() == start" check and return NULL if it doesn't match. The name is currently a bit misleading. I will fix that. >> >> -out_unlock: >> mutex_unlock(&dev->struct_mutex); >> >> return ret; >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c >> index ece72a8..bf09a6dc 100644 >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >> @@ -27,11 +27,7 @@ >> #include <drm/drmP.h> >> #include <drm/drm.h> >> #include <drm/drm_gem_cma_helper.h> >> - >> -static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) >> -{ >> - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; >> -} >> +#include <drm/drm_vma_manager.h> >> >> /* >> * __drm_gem_cma_create - Create a GEM CMA object without allocating memory >> @@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) >> { >> struct drm_gem_cma_object *cma_obj; >> >> - if (gem_obj->map_list.map) >> - drm_gem_free_mmap_offset(gem_obj); >> + drm_gem_free_mmap_offset(gem_obj); >> >> cma_obj = to_drm_gem_cma_obj(gem_obj); >> >> @@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv, >> return -EINVAL; >> } >> >> - *offset = get_gem_mmap_offset(gem_obj); >> + *offset = (unsigned int)drm_vma_node_offset_addr(&gem_obj->vma_node); > > *offset is an u64, same as drm_vma_node_offset_addr. Why do we need a cast > here? Copied from the helper above. I can fix it. >> >> drm_gem_object_unreference(gem_obj); >> >> @@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m >> { >> struct drm_gem_object *obj = &cma_obj->base; >> struct drm_device *dev = obj->dev; >> - uint64_t off = 0; >> + uint64_t off; >> >> WARN_ON(!mutex_is_locked(&dev->struct_mutex)); >> >> - if (obj->map_list.map) >> - off = (uint64_t)obj->map_list.hash.key; >> + off = drm_vma_node_start(&obj->vma_node); >> >> seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d", >> obj->name, obj->refcount.refcount.counter, >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c >> index 24c22a8..be32db1 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c >> @@ -10,6 +10,7 @@ >> */ >> >> #include <drm/drmP.h> >> +#include <drm/drm_vma_manager.h> >> >> #include <linux/shmem_fs.h> >> #include <drm/exynos_drm.h> >> @@ -152,8 +153,7 @@ out: >> exynos_drm_fini_buf(obj->dev, buf); >> exynos_gem_obj->buffer = NULL; >> >> - if (obj->map_list.map) >> - drm_gem_free_mmap_offset(obj); >> + drm_gem_free_mmap_offset(obj); >> >> /* release file pointer to gem object. */ >> drm_gem_object_release(obj); >> @@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv, >> goto unlock; >> } >> >> - if (!obj->map_list.map) { >> - ret = drm_gem_create_mmap_offset(obj); >> - if (ret) >> - goto out; >> - } >> + ret = drm_gem_create_mmap_offset(obj); >> + if (ret) >> + goto out; >> >> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; >> + *offset = drm_vma_node_offset_addr(&obj->vma_node); >> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset); >> >> out: >> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c >> index fe1d332..2f77bea 100644 >> --- a/drivers/gpu/drm/gma500/gem.c >> +++ b/drivers/gpu/drm/gma500/gem.c >> @@ -26,6 +26,7 @@ >> #include <drm/drmP.h> >> #include <drm/drm.h> >> #include <drm/gma_drm.h> >> +#include <drm/drm_vma_manager.h> >> #include "psb_drv.h" >> >> int psb_gem_init_object(struct drm_gem_object *obj) >> @@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) >> struct gtt_range *gtt = container_of(obj, struct gtt_range, gem); >> >> /* Remove the list map if one is present */ >> - if (obj->map_list.map) >> - drm_gem_free_mmap_offset(obj); >> + drm_gem_free_mmap_offset(obj); >> drm_gem_object_release(obj); >> >> /* This must occur last as it frees up the memory of the GEM object */ >> @@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, >> /* What validation is needed here ? */ >> >> /* Make it mmapable */ >> - if (!obj->map_list.map) { >> - ret = drm_gem_create_mmap_offset(obj); >> - if (ret) >> - goto out; >> - } >> - /* GEM should really work out the hash offsets for us */ >> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; >> + ret = drm_gem_create_mmap_offset(obj); >> + if (ret) >> + goto out; >> + *offset = drm_vma_node_offset_addr(&obj->vma_node); >> out: >> drm_gem_object_unreference(obj); >> unlock: >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 46bf7e3..53f81b3 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -26,6 +26,7 @@ >> */ >> >> #include <drm/drmP.h> >> +#include <drm/drm_vma_manager.h> >> #include <drm/i915_drm.h> >> #include "i915_drv.h" >> #include "i915_trace.h" >> @@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) >> >> if (obj->base.dev->dev_mapping) >> unmap_mapping_range(obj->base.dev->dev_mapping, >> - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT, >> + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node), >> obj->base.size, 1); >> >> obj->fault_mappable = false; >> @@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) >> struct drm_i915_private *dev_priv = obj->base.dev->dev_private; >> int ret; >> >> - if (obj->base.map_list.map) >> + if (drm_vma_node_has_offset(&obj->base.vma_node)) >> return 0; >> >> dev_priv->mm.shrinker_no_lock_stealing = true; >> @@ -1517,9 +1518,6 @@ out: >> >> static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj) >> { >> - if (!obj->base.map_list.map) >> - return; >> - >> drm_gem_free_mmap_offset(&obj->base); >> } >> >> @@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file, >> if (ret) >> goto out; >> >> - *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT; >> + *offset = drm_vma_node_offset_addr(&obj->base.vma_node); >> >> out: >> drm_gem_object_unreference(&obj->base); >> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c >> index cbcd71e..f90531fc00 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_gem.c >> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c >> @@ -20,6 +20,7 @@ >> >> #include <linux/spinlock.h> >> #include <linux/shmem_fs.h> >> +#include <drm/drm_vma_manager.h> >> >> #include "omap_drv.h" >> #include "omap_dmm_tiler.h" >> @@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj) >> static uint64_t mmap_offset(struct drm_gem_object *obj) >> { >> struct drm_device *dev = obj->dev; >> + int ret; >> + size_t size; >> >> WARN_ON(!mutex_is_locked(&dev->struct_mutex)); >> >> - if (!obj->map_list.map) { >> - /* Make it mmapable */ >> - size_t size = omap_gem_mmap_size(obj); >> - int ret = _drm_gem_create_mmap_offset_size(obj, size); >> - >> - if (ret) { >> - dev_err(dev->dev, "could not allocate mmap offset\n"); >> - return 0; >> - } >> + /* Make it mmapable */ >> + size = omap_gem_mmap_size(obj); >> + ret = _drm_gem_create_mmap_offset_size(obj, size); >> + if (ret) { >> + dev_err(dev->dev, "could not allocate mmap offset\n"); >> + return 0; >> } >> >> - return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT; >> + return drm_vma_node_offset_addr(&obj->vma_node); >> } >> >> uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj) >> @@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m) >> { >> struct drm_device *dev = obj->dev; >> struct omap_gem_object *omap_obj = to_omap_bo(obj); >> - uint64_t off = 0; >> + uint64_t off; >> >> WARN_ON(!mutex_is_locked(&dev->struct_mutex)); >> >> - if (obj->map_list.map) >> - off = (uint64_t)obj->map_list.hash.key; >> + off = drm_vma_node_start(&obj->vma_node); >> >> seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d", >> omap_obj->flags, obj->name, obj->refcount.refcount.counter, >> @@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj) >> >> list_del(&omap_obj->mm_list); >> >> - if (obj->map_list.map) >> - drm_gem_free_mmap_offset(obj); >> + drm_gem_free_mmap_offset(obj); >> >> /* this means the object is still pinned.. which really should >> * not happen. I think.. >> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c >> index f9eb679..dbb1575 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c >> +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c >> @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) >> { >> struct drm_device *dev = obj->dev; >> struct drm_gem_mm *mm = dev->mm_private; >> - struct drm_map_list *list; >> - struct drm_local_map *map; >> - int ret = 0; >> - >> - /* Set the object up for mmap'ing */ >> - list = &obj->map_list; >> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); >> - if (!list->map) >> - return -ENOMEM; >> - >> - map = list->map; >> - map->type = _DRM_GEM; >> - map->size = size; >> - map->handle = obj; >> - >> - /* Get a DRM GEM mmap offset allocated... */ >> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, >> - size / PAGE_SIZE, 0, 0); >> - >> - if (!list->file_offset_node) { >> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); >> - ret = -ENOSPC; >> - goto out_free_list; >> - } >> - >> - list->file_offset_node = drm_mm_get_block(list->file_offset_node, >> - size / PAGE_SIZE, 0); >> - if (!list->file_offset_node) { >> - ret = -ENOMEM; >> - goto out_free_list; >> - } >> - >> - list->hash.key = list->file_offset_node->start; >> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); >> - if (ret) { >> - DRM_ERROR("failed to add to map hash\n"); >> - goto out_free_mm; >> - } >> - >> - return 0; >> - >> -out_free_mm: >> - drm_mm_put_block(list->file_offset_node); >> -out_free_list: >> - kfree(list->map); >> - list->map = NULL; >> >> - return ret; >> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, >> + size / PAGE_SIZE); >> } >> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c >> index ef034fa..2a4cb2f 100644 >> --- a/drivers/gpu/drm/udl/udl_gem.c >> +++ b/drivers/gpu/drm/udl/udl_gem.c >> @@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj) >> if (obj->pages) >> udl_gem_put_pages(obj); >> >> - if (gem_obj->map_list.map) >> - drm_gem_free_mmap_offset(gem_obj); >> + drm_gem_free_mmap_offset(gem_obj); >> } >> >> /* the dumb interface doesn't work with the GEM straight MMAP >> @@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev, >> ret = udl_gem_get_pages(gobj, GFP_KERNEL); >> if (ret) >> goto out; >> - if (!gobj->base.map_list.map) { >> - ret = drm_gem_create_mmap_offset(obj); >> - if (ret) >> - goto out; >> - } >> + ret = drm_gem_create_mmap_offset(obj); >> + if (ret) >> + goto out; >> >> - *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT; >> + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node); >> >> out: >> drm_gem_object_unreference(&gobj->base); >> diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c >> index c5e9a9b..bc323b3 100644 >> --- a/drivers/gpu/host1x/drm/gem.c >> +++ b/drivers/gpu/host1x/drm/gem.c >> @@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo) >> >> unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo) >> { >> - return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT; >> + return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node); > > Ok, tegra is pretty funny here about mmap offsets, using an int. > Whatever, pre-existing fanciness so not our problem. All TTM helpers use "unsigned int", too. We can fix that in a followup patch. >> } >> >> struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size) >> @@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem) >> { >> struct tegra_bo *bo = to_tegra_bo(gem); >> >> - if (gem->map_list.map) >> - drm_gem_free_mmap_offset(gem); >> + drm_gem_free_mmap_offset(gem); >> >> drm_gem_object_release(gem); >> tegra_bo_destroy(gem->dev, bo); >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index 0ab6a09..4b518e0 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -71,6 +71,7 @@ >> #include <asm/pgalloc.h> >> #include <drm/drm.h> >> #include <drm/drm_sarea.h> >> +#include <drm/drm_vma_manager.h> >> >> #include <linux/idr.h> >> >> @@ -587,7 +588,6 @@ struct drm_map_list { >> struct drm_local_map *map; /**< mapping */ >> uint64_t user_token; >> struct drm_master *master; >> - struct drm_mm_node *file_offset_node; /**< fake offset */ >> }; >> >> /** >> @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info { >> * GEM specific mm private for tracking GEM objects >> */ >> struct drm_gem_mm { >> - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */ >> - struct drm_open_hash offset_hash; /**< User token hash table for maps */ >> + struct drm_vma_offset_manager vma_manager; >> }; >> >> /** >> @@ -644,7 +643,7 @@ struct drm_gem_object { >> struct file *filp; >> >> /* Mapping info for this object */ >> - struct drm_map_list map_list; >> + struct drm_vma_offset_node vma_node; >> >> /** >> * Size of the object, in bytes. Immutable over the object's >> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h >> index 238a166..272580c 100644 >> --- a/include/uapi/drm/drm.h >> +++ b/include/uapi/drm/drm.h >> @@ -181,7 +181,7 @@ enum drm_map_type { >> _DRM_AGP = 3, /**< AGP/GART */ >> _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */ >> _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */ >> - _DRM_GEM = 6, /**< GEM object */ >> + _DRM_GEM = 6, /**< GEM object (obsolete) */ >> }; >> >> /** >> -- >> 1.8.3.3 > > A few small things, with those addressed this is: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > I guess with the common infrastructure we'll have a bit of room for > follow-up cleanups, but that can be done on especially rainy autumn days > ;-) Thanks! David
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 1ad9e7e..8d3cdf3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -37,6 +37,7 @@ #include <linux/shmem_fs.h> #include <linux/dma-buf.h> #include <drm/drmP.h> +#include <drm/drm_vma_manager.h> /** @file drm_gem.c * @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev) } dev->mm_private = mm; - - if (drm_ht_create(&mm->offset_hash, 12)) { - kfree(mm); - return -ENOMEM; - } - - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START, - DRM_FILE_PAGE_OFFSET_SIZE); + drm_vma_offset_manager_init(&mm->vma_manager, + DRM_FILE_PAGE_OFFSET_START, + DRM_FILE_PAGE_OFFSET_SIZE); return 0; } @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev) { struct drm_gem_mm *mm = dev->mm_private; - drm_mm_takedown(&mm->offset_manager); - drm_ht_remove(&mm->offset_hash); + drm_vma_offset_manager_destroy(&mm->vma_manager); kfree(mm); dev->mm_private = NULL; } @@ -160,6 +155,7 @@ void drm_gem_private_object_init(struct drm_device *dev, kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0); + drm_vma_node_reset(&obj->vma_node); obj->size = size; } EXPORT_SYMBOL(drm_gem_private_object_init); @@ -302,12 +298,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list = &obj->map_list; - drm_ht_remove_item(&mm->offset_hash, &list->hash); - drm_mm_put_block(list->file_offset_node); - kfree(list->map); - list->map = NULL; + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node); } EXPORT_SYMBOL(drm_gem_free_mmap_offset); @@ -327,54 +319,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list; - struct drm_local_map *map; - int ret; - - /* Set the object up for mmap'ing */ - list = &obj->map_list; - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); - if (!list->map) - return -ENOMEM; - - map = list->map; - map->type = _DRM_GEM; - map->size = obj->size; - map->handle = obj; - - /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->size / PAGE_SIZE, 0, false); - - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); - ret = -ENOSPC; - goto out_free_list; - } - list->file_offset_node = drm_mm_get_block(list->file_offset_node, - obj->size / PAGE_SIZE, 0); - if (!list->file_offset_node) { - ret = -ENOMEM; - goto out_free_list; - } - - list->hash.key = list->file_offset_node->start; - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); - if (ret) { - DRM_ERROR("failed to add to map hash\n"); - goto out_free_mm; - } - - return 0; - -out_free_mm: - drm_mm_put_block(list->file_offset_node); -out_free_list: - kfree(list->map); - list->map = NULL; - - return ret; + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, + obj->size / PAGE_SIZE); } EXPORT_SYMBOL(drm_gem_create_mmap_offset); @@ -703,8 +650,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_file *priv = filp->private_data; struct drm_device *dev = priv->minor->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_local_map *map = NULL; - struct drm_hash_item *hash; + struct drm_gem_object *obj; + struct drm_vma_offset_node *node; int ret = 0; if (drm_device_is_unplugged(dev)) @@ -712,21 +659,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) mutex_lock(&dev->struct_mutex); - if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) { + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff); + if (!node) { mutex_unlock(&dev->struct_mutex); return drm_mmap(filp, vma); } - map = drm_hash_entry(hash, struct drm_map_list, hash)->map; - if (!map || - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) { - ret = -EPERM; - goto out_unlock; - } - - ret = drm_gem_mmap_obj(map->handle, map->size, vma); + obj = container_of(node, struct drm_gem_object, vma_node); + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma); -out_unlock: mutex_unlock(&dev->struct_mutex); return ret; diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index ece72a8..bf09a6dc 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -27,11 +27,7 @@ #include <drm/drmP.h> #include <drm/drm.h> #include <drm/drm_gem_cma_helper.h> - -static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) -{ - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; -} +#include <drm/drm_vma_manager.h> /* * __drm_gem_cma_create - Create a GEM CMA object without allocating memory @@ -172,8 +168,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) { struct drm_gem_cma_object *cma_obj; - if (gem_obj->map_list.map) - drm_gem_free_mmap_offset(gem_obj); + drm_gem_free_mmap_offset(gem_obj); cma_obj = to_drm_gem_cma_obj(gem_obj); @@ -237,7 +232,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv, return -EINVAL; } - *offset = get_gem_mmap_offset(gem_obj); + *offset = (unsigned int)drm_vma_node_offset_addr(&gem_obj->vma_node); drm_gem_object_unreference(gem_obj); @@ -301,12 +296,11 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m { struct drm_gem_object *obj = &cma_obj->base; struct drm_device *dev = obj->dev; - uint64_t off = 0; + uint64_t off; WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - if (obj->map_list.map) - off = (uint64_t)obj->map_list.hash.key; + off = drm_vma_node_start(&obj->vma_node); seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d", obj->name, obj->refcount.refcount.counter, diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 24c22a8..be32db1 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -10,6 +10,7 @@ */ #include <drm/drmP.h> +#include <drm/drm_vma_manager.h> #include <linux/shmem_fs.h> #include <drm/exynos_drm.h> @@ -152,8 +153,7 @@ out: exynos_drm_fini_buf(obj->dev, buf); exynos_gem_obj->buffer = NULL; - if (obj->map_list.map) - drm_gem_free_mmap_offset(obj); + drm_gem_free_mmap_offset(obj); /* release file pointer to gem object. */ drm_gem_object_release(obj); @@ -703,13 +703,11 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv, goto unlock; } - if (!obj->map_list.map) { - ret = drm_gem_create_mmap_offset(obj); - if (ret) - goto out; - } + ret = drm_gem_create_mmap_offset(obj); + if (ret) + goto out; - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&obj->vma_node); DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset); out: diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index fe1d332..2f77bea 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -26,6 +26,7 @@ #include <drm/drmP.h> #include <drm/drm.h> #include <drm/gma_drm.h> +#include <drm/drm_vma_manager.h> #include "psb_drv.h" int psb_gem_init_object(struct drm_gem_object *obj) @@ -38,8 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) struct gtt_range *gtt = container_of(obj, struct gtt_range, gem); /* Remove the list map if one is present */ - if (obj->map_list.map) - drm_gem_free_mmap_offset(obj); + drm_gem_free_mmap_offset(obj); drm_gem_object_release(obj); /* This must occur last as it frees up the memory of the GEM object */ @@ -81,13 +81,10 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, /* What validation is needed here ? */ /* Make it mmapable */ - if (!obj->map_list.map) { - ret = drm_gem_create_mmap_offset(obj); - if (ret) - goto out; - } - /* GEM should really work out the hash offsets for us */ - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; + ret = drm_gem_create_mmap_offset(obj); + if (ret) + goto out; + *offset = drm_vma_node_offset_addr(&obj->vma_node); out: drm_gem_object_unreference(obj); unlock: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 46bf7e3..53f81b3 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -26,6 +26,7 @@ */ #include <drm/drmP.h> +#include <drm/drm_vma_manager.h> #include <drm/i915_drm.h> #include "i915_drv.h" #include "i915_trace.h" @@ -1428,7 +1429,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) if (obj->base.dev->dev_mapping) unmap_mapping_range(obj->base.dev->dev_mapping, - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT, + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node), obj->base.size, 1); obj->fault_mappable = false; @@ -1486,7 +1487,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) struct drm_i915_private *dev_priv = obj->base.dev->dev_private; int ret; - if (obj->base.map_list.map) + if (drm_vma_node_has_offset(&obj->base.vma_node)) return 0; dev_priv->mm.shrinker_no_lock_stealing = true; @@ -1517,9 +1518,6 @@ out: static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj) { - if (!obj->base.map_list.map) - return; - drm_gem_free_mmap_offset(&obj->base); } @@ -1558,7 +1556,7 @@ i915_gem_mmap_gtt(struct drm_file *file, if (ret) goto out; - *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&obj->base.vma_node); out: drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index cbcd71e..f90531fc00 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -20,6 +20,7 @@ #include <linux/spinlock.h> #include <linux/shmem_fs.h> +#include <drm/drm_vma_manager.h> #include "omap_drv.h" #include "omap_dmm_tiler.h" @@ -308,21 +309,20 @@ uint32_t omap_gem_flags(struct drm_gem_object *obj) static uint64_t mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; + int ret; + size_t size; WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - if (!obj->map_list.map) { - /* Make it mmapable */ - size_t size = omap_gem_mmap_size(obj); - int ret = _drm_gem_create_mmap_offset_size(obj, size); - - if (ret) { - dev_err(dev->dev, "could not allocate mmap offset\n"); - return 0; - } + /* Make it mmapable */ + size = omap_gem_mmap_size(obj); + ret = _drm_gem_create_mmap_offset_size(obj, size); + if (ret) { + dev_err(dev->dev, "could not allocate mmap offset\n"); + return 0; } - return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT; + return drm_vma_node_offset_addr(&obj->vma_node); } uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj) @@ -997,12 +997,11 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m) { struct drm_device *dev = obj->dev; struct omap_gem_object *omap_obj = to_omap_bo(obj); - uint64_t off = 0; + uint64_t off; WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - if (obj->map_list.map) - off = (uint64_t)obj->map_list.hash.key; + off = drm_vma_node_start(&obj->vma_node); seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d", omap_obj->flags, obj->name, obj->refcount.refcount.counter, @@ -1309,8 +1308,7 @@ void omap_gem_free_object(struct drm_gem_object *obj) list_del(&omap_obj->mm_list); - if (obj->map_list.map) - drm_gem_free_mmap_offset(obj); + drm_gem_free_mmap_offset(obj); /* this means the object is still pinned.. which really should * not happen. I think.. diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c index f9eb679..dbb1575 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list; - struct drm_local_map *map; - int ret = 0; - - /* Set the object up for mmap'ing */ - list = &obj->map_list; - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); - if (!list->map) - return -ENOMEM; - - map = list->map; - map->type = _DRM_GEM; - map->size = size; - map->handle = obj; - - /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - size / PAGE_SIZE, 0, 0); - - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); - ret = -ENOSPC; - goto out_free_list; - } - - list->file_offset_node = drm_mm_get_block(list->file_offset_node, - size / PAGE_SIZE, 0); - if (!list->file_offset_node) { - ret = -ENOMEM; - goto out_free_list; - } - - list->hash.key = list->file_offset_node->start; - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); - if (ret) { - DRM_ERROR("failed to add to map hash\n"); - goto out_free_mm; - } - - return 0; - -out_free_mm: - drm_mm_put_block(list->file_offset_node); -out_free_list: - kfree(list->map); - list->map = NULL; - return ret; + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, + size / PAGE_SIZE); } diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index ef034fa..2a4cb2f 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -223,8 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj) if (obj->pages) udl_gem_put_pages(obj); - if (gem_obj->map_list.map) - drm_gem_free_mmap_offset(gem_obj); + drm_gem_free_mmap_offset(gem_obj); } /* the dumb interface doesn't work with the GEM straight MMAP @@ -247,13 +246,11 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev, ret = udl_gem_get_pages(gobj, GFP_KERNEL); if (ret) goto out; - if (!gobj->base.map_list.map) { - ret = drm_gem_create_mmap_offset(obj); - if (ret) - goto out; - } + ret = drm_gem_create_mmap_offset(obj); + if (ret) + goto out; - *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node); out: drm_gem_object_unreference(&gobj->base); diff --git a/drivers/gpu/host1x/drm/gem.c b/drivers/gpu/host1x/drm/gem.c index c5e9a9b..bc323b3 100644 --- a/drivers/gpu/host1x/drm/gem.c +++ b/drivers/gpu/host1x/drm/gem.c @@ -108,7 +108,7 @@ static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo) unsigned int tegra_bo_get_mmap_offset(struct tegra_bo *bo) { - return (unsigned int)bo->gem.map_list.hash.key << PAGE_SHIFT; + return (unsigned int)drm_vma_node_offset_addr(&bo->gem.vma_node); } struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size) @@ -182,8 +182,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem) { struct tegra_bo *bo = to_tegra_bo(gem); - if (gem->map_list.map) - drm_gem_free_mmap_offset(gem); + drm_gem_free_mmap_offset(gem); drm_gem_object_release(gem); tegra_bo_destroy(gem->dev, bo); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 0ab6a09..4b518e0 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -71,6 +71,7 @@ #include <asm/pgalloc.h> #include <drm/drm.h> #include <drm/drm_sarea.h> +#include <drm/drm_vma_manager.h> #include <linux/idr.h> @@ -587,7 +588,6 @@ struct drm_map_list { struct drm_local_map *map; /**< mapping */ uint64_t user_token; struct drm_master *master; - struct drm_mm_node *file_offset_node; /**< fake offset */ }; /** @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info { * GEM specific mm private for tracking GEM objects */ struct drm_gem_mm { - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */ - struct drm_open_hash offset_hash; /**< User token hash table for maps */ + struct drm_vma_offset_manager vma_manager; }; /** @@ -644,7 +643,7 @@ struct drm_gem_object { struct file *filp; /* Mapping info for this object */ - struct drm_map_list map_list; + struct drm_vma_offset_node vma_node; /** * Size of the object, in bytes. Immutable over the object's diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 238a166..272580c 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -181,7 +181,7 @@ enum drm_map_type { _DRM_AGP = 3, /**< AGP/GART */ _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */ _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */ - _DRM_GEM = 6, /**< GEM object */ + _DRM_GEM = 6, /**< GEM object (obsolete) */ }; /**