[PATCH 0/3] RFC: Common functions for GEM offset creation

[PATCH 0/3] RFC: Common functions for GEM offset creation

[Rob Clark]

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(-)

-- 
1.7.4.1

[PATCH 1/3] drm/gem: add functions for mmap offset creation

[Rob Clark]

---
 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);
-- 
1.7.4.1

[PATCH 2/3] drm/i915: use common functions for mmap offset creation

[Rob Clark]

---
 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);
-- 
1.7.4.1

[PATCH 3/3] drm/gma500: use common functions for mmap offset creation

[Rob Clark]

---
 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;
 	}
-- 
1.7.4.1

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Alan Cox]

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

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Rob Clark]

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
>

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Rob Clark]

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
>

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Alan Cox]

> 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

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Rob Clark]

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

Re: [PATCH 0/3] RFC: Common functions for GEM offset creation

[Daniel Vetter]

>> 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
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

[PATCH 1/9] drm/gem: add functions for mmap offset creation

[Rob Clark]

---
 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);
-- 
1.7.4.1

[PATCH 2/9] drm/i915: use common functions for mmap offset creation

[Rob Clark]

---
 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);
-- 
1.7.4.1

[PATCH 3/9] drm/gma500: use common functions for mmap offset creation

[Rob Clark]

---
 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;
 	}
-- 
1.7.4.1

Re: [PATCH 3/9] drm/gma500: use common functions for mmap offset creation

[Alan Cox]

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