In the process of adding GEM support for OMAP DRM driver, I noticed that I was adding code for creating/freeing mmap offsets which was virtually identical to what was already duplicated in i915 and gma500 drivers.
Rather than duplicating the code a 3rd time, it seemed like a good idea to move it to the GEM core.
Note that I don't actually have a way to test psb or i915, but the changes seem straightforward enough.
--
For the curious, OMAP DRM driver is here: https://github.com/robclark/kernel-omap4/commits/linux-omap-3.0-drm
I'll send patches when it's dependencies are merged and it is slightly more than half baked ;-)
Rob Clark (3): drm/gem: add functions for mmap offset creation drm/i915: use common functions for mmap offset creation drm/gma500: use common functions for mmap offset creation
drivers/gpu/drm/drm_gem.c | 88 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem.c | 85 +----------------------------------- drivers/staging/gma500/psb_gem.c | 63 +-------------------------- include/drm/drmP.h | 3 + 4 files changed, 95 insertions(+), 144 deletions(-)
--- drivers/gpu/drm/drm_gem.c | 88 +++++++++++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 3 ++ 2 files changed, 91 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 2b997d2..809358a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -273,6 +273,94 @@ again: } EXPORT_SYMBOL(drm_gem_handle_create);
+ +/** + * drm_gem_free_mmap_offset - release a fake mmap offset for an object + * @obj: obj in question + * + * This routine frees fake offsets allocated by drm_gem_create_mmap_offset(). + */ +void +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; +} +EXPORT_SYMBOL(drm_gem_free_mmap_offset); + +/** + * drm_gem_create_mmap_offset - create a fake mmap offset for an object + * @obj: obj in question + * + * GEM memory mapping works by handing back to userspace a fake mmap offset + * it can use in a subsequent mmap(2) call. The DRM core code then looks + * up the object based on the offset and sets up the various memory mapping + * structures. + * + * This routine allocates and attaches a fake offset for @obj. + */ +int +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 = 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 = 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, 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, + 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; +} +EXPORT_SYMBOL(drm_gem_create_mmap_offset); + /** Returns a reference to the object named by the handle. */ struct drm_gem_object * drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 111e98f..ec156c3 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1622,6 +1622,9 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) drm_gem_object_unreference_unlocked(obj); }
+void drm_gem_free_mmap_offset(struct drm_gem_object *obj); +int drm_gem_create_mmap_offset(struct drm_gem_object *obj); + struct drm_gem_object *drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp, u32 handle);
--- drivers/gpu/drm/i915/i915_gem.c | 85 +-------------------------------------- 1 files changed, 2 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5c0d124..5676eaa 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1265,74 +1265,6 @@ out: }
/** - * i915_gem_create_mmap_offset - create a fake mmap offset for an object - * @obj: obj in question - * - * GEM memory mapping works by handing back to userspace a fake mmap offset - * it can use in a subsequent mmap(2) call. The DRM core code then looks - * up the object based on the offset and sets up the various memory mapping - * structures. - * - * This routine allocates and attaches a fake offset for @obj. - */ -static int -i915_gem_create_mmap_offset(struct drm_i915_gem_object *obj) -{ - struct drm_device *dev = obj->base.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->base.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->base.size; - map->handle = obj; - - /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->base.size / PAGE_SIZE, - 0, 0); - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", - obj->base.name); - ret = -ENOSPC; - goto out_free_list; - } - - list->file_offset_node = drm_mm_get_block(list->file_offset_node, - obj->base.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; -} - -/** * i915_gem_release_mmap - remove physical page mappings * @obj: obj in question * @@ -1360,19 +1292,6 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) obj->fault_mappable = false; }
-static void -i915_gem_free_mmap_offset(struct drm_i915_gem_object *obj) -{ - struct drm_device *dev = obj->base.dev; - struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list = &obj->base.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; -} - static uint32_t i915_gem_get_gtt_size(struct drm_i915_gem_object *obj) { @@ -1493,7 +1412,7 @@ i915_gem_mmap_gtt(struct drm_file *file, }
if (!obj->base.map_list.map) { - ret = i915_gem_create_mmap_offset(obj); + ret = drm_gem_create_mmap_offset(&obj->base); if (ret) goto out; } @@ -3614,7 +3533,7 @@ static void i915_gem_free_object_tail(struct drm_i915_gem_object *obj) trace_i915_gem_object_destroy(obj);
if (obj->base.map_list.map) - i915_gem_free_mmap_offset(obj); + drm_gem_free_mmap_offset(&obj->base);
drm_gem_object_release(&obj->base); i915_gem_info_remove_obj(dev_priv, obj->base.size);
--- drivers/staging/gma500/psb_gem.c | 63 +------------------------------------ 1 files changed, 2 insertions(+), 61 deletions(-)
diff --git a/drivers/staging/gma500/psb_gem.c b/drivers/staging/gma500/psb_gem.c index 76ff7ba..3a397f5 100644 --- a/drivers/staging/gma500/psb_gem.c +++ b/drivers/staging/gma500/psb_gem.c @@ -42,13 +42,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) struct gtt_range *gtt = container_of(obj, struct gtt_range, gem); psb_gtt_free_range(obj->dev, gtt); if (obj->map_list.map) { - /* Do things GEM should do for us */ - struct drm_gem_mm *mm = obj->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_gem_free_mmap_offset(obj); } drm_gem_object_release(obj); } @@ -60,59 +54,6 @@ int psb_gem_get_aperture(struct drm_device *dev, void *data, }
/** - * psb_gem_create_mmap_offset - invent an mmap offset - * @obj: our object - * - * This is basically doing by hand a pile of ugly crap which should - * be done automatically by the GEM library code but isn't - */ -static int psb_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; - - list = &obj->map_list; - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); - if (list->map == NULL) - return -ENOMEM; - map = list->map; - map->type = _DRM_GEM; - map->size = obj->size; - map->handle =obj; - - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->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 free_it; - } - 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 free_it; - } - 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 free_mm; - } - return 0; - -free_mm: - drm_mm_put_block(list->file_offset_node); -free_it: - kfree(list->map); - list->map = NULL; - return ret; -} - -/** * psb_gem_dumb_map_gtt - buffer mapping for dumb interface * @file: our drm client file * @dev: drm device @@ -142,7 +83,7 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, /* Make it mmapable */ if (!obj->map_list.map) { - ret = psb_gem_create_mmap_offset(obj); + ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
On Mon, 18 Jul 2011 19:20:56 -0500 Rob Clark rob@ti.com wrote:
In the process of adding GEM support for OMAP DRM driver, I noticed that I was adding code for creating/freeing mmap offsets which was virtually identical to what was already duplicated in i915 and gma500 drivers.
The gma500 one was taken from i915 and I'd reach the same conclusion as you - the current GMA500 driver (see staging git) has the mmap offset patches broken out in gem_glue.c ready to do what you are proposing.
Rather than duplicating the code a 3rd time, it seemed like a good idea to move it to the GEM core.
Agreed entirely.
Note that I don't actually have a way to test psb or i915, but the changes seem straightforward enough.
Your patch doesn't apply versus gma500 but its trivial to remove it from gem_glue.c once it goes into the mainstream GEM.
(The other bits in the gma500 glue are to allow for GEM objects backed by things other than shmfs)
Alan
On Tue, Jul 19, 2011 at 3:57 AM, Alan Cox alan@lxorguk.ukuu.org.uk wrote:
On Mon, 18 Jul 2011 19:20:56 -0500 Rob Clark rob@ti.com wrote:
In the process of adding GEM support for OMAP DRM driver, I noticed that I was adding code for creating/freeing mmap offsets which was virtually identical to what was already duplicated in i915 and gma500 drivers.
The gma500 one was taken from i915 and I'd reach the same conclusion as you - the current GMA500 driver (see staging git) has the mmap offset patches broken out in gem_glue.c ready to do what you are proposing.
ahh, ok.. I should check this out (although I'm not entirely sure where your gma500 staging tree is)
I'm already using your patch to add drm_gem_private_object_init() for scanout buffers (which need to be physically contiguous). But perhaps you have some other useful "gems"
BR, -R
Rather than duplicating the code a 3rd time, it seemed like a good idea to move it to the GEM core.
Agreed entirely.
Note that I don't actually have a way to test psb or i915, but the changes seem straightforward enough.
Your patch doesn't apply versus gma500 but its trivial to remove it from gem_glue.c once it goes into the mainstream GEM.
(The other bits in the gma500 glue are to allow for GEM objects backed by things other than shmfs)
Alan _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
ahh, ok.. I should check this out (although I'm not entirely sure where your gma500 staging tree is)
Its in GregKH's staging tree or linux-next. It's basically what you've done so if your patch is submitted it'll take me 2 minutes to fix the gma500 tree.
I'm already using your patch to add drm_gem_private_object_init() for scanout buffers (which need to be physically contiguous). But perhaps you have some other useful "gems"
No I think that one and the offset patch is it.
I do need to attack the fb console code next because I need a GEM object as console in some cases and don't have enough virtual address space to vremap it to look linear.
Alan
On Mon, 18 Jul 2011 19:20:56 -0500, Rob Clark rob@ti.com wrote:
In the process of adding GEM support for OMAP DRM driver, I noticed that I was adding code for creating/freeing mmap offsets which was virtually identical to what was already duplicated in i915 and gma500 drivers.
Rather than duplicating the code a 3rd time, it seemed like a good idea to move it to the GEM core.
Note that I don't actually have a way to test psb or i915, but the changes seem straightforward enough.
My only concern is that for the common functions the mmap_offset to create should be passed in a parameter, so that we could support more than one mapping for an object. -Chris
On Tue, Jul 19, 2011 at 4:33 AM, Chris Wilson chris@chris-wilson.co.uk wrote:
On Mon, 18 Jul 2011 19:20:56 -0500, Rob Clark rob@ti.com wrote:
In the process of adding GEM support for OMAP DRM driver, I noticed that I was adding code for creating/freeing mmap offsets which was virtually identical to what was already duplicated in i915 and gma500 drivers.
Rather than duplicating the code a 3rd time, it seemed like a good idea to move it to the GEM core.
Note that I don't actually have a way to test psb or i915, but the changes seem straightforward enough.
My only concern is that for the common functions the mmap_offset to create should be passed in a parameter, so that we could support more than one mapping for an object.
I admit I've not got quite as far as dealing with this yet.. I'm just starting on the dri2 part in xorg driver. (Previous pvr xorg driver has some non-GEM way of sharing buffers.) So I'm figuring out some of this stuff as I go.
For me I think it isn't the end of the world to have same offset in all processes, although I'm interested if there is a better way. There is just one 'struct drm_local_map' in 'struct drm_gem_object', so I admit that I'm not quite sure how this was intended to work.
BR, -R
-Chris
-- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Jul 19, 2011 at 8:12 AM, Rob Clark robdclark@gmail.com wrote:
On Tue, Jul 19, 2011 at 4:33 AM, Chris Wilson chris@chris-wilson.co.uk wrote:
On Mon, 18 Jul 2011 19:20:56 -0500, Rob Clark rob@ti.com wrote:
In the process of adding GEM support for OMAP DRM driver, I noticed that I was adding code for creating/freeing mmap offsets which was virtually identical to what was already duplicated in i915 and gma500 drivers.
Rather than duplicating the code a 3rd time, it seemed like a good idea to move it to the GEM core.
Note that I don't actually have a way to test psb or i915, but the changes seem straightforward enough.
My only concern is that for the common functions the mmap_offset to create should be passed in a parameter, so that we could support more than one mapping for an object.
I admit I've not got quite as far as dealing with this yet.. I'm just starting on the dri2 part in xorg driver. (Previous pvr xorg driver has some non-GEM way of sharing buffers.) So I'm figuring out some of this stuff as I go.
For me I think it isn't the end of the world to have same offset in all processes, although I'm interested if there is a better way. There is just one 'struct drm_local_map' in 'struct drm_gem_object', so I admit that I'm not quite sure how this was intended to work.
Chris, any suggestions? I still haven't found a good excuse for offsets to be per-process.
I'm just wondering if I should go ahead and send a non-RFC version of the patches. I guess in the end it is not userspace visible so completely possible to change later. But it seems these util fxns should also be useful to a handful of other upcoming SoC DRM drivers (such as the Samsung one that was recently posted).
BR, -R
On Tue, Jul 19, 2011 at 4:33 AM, Chris Wilson chris@chris-wilson.co.uk wrote:
My only concern is that for the common functions the mmap_offset to create should be passed in a parameter, so that we could support more than one mapping for an object.
[ snip ]
On Tue, Jul 19, 2011 at 8:12 AM, Rob Clark robdclark@gmail.com wrote: Chris, any suggestions? I still haven't found a good excuse for offsets to be per-process.
I'm just wondering if I should go ahead and send a non-RFC version of the patches. I guess in the end it is not userspace visible so completely possible to change later. But it seems these util fxns should also be useful to a handful of other upcoming SoC DRM drivers (such as the Samsung one that was recently posted).
Imo you're patches looks nice and should go in as is. Fixing the mmap handling of gem objects for real looks like more work: For the second cpu-coherent mapping of i915 gem objects (as opposed to the gpu coherent mapping using the mmap_offset infrastructure) we directly create a vma for the underlying shmfs node in a hand-rolled mmap ioctl (using do_mmap), the core drm mmap handling is layered with a bit of historical cruft of it's own and ttm seems to do a bit of reinventing the wheel. So imo this needs some more cleanup to be nice and beautiful than just adding an additional argument. It's somewhere on one of my todo list ... ;-)
Cheers, Daniel
--- drivers/gpu/drm/drm_gem.c | 88 +++++++++++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 3 ++ 2 files changed, 91 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 2b997d2..809358a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -273,6 +273,94 @@ again: } EXPORT_SYMBOL(drm_gem_handle_create);
+ +/** + * drm_gem_free_mmap_offset - release a fake mmap offset for an object + * @obj: obj in question + * + * This routine frees fake offsets allocated by drm_gem_create_mmap_offset(). + */ +void +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; +} +EXPORT_SYMBOL(drm_gem_free_mmap_offset); + +/** + * drm_gem_create_mmap_offset - create a fake mmap offset for an object + * @obj: obj in question + * + * GEM memory mapping works by handing back to userspace a fake mmap offset + * it can use in a subsequent mmap(2) call. The DRM core code then looks + * up the object based on the offset and sets up the various memory mapping + * structures. + * + * This routine allocates and attaches a fake offset for @obj. + */ +int +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 = 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 = 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, 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, + 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; +} +EXPORT_SYMBOL(drm_gem_create_mmap_offset); + /** Returns a reference to the object named by the handle. */ struct drm_gem_object * drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 111e98f..ec156c3 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1622,6 +1622,9 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) drm_gem_object_unreference_unlocked(obj); }
+void drm_gem_free_mmap_offset(struct drm_gem_object *obj); +int drm_gem_create_mmap_offset(struct drm_gem_object *obj); + struct drm_gem_object *drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp, u32 handle);
--- drivers/gpu/drm/i915/i915_gem.c | 85 +-------------------------------------- 1 files changed, 2 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5c0d124..5676eaa 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1265,74 +1265,6 @@ out: }
/** - * i915_gem_create_mmap_offset - create a fake mmap offset for an object - * @obj: obj in question - * - * GEM memory mapping works by handing back to userspace a fake mmap offset - * it can use in a subsequent mmap(2) call. The DRM core code then looks - * up the object based on the offset and sets up the various memory mapping - * structures. - * - * This routine allocates and attaches a fake offset for @obj. - */ -static int -i915_gem_create_mmap_offset(struct drm_i915_gem_object *obj) -{ - struct drm_device *dev = obj->base.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->base.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->base.size; - map->handle = obj; - - /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->base.size / PAGE_SIZE, - 0, 0); - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", - obj->base.name); - ret = -ENOSPC; - goto out_free_list; - } - - list->file_offset_node = drm_mm_get_block(list->file_offset_node, - obj->base.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; -} - -/** * i915_gem_release_mmap - remove physical page mappings * @obj: obj in question * @@ -1360,19 +1292,6 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) obj->fault_mappable = false; }
-static void -i915_gem_free_mmap_offset(struct drm_i915_gem_object *obj) -{ - struct drm_device *dev = obj->base.dev; - struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list = &obj->base.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; -} - static uint32_t i915_gem_get_gtt_size(struct drm_i915_gem_object *obj) { @@ -1493,7 +1412,7 @@ i915_gem_mmap_gtt(struct drm_file *file, }
if (!obj->base.map_list.map) { - ret = i915_gem_create_mmap_offset(obj); + ret = drm_gem_create_mmap_offset(&obj->base); if (ret) goto out; } @@ -3614,7 +3533,7 @@ static void i915_gem_free_object_tail(struct drm_i915_gem_object *obj) trace_i915_gem_object_destroy(obj);
if (obj->base.map_list.map) - i915_gem_free_mmap_offset(obj); + drm_gem_free_mmap_offset(&obj->base);
drm_gem_object_release(&obj->base); i915_gem_info_remove_obj(dev_priv, obj->base.size);
--- drivers/staging/gma500/psb_gem.c | 63 +------------------------------------ 1 files changed, 2 insertions(+), 61 deletions(-)
diff --git a/drivers/staging/gma500/psb_gem.c b/drivers/staging/gma500/psb_gem.c index 76ff7ba..3a397f5 100644 --- a/drivers/staging/gma500/psb_gem.c +++ b/drivers/staging/gma500/psb_gem.c @@ -42,13 +42,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) struct gtt_range *gtt = container_of(obj, struct gtt_range, gem); psb_gtt_free_range(obj->dev, gtt); if (obj->map_list.map) { - /* Do things GEM should do for us */ - struct drm_gem_mm *mm = obj->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_gem_free_mmap_offset(obj); } drm_gem_object_release(obj); } @@ -60,59 +54,6 @@ int psb_gem_get_aperture(struct drm_device *dev, void *data, }
/** - * psb_gem_create_mmap_offset - invent an mmap offset - * @obj: our object - * - * This is basically doing by hand a pile of ugly crap which should - * be done automatically by the GEM library code but isn't - */ -static int psb_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; - - list = &obj->map_list; - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); - if (list->map == NULL) - return -ENOMEM; - map = list->map; - map->type = _DRM_GEM; - map->size = obj->size; - map->handle =obj; - - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->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 free_it; - } - 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 free_it; - } - 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 free_mm; - } - return 0; - -free_mm: - drm_mm_put_block(list->file_offset_node); -free_it: - kfree(list->map); - list->map = NULL; - return ret; -} - -/** * psb_gem_dumb_map_gtt - buffer mapping for dumb interface * @file: our drm client file * @dev: drm device @@ -142,7 +83,7 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, /* Make it mmapable */ if (!obj->map_list.map) { - ret = psb_gem_create_mmap_offset(obj); + ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
On Thu, 4 Aug 2011 12:07:40 -0500 Rob Clark rob@ti.com wrote:
I'm happy with these but it will need contributing with your Signed-off-by: to progress upstream of course
dri-devel@lists.freedesktop.org
-
Alan Cox -
Chris Wilson -
Daniel Vetter -
Rob Clark -
Rob Clark