From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Herrmann Subject: Re: [PATCH v4 2/4] drm/gem: convert to new unified vma manager Date: Wed, 24 Jul 2013 18:27:33 +0200 Message-ID: References: <1374084860-29768-1-git-send-email-dh.herrmann@gmail.com> <1374583636-14248-1-git-send-email-dh.herrmann@gmail.com> <1374583636-14248-3-git-send-email-dh.herrmann@gmail.com> <20130724155217.GV5939@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-oa0-f48.google.com (mail-oa0-f48.google.com [209.85.219.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 61E36E5C85 for ; Wed, 24 Jul 2013 09:27:34 -0700 (PDT) Received: by mail-oa0-f48.google.com with SMTP id f4so1519104oah.35 for ; Wed, 24 Jul 2013 09:27:33 -0700 (PDT) In-Reply-To: <20130724155217.GV5939@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , "dri-devel@lists.freedesktop.org" , Dave Airlie List-Id: dri-devel@lists.freedesktop.org Hi On Wed, Jul 24, 2013 at 5:52 PM, Daniel Vetter 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 >> Cc: Daniel Vetter >> Cc: Rob Clark >> Cc: Dave Airlie >> Cc: Thierry Reding >> Signed-off-by: David Herrmann >> Acked-by: Patrik Jakobsson >> --- >> 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 >> #include >> #include >> +#include >> >> /** @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 >> #include >> #include >> - >> -static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) >> -{ >> - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; >> -} >> +#include >> >> /* >> * __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 >> +#include >> >> #include >> #include >> @@ -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 >> #include >> #include >> +#include >> #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 >> +#include >> #include >> #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<> + (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 >> #include >> +#include >> >> #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 >> #include >> #include >> +#include >> >> #include >> >> @@ -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 > > 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