Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v2 00/10] video: da8xx-fb: runtime timing configuration
From: Afzal Mohammed @ 2013-01-07  5:13 UTC (permalink / raw)
  To: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, nsekhar, hvaibhav, linux-omap, linux-fbdev,
	linux-kernel, devicetree-discuss

Hi,

This series makes da8xx-fb driver (device found on DaVinci and AM335x)
capable of handling runtime timing configuration by adding fb_set_par.

The last change adds actual fb_set_par support. Other preceeding
changes makes the way clear for it as well as does certain cleanup's
on the way.

This has been tested on da850 evm as is. This was also tested on
am335x-evm & am335x-evmsk with a series that adds DT support which
follows shortly.

This is based on v3.8-rc2

Regards
Afzal

v2: disable raster in fb_set_par properly

Afzal Mohammed (10):
  video: da8xx-fb: fb_check_var enhancement
  video: da8xx-fb: simplify lcd_reset
  video: da8xx-fb: use modedb helper to update var
  video: da8xx-fb: remove unneeded "var" initialization
  video: da8xx-fb: store current display information
  video: da8xx-fb: store clk rate even if !CPUFREQ
  video: da8xx-fb: pix clk and clk div handling cleanup
  video: da8xx-fb: store struct device *
  video: da8xx-fb: report correct pixclock
  video: da8xx-fb: fb_set_par support

 drivers/video/da8xx-fb.c |  181 +++++++++++++++++++++++++++-------------------
 1 file changed, 105 insertions(+), 76 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Daniel Vetter @ 2013-01-06 17:46 UTC (permalink / raw)
  To: Rob Clark
  Cc: Thomas Petazzoni, linux-fbdev@vger.kernel.org, marcus.lorentzon,
	Tom Gall, Kyungmin Park, dri-devel@lists.freedesktop.org,
	Ragesh Radhakrishnan, Tomi Valkeinen, linux-media@vger.kernel.org,
	Laurent Pinchart, Philipp Zabel, Maxime Ripard, Vikas Sajjan,
	Sumit Semwal, Sebastien Guiriec, Benjamin Gaignard
In-Reply-To: <CAF6AEGsirt1iVfmpfT1pUCaj0KXO+Hycer5znUoCfH+81du1vA@mail.gmail.com>

On Thu, Dec 27, 2012 at 09:57:25AM -0600, Rob Clark wrote:
> On Mon, Dec 24, 2012 at 11:09 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On the topic of discussions, would anyone be interested in a
> > BoF/brainstorming/whatever session during the FOSDEM ?
> 
> I will be at FOSDEM.. and from http://wiki.x.org/wiki/fosdem2013 it
> looks like at least Daniel will be there.  If enough others are, it
> could be a good idea.

Seconded. Jesse should be there, too, and from the Helsinki guys Ville and
Andy should show up. Doesn't look like Jani will be able to make it. I
think something on Sunday (to not clash with the X devroom) would be good.

Should we apply for an offical BOF/Is there a process for tahat? Adding
Luc in case he knows ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* Re: [PATCH] fb: udlfb: fix scheduling while atomic.
From: Alexander Holler @ 2013-01-06 12:46 UTC (permalink / raw)
  To: Alexander Holler
  Cc: linux-kernel, linux-fbdev, Bernie Thompson,
	Florian Tobias Schandinat, Alan Cox, Steve Glendinning,
	Dave Airlie, stable
In-Reply-To: <1357386129-763-1-git-send-email-holler@ahsoftware.de>

Am 05.01.2013 12:42, schrieb Alexander Holler:
> The console functions are using spinlocks while calling fb-driver ops
> but udlfb waits for a semaphore in many ops. This results in the BUG
> "scheduling while atomic". One of those call flows is e.g.
>
> vt_console_print() (spinlock printing_lock)
> 	(...)
> 	dlfb_ops_imageblit()
>                          dlfb_handle_damage()
>                                  dlfb_get_urb()
> 					down_timeout(semaphore)
> BUG: scheduling while atomic
> (...)
> vt_console_print() (release spinlock printing_lock)
>
> Fix this through a workqueue for dlfb_handle_damage().
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>


Having had a second look at my patch for udlfb, I'm not sure it will 
work with more than one of those devices attached. I think my approach 
to just add one (static) workqueue might not work in such a case, at 
least it looks so to me. But I'm unable to test it, as I only have one 
of those devices.

Having had a look at udl, I wonder why udlfb still has to be around. But 
because udl currently doesn't work here too, I'm not sure what 
functionality udl misses which udlfb still has.

So to conclude, my patch works as a workaround if only one of those 
devices will be attached, but currently should not be included into the 
kernel.

I don't know if I will make another version of that patch, as I will 
first have a deeper look at udl (if I find the time).

Regards,

Alexander


^ permalink raw reply

* Re: 3.8-rc2 lockdep complains about console_lock vs. fb_notifier_list.rwsem
From: Sedat Dilek @ 2013-01-05 12:13 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-fbdev, LKML, alan, Andrew Morton

Hi Jiri,

...known issue (see thread in [1]), please feel free to test patches
from Alan and Andrew (see [1], [2] and [3]) and report.

Regards,
- Sedat -

[1] http://marc.info/?t\x135309396400003&r=1&w=2
[2] http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover.patch
[3] http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix.patch
[4] http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix-2.patch

^ permalink raw reply

* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alan Cox @ 2013-01-05 12:07 UTC (permalink / raw)
  To: Alexander Holler
  Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
	LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
	Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <50E81166.6050605@ahsoftware.de>

> So to add such an "I am crap" flag my idea would be to add an 
> .fb_handle_damage to struct fb_ops and then call that (if exists) 
> whenever something was changed.

I was thinking much higher level - ie at the printk kind of level

> My patch (for udlfb) follows as an reply to this message. If that patch 
> is ok, it should be applied to smscufx too (I would make it). In regard 
> to udl I don't know, I haven't had a deeper look at it nor used it up to 
> now.

Looks pretty clean as a solution to me.

^ permalink raw reply

* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alexander Holler @ 2013-01-05 12:06 UTC (permalink / raw)
  To: Alan Cox
  Cc: Borislav Petkov, Shawn Guo, Sasha Levin, Cong Wang, Josh Boyer,
	LKML, Florian Tobias Schandinat, Linus Torvalds, linux-fbdev,
	Bernie Thompson, Steve Glendinning, Dave Airlie
In-Reply-To: <20130105120729.7677c333@pyramind.ukuu.org.uk>

Am 05.01.2013 13:07, schrieb Alan Cox:
>> So to add such an "I am crap" flag my idea would be to add an
>> .fb_handle_damage to struct fb_ops and then call that (if exists)
>> whenever something was changed.
>
> I was thinking much higher level - ie at the printk kind of level
>
>> My patch (for udlfb) follows as an reply to this message. If that patch
>> is ok, it should be applied to smscufx too (I would make it). In regard
>> to udl I don't know, I haven't had a deeper look at it nor used it up to
>> now.
>
> Looks pretty clean as a solution to me.

Thanks and sorry for the two empty lines in the patch. I swear I had a 
look at the patch before sending it out, but haven't seen them.

So should I make the same patch for smscufx  and while beeing there, 
send out at v2 without those 2 empty lines?

Regards,

Alexander



^ permalink raw reply

* [PATCH] fb: udlfb: fix scheduling while atomic.
From: Alexander Holler @ 2013-01-05 11:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fbdev, Bernie Thompson, Florian Tobias Schandinat, Alan Cox,
	Steve Glendinning, Dave Airlie, Alexander Holler, stable
In-Reply-To: <50E81166.6050605@ahsoftware.de>

The console functions are using spinlocks while calling fb-driver ops
but udlfb waits for a semaphore in many ops. This results in the BUG
"scheduling while atomic". One of those call flows is e.g.

vt_console_print() (spinlock printing_lock)
	(...)
	dlfb_ops_imageblit()
                        dlfb_handle_damage()
                                dlfb_get_urb()
					down_timeout(semaphore)
BUG: scheduling while atomic
(...)
vt_console_print() (release spinlock printing_lock)

Fix this through a workqueue for dlfb_handle_damage().

Cc: <stable@vger.kernel.org>
Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/video/udlfb.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index 86d449e..5aadcb2 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -569,7 +569,7 @@ static int dlfb_render_hline(struct dlfb_data *dev, struct urb **urb_ptr,
 	return 0;
 }
 
-int dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
+int dlfb_handle_damage_queued(struct dlfb_data *dev, int x, int y,
 	       int width, int height, char *data)
 {
 	int i, ret;
@@ -630,6 +630,44 @@ error:
 	return 0;
 }
 
+static struct workqueue_struct *dlfb_handle_damage_wq;
+
+struct dlfb_handle_damage_work {
+	struct work_struct my_work;
+	struct dlfb_data *dev;
+	char *data;
+	int x, y, width, height;
+};
+
+static void dlfb_handle_damage_work(struct work_struct *work)
+{
+	struct dlfb_handle_damage_work *my_work +		(struct dlfb_handle_damage_work *)work;
+
+	dlfb_handle_damage_queued(my_work->dev, my_work->x, my_work->y,
+			my_work->width, my_work->height, my_work->data);
+	kfree(work);
+	return;
+}
+
+void dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
+	       int width, int height, char *data)
+{
+	struct dlfb_handle_damage_work *work +		kmalloc(sizeof(struct dlfb_handle_damage_work), GFP_KERNEL);
+
+	if (work) {
+		INIT_WORK((struct work_struct *)work, dlfb_handle_damage_work);
+		work->dev = dev;
+		work->x = x;
+		work->y = y;
+		work->width = width;
+		work->height = height;
+		work->data = data;
+		queue_work(dlfb_handle_damage_wq, (struct work_struct *)work);
+	}
+}
+
 /*
  * Path triggered by usermode clients who write to filesystem
  * e.g. cat filename > /dev/fb1
@@ -945,6 +983,9 @@ static void dlfb_free_framebuffer(struct dlfb_data *dev)
 
 		unregister_framebuffer(info);
 
+		if (dlfb_handle_damage_wq)
+			destroy_workqueue(dlfb_handle_damage_wq);
+
 		if (info->cmap.len != 0)
 			fb_dealloc_cmap(&info->cmap);
 		if (info->monspecs.modedb)
@@ -1626,13 +1667,13 @@ static int dlfb_usb_probe(struct usb_interface *interface,
 		dev->sku_pixel_limit = pixel_limit;
 	}
 
-
 	if (!dlfb_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
 		retval = -ENOMEM;
 		pr_err("dlfb_alloc_urb_list failed\n");
 		goto error;
 	}
 
+
 	kref_get(&dev->kref); /* matching kref_put in free_framebuffer_work */
 
 	/* We don't register a new USB class. Our client interface is fbdev */
@@ -1694,6 +1735,13 @@ static void dlfb_init_framebuffer_work(struct work_struct *work)
 		goto error;
 	}
 
+	dlfb_handle_damage_wq = alloc_workqueue("udlfb_damage",
+						WQ_MEM_RECLAIM, 0);
+	if (dlfb_handle_damage_wq = NULL) {
+		pr_err("unable to allocate workqueue\n");
+		goto error;
+	}
+
 	/* ready to begin using device */
 
 	atomic_set(&dev->usb_active, 1);
@@ -1702,6 +1750,7 @@ static void dlfb_init_framebuffer_work(struct work_struct *work)
 	dlfb_ops_check_var(&info->var, info);
 	dlfb_ops_set_par(info);
 
+
 	retval = register_framebuffer(info);
 	if (retval < 0) {
 		pr_err("register_framebuffer failed %d\n", retval);
-- 
1.7.11.7


^ permalink raw reply related

* Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
From: Alexander Holler @ 2013-01-05 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox