[PATCH 1/7] drm: add unpin function to prime helpers

[PATCH 1/7] drm: add unpin function to prime helpers

[Maarten Lankhorst]

Prevents buffers from being pinned forever.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/drm_prime.c | 13 +++++++++++--
 include/drm/drmP.h          |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 366910d..85bbc52 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -91,10 +91,13 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
 static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
 
 	if (obj->export_dma_buf == dma_buf) {
 		/* drop the reference on the export fd holds */
 		obj->export_dma_buf = NULL;
+		if (dev->driver->gem_prime_unpin)
+			dev->driver->gem_prime_unpin(obj);
 		drm_gem_object_unreference_unlocked(obj);
 	}
 }
@@ -184,13 +187,19 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
 struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
 				     struct drm_gem_object *obj, int flags)
 {
+	struct dma_buf *buf;
+
 	if (dev->driver->gem_prime_pin) {
 		int ret = dev->driver->gem_prime_pin(obj);
 		if (ret)
 			return ERR_PTR(ret);
 	}
-	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
-			      0600);
+	buf = dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+			     0600);
+
+	if (IS_ERR(buf) && dev->driver->gem_prime_unpin)
+		dev->driver->gem_prime_unpin(obj);
+	return buf;
 }
 EXPORT_SYMBOL(drm_gem_prime_export);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..99dc622 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -932,6 +932,7 @@ struct drm_driver {
 				struct dma_buf *dma_buf);
 	/* low-level interface used by drm_gem_prime_{import,export} */
 	int (*gem_prime_pin)(struct drm_gem_object *obj);
+	void (*gem_prime_unpin)(struct drm_gem_object *obj);
 	struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
 	struct drm_gem_object *(*gem_prime_import_sg_table)(
 				struct drm_device *dev, size_t size,
-- 
1.8.2

[PATCH 2/7] drm: move pinning/unpinning to buffer attach

[Maarten Lankhorst]

This allows importing bo's to own device to work without requiring that the buffer is pinned in GART.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/drm_prime.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 85bbc52..16b302a 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -63,6 +63,29 @@ struct drm_prime_member {
 	uint32_t handle;
 };
 
+static int drm_gem_map_attach(struct dma_buf *dma_buf,
+			      struct device *target_dev,
+			      struct dma_buf_attachment *attach)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
+
+	if (!dev->driver->gem_prime_pin)
+		return 0;
+
+	return dev->driver->gem_prime_pin(obj);
+}
+
+static void drm_gem_map_detach(struct dma_buf *dma_buf,
+			       struct dma_buf_attachment *attach)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
+
+	if (dev->driver->gem_prime_unpin)
+		dev->driver->gem_prime_unpin(obj);
+}
+
 static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
 		enum dma_data_direction dir)
 {
@@ -91,13 +114,10 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
 static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
 
 	if (obj->export_dma_buf == dma_buf) {
 		/* drop the reference on the export fd holds */
 		obj->export_dma_buf = NULL;
-		if (dev->driver->gem_prime_unpin)
-			dev->driver->gem_prime_unpin(obj);
 		drm_gem_object_unreference_unlocked(obj);
 	}
 }
@@ -148,6 +168,8 @@ static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+	.attach = drm_gem_map_attach,
+	.detach = drm_gem_map_detach,
 	.map_dma_buf = drm_gem_map_dma_buf,
 	.unmap_dma_buf = drm_gem_unmap_dma_buf,
 	.release = drm_gem_dmabuf_release,
@@ -187,19 +209,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
 struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
 				     struct drm_gem_object *obj, int flags)
 {
-	struct dma_buf *buf;
-
-	if (dev->driver->gem_prime_pin) {
-		int ret = dev->driver->gem_prime_pin(obj);
-		if (ret)
-			return ERR_PTR(ret);
-	}
-	buf = dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
 			     0600);
-
-	if (IS_ERR(buf) && dev->driver->gem_prime_unpin)
-		dev->driver->gem_prime_unpin(obj);
-	return buf;
 }
 EXPORT_SYMBOL(drm_gem_prime_export);
 
-- 
1.8.2

[PATCH 3/7] drm/radeon: implement unpin function

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c   |  2 ++
 drivers/gpu/drm/radeon/radeon_prime.c | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 66a7f0f..6800c5e 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -124,6 +124,7 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev,
 							size_t size,
 							struct sg_table *sg);
 int radeon_gem_prime_pin(struct drm_gem_object *obj);
+void radeon_gem_prime_unpin(struct drm_gem_object *obj);
 void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
 void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
 extern long radeon_kms_compat_ioctl(struct file *filp, unsigned int cmd,
@@ -415,6 +416,7 @@ static struct drm_driver kms_driver = {
 	.gem_prime_export = drm_gem_prime_export,
 	.gem_prime_import = drm_gem_prime_import,
 	.gem_prime_pin = radeon_gem_prime_pin,
+	.gem_prime_unpin = radeon_gem_prime_unpin,
 	.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
 	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
 	.gem_prime_vmap = radeon_gem_prime_vmap,
diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c
index 4940af7..e557da5 100644
--- a/drivers/gpu/drm/radeon/radeon_prime.c
+++ b/drivers/gpu/drm/radeon/radeon_prime.c
@@ -88,10 +88,20 @@ int radeon_gem_prime_pin(struct drm_gem_object *obj)
 
 	/* pin buffer into GTT */
 	ret = radeon_bo_pin(bo, RADEON_GEM_DOMAIN_GTT, NULL);
-	if (ret) {
-		radeon_bo_unreserve(bo);
-		return ret;
-	}
+	radeon_bo_unreserve(bo);
+	return ret;
+}
+
+void radeon_gem_prime_unpin(struct drm_gem_object *obj)
+{
+	struct radeon_bo *bo = gem_to_radeon_bo(obj);
+	int ret = 0;
+
+	ret = radeon_bo_reserve(bo, false);
+	if (unlikely(ret != 0))
+		return;
+
+	radeon_bo_unpin(bo);
 	radeon_bo_unreserve(bo);
 
 	return 0;
-- 
1.8.2

Re: [PATCH 3/7] drm/radeon: implement unpin function

[Jerome Glisse]

[-- Attachment #1.1: Type: text/plain, Size: 2799 bytes --]

On Tue, Apr 9, 2013 at 7:37 AM, Maarten Lankhorst
<m.b.lankhorst@gmail.com>wrote:

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
>

Can userspace pin directly ? If so then that sounds as a bad idea.

Reviewed-by: Jerome Glisse <jglisse@redhat.com>


> ---
>  drivers/gpu/drm/radeon/radeon_drv.c   |  2 ++
>  drivers/gpu/drm/radeon/radeon_prime.c | 18 ++++++++++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 66a7f0f..6800c5e 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -124,6 +124,7 @@ struct drm_gem_object
> *radeon_gem_prime_import_sg_table(struct drm_device *dev,
>                                                         size_t size,
>                                                         struct sg_table
> *sg);
>  int radeon_gem_prime_pin(struct drm_gem_object *obj);
> +void radeon_gem_prime_unpin(struct drm_gem_object *obj);
>  void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
>  void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
>  extern long radeon_kms_compat_ioctl(struct file *filp, unsigned int cmd,
> @@ -415,6 +416,7 @@ static struct drm_driver kms_driver = {
>         .gem_prime_export = drm_gem_prime_export,
>         .gem_prime_import = drm_gem_prime_import,
>         .gem_prime_pin = radeon_gem_prime_pin,
> +       .gem_prime_unpin = radeon_gem_prime_unpin,
>         .gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
>         .gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
>         .gem_prime_vmap = radeon_gem_prime_vmap,
> diff --git a/drivers/gpu/drm/radeon/radeon_prime.c
> b/drivers/gpu/drm/radeon/radeon_prime.c
> index 4940af7..e557da5 100644
> --- a/drivers/gpu/drm/radeon/radeon_prime.c
> +++ b/drivers/gpu/drm/radeon/radeon_prime.c
> @@ -88,10 +88,20 @@ int radeon_gem_prime_pin(struct drm_gem_object *obj)
>
>         /* pin buffer into GTT */
>         ret = radeon_bo_pin(bo, RADEON_GEM_DOMAIN_GTT, NULL);
> -       if (ret) {
> -               radeon_bo_unreserve(bo);
> -               return ret;
> -       }
> +       radeon_bo_unreserve(bo);
> +       return ret;
> +}
> +
> +void radeon_gem_prime_unpin(struct drm_gem_object *obj)
> +{
> +       struct radeon_bo *bo = gem_to_radeon_bo(obj);
> +       int ret = 0;
> +
> +       ret = radeon_bo_reserve(bo, false);
> +       if (unlikely(ret != 0))
> +               return;
> +
> +       radeon_bo_unpin(bo);
>         radeon_bo_unreserve(bo);
>
>         return 0;
> --
> 1.8.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 3664 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm/radeon: implement unpin function

[Maarten Lankhorst]

Op 09-04-13 16:16, Jerome Glisse schreef:
> On Tue, Apr 9, 2013 at 7:37 AM, Maarten Lankhorst
> <m.b.lankhorst@gmail.com>wrote:
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
>>
> Can userspace pin directly ? If so then that sounds as a bad idea.
>
> Reviewed-by: Jerome Glisse <jglisse@redhat.com>
>
It's slightly better than before, it used to pin as soon as you export a buffer for as long as the buffer exists, with the series the pinning will only happen when importing to another device, until the device releases their reference.

~Maarten

Re: [PATCH 3/7] drm/radeon: implement unpin function

[Daniel Vetter]

On Tue, Apr 9, 2013 at 4:26 PM, Maarten Lankhorst
<maarten.lankhorst@canonical.com> wrote:
> Op 09-04-13 16:16, Jerome Glisse schreef:
>> On Tue, Apr 9, 2013 at 7:37 AM, Maarten Lankhorst
>> <m.b.lankhorst@gmail.com>wrote:
>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
>>>
>> Can userspace pin directly ? If so then that sounds as a bad idea.
>>
>> Reviewed-by: Jerome Glisse <jglisse@redhat.com>
>>
> It's slightly better than before, it used to pin as soon as you export a buffer for as long as the buffer exists, with the series the pinning will only happen when importing to another device, until the device releases their reference.

To make fully dynamic pinning work (and especially reclaiming) we
first need dma_buf fences, the new ww_mutexes and all that stuff since
otherwise you can trivially deadlock in buffer reservations. But I
think with Maarten's stuff we have a plan to get there, and this is
just the first tiny step of a long journey.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 4/7] drm/nouveau: implement prime helper unpin function

[Maarten Lankhorst]

---
 drivers/gpu/drm/nouveau/nouveau_drm.c   | 1 +
 drivers/gpu/drm/nouveau/nouveau_gem.h   | 1 +
 drivers/gpu/drm/nouveau/nouveau_prime.c | 9 ++++++++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index d109936..476f792 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -693,6 +693,7 @@ driver = {
 	.gem_prime_export = drm_gem_prime_export,
 	.gem_prime_import = drm_gem_prime_import,
 	.gem_prime_pin = nouveau_gem_prime_pin,
+	.gem_prime_unpin = nouveau_gem_prime_unpin,
 	.gem_prime_get_sg_table = nouveau_gem_prime_get_sg_table,
 	.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
 	.gem_prime_vmap = nouveau_gem_prime_vmap,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.h b/drivers/gpu/drm/nouveau/nouveau_gem.h
index 8d7a3f0..502e429 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.h
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.h
@@ -36,6 +36,7 @@ extern int nouveau_gem_ioctl_info(struct drm_device *, void *,
 				  struct drm_file *);
 
 extern int nouveau_gem_prime_pin(struct drm_gem_object *);
+extern void nouveau_gem_prime_unpin(struct drm_gem_object *);
 extern struct sg_table *nouveau_gem_prime_get_sg_table(struct drm_gem_object *);
 extern struct drm_gem_object *nouveau_gem_prime_import_sg_table(
 	struct drm_device *, size_t size, struct sg_table *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c
index f53e108..e90468d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_prime.c
+++ b/drivers/gpu/drm/nouveau/nouveau_prime.c
@@ -84,7 +84,7 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev,
 int nouveau_gem_prime_pin(struct drm_gem_object *obj)
 {
 	struct nouveau_bo *nvbo = nouveau_gem_object(obj);
-	int ret = 0;
+	int ret;
 
 	/* pin buffer into GTT */
 	ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_TT);
@@ -93,3 +93,10 @@ int nouveau_gem_prime_pin(struct drm_gem_object *obj)
 
 	return 0;
 }
+
+void nouveau_gem_prime_unpin(struct drm_gem_object *obj)
+{
+	struct nouveau_bo *nvbo = nouveau_gem_object(obj);
+
+	nouveau_bo_unpin(nvbo);
+}
-- 
1.8.2

[PATCH 5/7] drm/nouveau: unpin notify object in chan_fini

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/nouveau/nouveau_abi16.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 3b6dc88..9750bb9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -128,6 +128,7 @@ nouveau_abi16_chan_fini(struct nouveau_abi16 *abi16,
 
 	if (chan->ntfy) {
 		nouveau_bo_vma_del(chan->ntfy, &chan->ntfy_vma);
+		nouveau_bo_unpin(chan->ntfy);
 		drm_gem_object_unreference_unlocked(chan->ntfy->gem);
 	}
 
-- 
1.8.2

[PATCH 6/7] drm/nouveau: fixup fbcon failure paths

[Maarten Lankhorst]

Add missing calls, and fix a leak from forgetting to call the unpin function.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index b035317..ecbfe69 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -289,16 +289,13 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
 	ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_VRAM);
 	if (ret) {
 		NV_ERROR(drm, "failed to pin fb: %d\n", ret);
-		nouveau_bo_ref(NULL, &nvbo);
-		goto out;
+		goto out_unref;
 	}
 
 	ret = nouveau_bo_map(nvbo);
 	if (ret) {
 		NV_ERROR(drm, "failed to map fb: %d\n", ret);
-		nouveau_bo_unpin(nvbo);
-		nouveau_bo_ref(NULL, &nvbo);
-		goto out;
+		goto out_unpin;
 	}
 
 	chan = nouveau_nofbaccel ? NULL : drm->channel;
@@ -316,13 +313,14 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
 	info = framebuffer_alloc(0, &pdev->dev);
 	if (!info) {
 		ret = -ENOMEM;
-		goto out_unref;
+		goto out_unlock;
 	}
 
 	ret = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (ret) {
 		ret = -ENOMEM;
-		goto out_unref;
+		framebuffer_release(info);
+		goto out_unlock;
 	}
 
 	info->par = fbcon;
@@ -337,7 +335,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
 	fbcon->helper.fbdev = info;
 
 	strcpy(info->fix.id, "nouveaufb");
-	if (nouveau_nofbaccel)
+	if (!chan)
 		info->flags = FBINFO_DEFAULT | FBINFO_HWACCEL_DISABLED;
 	else
 		info->flags = FBINFO_DEFAULT | FBINFO_HWACCEL_COPYAREA |
@@ -383,8 +381,14 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
 	vga_switcheroo_client_fb_set(dev->pdev, info);
 	return 0;
 
-out_unref:
+out_unlock:
 	mutex_unlock(&dev->struct_mutex);
+	if (chan)
+		nouveau_bo_vma_del(nvbo, &fbcon->nouveau_fb.vma);
+out_unpin:
+	nouveau_bo_unpin(nvbo);
+out_unref:
+	nouveau_bo_ref(NULL, &nvbo);
 out:
 	return ret;
 }
@@ -413,6 +417,7 @@ nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon)
 	if (nouveau_fb->nvbo) {
 		nouveau_bo_unmap(nouveau_fb->nvbo);
 		nouveau_bo_vma_del(nouveau_fb->nvbo, &nouveau_fb->vma);
+		nouveau_bo_unpin(nouveau_fb->nvbo);
 		drm_gem_object_unreference_unlocked(nouveau_fb->nvbo->gem);
 		nouveau_fb->nvbo = NULL;
 	}
-- 
1.8.2

[PATCH 7/7] drm/nouveau: complain loudly if buffer is pinned during destruction

[Maarten Lankhorst]

Shouldn't happen, and we invert the struct_mutex with reservation here,
potentially leading to deadlocks. Once reservations become lockdep annotated,
lockdep will go splat on this.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index b4b4d0c..7054706 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -50,7 +50,8 @@ nouveau_gem_object_del(struct drm_gem_object *gem)
 		return;
 	nvbo->gem = NULL;
 
-	if (unlikely(nvbo->pin_refcnt)) {
+	/* Lockdep hates you for doing reserve with gem object lock held */
+	if (WARN_ON_ONCE(nvbo->pin_refcnt)) {
 		nvbo->pin_refcnt = 1;
 		nouveau_bo_unpin(nvbo);
 	}
-- 
1.8.2

Re: [PATCH 1/7] drm: add unpin function to prime helpers

[Daniel Vetter]

On Tue, Apr 09, 2013 at 01:37:40PM +0200, Maarten Lankhorst wrote:
> Prevents buffers from being pinned forever.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

Did I mention in my review of Aaron's patch that is feels a bit inflexible
to move the dma_buf vtable to here and has a midlayer touch to it? Looks
like we fix this by adding an vtable which pretty much just forwards the
dma-buf calls 1:1 ;-)

Whatever, i915.ko does use this, so patches 1-2 are

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

The others are acked on principle, but I'm too lazy to dig into that
driver code to do a real review.
-Daniel

> ---
>  drivers/gpu/drm/drm_prime.c | 13 +++++++++++--
>  include/drm/drmP.h          |  1 +
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 366910d..85bbc52 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -91,10 +91,13 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
>  static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
>  {
>  	struct drm_gem_object *obj = dma_buf->priv;
> +	struct drm_device *dev = obj->dev;
>  
>  	if (obj->export_dma_buf == dma_buf) {
>  		/* drop the reference on the export fd holds */
>  		obj->export_dma_buf = NULL;
> +		if (dev->driver->gem_prime_unpin)
> +			dev->driver->gem_prime_unpin(obj);
>  		drm_gem_object_unreference_unlocked(obj);
>  	}
>  }
> @@ -184,13 +187,19 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
>  struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
>  				     struct drm_gem_object *obj, int flags)
>  {
> +	struct dma_buf *buf;
> +
>  	if (dev->driver->gem_prime_pin) {
>  		int ret = dev->driver->gem_prime_pin(obj);
>  		if (ret)
>  			return ERR_PTR(ret);
>  	}
> -	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
> -			      0600);
> +	buf = dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
> +			     0600);
> +
> +	if (IS_ERR(buf) && dev->driver->gem_prime_unpin)
> +		dev->driver->gem_prime_unpin(obj);
> +	return buf;
>  }
>  EXPORT_SYMBOL(drm_gem_prime_export);
>  
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 2d94d74..99dc622 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -932,6 +932,7 @@ struct drm_driver {
>  				struct dma_buf *dma_buf);
>  	/* low-level interface used by drm_gem_prime_{import,export} */
>  	int (*gem_prime_pin)(struct drm_gem_object *obj);
> +	void (*gem_prime_unpin)(struct drm_gem_object *obj);
>  	struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
>  	struct drm_gem_object *(*gem_prime_import_sg_table)(
>  				struct drm_device *dev, size_t size,
> -- 
> 1.8.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch