* [PATCH 09/29] g364fb: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:58 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/g364fb.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/g364fb.c b/drivers/video/g364fb.c
index d662317..223896c 100644
--- a/drivers/video/g364fb.c
+++ b/drivers/video/g364fb.c
@@ -149,10 +149,11 @@ int g364fb_cursor(struct fb_info *info, struct fb_cursor *cursor)
static int g364fb_pan_display(struct fb_var_screeninfo *var,
struct fb_info *info)
{
- if (var->xoffset || var->yoffset + var->yres > var->yres_virtual)
+ if (var->xoffset ||
+ var->yoffset + info->var.yres > info->var.yres_virtual)
return -EINVAL;
- *(unsigned int *) TOP_REG = var->yoffset * var->xres;
+ *(unsigned int *) TOP_REG = var->yoffset * info->var.xres;
return 0;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH 08/29] fbdev: unicore32: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:58 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
---
drivers/video/fb-puv3.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fb-puv3.c b/drivers/video/fb-puv3.c
index 27f2c57..60a787f 100644
--- a/drivers/video/fb-puv3.c
+++ b/drivers/video/fb-puv3.c
@@ -624,8 +624,8 @@ static int unifb_pan_display(struct fb_var_screeninfo *var,
|| var->xoffset)
return -EINVAL;
} else {
- if (var->xoffset + var->xres > info->var.xres_virtual ||
- var->yoffset + var->yres > info->var.yres_virtual)
+ if (var->xoffset + info->var.xres > info->var.xres_virtual ||
+ var->yoffset + info->var.yres > info->var.yres_virtual)
return -EINVAL;
}
info->var.xoffset = var->xoffset;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 07/29] fbdev: da8xx: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Cc: Martin Ambrose <martin@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 8b7d473..3470217 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -877,8 +877,8 @@ static int da8xx_pan_display(struct fb_var_screeninfo *var,
start = fix->smem_start +
new_var.yoffset * fix->line_length +
- new_var.xoffset * var->bits_per_pixel / 8;
- end = start + var->yres * fix->line_length - 1;
+ new_var.xoffset * fbi->var.bits_per_pixel / 8;
+ end = start + fbi->var.yres * fix->line_length - 1;
par->dma_start = start;
par->dma_end = end;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH 06/29] radeonfb: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it. Also use the
aligned fix.line_length and not the (possible) unaligned xres_virtual.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/video/aty/radeon_base.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/aty/radeon_base.c b/drivers/video/aty/radeon_base.c
index 32f8cf6..1506848 100644
--- a/drivers/video/aty/radeon_base.c
+++ b/drivers/video/aty/radeon_base.c
@@ -845,16 +845,16 @@ static int radeonfb_pan_display (struct fb_var_screeninfo *var,
{
struct radeonfb_info *rinfo = info->par;
- if ((var->xoffset + var->xres > var->xres_virtual)
- || (var->yoffset + var->yres > var->yres_virtual))
- return -EINVAL;
+ if ((var->xoffset + info->var.xres > info->var.xres_virtual)
+ || (var->yoffset + info->var.yres > info->var.yres_virtual))
+ return -EINVAL;
if (rinfo->asleep)
return 0;
radeon_fifo_wait(2);
- OUTREG(CRTC_OFFSET, ((var->yoffset * var->xres_virtual + var->xoffset)
- * var->bits_per_pixel / 8) & ~7);
+ OUTREG(CRTC_OFFSET, (var->yoffset * info->fix.line_length +
+ var->xoffset * info->var.bits_per_pixel / 8) & ~7);
return 0;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH 05/29] atmel_lcdfb: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/video/atmel_lcdfb.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 4484c72..8b5d755 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -39,7 +39,8 @@
| FBINFO_HWACCEL_YPAN)
static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
- struct fb_var_screeninfo *var)
+ struct fb_var_screeninfo *var,
+ struct fb_info *info)
{
}
@@ -50,14 +51,16 @@ static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
| FBINFO_HWACCEL_YPAN)
static void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
- struct fb_var_screeninfo *var)
+ struct fb_var_screeninfo *var,
+ struct fb_info *info)
{
u32 dma2dcfg;
u32 pixeloff;
- pixeloff = (var->xoffset * var->bits_per_pixel) & 0x1f;
+ pixeloff = (var->xoffset * info->var.bits_per_pixel) & 0x1f;
- dma2dcfg = ((var->xres_virtual - var->xres) * var->bits_per_pixel) / 8;
+ dma2dcfg = (info-var.xres_virtual - info->var.xres)
+ * info->var.bits_per_pixel / 8;
dma2dcfg |= pixeloff << ATMEL_LCDC_PIXELOFF_OFFSET;
lcdc_writel(sinfo, ATMEL_LCDC_DMA2DCFG, dma2dcfg);
@@ -249,14 +252,14 @@ static void atmel_lcdfb_update_dma(struct fb_info *info,
unsigned long dma_addr;
dma_addr = (fix->smem_start + var->yoffset * fix->line_length
- + var->xoffset * var->bits_per_pixel / 8);
+ + var->xoffset * info->var.bits_per_pixel / 8);
dma_addr &= ~3UL;
/* Set framebuffer DMA base address and pixel offset */
lcdc_writel(sinfo, ATMEL_LCDC_DMABADDR1, dma_addr);
- atmel_lcdfb_update_dma2d(sinfo, var);
+ atmel_lcdfb_update_dma2d(sinfo, var, info);
}
static inline void atmel_lcdfb_free_video_memory(struct atmel_lcdfb_info *sinfo)
--
1.7.3.4
^ permalink raw reply related
* [PATCH 04/29] arkfb: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Ondrej Zajicek <santiago@crfreenet.org>
---
drivers/video/arkfb.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/video/arkfb.c b/drivers/video/arkfb.c
index 8686429..555dd4c 100644
--- a/drivers/video/arkfb.c
+++ b/drivers/video/arkfb.c
@@ -908,13 +908,14 @@ static int arkfb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info
unsigned int offset;
/* Calculate the offset */
- if (var->bits_per_pixel = 0) {
- offset = (var->yoffset / 16) * (var->xres_virtual / 2) + (var->xoffset / 2);
+ if (info->var.bits_per_pixel = 0) {
+ offset = (var->yoffset / 16) * (info->var.xres_virtual / 2)
+ + (var->xoffset / 2);
offset = offset >> 2;
} else {
offset = (var->yoffset * info->fix.line_length) +
- (var->xoffset * var->bits_per_pixel / 8);
- offset = offset >> ((var->bits_per_pixel = 4) ? 2 : 3);
+ (var->xoffset * info->var.bits_per_pixel / 8);
+ offset = offset >> ((info->var.bits_per_pixel = 4) ? 2 : 3);
}
/* Set the offset */
--
1.7.3.4
^ permalink raw reply related
* [PATCH 03/29] acornfb: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/acornfb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/acornfb.c b/drivers/video/acornfb.c
index 3bacc12..b303f17 100644
--- a/drivers/video/acornfb.c
+++ b/drivers/video/acornfb.c
@@ -850,9 +850,9 @@ acornfb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
u_int y_bottom = var->yoffset;
if (!(var->vmode & FB_VMODE_YWRAP))
- y_bottom += var->yres;
+ y_bottom += info->var.yres;
- if (y_bottom > var->yres_virtual)
+ if (y_bottom > info->var.yres_virtual)
return -EINVAL;
acornfb_update_dma(info, var);
--
1.7.3.4
^ permalink raw reply related
* [PATCH 02/29] acornfb: Dont BUG() on invalid pan parameters
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
The driver currently BUG()s if the pan parameters passed directly from
userspace are invalid. Return -EINVAL instead.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/acornfb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/video/acornfb.c b/drivers/video/acornfb.c
index 6183a57..3bacc12 100644
--- a/drivers/video/acornfb.c
+++ b/drivers/video/acornfb.c
@@ -852,7 +852,8 @@ acornfb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
if (!(var->vmode & FB_VMODE_YWRAP))
y_bottom += var->yres;
- BUG_ON(y_bottom > var->yres_virtual);
+ if (y_bottom > var->yres_virtual)
+ return -EINVAL;
acornfb_update_dma(info, var);
--
1.7.3.4
^ permalink raw reply related
* [PATCH 01/29] 68328fb: use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
We must not use any information in the passed var besides xoffset,
yoffset and vmode as otherwise applications might abuse it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/68328fb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/68328fb.c b/drivers/video/68328fb.c
index 75a39ea..a425d65 100644
--- a/drivers/video/68328fb.c
+++ b/drivers/video/68328fb.c
@@ -378,8 +378,8 @@ static int mc68x328fb_pan_display(struct fb_var_screeninfo *var,
|| var->xoffset)
return -EINVAL;
} else {
- if (var->xoffset + var->xres > info->var.xres_virtual ||
- var->yoffset + var->yres > info->var.yres_virtual)
+ if (var->xoffset + info->var.xres > info->var.xres_virtual ||
+ var->yoffset + info->var.yres > info->var.yres_virtual)
return -EINVAL;
}
info->var.xoffset = var->xoffset;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 00/29] Use display information in info not in var for panning
From: Laurent Pinchart @ 2011-05-25 22:57 UTC (permalink / raw)
To: linux-fbdev
Hi everybody,
While playing with the FBIOPAN_DISPLAY ioctl I noticed that many drivers use
information from the ioctl argument such as the display resolution when they
should use the current settings from the fb_info structure.
These 29 patches fix most of those drivers. Missing drivers are ivtv (for
which I've already sent a patch), sh_mobile_lcdcfb (for which the fix is a
bit more complex, I will work on it later) and viafb (for which Florian Tobias
Schandinat has already sent a patch).
The patches have been compile-tested on architectures for which I have a
working compiler (x86 and ARM) with allyesconfig. I don't own any hardware
supported by these drivers so I haven't been able to perform additional tests.
I've split the fixes in one patch per driver for easier review and handling of
potential problems. The patches can be squashed together if needed. I've tried
to CC individual driver maintainers when possible.
Laurent Pinchart (29):
68328fb: use display information in info not in var for panning
acornfb: Dont BUG() on invalid pan parameters
acornfb: use display information in info not in var for panning
arkfb: use display information in info not in var for panning
atmel_lcdfb: use display information in info not in var for panning
radeonfb: use display information in info not in var for panning
fbdev: da8xx: use display information in info not in var for panning
fbdev: unicore32: use display information in info not in var for
panning
g364fb: use display information in info not in var for panning
gxt4500: use display information in info not in var for panning
hgafb: use display information in info not in var for panning
imsttfb: use display information in info not in var for panning
intelfb: use display information in info not in var for panning
mb862xxfb: use display information in info not in var for panning
mx3fb: use display information in info not in var for panning
neofb: use display information in info not in var for panning
pm2fb: use display information in info not in var for panning
pm3fb: use display information in info not in var for panning
s3c-fb: use display information in info not in var for panning
s3c-fb: use display information in info not in var for panning
s3fb: use display information in info not in var for panning
sisfb: use display information in info not in var for panning
sm501fb: use display information in info not in var for panning
tridentfb: use display information in info not in var for panning
vfb: use display information in info not in var for panning
vga16fb: use display information in info not in var for panning
vt8500lcdfb: use display information in info not in var for panning
vt8623fb: use display information in info not in var for panning
staging: xgifb: use display information in info not in var for
panning
drivers/staging/xgifb/XGI_main_26.c | 22 ++++++----------------
drivers/video/68328fb.c | 4 ++--
drivers/video/acornfb.c | 5 +++--
drivers/video/arkfb.c | 9 +++++----
drivers/video/atmel_lcdfb.c | 15 +++++++++------
drivers/video/aty/radeon_base.c | 10 +++++-----
drivers/video/da8xx-fb.c | 4 ++--
drivers/video/fb-puv3.c | 4 ++--
drivers/video/g364fb.c | 5 +++--
drivers/video/gxt4500.c | 4 ++--
drivers/video/hgafb.c | 4 ++--
drivers/video/imsttfb.c | 2 +-
drivers/video/intelfb/intelfbhw.c | 6 +++---
drivers/video/mb862xx/mb862xxfb.c | 2 +-
drivers/video/mx3fb.c | 6 +++---
drivers/video/neofb.c | 4 ++--
drivers/video/pm2fb.c | 4 ++--
drivers/video/pm3fb.c | 4 ++--
drivers/video/s3c-fb.c | 2 +-
drivers/video/s3fb.c | 7 ++++---
drivers/video/savage/savagefb_driver.c | 16 +++++++---------
drivers/video/sis/sis_main.c | 30 ++++++++++--------------------
drivers/video/sm501fb.c | 6 +++---
drivers/video/tridentfb.c | 4 ++--
drivers/video/vfb.c | 4 ++--
drivers/video/vga16fb.c | 2 +-
drivers/video/vt8500lcdfb.c | 4 ++--
drivers/video/vt8623fb.c | 9 +++++----
28 files changed, 92 insertions(+), 106 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Fabio Erculiani @ 2011-05-25 19:16 UTC (permalink / raw)
To: Bruno Prémont; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <20110525211253.4985ee79@neptune.home>
On Wed, May 25, 2011 at 9:12 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
>> On Wed, May 25, 2011 at 8:57 PM, Bruno Prémont wrote:
>> > On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
>> >> On Wed, May 25, 2011 at 8:46 PM, Bruno Prémont wrote:
>> >> > On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
>> >> >> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
>> >> >> It is causing deadlock during boot, so I would consider it quite critical.
>> >> >> Users using any fb driver will get into troubles.
>> >> >> The workaround is to boot with vga=normal.
>> >> >
>> >> > What is your system doing during boot? I've never seen it here but maybe
>> >> > my boot sequence is too simple.
>> >>
>> >> I'm using vesafb and vgay1. It is quite simple to reproduce.
>> >> Also see: http://bugs.gentoo.org/show_bug.cgi?id68109
>> >
>> > Looks like gentoo kernel, might be splash is related to the hang
>>
>> Then, if you say so, it must be the fbsplash patch for sure, I keep
>> forgetting of that :-/
>
> I've had a look at the bug report which points at fbcon_decore
> patch.
>
> Looking into that patch confirms my impression:
> fbcon_decor calls a userspace helper at the time fbcon takes over
> console and that userspace helper then tries to open fb device
> with the aim of calling some IOCTLs.
>
> Probably changing fbcon_decor to just call the userspace helper in
> non-blocking mode (or having userspace helper "fork and detach") would
> avoid the deadlock as well.
> Though fbcon_decor seems to rely on helper's return code...
>
> What is the matching piece on userspace side so I can look at it as well?
I guess you're talking about this:
http://fbsplash.berlios.de/wiki/doku.php
the package is named splashutils, and contains userspace helpers.
>
> Bruno
>
--
Fabio Erculiani
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between
From: Bruno Prémont @ 2011-05-25 19:12 UTC (permalink / raw)
To: Fabio Erculiani; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <BANLkTimtYvcN8kj+jnaKHNGmN36YvMt7Rg@mail.gmail.com>
On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> On Wed, May 25, 2011 at 8:57 PM, Bruno Prémont wrote:
> > On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> >> On Wed, May 25, 2011 at 8:46 PM, Bruno Prémont wrote:
> >> > On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> >> >> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
> >> >> It is causing deadlock during boot, so I would consider it quite critical.
> >> >> Users using any fb driver will get into troubles.
> >> >> The workaround is to boot with vga=normal.
> >> >
> >> > What is your system doing during boot? I've never seen it here but maybe
> >> > my boot sequence is too simple.
> >>
> >> I'm using vesafb and vgay1. It is quite simple to reproduce.
> >> Also see: http://bugs.gentoo.org/show_bug.cgi?id68109
> >
> > Looks like gentoo kernel, might be splash is related to the hang
>
> Then, if you say so, it must be the fbsplash patch for sure, I keep
> forgetting of that :-/
I've had a look at the bug report which points at fbcon_decore
patch.
Looking into that patch confirms my impression:
fbcon_decor calls a userspace helper at the time fbcon takes over
console and that userspace helper then tries to open fb device
with the aim of calling some IOCTLs.
Probably changing fbcon_decor to just call the userspace helper in
non-blocking mode (or having userspace helper "fork and detach") would
avoid the deadlock as well.
Though fbcon_decor seems to rely on helper's return code...
What is the matching piece on userspace side so I can look at it as well?
Bruno
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Fabio Erculiani @ 2011-05-25 19:04 UTC (permalink / raw)
To: Bruno Prémont; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <20110525205707.12abc68c@neptune.home>
On Wed, May 25, 2011 at 8:57 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
>> On Wed, May 25, 2011 at 8:46 PM, Bruno Prémont wrote:
>> > On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
>> >> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
>> >> It is causing deadlock during boot, so I would consider it quite critical.
>> >> Users using any fb driver will get into troubles.
>> >> The workaround is to boot with vga=normal.
>> >
>> > What is your system doing during boot? I've never seen it here but maybe
>> > my boot sequence is too simple.
>>
>> I'm using vesafb and vgay1. It is quite simple to reproduce.
>> Also see: http://bugs.gentoo.org/show_bug.cgi?id68109
>
> Looks like gentoo kernel, might be splash is related to the hang
Then, if you say so, it must be the fbsplash patch for sure, I keep
forgetting of that :-/
>
>> > Could you tell if it deadlocks before init gets started or afterwards,
>> > which fb drivers (and extra kernel patches if any) are in use.
>>
>> Exactly when register_framebuffer() is called, in my case, early in
>> the boot phase, before init.
>>
>> >
>> > If you have the complete backtrace of the deadlocked processes it would
>> > help getting a better idea of what is affected and how (and why just the
>> > framebuffer's lock is not causing trouble with earlier kernel versions).
>>
>> Because that code got a HUGE rewrite in the latest cycle, where
>> registration_lock has been introduce.
>> Just make a diff between 2.6.38 and 2.6.39. It will be easy to see
>> that SO MANY lines have changed ;-)
>> Anyway, since I'm out of office these days, I won't be able to send
>> you the traceback this week, but since so many people have run into
>> it, I guess it's fairly simple to reproduce.
>
> Rewrite was not so huge, and I'm not affected here (running with KMS
> compiled in though and vanilla kernel).
> Probably the cause is related to one of the extra patches provided by
> Gentoo and subscribing to framebuffer registration notifications.
>
> Will have a look.
Thanks a lot, let me know if I can help out next week.
>
> Bruno
>
--
Fabio Erculiani
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between
From: Bruno Prémont @ 2011-05-25 18:57 UTC (permalink / raw)
To: Fabio Erculiani; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <BANLkTi=207-bVi3pnOWk7zG61u4=YU4GFg@mail.gmail.com>
On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> On Wed, May 25, 2011 at 8:46 PM, Bruno Prémont wrote:
> > On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> >> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
> >> It is causing deadlock during boot, so I would consider it quite critical.
> >> Users using any fb driver will get into troubles.
> >> The workaround is to boot with vga=normal.
> >
> > What is your system doing during boot? I've never seen it here but maybe
> > my boot sequence is too simple.
>
> I'm using vesafb and vgay1. It is quite simple to reproduce.
> Also see: http://bugs.gentoo.org/show_bug.cgi?id68109
Looks like gentoo kernel, might be splash is related to the hang
> > Could you tell if it deadlocks before init gets started or afterwards,
> > which fb drivers (and extra kernel patches if any) are in use.
>
> Exactly when register_framebuffer() is called, in my case, early in
> the boot phase, before init.
>
> >
> > If you have the complete backtrace of the deadlocked processes it would
> > help getting a better idea of what is affected and how (and why just the
> > framebuffer's lock is not causing trouble with earlier kernel versions).
>
> Because that code got a HUGE rewrite in the latest cycle, where
> registration_lock has been introduce.
> Just make a diff between 2.6.38 and 2.6.39. It will be easy to see
> that SO MANY lines have changed ;-)
> Anyway, since I'm out of office these days, I won't be able to send
> you the traceback this week, but since so many people have run into
> it, I guess it's fairly simple to reproduce.
Rewrite was not so huge, and I'm not affected here (running with KMS
compiled in though and vanilla kernel).
Probably the cause is related to one of the extra patches provided by
Gentoo and subscribing to framebuffer registration notifications.
Will have a look.
Bruno
^ permalink raw reply
* Re: efifb not detected on Intel DQ67SW
From: Andrew Lutomirski @ 2011-05-25 18:54 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <BANLkTikxX577VKmPRVGcp_Chj-udCf-KaQ@mail.gmail.com>
On Wed, May 25, 2011 at 2:32 PM, Matthew Garrett <mjg@redhat.com> wrote:
> On Wed, May 25, 2011 at 02:21:17PM -0400, Andrew Lutomirski wrote:
>
>> Looking at the code, I'm a little confused how it's supposed to work.
>> AFAICT, unless there's a DMI match, then the driver will only load
>> ifthe boot code sets VIDEO_TYPE_EFI, but nothing sets that.
>
> grub should be setting that.
It looks like grub-fedora (the git version, anyway) has a function
set_kernel_params that tries to do this. Peter, the git tree claims
that you maintain it. Are there any experiments you'd like me to do?
(Dumping boot_params, perhaps?)
--Andy
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Fabio Erculiani @ 2011-05-25 18:53 UTC (permalink / raw)
To: Bruno Prémont; +Cc: Anca Emanuel, linux-fbdev, lethal, linux-kernel
In-Reply-To: <20110525205105.03e2d911@neptune.home>
On Wed, May 25, 2011 at 8:51 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
>
> Your patch was against 2.6.39 or shortly before as Linus's code
> did introduce the do_ variants of register/unregister/conflict functions.
>
> Bruno
>
I made that patch against the 2.6.39 tag.
Git ain't lying ;-)
--
Fabio Erculiani
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Fabio Erculiani @ 2011-05-25 18:52 UTC (permalink / raw)
To: Bruno Prémont; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <20110525204648.58983270@neptune.home>
On Wed, May 25, 2011 at 8:46 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
>> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
>> It is causing deadlock during boot, so I would consider it quite critical.
>> Users using any fb driver will get into troubles.
>> The workaround is to boot with vga=normal.
>
> What is your system doing during boot? I've never seen it here but maybe
> my boot sequence is too simple.
I'm using vesafb and vgay1. It is quite simple to reproduce.
Also see: http://bugs.gentoo.org/show_bug.cgi?id68109
>
> Could you tell if it deadlocks before init gets started or afterwards,
> which fb drivers (and extra kernel patches if any) are in use.
Exactly when register_framebuffer() is called, in my case, early in
the boot phase, before init.
>
> If you have the complete backtrace of the deadlocked processes it would
> help getting a better idea of what is affected and how (and why just the
> framebuffer's lock is not causing trouble with earlier kernel versions).
Because that code got a HUGE rewrite in the latest cycle, where
registration_lock has been introduce.
Just make a diff between 2.6.38 and 2.6.39. It will be easy to see
that SO MANY lines have changed ;-)
Anyway, since I'm out of office these days, I won't be able to send
you the traceback this week, but since so many people have run into
it, I guess it's fairly simple to reproduce.
>
> Bruno
>
>
>> Cheers,
>
--
Fabio Erculiani
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between
From: Bruno Prémont @ 2011-05-25 18:51 UTC (permalink / raw)
To: Fabio Erculiani; +Cc: Anca Emanuel, linux-fbdev, lethal, linux-kernel
In-Reply-To: <BANLkTikk2BbOWHQN+UPWD71QoM6g-QpHZg@mail.gmail.com>
On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> On Wed, May 25, 2011 at 8:41 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
> >
> > Did you test 2.6.39 ? The fix from Linus is not ok ?
> >
>
> Sorry, maybe I missed it?
Your patch was against 2.6.39 or shortly before as Linus's code
did introduce the do_ variants of register/unregister/conflict functions.
Bruno
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Fabio Erculiani @ 2011-05-25 18:48 UTC (permalink / raw)
To: Anca Emanuel; +Cc: Bruno Prémont, linux-fbdev, lethal, linux-kernel
In-Reply-To: <BANLkTi=Q5EzN+wP9jDNCxCnZBQ0XHOmOhQ@mail.gmail.com>
On Wed, May 25, 2011 at 8:41 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>
> Did you test 2.6.39 ? The fix from Linus is not ok ?
>
Sorry, maybe I missed it?
--
Fabio Erculiani
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between
From: Bruno Prémont @ 2011-05-25 18:46 UTC (permalink / raw)
To: Fabio Erculiani; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <BANLkTimxi-LZFzwv=z6RRBCX098hr-ViUg@mail.gmail.com>
On Wed, 25 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
> It is causing deadlock during boot, so I would consider it quite critical.
> Users using any fb driver will get into troubles.
> The workaround is to boot with vga=normal.
What is your system doing during boot? I've never seen it here but maybe
my boot sequence is too simple.
Could you tell if it deadlocks before init gets started or afterwards,
which fb drivers (and extra kernel patches if any) are in use.
If you have the complete backtrace of the deadlocked processes it would
help getting a better idea of what is affected and how (and why just the
framebuffer's lock is not causing trouble with earlier kernel versions).
Bruno
> Cheers,
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Anca Emanuel @ 2011-05-25 18:41 UTC (permalink / raw)
To: Fabio Erculiani; +Cc: Bruno Prémont, linux-fbdev, lethal, linux-kernel
In-Reply-To: <BANLkTimxi-LZFzwv=z6RRBCX098hr-ViUg@mail.gmail.com>
On Wed, May 25, 2011 at 9:17 PM, Fabio Erculiani <lxnay@sabayon.org> wrote:
> I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
> It is causing deadlock during boot, so I would consider it quite critical.
> Users using any fb driver will get into troubles.
> The workaround is to boot with vga=normal.
Did you test 2.6.39 ? The fix from Linus is not ok ?
^ permalink raw reply
* Re: efifb not detected on Intel DQ67SW
From: Matthew Garrett @ 2011-05-25 18:32 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <BANLkTikxX577VKmPRVGcp_Chj-udCf-KaQ@mail.gmail.com>
On Wed, May 25, 2011 at 02:21:17PM -0400, Andrew Lutomirski wrote:
> Looking at the code, I'm a little confused how it's supposed to work.
> AFAICT, unless there's a DMI match, then the driver will only load
> ifthe boot code sets VIDEO_TYPE_EFI, but nothing sets that.
grub should be setting that.
> Should there be something that looks at
> EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE to find the frame buffer? (Does
> that not work on Apple machines?)
By the time we get into the kernel we've called ExitBootServices(), so
at that point we can't do anything with GOP. grub should have grabbed
that information and stashed it in the appropriate field.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* efifb not detected on Intel DQ67SW
From: Andrew Lutomirski @ 2011-05-25 18:21 UTC (permalink / raw)
To: linux-fbdev
Hi-
For mostly masochistic reasons, I boot my DQ67SW box in UEFI mode.
efifb doesn't detect the anything, so I have no console until i915
loads.
Looking at the code, I'm a little confused how it's supposed to work.
AFAICT, unless there's a DMI match, then the driver will only load
ifthe boot code sets VIDEO_TYPE_EFI, but nothing sets that.
Should there be something that looks at
EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE to find the frame buffer? (Does
that not work on Apple machines?)
--Andy
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between register_framebuffer()
From: Fabio Erculiani @ 2011-05-25 18:17 UTC (permalink / raw)
To: Bruno Prémont; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <20110525181917.12cf97b8@neptune.home>
I'm not a fbdev expert. So I leave the real fix to real men ( ;-) ).
It is causing deadlock during boot, so I would consider it quite critical.
Users using any fb driver will get into troubles.
The workaround is to boot with vga=normal.
Cheers,
--
Fabio Erculiani
^ permalink raw reply
* Re: [PATCH] fbmem: fix race condition between
From: Bruno Prémont @ 2011-05-25 16:19 UTC (permalink / raw)
To: Fabio Erculiani; +Cc: linux-fbdev, lethal, linux-kernel
In-Reply-To: <20110524224545.08c53b1d@neptune.home>
On Tue, 24 May 2011 Bruno Prémont <bonbons@linux-vserver.org> wrote:
> On Tue, 24 May 2011 Fabio Erculiani <lxnay@sabayon.org> wrote:
> > register_framebuffer() acquires registration_lock, then calls
> > do_register_framebuffer() which calls fb_notifier_call_chain()
> > which calls fb_open() which calls get_fb_info() which tries
> > (at last) to acquire registration_lock again.
> > This is a workaround.
>
> What calls fb_open() instead of just calling fb_info->fbops->fb_open
> when it is set?
> In other words what kernel handler of fb_notifier chain does file-based
> open of the framebuffer or is the event pushed out synchronously to
> userspace?
Note that fb_info->lock is also taken on both sides:
- per definition around all fb_notifier_call_chain() calls
- as well in fb_open() to protect fb_info->fb_ops->fb_open call.
Bruno
> > Signed-off-by: Fabio Erculiani <lxnay@sabayon.org>
> > ---
> > drivers/video/fbmem.c | 13 ++++++++++++-
> > 1 files changed, 12 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 5aac00e..b9831a0 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1628,6 +1628,11 @@ static int do_register_framebuffer(struct fb_info *fb_info)
> > event.info = fb_info;
> > if (!lock_fb_info(fb_info))
> > return -ENODEV;
> > +
> > + /* FIXME: unlock registration mutex before registration
> > + * notification is sent, in order to avoid deadlock.
> > + */
> > + mutex_unlock(®istration_lock);
> > fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
> > unlock_fb_info(fb_info);
> > return 0;
> > @@ -1692,7 +1697,13 @@ register_framebuffer(struct fb_info *fb_info)
> >
> > mutex_lock(®istration_lock);
> > ret = do_register_framebuffer(fb_info);
> > - mutex_unlock(®istration_lock);
> > + if (ret != 0)
> > + /*
> > + * FIXME: mutex is unlocked only if ret = 0.
> > + * This is the second part of the workaround
> > + * that prevents deadlocking.
> > + */
> > + mutex_unlock(®istration_lock);
> >
> > return ret;
> > }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox