* Re: [PATCH] backlight: lm3649_backlight: remove ret = -EIO at error paths of probe
From: gshark @ 2013-01-02 8:07 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1356977911-13521-1-git-send-email-devendra.aaru@gmail.com>
2013년 01월 02일 08:47, Jingoo Han 쓴 글:
> On Tuesday, January 01, 2013 3:19 AM, Devendra Naga wrote
>> the APIs are returning correctly the err codes, no need of assigning -EIO to the
>> ret again
>>
>> Cc: Daniel Jeong <daniel.jeong@ti.com>
>> Cc: G.Shark Jeong <gshark.jeong@gmail.com>
>> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> It looks good.
> Acked-by: Jingoo Han <jg1.han@samsung.com>
>
>
> Best regards,
> Jingoo Han
>
>> ---
>> drivers/video/backlight/lm3639_bl.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/backlight/lm3639_bl.c b/drivers/video/backlight/lm3639_bl.c
>> index 7ab2d2a..053964d 100644
>> --- a/drivers/video/backlight/lm3639_bl.c
>> +++ b/drivers/video/backlight/lm3639_bl.c
>> @@ -350,14 +350,13 @@ static int lm3639_probe(struct i2c_client *client,
>> &lm3639_bled_ops, &props);
>> if (IS_ERR(pchip->bled)) {
>> dev_err(&client->dev, "fail : backlight register\n");
>> - ret = -EIO;
>> + ret = PTR_ERR(pchip->bled);
>> goto err_out;
>> }
>>
>> ret = device_create_file(&(pchip->bled->dev), &dev_attr_bled_mode);
>> if (ret < 0) {
>> dev_err(&client->dev, "failed : add sysfs entries\n");
>> - ret = -EIO;
>> goto err_bled_mode;
>> }
>>
>> @@ -369,7 +368,6 @@ static int lm3639_probe(struct i2c_client *client,
>> &client->dev, &pchip->cdev_flash);
>> if (ret < 0) {
>> dev_err(&client->dev, "fail : flash register\n");
>> - ret = -EIO;
>> goto err_flash;
>> }
>>
>> @@ -381,7 +379,6 @@ static int lm3639_probe(struct i2c_client *client,
>> &client->dev, &pchip->cdev_torch);
>> if (ret < 0) {
>> dev_err(&client->dev, "fail : torch register\n");
>> - ret = -EIO;
>> goto err_torch;
>> }
>>
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thank you. This patch looks good for us.
lm3630 has same thing and I will fix it.
Acked-by: Daniel Jeong <daniel.jeong@ti.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/1] video: exynos_mipi_dsi: Fix an error check condition
From: Sachin Kamat @ 2013-01-02 11:12 UTC (permalink / raw)
To: linux-fbdev
Checking an unsigned variable for negative value returns false.
Hence use the macro to fix it.
Fixes the following smatch warning:
drivers/video/exynos/exynos_mipi_dsi.c:417 exynos_mipi_dsi_probe()
warn: unsigned 'dsim->irq' is never less than zero.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/exynos/exynos_mipi_dsi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 4a17cdc..f623dfc 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -414,7 +414,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
}
dsim->irq = platform_get_irq(pdev, 0);
- if (dsim->irq < 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;
--
1.7.4.1
^ permalink raw reply related
* Re: backlight_register_device can oops if name is null ?
From: devendra.aaru @ 2013-01-02 16:40 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAHdPZaMKfMA=Z6n-X+3NMR-CScfwQmKYuyoJHBeK2bYFn8EEdg@mail.gmail.com>
Hello,
Thanks :)
On Tue, Jan 1, 2013 at 7:44 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Tuesday, January 01, 2013 6:05 AM, Devendra Naga wrote
>> Hello,
>>
>> while reading through the backlight_register_device function, there
>
> backlight_register_device -> backlight_device_register
>
i am sorry i would have really written correctly,
>> seems to be a problem if the new object name is null, the pr_debug
>> print goes and dereferences a null pointer causing an oops,
>>
>> the print should be removed or the check must be placed to ensure that
>> name is not null,
>
> Hi,
>
> If name is null in backlight_device_register(),
> pr_debug() will work properly and won't cause an oops.
>
so if i understand the code pr_debug code, i see a printk macro assignment,
is it something like printk or a wrapper macro around printk are
making sure that the strings are non null ?
i am sorry to ask this as i dont have proper tags in vim ubuntu, its
not allowing me to a tag list when i do in fedora :(,
> However, dev_set_name() will cause oops, instead of pr_debug().
> In this case, NULL check would be better.
>
the documentation of the backlight_device_register says that "name
must be same as the name of the respected frame buffer device", but do
we really add a check for the null to do dev_set_name? or fail the
registering ?
>
> Best regards,
> Jingoo Han
>
>>
>> any ideas?
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 1/4] drivers/video/wm8505fb.c: use devm_ functions
From: Tony Prisk @ 2013-01-02 20:23 UTC (permalink / raw)
To: linux-arm-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
The various devm_ functions allocate memory that is released when a driver
detaches. This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.
The patch makes some other cleanups. First, the original code used
devm_kzalloc, but kfree. This would lead to a double free. The problem
was found using the following semantic match (http://coccinelle.lip6.fr/):
// <smpl>
@@
expression x,e;
@@
x = devm_kzalloc(...)
... when != x = e
?-kfree(x,...);
// </smpl>
The error-handing code of devm_request_and_ioremap does not print any
warning message, because devm_request_and_ioremap does this.
The call to dma_alloc_coherent is converted to its devm equivalent,
dmam_alloc_coherent. This implicitly introduces a call to
dmam_free_coherent, which was completly missing in the original code.
A semicolon is removed at the end of the error-handling code for the call
to dma_alloc_coherent.
The block of code calling fb_alloc_cmap is moved below the block of code
calling wm8505fb_set_par, so that the error-handing code of the call to
wm8505fb_set_par can just return ret. This way there is only one block of
error-handling code that needs to call fb_dealloc_cmap, and so this is
moved up to the place where it is needed, eliminating the need for all
gotos and labels in the function. This was suggested by Tony Prisk.
The initializations of fbi and ret at the beginning of the function are not
necessary and are removed. The call platform_set_drvdata(pdev, NULL); at
the end of the function is also removed.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
drivers/video/wm8505fb.c | 78 +++++++++++-----------------------------------
1 file changed, 19 insertions(+), 59 deletions(-)
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index 77539c1..1c3ce2c 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -274,15 +274,11 @@ static int __devinit wm8505fb_probe(struct platform_device *pdev)
unsigned long fb_mem_len;
void *fb_mem_virt;
- ret = -ENOMEM;
- fbi = NULL;
-
fbi = devm_kzalloc(&pdev->dev, sizeof(struct wm8505fb_info) +
sizeof(u32) * 16, GFP_KERNEL);
if (!fbi) {
dev_err(&pdev->dev, "Failed to initialize framebuffer device\n");
- ret = -ENOMEM;
- goto failed;
+ return -ENOMEM;
}
strcpy(fbi->fb.fix.id, DRIVER_NAME);
@@ -308,31 +304,15 @@ static int __devinit wm8505fb_probe(struct platform_device *pdev)
fbi->fb.pseudo_palette = addr;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res = NULL) {
- dev_err(&pdev->dev, "no I/O memory resource defined\n");
- ret = -ENODEV;
- goto failed_fbi;
- }
-
- res = request_mem_region(res->start, resource_size(res), DRIVER_NAME);
- if (res = NULL) {
- dev_err(&pdev->dev, "failed to request I/O memory\n");
- ret = -EBUSY;
- goto failed_fbi;
- }
- fbi->regbase = ioremap(res->start, resource_size(res));
- if (fbi->regbase = NULL) {
- dev_err(&pdev->dev, "failed to map I/O memory\n");
- ret = -EBUSY;
- goto failed_free_res;
- }
+ fbi->regbase = devm_request_and_ioremap(&pdev->dev, res);
+ if (fbi->regbase = NULL)
+ return -EBUSY;
np = of_parse_phandle(pdev->dev.of_node, "default-mode", 0);
if (!np) {
pr_err("%s: No display description in Device Tree\n", __func__);
- ret = -EINVAL;
- goto failed_free_res;
+ return -EINVAL;
}
/*
@@ -351,7 +331,7 @@ static int __devinit wm8505fb_probe(struct platform_device *pdev)
ret |= of_property_read_u32(np, "bpp", &bpp);
if (ret) {
pr_err("%s: Unable to read display properties\n", __func__);
- goto failed_free_res;
+ return ret;
}
of_mode.vmode = FB_VMODE_NONINTERLACED;
@@ -365,12 +345,12 @@ static int __devinit wm8505fb_probe(struct platform_device *pdev)
/* try allocating the framebuffer */
fb_mem_len = of_mode.xres * of_mode.yres * 2 * (bpp / 8);
- fb_mem_virt = dma_alloc_coherent(&pdev->dev, fb_mem_len, &fb_mem_phys,
+ fb_mem_virt = dmam_alloc_coherent(&pdev->dev, fb_mem_len, &fb_mem_phys,
GFP_KERNEL);
if (!fb_mem_virt) {
pr_err("%s: Failed to allocate framebuffer\n", __func__);
return -ENOMEM;
- };
+ }
fbi->fb.var.xres_virtual = of_mode.xres;
fbi->fb.var.yres_virtual = of_mode.yres * 2;
@@ -381,28 +361,29 @@ static int __devinit wm8505fb_probe(struct platform_device *pdev)
fbi->fb.screen_base = fb_mem_virt;
fbi->fb.screen_size = fb_mem_len;
- if (fb_alloc_cmap(&fbi->fb.cmap, 256, 0) < 0) {
- dev_err(&pdev->dev, "Failed to allocate color map\n");
- ret = -ENOMEM;
- goto failed_free_io;
- }
-
- wm8505fb_init_hw(&fbi->fb);
-
fbi->contrast = 0x80;
ret = wm8505fb_set_par(&fbi->fb);
if (ret) {
dev_err(&pdev->dev, "Failed to set parameters\n");
- goto failed_free_cmap;
+ return ret;
}
+ if (fb_alloc_cmap(&fbi->fb.cmap, 256, 0) < 0) {
+ dev_err(&pdev->dev, "Failed to allocate color map\n");
+ return -ENOMEM;
+ }
+
+ wm8505fb_init_hw(&fbi->fb);
+
platform_set_drvdata(pdev, fbi);
ret = register_framebuffer(&fbi->fb);
if (ret < 0) {
dev_err(&pdev->dev,
"Failed to register framebuffer device: %d\n", ret);
- goto failed_free_cmap;
+ if (fbi->fb.cmap.len)
+ fb_dealloc_cmap(&fbi->fb.cmap);
+ return ret;
}
ret = device_create_file(&pdev->dev, &dev_attr_contrast);
@@ -416,25 +397,11 @@ static int __devinit wm8505fb_probe(struct platform_device *pdev)
fbi->fb.fix.smem_start + fbi->fb.fix.smem_len - 1);
return 0;
-
-failed_free_cmap:
- if (fbi->fb.cmap.len)
- fb_dealloc_cmap(&fbi->fb.cmap);
-failed_free_io:
- iounmap(fbi->regbase);
-failed_free_res:
- release_mem_region(res->start, resource_size(res));
-failed_fbi:
- platform_set_drvdata(pdev, NULL);
- kfree(fbi);
-failed:
- return ret;
}
static int __devexit wm8505fb_remove(struct platform_device *pdev)
{
struct wm8505fb_info *fbi = platform_get_drvdata(pdev);
- struct resource *res;
device_remove_file(&pdev->dev, &dev_attr_contrast);
@@ -445,13 +412,6 @@ static int __devexit wm8505fb_remove(struct platform_device *pdev)
if (fbi->fb.cmap.len)
fb_dealloc_cmap(&fbi->fb.cmap);
- iounmap(fbi->regbase);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
- kfree(fbi);
-
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/4] video: vt8500: Make wmt_ge_rops optional
From: Tony Prisk @ 2013-01-02 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357158184-18213-1-git-send-email-linux@prisktech.co.nz>
At the moment, accelerated raster ops are always enabled on VT8500
and WM8xxx series SoCs. This patch makes them optional.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
drivers/video/Kconfig | 23 +++++++++++++----------
drivers/video/vt8500lcdfb.c | 15 +++++++++++++++
drivers/video/wm8505fb.c | 15 +++++++++++++++
3 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index e7068c5..6678daf 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -190,14 +190,6 @@ config FB_SYS_FOPS
depends on FB
default n
-config FB_WMT_GE_ROPS
- tristate
- depends on FB
- default n
- ---help---
- Include functions for accelerated rectangle filling and area
- copying using WonderMedia Graphics Engine operations.
-
config FB_DEFERRED_IO
bool
depends on FB
@@ -1777,7 +1769,8 @@ config FB_AU1200
config FB_VT8500
bool "VT8500 LCD Driver"
depends on (FB = y) && ARM && ARCH_VT8500
- select FB_WMT_GE_ROPS
+ select FB_SYS_FILLRECT if (!FB_WMT_GE_ROPS)
+ select FB_SYS_COPYAREA if (!FB_WMT_GE_ROPS)
select FB_SYS_IMAGEBLIT
help
This is the framebuffer driver for VIA VT8500 integrated LCD
@@ -1786,12 +1779,22 @@ config FB_VT8500
config FB_WM8505
bool "WM8505 frame buffer support"
depends on (FB = y) && ARM && ARCH_VT8500
- select FB_WMT_GE_ROPS
+ select FB_SYS_FILLRECT if (!FB_WMT_GE_ROPS)
+ select FB_SYS_COPYAREA if (!FB_WMT_GE_ROPS)
select FB_SYS_IMAGEBLIT
help
This is the framebuffer driver for WonderMedia WM8505/WM8650
integrated LCD controller.
+config FB_WMT_GE_ROPS
+ bool "VT8500/WM8xxx accelerated raster ops support"
+ depends on (FB = y) && (FB_VT8500 || FB_WM8505)
+ default n
+ help
+ This adds support for accelerated raster operations on the
+ VIA VT8500 and Wondermedia 8xxx series SoCs.
+
+
source "drivers/video/geode/Kconfig"
config FB_HIT
diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index 9af8da7..e8853ac 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -33,7 +33,10 @@
#include <linux/platform_data/video-vt8500lcdfb.h>
#include "vt8500lcdfb.h"
+
+#ifdef CONFIG_FB_WMT_GE_ROPS
#include "wmt_ge_rops.h"
+#endif
#ifdef CONFIG_OF
#include <linux/of.h>
@@ -249,12 +252,24 @@ static int vt8500lcd_blank(int blank, struct fb_info *info)
return 0;
}
+#ifndef CONFIG_FB_WMT_GE_ROPS
+static int wmt_ge_sync(struct fb_info *p)
+{
+ return 0;
+}
+#endif
+
static struct fb_ops vt8500lcd_ops = {
.owner = THIS_MODULE,
.fb_set_par = vt8500lcd_set_par,
.fb_setcolreg = vt8500lcd_setcolreg,
+#ifdef CONFIG_FB_WMT_GE_ROPS
.fb_fillrect = wmt_ge_fillrect,
.fb_copyarea = wmt_ge_copyarea,
+#else
+ .fb_fillrect = sys_fillrect,
+ .fb_copyarea = sys_copyarea,
+#endif
.fb_imageblit = sys_imageblit,
.fb_sync = wmt_ge_sync,
.fb_ioctl = vt8500lcd_ioctl,
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index 77539c1..a990708 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -35,7 +35,10 @@
#include <linux/platform_data/video-vt8500lcdfb.h>
#include "wm8505fb_regs.h"
+
+#ifdef CONFIG_FB_WMT_GE_ROPS
#include "wmt_ge_rops.h"
+#endif
#define DRIVER_NAME "wm8505-fb"
@@ -248,12 +251,24 @@ static int wm8505fb_blank(int blank, struct fb_info *info)
return 0;
}
+#ifndef CONFIG_FB_WMT_GE_ROPS
+static int wmt_ge_sync(struct fb_info *p)
+{
+ return 0;
+}
+#endif
+
static struct fb_ops wm8505fb_ops = {
.owner = THIS_MODULE,
.fb_set_par = wm8505fb_set_par,
.fb_setcolreg = wm8505fb_setcolreg,
+#ifdef CONFIG_FB_WMT_GE_ROPS
.fb_fillrect = wmt_ge_fillrect,
.fb_copyarea = wmt_ge_copyarea,
+#else
+ .fb_fillrect = sys_fillrect,
+ .fb_copyarea = sys_copyarea,
+#endif
.fb_imageblit = sys_imageblit,
.fb_sync = wmt_ge_sync,
.fb_pan_display = wm8505fb_pan_display,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/4] video: vt8500: Remove unused platform_data/video-vt8500lcdfb.h
From: Tony Prisk @ 2013-01-02 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357158184-18213-1-git-send-email-linux@prisktech.co.nz>
With the conversion to devicetree only for arch-vt8500, this
header is no longer required. This patch removes the #include
from the two framebuffer drivers that used it, and the header file.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
drivers/video/vt8500lcdfb.c | 2 --
drivers/video/wm8505fb.c | 2 --
include/linux/platform_data/video-vt8500lcdfb.h | 31 -----------------------
3 files changed, 35 deletions(-)
delete mode 100644 include/linux/platform_data/video-vt8500lcdfb.h
diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index e8853ac..b1fbe20 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -30,8 +30,6 @@
#include <linux/platform_device.h>
#include <linux/wait.h>
-#include <linux/platform_data/video-vt8500lcdfb.h>
-
#include "vt8500lcdfb.h"
#ifdef CONFIG_FB_WMT_GE_ROPS
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index a990708..f0185cd 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -32,8 +32,6 @@
#include <linux/of_fdt.h>
#include <linux/memblock.h>
-#include <linux/platform_data/video-vt8500lcdfb.h>
-
#include "wm8505fb_regs.h"
#ifdef CONFIG_FB_WMT_GE_ROPS
diff --git a/include/linux/platform_data/video-vt8500lcdfb.h b/include/linux/platform_data/video-vt8500lcdfb.h
deleted file mode 100644
index 7f399c3..0000000
--- a/include/linux/platform_data/video-vt8500lcdfb.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * VT8500/WM8505 Frame Buffer platform data definitions
- *
- * Copyright (C) 2010 Ed Spiridonov <edo.rus@gmail.com>
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef _VT8500FB_H
-#define _VT8500FB_H
-
-#include <linux/fb.h>
-
-struct vt8500fb_platform_data {
- struct fb_videomode mode;
- u32 xres_virtual;
- u32 yres_virtual;
- u32 bpp;
- unsigned long video_mem_phys;
- void *video_mem_virt;
- unsigned long video_mem_len;
-};
-
-#endif /* _VT8500FB_H */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 4/4] video: vt8500: Update descriptions in video/Kconfig
From: Tony Prisk @ 2013-01-02 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357158184-18213-1-git-send-email-linux@prisktech.co.nz>
This patch updates the descriptions for the VIA VT8500 and
Wondermedia WM8xxx-series framebuffer drivers to correctly reflect
which hardware they support.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
drivers/video/Kconfig | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6678daf..3bbb3c1 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1767,7 +1767,7 @@ config FB_AU1200
option au1200fb:panel=<name>.
config FB_VT8500
- bool "VT8500 LCD Driver"
+ bool "VIA VT8500 Framebuffer Driver"
depends on (FB = y) && ARM && ARCH_VT8500
select FB_SYS_FILLRECT if (!FB_WMT_GE_ROPS)
select FB_SYS_COPYAREA if (!FB_WMT_GE_ROPS)
@@ -1777,14 +1777,15 @@ config FB_VT8500
controller.
config FB_WM8505
- bool "WM8505 frame buffer support"
+ bool "Wondermedia WM8xxx-series framebuffer support"
depends on (FB = y) && ARM && ARCH_VT8500
select FB_SYS_FILLRECT if (!FB_WMT_GE_ROPS)
select FB_SYS_COPYAREA if (!FB_WMT_GE_ROPS)
select FB_SYS_IMAGEBLIT
help
- This is the framebuffer driver for WonderMedia WM8505/WM8650
- integrated LCD controller.
+ This is the framebuffer driver for WonderMedia WM8xxx-series
+ integrated LCD controller. This driver covers the WM8505, WM8650
+ and WM8850 SoCs.
config FB_WMT_GE_ROPS
bool "VT8500/WM8xxx accelerated raster ops support"
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/1] video: exynos_mipi_dsi: Fix an error check condition
From: Donghwa Lee @ 2013-01-03 0:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1357124433-24455-1-git-send-email-sachin.kamat@linaro.org>
On 01/02/2013 20:00, Sachin Kamat wrote:
> Checking an unsigned variable for negative value returns false.
> Hence use the macro to fix it.
>
> Fixes the following smatch warning:
> drivers/video/exynos/exynos_mipi_dsi.c:417 exynos_mipi_dsi_probe()
> warn: unsigned 'dsim->irq' is never less than zero.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/video/exynos/exynos_mipi_dsi.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> index 4a17cdc..f623dfc 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -414,7 +414,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
> }
>
> dsim->irq = platform_get_irq(pdev, 0);
> - if (dsim->irq < 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;
Hi,
It looks good to me.
Acked-by: Donghwa Lee <dh09.lee@samsung.com>
Thank you,
Donghwa Lee
^ permalink raw reply
* Re: backlight_register_device can oops if name is null ?
From: Jingoo Han @ 2013-01-03 2:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAHdPZaMKfMA=Z6n-X+3NMR-CScfwQmKYuyoJHBeK2bYFn8EEdg@mail.gmail.com>
On Thursday, January 03, 2013 1:41 AM, Devendra Naga wrote"
> On Tue, Jan 1, 2013 at 7:44 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Tuesday, January 01, 2013 6:05 AM, Devendra Naga wrote
> >> Hello,
> >>
> >> while reading through the backlight_register_device function, there
> >
> > backlight_register_device -> backlight_device_register
> >
>
> i am sorry i would have really written correctly,
>
> >> seems to be a problem if the new object name is null, the pr_debug
> >> print goes and dereferences a null pointer causing an oops,
> >>
> >> the print should be removed or the check must be placed to ensure that
> >> name is not null,
> >
> > Hi,
> >
> > If name is null in backlight_device_register(),
> > pr_debug() will work properly and won't cause an oops.
> >
>
> so if i understand the code pr_debug code, i see a printk macro assignment,
>
> is it something like printk or a wrapper macro around printk are
> making sure that the strings are non null ?
Sorry, I don't know how printk deals this null.
But I tested null by using ams369fg06 driver as below:
- bd = backlight_device_register("ams369fg06-bl", &spi->dev, lcd,
+ bd = backlight_device_register(NULL, &spi->dev, lcd,
Also, I replaced pr_debug with printk as below:
- pr_debug("backlight_device_register: name=%s\n", name);
+ printk("backlight_device_register: name=%s\n", name);
In this case, printk message is as below:
backlight_device_register: name=(null)
>
> i am sorry to ask this as i dont have proper tags in vim ubuntu, its
> not allowing me to a tag list when i do in fedora :(,
>
> > However, dev_set_name() will cause oops, instead of pr_debug().
> > In this case, NULL check would be better.
> >
>
> the documentation of the backlight_device_register says that "name
> must be same as the name of the respected frame buffer device", but do
> we really add a check for the null to do dev_set_name? or fail the
> registering ?
"name must be same as the name of the respected frame buffer device" is
not correct. Most backlight drivers are using their driver name as 'name'.
Anyway, in my opinion, deference to null does not seem to happen.
Now, backlight_device_register() calls are using 'name' properly.
However, if a check for the null is added, it will be added to
backlight_device_register() as below:
@@ -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(".....");
+ return -EINVAL
+ }
+
>
> >
> > Best regards,
> > Jingoo Han
> >
> >>
> >> any ideas?
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply
* Re: backlight_register_device can oops if name is null ?
From: devendra.aaru @ 2013-01-03 5:22 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAHdPZaMKfMA=Z6n-X+3NMR-CScfwQmKYuyoJHBeK2bYFn8EEdg@mail.gmail.com>
>>>
>> so if i understand the code pr_debug code, i see a printk macro assignment,
>>
>> is it something like printk or a wrapper macro around printk are
>> making sure that the strings are non null ?
>
> Sorry, I don't know how printk deals this null.
>
> But I tested null by using ams369fg06 driver as below:
>
> - bd = backlight_device_register("ams369fg06-bl", &spi->dev, lcd,
> + bd = backlight_device_register(NULL, &spi->dev, lcd,
>
> Also, I replaced pr_debug with printk as below:
>
> - pr_debug("backlight_device_register: name=%s\n", name);
> + printk("backlight_device_register: name=%s\n", name);
>
> In this case, printk message is as below:
>
> backlight_device_register: name=(null)
>
>>
wow, you tested the code, i would have done it with a single module,
great and thanks for lot of information you are putting .
>> the documentation of the backlight_device_register says that "name
>> must be same as the name of the respected frame buffer device", but do
>> we really add a check for the null to do dev_set_name? or fail the
>> registering ?
>
> "name must be same as the name of the respected frame buffer device" is
> not correct. Most backlight drivers are using their driver name as 'name'.
>
> Anyway, in my opinion, deference to null does not seem to happen.
> Now, backlight_device_register() calls are using 'name' properly.
>
>
> However, if a check for the null is added, it will be added to
> backlight_device_register() as below:
>
>
> @@ -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(".....");
> + return -EINVAL
> + }
> +
This is what exactly i was thinking, thanks for making a patch too.
If you want to submit it then please take my
Acked-By: Devendra Naga <devendra.aaru@gmail.com>
^ permalink raw reply
* [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
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