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 Acked-by: Alex Deucher alexander.deucher@amd.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 094e7e5..f191e97 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -127,6 +127,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, @@ -422,6 +423,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); }
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com --- 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 383f4e6..218a4b5 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -702,6 +702,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 1c4c6c9..8f467e7 100644 --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c @@ -129,6 +129,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); }
Having nouveau builtin would still allow ACPI_VIDEO to be used as external module if some of the deps for acpi_video have not been met, which would result in a linking failure. Solve this by selecting all dependencies as well.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/nouveau/Kconfig | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 71ca63b..a7c54c8 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -139,6 +139,7 @@ config DRM_I915 select BACKLIGHT_CLASS_DEVICE if ACPI select VIDEO_OUTPUT_CONTROL if ACPI select INPUT if ACPI + select THERMAL if ACPI select ACPI_VIDEO if ACPI select ACPI_BUTTON if ACPI help diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index a7ff6d5..ff80f12 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -15,6 +15,13 @@ config DRM_NOUVEAU select ACPI_WMI if ACPI && X86 select MXM_WMI if ACPI && X86 select POWER_SUPPLY + # Similar to i915, we need to select ACPI_VIDEO and it's dependencies + select BACKLIGHT_LCD_SUPPORT if ACPI && X86 + select BACKLIGHT_CLASS_DEVICE if ACPI && X86 + select VIDEO_OUTPUT_CONTROL if ACPI && X86 + select INPUT if ACPI && X86 + select THERMAL if ACPI && X86 + select ACPI_VIDEO if ACPI && X86 help Choose this option for open-source nVidia support.
Mutexes should not be acquired in interrupt context. While the trylock fastpath is arguably safe on all implementations, the slowpath unlock path definitely isn't. This fixes the following lockdep splat:
[ 13.044313] ------------[ cut here ]------------ [ 13.044367] WARNING: at /c/kernel-tests/src/tip/kernel/mutex.c:858 mutex_trylock+0x87/0x220() [ 13.044378] DEBUG_LOCKS_WARN_ON(in_interrupt()) [ 13.044378] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-rc4-00296-ga2963dd #20 [ 13.044379] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 13.044390] 0000000000000009 ffff88000de039f8 ffffffff81fc86d5 ffff88000de03a38 [ 13.044395] ffffffff810d511b ffff880000000018 ffff88000f33c690 0000000000000001 [ 13.044398] 00000000000003f0 ffff88000f4677c8 0000000000000000 ffff88000de03a98 [ 13.044400] Call Trace: [ 13.044412] <IRQ> [<ffffffff81fc86d5>] dump_stack+0x19/0x1b [ 13.044441] [<ffffffff810d511b>] warn_slowpath_common+0x6b/0x90 [ 13.044445] [<ffffffff810d51a6>] warn_slowpath_fmt+0x46/0x50 [ 13.044448] [<ffffffff81fd34d7>] mutex_trylock+0x87/0x220 [ 13.044482] [<ffffffff8186484d>] cirrus_dirty_update+0x1cd/0x330 [ 13.044486] [<ffffffff818649e8>] cirrus_imageblit+0x38/0x50 [ 13.044506] [<ffffffff8165782e>] soft_cursor+0x22e/0x240 [ 13.044510] [<ffffffff81656c31>] bit_cursor+0x581/0x5b0 [ 13.044525] [<ffffffff815de9f4>] ? vsnprintf+0x124/0x670 [ 13.044529] [<ffffffff81651333>] ? get_color.isra.16+0x43/0x130 [ 13.044532] [<ffffffff81653fca>] fbcon_cursor+0x18a/0x1d0 [ 13.044535] [<ffffffff816566b0>] ? update_attr.isra.2+0xa0/0xa0 [ 13.044556] [<ffffffff81754b82>] hide_cursor+0x32/0xa0 [ 13.044565] [<ffffffff81755bd3>] vt_console_print+0x103/0x3b0 [ 13.044569] [<ffffffff810d58ac>] ? print_time+0x9c/0xb0 [ 13.044576] [<ffffffff810d5960>] ? print_prefix+0xa0/0xc0 [ 13.044580] [<ffffffff810d63f6>] call_console_drivers.constprop.6+0x146/0x1f0 [ 13.044593] [<ffffffff815f9b38>] ? do_raw_spin_unlock+0xc8/0x100 [ 13.044597] [<ffffffff810d6f27>] console_unlock+0x2f7/0x460 [ 13.044600] [<ffffffff810d787a>] vprintk_emit+0x59a/0x5e0 [ 13.044615] [<ffffffff81fb676c>] printk+0x4d/0x4f [ 13.044650] [<ffffffff82ba5511>] print_local_APIC+0x28/0x41c [ 13.044672] [<ffffffff8114db55>] generic_smp_call_function_single_interrupt+0x145/0x2b0 [ 13.044688] [<ffffffff8106f9e7>] smp_call_function_single_interrupt+0x27/0x40 [ 13.044697] [<ffffffff81fd8f72>] call_function_single_interrupt+0x72/0x80 [ 13.044707] <EOI> [<ffffffff81078166>] ? native_safe_halt+0x6/0x10 [ 13.044717] [<ffffffff811425cd>] ? trace_hardirqs_on+0xd/0x10 [ 13.044738] [<ffffffff8104f669>] default_idle+0x59/0x120 [ 13.044742] [<ffffffff810501e8>] arch_cpu_idle+0x18/0x40 [ 13.044754] [<ffffffff811320c5>] cpu_startup_entry+0x235/0x410 [ 13.044763] [<ffffffff81f9e781>] rest_init+0xd1/0xe0 [ 13.044766] [<ffffffff81f9e6b5>] ? rest_init+0x5/0xe0 [ 13.044778] [<ffffffff82b93ec2>] start_kernel+0x425/0x493 [ 13.044781] [<ffffffff82b93810>] ? repair_env_string+0x5e/0x5e [ 13.044786] [<ffffffff82b93595>] x86_64_start_reservations+0x2a/0x2c [ 13.044789] [<ffffffff82b93688>] x86_64_start_kernel+0xf1/0x100 [ 13.044799] ---[ end trace 113ad28772af4058 ]---
Reported-by: Fengguang Wu fengguang.wu@intel.com Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com --- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 3541b56..b27e956 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -25,7 +25,7 @@ static void cirrus_dirty_update(struct cirrus_fbdev *afbdev, struct cirrus_bo *bo; int src_offset, dst_offset; int bpp = (afbdev->gfb.base.bits_per_pixel + 7)/8; - int ret; + int ret = -EBUSY; bool unmap = false; bool store_for_later = false; int x2, y2; @@ -39,7 +39,8 @@ static void cirrus_dirty_update(struct cirrus_fbdev *afbdev, * then the BO is being moved and we should * store up the damage until later. */ - ret = cirrus_bo_reserve(bo, true); + if (!in_interrupt()) + ret = cirrus_bo_reserve(bo, true); if (ret) { if (ret != -EBUSY) return;
Mutexes should not be acquired in interrupt context. While the trylock fastpath is arguably safe on all implementations, the slowpath unlock path definitely isn't.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com --- drivers/gpu/drm/mgag200/mgag200_fb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 5da824c..964f58c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -27,7 +27,7 @@ static void mga_dirty_update(struct mga_fbdev *mfbdev, struct mgag200_bo *bo; int src_offset, dst_offset; int bpp = (mfbdev->mfb.base.bits_per_pixel + 7)/8; - int ret; + int ret = -EBUSY; bool unmap = false; bool store_for_later = false; int x2, y2; @@ -41,7 +41,8 @@ static void mga_dirty_update(struct mga_fbdev *mfbdev, * then the BO is being moved and we should * store up the damage until later. */ - ret = mgag200_bo_reserve(bo, true); + if (!in_interrupt()) + ret = mgag200_bo_reserve(bo, true); if (ret) { if (ret != -EBUSY) return;
Mutexes should not be acquired in interrupt context. While the trylock fastpath is arguably safe on all implementations, the slowpath unlock path definitely isn't.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com --- drivers/gpu/drm/ast/ast_fb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c index fbc0823..7b33e14 100644 --- a/drivers/gpu/drm/ast/ast_fb.c +++ b/drivers/gpu/drm/ast/ast_fb.c @@ -51,7 +51,7 @@ static void ast_dirty_update(struct ast_fbdev *afbdev, struct ast_bo *bo; int src_offset, dst_offset; int bpp = (afbdev->afb.base.bits_per_pixel + 7)/8; - int ret; + int ret = -EBUSY; bool unmap = false; bool store_for_later = false; int x2, y2; @@ -65,7 +65,8 @@ static void ast_dirty_update(struct ast_fbdev *afbdev, * then the BO is being moved and we should * store up the damage until later. */ - ret = ast_bo_reserve(bo, true); + if (!in_interrupt()) + ret = ast_bo_reserve(bo, true); if (ret) { if (ret != -EBUSY) return;
dri-devel@lists.freedesktop.org
-
Maarten Lankhorst