Prevents buffers from being pinned forever.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- 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,
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 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- 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);
Changes since v1: - Fixup compiler warning in unpin function.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com 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, 15 insertions(+), 5 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..65b9eab 100644 --- a/drivers/gpu/drm/radeon/radeon_prime.c +++ b/drivers/gpu/drm/radeon/radeon_prime.c @@ -88,11 +88,19 @@ 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;
- return 0; + ret = radeon_bo_reserve(bo, false); + if (unlikely(ret != 0)) + return; + + radeon_bo_unpin(bo); + radeon_bo_unreserve(bo); }
--- 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); +}
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); }
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; }
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); }
dri-devel@lists.freedesktop.org
-
Maarten Lankhorst