* [PATCH] video: exynos_mipi_dsi: Use devm_* APIs
From: Sachin Kamat @ 2013-01-03 7:12 UTC (permalink / raw)
To: linux-fbdev
devm_* APIs are device managed and make exit and cleanup code simpler.
While at it also remove some unused labels and fix an error path.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Compile tested using the latest linux-next tree.
This patch should be applied on top of the following patch:
http://www.spinics.net/lists/linux-fbdev/msg09303.html
---
drivers/video/exynos/exynos_mipi_dsi.c | 68 ++++++++------------------------
include/video/exynos_mipi_dsim.h | 1 -
2 files changed, 17 insertions(+), 52 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index f623dfc..fac7df6 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -338,7 +338,8 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
struct mipi_dsim_ddi *dsim_ddi;
int ret = -EINVAL;
- dsim = kzalloc(sizeof(struct mipi_dsim_device), GFP_KERNEL);
+ dsim = devm_kzalloc(&pdev->dev, sizeof(struct mipi_dsim_device),
+ GFP_KERNEL);
if (!dsim) {
dev_err(&pdev->dev, "failed to allocate dsim object.\n");
return -ENOMEM;
@@ -352,13 +353,13 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
dsim_pd = (struct mipi_dsim_platform_data *)dsim->pd;
if (dsim_pd = NULL) {
dev_err(&pdev->dev, "failed to get platform data for dsim.\n");
- goto err_clock_get;
+ return -EINVAL;
}
/* get mipi_dsim_config. */
dsim_config = dsim_pd->dsim_config;
if (dsim_config = NULL) {
dev_err(&pdev->dev, "failed to get dsim config data.\n");
- goto err_clock_get;
+ return -EINVAL;
}
dsim->dsim_config = dsim_config;
@@ -366,41 +367,28 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
mutex_init(&dsim->lock);
- ret = regulator_bulk_get(&pdev->dev, ARRAY_SIZE(supplies), supplies);
+ ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(supplies),
+ supplies);
if (ret) {
dev_err(&pdev->dev, "Failed to get regulators: %d\n", ret);
- goto err_clock_get;
+ return ret;
}
- dsim->clock = clk_get(&pdev->dev, "dsim0");
+ dsim->clock = devm_clk_get(&pdev->dev, "dsim0");
if (IS_ERR(dsim->clock)) {
dev_err(&pdev->dev, "failed to get dsim clock source\n");
- ret = -ENODEV;
- goto err_clock_get;
+ return -ENODEV;
}
clk_enable(dsim->clock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "failed to get io memory region\n");
- ret = -ENODEV;
- goto err_platform_get;
- }
-
- dsim->res = request_mem_region(res->start, resource_size(res),
- dev_name(&pdev->dev));
- if (!dsim->res) {
- dev_err(&pdev->dev, "failed to request io memory region\n");
- ret = -ENOMEM;
- goto err_mem_region;
- }
- dsim->reg_base = ioremap(res->start, resource_size(res));
+ dsim->reg_base = devm_request_and_ioremap(&pdev->dev, res);
if (!dsim->reg_base) {
dev_err(&pdev->dev, "failed to remap io region\n");
ret = -ENOMEM;
- goto err_ioremap;
+ goto error;
}
mutex_init(&dsim->lock);
@@ -410,26 +398,27 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
if (!dsim_ddi) {
dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
ret = -EINVAL;
- goto err_bind;
+ goto error;
}
dsim->irq = platform_get_irq(pdev, 0);
if (IS_ERR_VALUE(dsim->irq)) {
dev_err(&pdev->dev, "failed to request dsim irq resource\n");
ret = -EINVAL;
- goto err_platform_get_irq;
+ goto error;
}
init_completion(&dsim_wr_comp);
init_completion(&dsim_rd_comp);
platform_set_drvdata(pdev, dsim);
- ret = request_irq(dsim->irq, exynos_mipi_dsi_interrupt_handler,
+ ret = devm_request_irq(&pdev->dev, dsim->irq,
+ exynos_mipi_dsi_interrupt_handler,
IRQF_SHARED, dev_name(&pdev->dev), dsim);
if (ret != 0) {
dev_err(&pdev->dev, "failed to request dsim irq\n");
ret = -EINVAL;
- goto err_bind;
+ goto error;
}
/* enable interrupts */
@@ -471,22 +460,8 @@ done:
return 0;
-err_bind:
- iounmap(dsim->reg_base);
-
-err_ioremap:
- release_mem_region(dsim->res->start, resource_size(dsim->res));
-
-err_mem_region:
- release_resource(dsim->res);
-
-err_platform_get:
+error:
clk_disable(dsim->clock);
- clk_put(dsim->clock);
-err_clock_get:
- kfree(dsim);
-
-err_platform_get_irq:
return ret;
}
@@ -496,13 +471,7 @@ static int exynos_mipi_dsi_remove(struct platform_device *pdev)
struct mipi_dsim_ddi *dsim_ddi, *next;
struct mipi_dsim_lcd_driver *dsim_lcd_drv;
- iounmap(dsim->reg_base);
-
clk_disable(dsim->clock);
- clk_put(dsim->clock);
-
- release_resource(dsim->res);
- release_mem_region(dsim->res->start, resource_size(dsim->res));
list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
if (dsim_ddi) {
@@ -518,9 +487,6 @@ static int exynos_mipi_dsi_remove(struct platform_device *pdev)
}
}
- regulator_bulk_free(ARRAY_SIZE(supplies), supplies);
- kfree(dsim);
-
return 0;
}
diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h
index 83ce5e6..89dc88a 100644
--- a/include/video/exynos_mipi_dsim.h
+++ b/include/video/exynos_mipi_dsim.h
@@ -220,7 +220,6 @@ struct mipi_dsim_config {
struct mipi_dsim_device {
struct device *dev;
int id;
- struct resource *res;
struct clk *clock;
unsigned int irq;
void __iomem *reg_base;
--
1.7.4.1
^ permalink raw reply related
* [PATCH -v2 11/26] video/uvesafb: rename random32() to prandom_u32()
From: Akinobu Mita @ 2013-01-03 12:19 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: Akinobu Mita, Michal Januszewski, Florian Tobias Schandinat,
linux-fbdev
In-Reply-To: <1357215562-6288-1-git-send-email-akinobu.mita@gmail.com>
Use more preferable function name which implies using a pseudo-random
number generator.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Michal Januszewski <spock@gentoo.org>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
No change from v1
drivers/video/uvesafb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 2f8f82d..d120b11 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -166,7 +166,7 @@ static int uvesafb_exec(struct uvesafb_ktask *task)
memcpy(&m->id, &uvesafb_cn_id, sizeof(m->id));
m->seq = seq;
m->len = len;
- m->ack = random32();
+ m->ack = prandom_u32();
/* uvesafb_task structure */
memcpy(m + 1, &task->t, sizeof(task->t));
--
1.7.11.7
^ permalink raw reply related
* 3.8-rc2: EFI framebuffer lock inversion...
From: Daniel J Blueman @ 2013-01-03 12:56 UTC (permalink / raw)
To: Peter Jones, linux-fbdev; +Cc: nouveau, Linux Kernel
On 3.8-rc2 with lockdep enabled and dual-GPU setup (Macbook Pro
Retina), I see two releated lock inversion issues with the EFI
framebuffer, leading to possible deadlock: when X takes over from the
EFI framebuffer [1] and when nouveau releases the framebuffer when
being vgaswitcherood [2].
Let me know if you'd like any testing or analysis when I can get the time.
Many thanks,
Daniel
--- [1]
init: lightdm main process (950) terminated with status 1
===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc2-expert #1 Not tainted
-------------------------------------------------------
Xorg/1193 is trying to acquire lock:
((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0
but task is already holding lock:
(console_lock){+.+.+.}, at: [<ffffffff81263f95>] do_fb_ioctl+0x2e5/0x5f0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (console_lock){+.+.+.}:
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff810407a7>] console_lock+0x77/0x80
[<ffffffff812c6d84>] register_con_driver+0x34/0x140
[<ffffffff812c84e9>] take_over_console+0x29/0x60
[<ffffffff8126e76b>] fbcon_takeover+0x5b/0xb0
[<ffffffff81272bb5>] fbcon_event_notify+0x715/0x820
[<ffffffff810693a5>] notifier_call_chain+0x55/0x110
[<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81264c1d>] register_framebuffer+0x1bd/0x2f0
[<ffffffff81ac2bd4>] efifb_probe+0x40f/0x496
[<ffffffff81308dfe>] platform_drv_probe+0x3e/0x70
[<ffffffff81306dc6>] driver_probe_device+0x76/0x240
[<ffffffff81307033>] __driver_attach+0xa3/0xb0
[<ffffffff8130503d>] bus_for_each_dev+0x4d/0x90
[<ffffffff81306929>] driver_attach+0x19/0x20
[<ffffffff813064e0>] bus_add_driver+0x1a0/0x270
[<ffffffff813076c2>] driver_register+0x72/0x170
[<ffffffff81308671>] platform_driver_register+0x41/0x50
[<ffffffff81308696>] platform_driver_probe+0x16/0xa0
[<ffffffff81ac2ece>] efifb_init+0x273/0x292
[<ffffffff810002da>] do_one_initcall+0x11a/0x170
[<ffffffff8154187c>] kernel_init+0x11c/0x290
[<ffffffff8155acac>] ret_from_fork+0x7c/0xb0
-> #0 ((fb_notifier_list).rwsem){++++.+}:
[<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff81557ad7>] down_read+0x47/0x5c
[<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81263196>] fb_blank+0x36/0xc0
[<ffffffff81263fa7>] do_fb_ioctl+0x2f7/0x5f0
[<ffffffff812646e1>] fb_ioctl+0x41/0x50
[<ffffffff811209d7>] do_vfs_ioctl+0x97/0x580
[<ffffffff81120f0b>] sys_ioctl+0x4b/0x90
[<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(console_lock);
lock((fb_notifier_list).rwsem);
lock(console_lock);
lock((fb_notifier_list).rwsem);
*** DEADLOCK ***
2 locks held by Xorg/1193:
#0: (&fb_info->lock){+.+.+.}, at: [<ffffffff81262ef1>] lock_fb_info+0x21/0x60
#1: (console_lock){+.+.+.}, at: [<ffffffff81263f95>] do_fb_ioctl+0x2e5/0x5f0
stack backtrace:
Pid: 1193, comm: Xorg Not tainted 3.8.0-rc2-expert #1
Call Trace:
[<ffffffff8154f6c6>] print_circular_bug+0x28e/0x29f
[<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff8108d3a4>] ? __lock_is_held+0x54/0x80
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81557ad7>] down_read+0x47/0x5c
[<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81263196>] fb_blank+0x36/0xc0
[<ffffffff81263fa7>] do_fb_ioctl+0x2f7/0x5f0
[<ffffffff810e8d1a>] ? mmap_region+0x1aa/0x620
[<ffffffff812646e1>] fb_ioctl+0x41/0x50
[<ffffffff811209d7>] do_vfs_ioctl+0x97/0x580
[<ffffffff8112c49a>] ? fget_light+0x3da/0x4d0
[<ffffffff8155ad7b>] ? sysret_check+0x1b/0x56
[<ffffffff81120f0b>] sys_ioctl+0x4b/0x90
[<ffffffff8122c03e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f
[drm] Enabling RC6 states: RC6 on, RC6p on, RC6pp off
--- [2]
hda-intel 0000:01:00.1: Disabling via VGA-switcheroo
hda-intel 0000:01:00.1: Cannot lock devices!
VGA switcheroo: switched nouveau off
nouveau [ DRM] suspending fbcon...
===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc2-expert #1 Not tainted
-------------------------------------------------------
sh/1017 is trying to acquire lock:
((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0
but task is already holding lock:
(console_lock){+.+.+.}, at: [<ffffffffa0204d35>]
nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (console_lock){+.+.+.}:
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff810407a7>] console_lock+0x77/0x80
[<ffffffff812c6d84>] register_con_driver+0x34/0x140
[<ffffffff812c84e9>] take_over_console+0x29/0x60
[<ffffffff8126e76b>] fbcon_takeover+0x5b/0xb0
[<ffffffff81272bb5>] fbcon_event_notify+0x715/0x820
[<ffffffff810693a5>] notifier_call_chain+0x55/0x110
[<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81264c1d>] register_framebuffer+0x1bd/0x2f0
[<ffffffff81ac2bd4>] efifb_probe+0x40f/0x496
[<ffffffff81308dfe>] platform_drv_probe+0x3e/0x70
[<ffffffff81306dc6>] driver_probe_device+0x76/0x240
[<ffffffff81307033>] __driver_attach+0xa3/0xb0
[<ffffffff8130503d>] bus_for_each_dev+0x4d/0x90
[<ffffffff81306929>] driver_attach+0x19/0x20
[<ffffffff813064e0>] bus_add_driver+0x1a0/0x270
[<ffffffff813076c2>] driver_register+0x72/0x170
[<ffffffff81308671>] platform_driver_register+0x41/0x50
[<ffffffff81308696>] platform_driver_probe+0x16/0xa0
[<ffffffff81ac2ece>] efifb_init+0x273/0x292
[<ffffffff810002da>] do_one_initcall+0x11a/0x170
[<ffffffff8154187c>] kernel_init+0x11c/0x290
[<ffffffff8155acac>] ret_from_fork+0x7c/0xb0
-> #0 ((fb_notifier_list).rwsem){++++.+}:
[<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff81557ad7>] down_read+0x47/0x5c
[<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81263146>] fb_set_suspend+0x46/0x60
[<ffffffffa0204da2>] nouveau_fbcon_set_suspend+0x92/0xc0 [nouveau]
[<ffffffffa01f5451>] nouveau_do_suspend+0x51/0x200 [nouveau]
[<ffffffffa01f564f>] nouveau_pmops_suspend+0x2f/0x80 [nouveau]
[<ffffffffa01f723c>] nouveau_switcheroo_set_state+0x5c/0xc0 [nouveau]
[<ffffffff81300877>] vga_switchoff+0x17/0x40
[<ffffffff81300f1a>] vga_switcheroo_debugfs_write+0xca/0x380
[<ffffffff8110ec93>] vfs_write+0xa3/0x160
[<ffffffff8110ef9d>] sys_write+0x4d/0xa0
[<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(console_lock);
lock((fb_notifier_list).rwsem);
lock(console_lock);
lock((fb_notifier_list).rwsem);
*** DEADLOCK ***
2 locks held by sh/1017:
#0: (vgasr_mutex){+.+.+.}, at: [<ffffffff81300ea7>]
vga_switcheroo_debugfs_write+0x57/0x380
#1: (console_lock){+.+.+.}, at: [<ffffffffa0204d35>]
nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau]
stack backtrace:
Pid: 1017, comm: sh Not tainted 3.8.0-rc2-expert #1
Call Trace:
[<ffffffff8154f6c6>] print_circular_bug+0x28e/0x29f
[<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff8108d3a4>] ? __lock_is_held+0x54/0x80
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81557ad7>] down_read+0x47/0x5c
[<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81263146>] fb_set_suspend+0x46/0x60
[<ffffffff810407a7>] ? console_lock+0x77/0x80
[<ffffffffa0204d35>] ? nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau]
[<ffffffffa0204da2>] nouveau_fbcon_set_suspend+0x92/0xc0 [nouveau]
[<ffffffffa01f5451>] nouveau_do_suspend+0x51/0x200 [nouveau]
[<ffffffffa01f564f>] nouveau_pmops_suspend+0x2f/0x80 [nouveau]
[<ffffffffa01f723c>] nouveau_switcheroo_set_state+0x5c/0xc0 [nouveau]
[<ffffffff81300877>] vga_switchoff+0x17/0x40
[<ffffffff81300f1a>] vga_switcheroo_debugfs_write+0xca/0x380
[<ffffffff8110ec93>] vfs_write+0xa3/0x160
[<ffffffff8110ef9d>] sys_write+0x4d/0xa0
[<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f
nouveau [ DRM] suspending display...
nouveau [ DRM] unpinning framebuffer(s)...
nouveau [ DRM] evicting buffers...
nouveau [ DRM] suspending client object trees...
tg3 0000:0a:00.0 eth0: Link is up at 1000 Mbps, full duplex
tg3 0000:0a:00.0 eth0: Flow control is on for TX and on for RX
nouveau E[ I2C][0000:01:00.0] AUXCH(3): begin idle timeout 0xffffffff
nouveau E[ I2C][0000:01:00.0] AUXCH(2): begin idle timeout 0xffffffff
nouveau E[ I2C][0000:01:00.0] AUXCH(1): begin idle timeout 0xffffffff
--
Daniel J Blueman
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Alan Cox @ 2013-01-03 13:11 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: Peter Jones, linux-fbdev, nouveau, Linux Kernel, akpm
In-Reply-To: <CAMVG2ssA-B6s+F-g4mL0dbBSVsNupzT1nm1Kc70dXHGsNQuNYA@mail.gmail.com>
On Thu, 3 Jan 2013 20:56:30 +0800
Daniel J Blueman <daniel@quora.org> wrote:
> On 3.8-rc2 with lockdep enabled and dual-GPU setup (Macbook Pro
> Retina), I see two releated lock inversion issues with the EFI
> framebuffer, leading to possible deadlock: when X takes over from the
> EFI framebuffer [1] and when nouveau releases the framebuffer when
> being vgaswitcherood [2].
>
> Let me know if you'd like any testing or analysis when I can get the time.
The fb layer locking was broken. I posted patches early December which
should have fixed the ones we know about. ('fb: Rework locking to fix
lock ordering on takeover').
Alan
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Daniel J Blueman @ 2013-01-03 13:42 UTC (permalink / raw)
To: Alan Cox; +Cc: Peter Jones, linux-fbdev, nouveau, Linux Kernel, akpm
In-Reply-To: <20130103131156.214bd7a5@pyramind.ukuu.org.uk>
On 3 January 2013 21:11, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Thu, 3 Jan 2013 20:56:30 +0800
> Daniel J Blueman <daniel@quora.org> wrote:
>
>> On 3.8-rc2 with lockdep enabled and dual-GPU setup (Macbook Pro
>> Retina), I see two releated lock inversion issues with the EFI
>> framebuffer, leading to possible deadlock: when X takes over from the
>> EFI framebuffer [1] and when nouveau releases the framebuffer when
>> being vgaswitcherood [2].
>>
>> Let me know if you'd like any testing or analysis when I can get the time.
>
> The fb layer locking was broken. I posted patches early December which
> should have fixed the ones we know about. ('fb: Rework locking to fix
> lock ordering on takeover').
Superb work, Alan!
The only patch I could find [1] (mid Nov) looks like it needs another
sites updating, since we now see an i915 vs efifb lock ordering issue
[2].
I can get some time next week to take a look if it helps.
Thanks,
Daniel
--- [1] https://patchwork.kernel.org/patch/1757061/
--- [2]
[drm] Memory usable by graphics device = 2048M
checking generic (b0000000 1440000) vs hw (b0000000 10000000)
fb: conflicting fb hw usage inteldrmfb vs EFI VGA - removing generic driver
===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc2-expert+ #2 Not tainted
-------------------------------------------------------
modprobe/603 is trying to acquire lock:
(console_lock){+.+.+.}, at: [<ffffffff812c869f>] unbind_con_driver+0x3f/0x200
but task is already holding lock:
((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 ((fb_notifier_list).rwsem){++++.+}:
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff81557c97>] down_read+0x47/0x5c
[<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff81264c20>] register_framebuffer+0x1c0/0x300
[<ffffffff81ac2bd4>] efifb_probe+0x40f/0x496
[<ffffffff81308fbe>] platform_drv_probe+0x3e/0x70
[<ffffffff81306f86>] driver_probe_device+0x76/0x240
[<ffffffff813071f3>] __driver_attach+0xa3/0xb0
[<ffffffff813051fd>] bus_for_each_dev+0x4d/0x90
[<ffffffff81306ae9>] driver_attach+0x19/0x20
[<ffffffff813066a0>] bus_add_driver+0x1a0/0x270
[<ffffffff81307882>] driver_register+0x72/0x170
[<ffffffff81308831>] platform_driver_register+0x41/0x50
[<ffffffff81308856>] platform_driver_probe+0x16/0xa0
[<ffffffff81ac2ece>] efifb_init+0x273/0x292
[<ffffffff810002da>] do_one_initcall+0x11a/0x170
[<ffffffff81541a3c>] kernel_init+0x11c/0x290
[<ffffffff8155ae6c>] ret_from_fork+0x7c/0xb0
-> #0 (console_lock){+.+.+.}:
[<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff810407a7>] console_lock+0x77/0x80
[<ffffffff812c869f>] unbind_con_driver+0x3f/0x200
[<ffffffff81272bc7>] fbcon_event_notify+0x447/0x8b0
[<ffffffff810693a5>] notifier_call_chain+0x55/0x110
[<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff812647db>] do_unregister_framebuffer+0x5b/0x110
[<ffffffff81264a28>] do_remove_conflicting_framebuffers+0x158/0x190
[<ffffffff81264d9a>] remove_conflicting_framebuffers+0x3a/0x60
[<ffffffffa007dbe4>] i915_driver_load+0x7d4/0xe70 [i915]
[<ffffffff812ee1ee>] drm_get_pci_dev+0x17e/0x2b0
[<ffffffffa0079616>] i915_pci_probe+0x36/0x90 [i915]
[<ffffffff8124a146>] local_pci_probe+0x46/0x80
[<ffffffff8124a9d1>] pci_device_probe+0x101/0x110
[<ffffffff81306f86>] driver_probe_device+0x76/0x240
[<ffffffff813071f3>] __driver_attach+0xa3/0xb0
[<ffffffff813051fd>] bus_for_each_dev+0x4d/0x90
[<ffffffff81306ae9>] driver_attach+0x19/0x20
[<ffffffff813066a0>] bus_add_driver+0x1a0/0x270
[<ffffffff81307882>] driver_register+0x72/0x170
[<ffffffff8124aacf>] __pci_register_driver+0x5f/0x70
[<ffffffff812ee435>] drm_pci_init+0x115/0x130
[<ffffffffa00ff066>] i915_init+0x66/0x68 [i915]
[<ffffffff810002da>] do_one_initcall+0x11a/0x170
[<ffffffff8109cf84>] load_module+0xfd4/0x13c0
[<ffffffff8109d427>] sys_init_module+0xb7/0xe0
[<ffffffff8155af16>] system_call_fastpath+0x1a/0x1f
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock((fb_notifier_list).rwsem);
lock(console_lock);
lock((fb_notifier_list).rwsem);
lock(console_lock);
*** DEADLOCK ***
6 locks held by modprobe/603:
#0: (&__lockdep_no_validate__){......}, at: [<ffffffff813071a3>]
__driver_attach+0x53/0xb0
#1: (&__lockdep_no_validate__){......}, at: [<ffffffff813071b1>]
__driver_attach+0x61/0xb0
#2: (drm_global_mutex){+.+.+.}, at: [<ffffffff812ee12c>]
drm_get_pci_dev+0xbc/0x2b0
#3: (registration_lock){+.+.+.}, at: [<ffffffff81264d8b>]
remove_conflicting_framebuffers+0x2b/0x60
#4: (&fb_info->lock){+.+.+.}, at: [<ffffffff81262ef1>] lock_fb_info+0x21/0x60
#5: ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0
stack backtrace:
Pid: 603, comm: modprobe Not tainted 3.8.0-rc2-expert+ #2
Call Trace:
[<ffffffff8154f886>] print_circular_bug+0x28e/0x29f
[<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
[<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
[<ffffffff8155a12a>] ? _raw_spin_unlock_irqrestore+0x3a/0x70
[<ffffffff8109209d>] ? trace_hardirqs_on_caller+0x10d/0x1a0
[<ffffffff810916ea>] lock_acquire+0x5a/0x70
[<ffffffff812c869f>] ? unbind_con_driver+0x3f/0x200
[<ffffffff810407a7>] console_lock+0x77/0x80
[<ffffffff812c869f>] ? unbind_con_driver+0x3f/0x200
[<ffffffff812c869f>] unbind_con_driver+0x3f/0x200
[<ffffffff81090a61>] ? __lock_acquire+0x3a1/0xb60
[<ffffffff81272bc7>] fbcon_event_notify+0x447/0x8b0
[<ffffffff810693a5>] notifier_call_chain+0x55/0x110
[<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
[<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
[<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
[<ffffffff812647db>] do_unregister_framebuffer+0x5b/0x110
[<ffffffff81264a28>] do_remove_conflicting_framebuffers+0x158/0x190
[<ffffffff81264d9a>] remove_conflicting_framebuffers+0x3a/0x60
[<ffffffffa007dbe4>] i915_driver_load+0x7d4/0xe70 [i915]
[<ffffffff812ee1ee>] drm_get_pci_dev+0x17e/0x2b0
[<ffffffff8109213d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffffa0079616>] i915_pci_probe+0x36/0x90 [i915]
[<ffffffff8124a146>] local_pci_probe+0x46/0x80
[<ffffffff8124a9d1>] pci_device_probe+0x101/0x110
[<ffffffff81306f86>] driver_probe_device+0x76/0x240
[<ffffffff813071f3>] __driver_attach+0xa3/0xb0
[<ffffffff81307150>] ? driver_probe_device+0x240/0x240
[<ffffffff813051fd>] bus_for_each_dev+0x4d/0x90
[<ffffffff81306ae9>] driver_attach+0x19/0x20
[<ffffffff813066a0>] bus_add_driver+0x1a0/0x270
[<ffffffffa00ff000>] ? 0xffffffffa00fefff
[<ffffffff81307882>] driver_register+0x72/0x170
[<ffffffffa00ff000>] ? 0xffffffffa00fefff
[<ffffffff8124aacf>] __pci_register_driver+0x5f/0x70
[<ffffffff812ee435>] drm_pci_init+0x115/0x130
[<ffffffffa00ff000>] ? 0xffffffffa00fefff
[<ffffffffa00ff066>] i915_init+0x66/0x68 [i915]
[<ffffffff810002da>] do_one_initcall+0x11a/0x170
[<ffffffff8109cf84>] load_module+0xfd4/0x13c0
[<ffffffff81098ab0>] ? in_lock_functions+0x20/0x20
[<ffffffff8109d427>] sys_init_module+0xb7/0xe0
[<ffffffff8155af16>] system_call_fastpath+0x1a/0x1f
Console: switching to colour dummy device 80x25
--
Daniel J Blueman
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Sedat Dilek @ 2013-01-03 14:11 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: alan, linux-fbdev, LKML, Daniel Vetter
In-Reply-To: <CAMVG2ssA-B6s+F-g4mL0dbBSVsNupzT1nm1Kc70dXHGsNQuNYA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 724 bytes --]
Hi Daniel,
just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
Me is also working with i915 KMS.
I looked at nouveau KMS driver and adapted the part for i915:
drivers/gpu/drm/nouveau/nouveau_drm.c-200- /* remove conflicting
drivers (vesafb, efifb etc) */
drivers/gpu/drm/nouveau/nouveau_drm.c:201: aper = alloc_apertures(3);
drivers/gpu/drm/nouveau/nouveau_drm.c-202- if (!aper)
drivers/gpu/drm/nouveau/nouveau_drm.c-203- return -ENOMEM;
Untested by me, feel free to test.
Maybe some of the i915 and/or fb driver experts can comment on the problem.
Regards,
- Sedat -
[1] http://marc.info/?t=135721787600001&r=1&w=2
[2] https://patchwork.kernel.org/patch/1757061/
[-- Attachment #2: i915-Remove-conflicting-fb-drivers.patch --]
[-- Type: application/octet-stream, Size: 508 bytes --]
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 99daa89..d9ae93d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1396,6 +1396,11 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
struct pci_dev *pdev = dev_priv->dev->pdev;
bool primary;
+ /* remove conflicting drivers (vesafb, efifb etc) */
+ ap = alloc_apertures(3);
+ if (!ap)
+ return -ENOMEM;
+
ap = alloc_apertures(1);
if (!ap)
return;
^ permalink raw reply related
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Alan Cox @ 2013-01-03 14:28 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: Peter Jones, linux-fbdev, nouveau, Linux Kernel, akpm
In-Reply-To: <CAMVG2sv8p1wFXqOJObfKg9OvyQCsNqw1QmiwXsk7nwZS8ymcbw@mail.gmail.com>
> The only patch I could find [1] (mid Nov) looks like it needs another
> sites updating, since we now see an i915 vs efifb lock ordering issue
> [2].
>
> I can get some time next week to take a look if it helps.
That would be great. I've not got any EFI afflicted hardware and I'm
doing my best to avoid it.
Alan
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Daniel J Blueman @ 2013-01-03 14:39 UTC (permalink / raw)
To: sedat.dilek; +Cc: alan, linux-fbdev, LKML, Daniel Vetter
In-Reply-To: <CA+icZUVEBEerN=YRSS8Rx-2Xvx4byFuBDbVcE45qq4QZ67+Q-A@mail.gmail.com>
On 3 January 2013 22:11, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> Hi Daniel,
>
> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
> Me is also working with i915 KMS.
>
> I looked at nouveau KMS driver and adapted the part for i915:
>
> drivers/gpu/drm/nouveau/nouveau_drm.c-200- /* remove conflicting
> drivers (vesafb, efifb etc) */
> drivers/gpu/drm/nouveau/nouveau_drm.c:201: aper = alloc_apertures(3);
> drivers/gpu/drm/nouveau/nouveau_drm.c-202- if (!aper)
> drivers/gpu/drm/nouveau/nouveau_drm.c-203- return -ENOMEM;
>
> Untested by me, feel free to test.
>
> Maybe some of the i915 and/or fb driver experts can comment on the problem.
The structure array from alloc_apertures is just used for the PCI base
address registers, so it's important here.
I'll take a look at the efifb locking later.
Thanks,
Daniel
--
Daniel J Blueman
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Sedat Dilek @ 2013-01-03 15:09 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: alan, linux-fbdev, LKML, Daniel Vetter, Dave Airlie
In-Reply-To: <CAMVG2svQkEZtz_kod-ifp1y=pwOPa4ZyRnsayf=oyzHzyRsotw@mail.gmail.com>
On Thu, Jan 3, 2013 at 3:39 PM, Daniel J Blueman <daniel@quora.org> wrote:
> On 3 January 2013 22:11, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> Hi Daniel,
>>
>> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
>> Me is also working with i915 KMS.
>>
>> I looked at nouveau KMS driver and adapted the part for i915:
>>
>> drivers/gpu/drm/nouveau/nouveau_drm.c-200- /* remove conflicting
>> drivers (vesafb, efifb etc) */
>> drivers/gpu/drm/nouveau/nouveau_drm.c:201: aper = alloc_apertures(3);
>> drivers/gpu/drm/nouveau/nouveau_drm.c-202- if (!aper)
>> drivers/gpu/drm/nouveau/nouveau_drm.c-203- return -ENOMEM;
>>
>> Untested by me, feel free to test.
>>
>> Maybe some of the i915 and/or fb driver experts can comment on the problem.
>
> The structure array from alloc_apertures is just used for the PCI base
> address registers, so it's important here.
>
> I'll take a look at the efifb locking later.
>
That is the code part [1] I looked at.
Maybe the next lines with ap(er)->ranges || pci_resource_start() and
pci_resource_len() are missing?
I also looked at "include/linux/fb.h" but could not get wiser.
Also I can't say what the value "1" or "3" means in alloc_apertures().
Wouldn't it make sense to remove the conflicting fb-drivers globally
not in each affected DRM/KMS driver?
Just an idea.
- Sedat -
[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/nouveau/nouveau_drm.c;hb=refs/tags/v3.8-rc2#l192
> Thanks,
> Daniel
> --
> Daniel J Blueman
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Sedat Dilek @ 2013-01-03 15:12 UTC (permalink / raw)
To: Alan Cox, Linus Torvalds; +Cc: linux-fbdev, LKML, Borislav Petkov
In-Reply-To: <CAMVG2ssA-B6s+F-g4mL0dbBSVsNupzT1nm1Kc70dXHGsNQuNYA@mail.gmail.com>
Hi,
I have tested the fb-fix [1] from Alan on top of Linux v3.8-rc2.
Unfortunately, several people still complain about it.
As Boris shouted out... Apply directly to mainline?
Feel free to add a "Tested-by".
Just FYI: There seems to exist a locking problem with efifb reported
by Daniel J (also see [1]).
Regards,
- Sedat -
[1] http://marc.info/?t\x135721787600001&r=1&w=2
[2] https://patchwork.kernel.org/patch/1757061/
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Sedat Dilek @ 2013-01-03 15:36 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: alan, linux-fbdev, LKML, Daniel Vetter
In-Reply-To: <CA+icZUVEBEerN=YRSS8Rx-2Xvx4byFuBDbVcE45qq4QZ67+Q-A@mail.gmail.com>
On Thu, Jan 3, 2013 at 3:11 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> Hi Daniel,
>
> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
> Me is also working with i915 KMS.
>
> I looked at nouveau KMS driver and adapted the part for i915:
>
> drivers/gpu/drm/nouveau/nouveau_drm.c-200- /* remove conflicting
> drivers (vesafb, efifb etc) */
> drivers/gpu/drm/nouveau/nouveau_drm.c:201: aper = alloc_apertures(3);
> drivers/gpu/drm/nouveau/nouveau_drm.c-202- if (!aper)
> drivers/gpu/drm/nouveau/nouveau_drm.c-203- return -ENOMEM;
>
> Untested by me, feel free to test.
>
> Maybe some of the i915 and/or fb driver experts can comment on the problem.
>
That is the i915 part [---> i915_kick_out_firmware_fb()] where I looked at [1].
- Sedat -
[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/i915/i915_dma.c;hb=refs/tags/v3.8-rc2#l1393
> Regards,
> - Sedat -
>
> [1] http://marc.info/?t\x135721787600001&r=1&w=2
> [2] https://patchwork.kernel.org/patch/1757061/
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Linus Torvalds @ 2013-01-03 19:40 UTC (permalink / raw)
To: Alan Cox
Cc: Daniel J Blueman, Peter Jones, linux-fbdev@vger.kernel.org,
nouveau, Linux Kernel, Andrew Morton
In-Reply-To: <20130103131156.214bd7a5@pyramind.ukuu.org.uk>
On Thu, Jan 3, 2013 at 5:11 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>
> The fb layer locking was broken. I posted patches early December which
> should have fixed the ones we know about. ('fb: Rework locking to fix
> lock ordering on takeover').
That patch causes compile errors with "allmodconfig":
ERROR: "do_take_over_console" [drivers/video/console/fbcon.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....
Hmm?
Linus
^ permalink raw reply
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Andrew Morton @ 2013-01-03 20:06 UTC (permalink / raw)
To: Linus Torvalds
Cc: Alan Cox, Daniel J Blueman, Peter Jones,
linux-fbdev@vger.kernel.org, nouveau, Linux Kernel,
Florian Tobias Schandinat
In-Reply-To: <CA+55aFyseLhQj-u7XOdCayMN2dJ+LaKKxSN2NrfqFA98o1WW+A@mail.gmail.com>
On Thu, 3 Jan 2013 11:40:47 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Jan 3, 2013 at 5:11 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >
> > The fb layer locking was broken. I posted patches early December which
> > should have fixed the ones we know about. ('fb: Rework locking to fix
> > lock ordering on takeover').
>
> That patch causes compile errors with "allmodconfig":
>
> ERROR: "do_take_over_console" [drivers/video/console/fbcon.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> make: *** Waiting for unfinished jobs....
>
> Hmm?
I have a couple of fixes against
fb-rework-locking-to-fix-lock-ordering-on-takeover.patch:
http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix.patch
http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix-2.patch
Florian has been busy for a month or two - I've been waiting for him to
reappear to consider this patch.
^ permalink raw reply
* Re: [BUGFIX PATCH] video: mxsfb: fix crash when unblanking the display
From: Lauri Hintsala @ 2013-01-04 7:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20663.19785.221750.909491@ipc1.ka-ro>
On 11/29/2012 01:55 PM, Lothar Waßmann wrote:
> Hi,
>
> adding Juergen Beisert as the driver's author to the recipient list.
>
> Lothar Waßmann writes:
>> The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
>> The write in mxsfb_disable_controller() sets the data_cnt for the LCD
>> DMA to 0 which obviously means the max. count for the LCD DMA and
>> leads to overwriting arbitrary memory when the display is unblanked.
>>
>> Note: I already sent this patch in December 2011 but noone picked it up obviously.
>>
>> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Tested-by: Lauri Hintsala <lauri.hintsala@bluegiga.net>
This fixes the CPU freezing. I really recommend to apply this patch into
mainline soon because unblanking freezes the whole system!!!
Default value of consoleblank parameter is 600 seconds and the whole
system crashes every time when framebuffer console is activated after
that timeout. I wonder why this issue is still open in mainline kernel.
Should this patch be added into stable versions too?
Best Regards,
Lauri Hintsala
^ permalink raw reply
* Re: [BUGFIX PATCH] video: mxsfb: fix crash when unblanking the display
From: Shawn Guo @ 2013-01-04 7:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50E67FDC.3000905@bluegiga.com>
On Fri, Jan 04, 2013 at 09:08:12AM +0200, Lauri Hintsala wrote:
> On 11/29/2012 01:55 PM, Lothar Waßmann wrote:
> >Hi,
> >
> >adding Juergen Beisert as the driver's author to the recipient list.
> >
> >Lothar Waßmann writes:
> >>The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
> >>The write in mxsfb_disable_controller() sets the data_cnt for the LCD
> >>DMA to 0 which obviously means the max. count for the LCD DMA and
> >>leads to overwriting arbitrary memory when the display is unblanked.
> >>
> >>Note: I already sent this patch in December 2011 but noone picked it up obviously.
> >>
> >>Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
>
> Tested-by: Lauri Hintsala <lauri.hintsala@bluegiga.net>
>
> This fixes the CPU freezing. I really recommend to apply this patch
> into mainline soon because unblanking freezes the whole system!!!
>
> Default value of consoleblank parameter is 600 seconds and the whole
> system crashes every time when framebuffer console is activated
> after that timeout. I wonder why this issue is still open in
> mainline kernel.
Part of the reason is the FB maintainer's absence. Let me try to send
it through arm-soc tree.
>
> Should this patch be added into stable versions too?
>
Yes, will do.
Shawn
^ permalink raw reply
* [PATCH] backlight: check null deference of name when device is registered
From: Jingoo Han @ 2013-01-04 8:29 UTC (permalink / raw)
To: 'Andrew Morton', 'LKML'
Cc: linux-fbdev, 'Richard Purdie', 'Devendra Naga',
'Jingoo Han'
NULL deference of name is checked when device is registered.
If the name is null, it will cause a kernel oops in dev_set_name().
Acked-by: Devendra Naga <devendra.aaru@gmail.com>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/backlight/backlight.c | 5 +++++
drivers/video/backlight/lcd.c | 5 +++++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 345f666..f2db904 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name,
struct backlight_device *new_bd;
int rc;
+ if (name = NULL) {
+ pr_err("backlight name is null\n");
+ return ERR_PTR(-EINVAL);
+ }
+
pr_debug("backlight_device_register: name=%s\n", name);
new_bd = kzalloc(sizeof(struct backlight_device), GFP_KERNEL);
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 34fb6bd..3d0ac0c 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -207,6 +207,11 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
struct lcd_device *new_ld;
int rc;
+ if (name = NULL) {
+ pr_err("lcd name is null\n");
+ return ERR_PTR(-EINVAL);
+ }
+
pr_debug("lcd_device_register: name=%s\n", name);
new_ld = kzalloc(sizeof(struct lcd_device), GFP_KERNEL);
--
1.7.2.5
^ permalink raw reply related
* Re: 3.8-rc2: EFI framebuffer lock inversion...
From: Sedat Dilek @ 2013-01-04 10:49 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: alan, linux-fbdev, LKML, Daniel Vetter
In-Reply-To: <CAMVG2svQkEZtz_kod-ifp1y=pwOPa4ZyRnsayf=oyzHzyRsotw@mail.gmail.com>
On Thu, Jan 3, 2013 at 3:39 PM, Daniel J Blueman <daniel@quora.org> wrote:
> On 3 January 2013 22:11, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> Hi Daniel,
>>
>> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
>> Me is also working with i915 KMS.
>>
>> I looked at nouveau KMS driver and adapted the part for i915:
>>
>> drivers/gpu/drm/nouveau/nouveau_drm.c-200- /* remove conflicting
>> drivers (vesafb, efifb etc) */
>> drivers/gpu/drm/nouveau/nouveau_drm.c:201: aper = alloc_apertures(3);
>> drivers/gpu/drm/nouveau/nouveau_drm.c-202- if (!aper)
>> drivers/gpu/drm/nouveau/nouveau_drm.c-203- return -ENOMEM;
>>
>> Untested by me, feel free to test.
>>
>> Maybe some of the i915 and/or fb driver experts can comment on the problem.
>
> The structure array from alloc_apertures is just used for the PCI base
> address registers, so it's important here.
>
> I'll take a look at the efifb locking later.
>
You had a chance to look at this?
- Sedat -
> Thanks,
> Daniel
> --
> Daniel J Blueman
^ permalink raw reply
* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alexander Holler @ 2013-01-04 12:50 UTC (permalink / raw)
To: Borislav Petkov
Cc: Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer, Alan Cox, LKML,
Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <20121228124026.GB12918@x1.alien8.de>
Am 28.12.2012 13:40, schrieb Borislav Petkov:
> On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote:
>> +1
>>
>> http://thread.gmane.org/gmane.linux.kernel/1413953/focus\x1415070
>
> Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is,
> good.
>
> Linus, Alan's patch works at least in 2 cases, you might consider
> picking it up directly since the fb maintainer is absent, reportedly.
Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at
least on ARM(v5). When I have linked in udlfb the following happens on
boot (with an attached USB-LCD and with or without the "Rework locking
patch):
dlfb_init_framebuffer_work() (work)
register_framebuffer() (lock mutex registration_lock)
(vt_console_print) (spinlock printing_lock)
(fbcon_scroll)
(fbcon_redraw)
(fbcon_putcs)
(bit_putcs)
dlfb_ops_imageblit()
dlfb_handle_damage()
dlfb_get_urb()
down_timeout(semaphore)
BUG: scheduling while atomic
(vt_console_print) (release spinlock printing_lock)
register_framebuffer() (unlock mutex registration_lock)
The above is without the "Rework locking" patch. But I get the same BUG
with the patch (so the patch doesn't do any harm), I just haven't looked
in detail what changed with the patch.
I don't get the BUG when I try the same on a x86_64 machine, not sure
why. I've just started to read me through the code, documentation and
the "Rework locking" patch. And I'm slow in doing so (spare time).
But maybe someone else with more knowledge about the inner workings of
this stuff is faster than I.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alan Cox @ 2013-01-04 13:25 UTC (permalink / raw)
To: Alexander Holler
Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <50E6D01D.6040304@ahsoftware.de>
On Fri, 04 Jan 2013 13:50:37 +0100
Alexander Holler <holler@ahsoftware.de> wrote:
> Am 28.12.2012 13:40, schrieb Borislav Petkov:
> > On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote:
> >> +1
> >>
> >> http://thread.gmane.org/gmane.linux.kernel/1413953/focus\x1415070
> >
> > Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is,
> > good.
> >
> > Linus, Alan's patch works at least in 2 cases, you might consider
> > picking it up directly since the fb maintainer is absent, reportedly.
>
>
> Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at
> least on ARM(v5). When I have linked in udlfb the following happens on
> boot (with an attached USB-LCD and with or without the "Rework locking
> patch):
They are broken if used as the system console (has been known for years).
Perhaps your x86 test has the system console still on another device ?
For the udl layer it shouldn't matter as Dave Airlie wrote a DRM driver
for udl which obsoletes the old fb layer one and works much better
(although the error handling is still totally broken and leaks like a
sieve if it fails)
Fixing the console isn't that difficult - you just need to make your
device queue the console I/O to a worker thread of some kind. We don't
want to do that by default because we want to get the messages out
reliably and immediately on saner hardware. Given there are several
such cases a general helper and a console "I am crap" flag might be better
than hacking each driver.
Alan
^ permalink raw reply
* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alexander Holler @ 2013-01-04 13:36 UTC (permalink / raw)
To: Alan Cox
Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <20130104132557.70e0c527@pyramind.ukuu.org.uk>
Am 04.01.2013 14:25, schrieb Alan Cox:
> On Fri, 04 Jan 2013 13:50:37 +0100
> Alexander Holler <holler@ahsoftware.de> wrote:
>
>> Am 28.12.2012 13:40, schrieb Borislav Petkov:
>>> On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote:
>>>> +1
>>>>
>>>> http://thread.gmane.org/gmane.linux.kernel/1413953/focus\x1415070
>>>
>>> Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is,
>>> good.
>>>
>>> Linus, Alan's patch works at least in 2 cases, you might consider
>>> picking it up directly since the fb maintainer is absent, reportedly.
>>
>>
>> Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at
>> least on ARM(v5). When I have linked in udlfb the following happens on
>> boot (with an attached USB-LCD and with or without the "Rework locking
>> patch):
>
> They are broken if used as the system console (has been known for years).
Ah. Thats why I didn't see it before. Usually I've used the serial as
system console. So thats why it worked before. ;)
> Perhaps your x86 test has the system console still on another device ?
Exactly thats the case. Thanks for pointing it out.
> For the udl layer it shouldn't matter as Dave Airlie wrote a DRM driver
> for udl which obsoletes the old fb layer one and works much better
> (although the error handling is still totally broken and leaks like a
> sieve if it fails)
>
> Fixing the console isn't that difficult - you just need to make your
> device queue the console I/O to a worker thread of some kind. We don't
That is what I wanted to try next. ;)
> want to do that by default because we want to get the messages out
> reliably and immediately on saner hardware. Given there are several
> such cases a general helper and a console "I am crap" flag might be better
> than hacking each driver.
All those drivers look very similiar. I will see if I'm successfull in
writing such an IamCrapHelper. Might need some time, but I will post a
patch for review, if I've done and tested it. I'm only using the USB-LCD
on occasion, so it doesn't have high priority for me because I don't
really need it.
Thanks for the hints.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alexander Holler @ 2013-01-05 11:41 UTC (permalink / raw)
To: Alan Cox
Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <50E6DAC9.802@ahsoftware.de>
Am 04.01.2013 14:36, schrieb Alexander Holler:
> Am 04.01.2013 14:25, schrieb Alan Cox:
>> On Fri, 04 Jan 2013 13:50:37 +0100
>> Alexander Holler <holler@ahsoftware.de> wrote:
...
>>> Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at
>>> least on ARM(v5). When I have linked in udlfb the following happens on
>>> boot (with an attached USB-LCD and with or without the "Rework locking
>>> patch):
>>
>> They are broken if used as the system console (has been known for years).
>> Fixing the console isn't that difficult - you just need to make your
>> device queue the console I/O to a worker thread of some kind. We don't
>
> That is what I wanted to try next. ;)
>
>> want to do that by default because we want to get the messages out
>> reliably and immediately on saner hardware. Given there are several
>> such cases a general helper and a console "I am crap" flag might be
>> better
>> than hacking each driver.
>
> All those drivers look very similiar. I will see if I'm successfull in
> writing such an IamCrapHelper. Might need some time, but I will post a
> patch for review, if I've done and tested it. I'm only using the USB-LCD
> on occasion, so it doesn't have high priority for me because I don't
> really need it.
I've just added a work queue for dlfb_handle_damage. Up to now I've only
tested the console, seems to work without any problems (even with lock
checking on).
In regard to that "I am crap" handler, I'm not sure how to do that.
Just queuing the ops wherever they are used outside the drivers doesn't
work, because e.g.
if(i_am_crap)
queue_work(ops.fb_imageblit(..., image))
else
ops.fb_imageblit(..., image)
doesn't work, because e.g. image will become destroyed before the work
gets executed. And copying the whole image doesn't make sense.
handle_damage() in contrast just needs the coordinates for a rectangle
(x, y, w, h).
So to add such an "I am crap" flag my idea would be to add an
.fb_handle_damage to struct fb_ops and then call that (if exists)
whenever something was changed.
But I don't like that very much. I think that might end up in more
changes than just changing those 3 very similiar drivers (I'm not sure
if the queuing is needed for udl at all).
Maybe it would make sense, to unify the stuff in those 3 similiar
drivers moving the shared functions to one file which is used by them
all. udl seems to have already split some stuff into different files.
My patch (for udlfb) follows as an reply to this message. If that patch
is ok, it should be applied to smscufx too (I would make it). In regard
to udl I don't know, I haven't had a deeper look at it nor used it up to
now.
Regards,
Alexander
^ permalink raw reply
* [PATCH] fb: udlfb: fix scheduling while atomic.
From: Alexander Holler @ 2013-01-05 11:42 UTC (permalink / raw)
To: linux-kernel
Cc: linux-fbdev, Bernie Thompson, Florian Tobias Schandinat, Alan Cox,
Steve Glendinning, Dave Airlie, Alexander Holler, stable
In-Reply-To: <50E81166.6050605@ahsoftware.de>
The console functions are using spinlocks while calling fb-driver ops
but udlfb waits for a semaphore in many ops. This results in the BUG
"scheduling while atomic". One of those call flows is e.g.
vt_console_print() (spinlock printing_lock)
(...)
dlfb_ops_imageblit()
dlfb_handle_damage()
dlfb_get_urb()
down_timeout(semaphore)
BUG: scheduling while atomic
(...)
vt_console_print() (release spinlock printing_lock)
Fix this through a workqueue for dlfb_handle_damage().
Cc: <stable@vger.kernel.org>
Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
drivers/video/udlfb.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 2 deletions(-)
diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index 86d449e..5aadcb2 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -569,7 +569,7 @@ static int dlfb_render_hline(struct dlfb_data *dev, struct urb **urb_ptr,
return 0;
}
-int dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
+int dlfb_handle_damage_queued(struct dlfb_data *dev, int x, int y,
int width, int height, char *data)
{
int i, ret;
@@ -630,6 +630,44 @@ error:
return 0;
}
+static struct workqueue_struct *dlfb_handle_damage_wq;
+
+struct dlfb_handle_damage_work {
+ struct work_struct my_work;
+ struct dlfb_data *dev;
+ char *data;
+ int x, y, width, height;
+};
+
+static void dlfb_handle_damage_work(struct work_struct *work)
+{
+ struct dlfb_handle_damage_work *my_work + (struct dlfb_handle_damage_work *)work;
+
+ dlfb_handle_damage_queued(my_work->dev, my_work->x, my_work->y,
+ my_work->width, my_work->height, my_work->data);
+ kfree(work);
+ return;
+}
+
+void dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
+ int width, int height, char *data)
+{
+ struct dlfb_handle_damage_work *work + kmalloc(sizeof(struct dlfb_handle_damage_work), GFP_KERNEL);
+
+ if (work) {
+ INIT_WORK((struct work_struct *)work, dlfb_handle_damage_work);
+ work->dev = dev;
+ work->x = x;
+ work->y = y;
+ work->width = width;
+ work->height = height;
+ work->data = data;
+ queue_work(dlfb_handle_damage_wq, (struct work_struct *)work);
+ }
+}
+
/*
* Path triggered by usermode clients who write to filesystem
* e.g. cat filename > /dev/fb1
@@ -945,6 +983,9 @@ static void dlfb_free_framebuffer(struct dlfb_data *dev)
unregister_framebuffer(info);
+ if (dlfb_handle_damage_wq)
+ destroy_workqueue(dlfb_handle_damage_wq);
+
if (info->cmap.len != 0)
fb_dealloc_cmap(&info->cmap);
if (info->monspecs.modedb)
@@ -1626,13 +1667,13 @@ static int dlfb_usb_probe(struct usb_interface *interface,
dev->sku_pixel_limit = pixel_limit;
}
-
if (!dlfb_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
retval = -ENOMEM;
pr_err("dlfb_alloc_urb_list failed\n");
goto error;
}
+
kref_get(&dev->kref); /* matching kref_put in free_framebuffer_work */
/* We don't register a new USB class. Our client interface is fbdev */
@@ -1694,6 +1735,13 @@ static void dlfb_init_framebuffer_work(struct work_struct *work)
goto error;
}
+ dlfb_handle_damage_wq = alloc_workqueue("udlfb_damage",
+ WQ_MEM_RECLAIM, 0);
+ if (dlfb_handle_damage_wq = NULL) {
+ pr_err("unable to allocate workqueue\n");
+ goto error;
+ }
+
/* ready to begin using device */
atomic_set(&dev->usb_active, 1);
@@ -1702,6 +1750,7 @@ static void dlfb_init_framebuffer_work(struct work_struct *work)
dlfb_ops_check_var(&info->var, info);
dlfb_ops_set_par(info);
+
retval = register_framebuffer(info);
if (retval < 0) {
pr_err("register_framebuffer failed %d\n", retval);
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alexander Holler @ 2013-01-05 12:06 UTC (permalink / raw)
To: Alan Cox
Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <20130105120729.7677c333@pyramind.ukuu.org.uk>
Am 05.01.2013 13:07, schrieb Alan Cox:
>> So to add such an "I am crap" flag my idea would be to add an
>> .fb_handle_damage to struct fb_ops and then call that (if exists)
>> whenever something was changed.
>
> I was thinking much higher level - ie at the printk kind of level
>
>> My patch (for udlfb) follows as an reply to this message. If that patch
>> is ok, it should be applied to smscufx too (I would make it). In regard
>> to udl I don't know, I haven't had a deeper look at it nor used it up to
>> now.
>
> Looks pretty clean as a solution to me.
Thanks and sorry for the two empty lines in the patch. I swear I had a
look at the patch before sending it out, but haven't seen them.
So should I make the same patch for smscufx and while beeing there,
send out at v2 without those 2 empty lines?
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alan Cox @ 2013-01-05 12:07 UTC (permalink / raw)
To: Alexander Holler
Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <50E81166.6050605@ahsoftware.de>
> So to add such an "I am crap" flag my idea would be to add an
> .fb_handle_damage to struct fb_ops and then call that (if exists)
> whenever something was changed.
I was thinking much higher level - ie at the printk kind of level
> My patch (for udlfb) follows as an reply to this message. If that patch
> is ok, it should be applied to smscufx too (I would make it). In regard
> to udl I don't know, I haven't had a deeper look at it nor used it up to
> now.
Looks pretty clean as a solution to me.
^ permalink raw reply
* Re: 3.8-rc2 lockdep complains about console_lock vs. fb_notifier_list.rwsem
From: Sedat Dilek @ 2013-01-05 12:13 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-fbdev, LKML, alan, Andrew Morton
Hi Jiri,
...known issue (see thread in [1]), please feel free to test patches
from Alan and Andrew (see [1], [2] and [3]) and report.
Regards,
- Sedat -
[1] http://marc.info/?t\x135309396400003&r=1&w=2
[2] http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover.patch
[3] http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix.patch
[4] http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix-2.patch
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox