* 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
* 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: 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 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: 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
* [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: [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
* 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: 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: 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: 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: 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: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: 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: 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: 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: 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: 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
* 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
* [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
* [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
* 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
* 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: [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
* [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
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