Re: [PATCH] fb: Rework locking to fix lock ordering on takeover

[Alexander Holler]

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@xxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/