Re: Oops in cgsixfb.c, SparcStation 20/SMP [2.2.12] (fixed, patch)

Chris Noe (stiker@northlink.com)
Fri, 24 Sep 1999 23:08:18 -0400 (EDT)


I spent the night hacking out this patch which now keeps my ss20 up in
even in the face of 'make config', multiple console switches, etc.

Tis my first jaunder with spinlocks though, so it could very well be
wrong. Hopefully I've done it somewhat right, but if not, please apply
ASAP because this bug will trigger an oops *very* easily if not fixed.

Question: I used the irqsave/restore versions of the spinlocks to err on
the side of caution. How can I tell in which sections I can use the
non-irq versions? And the only reason to do that anyway would be speed,
correct? From my reading of spinlocks.txt, it's "only when interrupts are
disabled" to avoid recursing on the same lock. How can I tell if
interrupts are disabled when someone enters the cg6 fb code?

The fb seems to update plenty fast at the moment so I didn't want to take
any chance at leaving a window of opportunity open for a memory stomping.

Also, I wondered about this: the newer kernel (with this patch) appears a
good 5k smaller than the original. Why did _adding_ a few calls shrink my
kernel? I love the idea, but it makes me wonder if I didn't just diff away
half of the tree or something stupid :) I'm sure a good look at spinlock.h
will tell the tale (I'll go to it now).

Chris Noe
(stiker@northlink.com)

--- linux/include/video/sbusfb.h.orig Sat Sep 25 09:43:16 1999
+++ linux/include/video/sbusfb.h Sat Sep 25 10:48:27 1999
@@ -1,7 +1,9 @@
#include <linux/timer.h>
+
#include <asm/sbus.h>
#include <asm/oplib.h>
#include <asm/fbio.h>
+#include <asm/spinlock.h>

#include <video/fbcon.h>

@@ -25,6 +27,7 @@
struct fb_info_cgsix {
struct bt_regs *bt;
struct cg6_fbc *fbc;
+ spinlock_t fbc_lock; /* do we need to lock the thc/tec as well? */
struct cg6_thc *thc;
struct cg6_tec *tec;
volatile u32 *fhc;
--- linux/drivers/video/cgsixfb.c.orig Sat May 29 11:10:15 1999
+++ linux/drivers/video/cgsixfb.c Sat Sep 25 10:35:29 1999
@@ -1,6 +1,8 @@
/* $Id: cgsixfb.c,v 1.16.2.1 1999/05/25 00:59:35 davem Exp $
* cgsixfb.c: CGsix (GX,GXplus) frame buffer driver
*
+ * 9/24/99: SMP locking issues semi-fixed, Chris Noe <stiker@northlink.com>
+ *
* Copyright (C) 1996,1998 Jakub Jelinek (jj@ultra.linux.cz)
* Copyright (C) 1996 Miguel de Icaza (miguel@nuclecu.unam.mx)
* Copyright (C) 1996 Eddie C. Dost (ecd@skynet.be)
@@ -231,9 +233,11 @@
{
struct fb_info_sbusfb *fb = (struct fb_info_sbusfb *)p->fb_info;
register struct cg6_fbc *fbc = fb->s.cg6.fbc;
+ unsigned long flags;
int x, y, w, h;
int i;

+ spin_lock_irqsave(&fb->s.cg6.fbc_lock, flags);
do {
i = fbc->s;
} while (i & 0x10000000);
@@ -262,14 +266,17 @@
do {
i = fbc->draw;
} while (i < 0 && (i & 0x20000000));
+ spin_unlock_irqrestore(&fb->s.cg6.fbc_lock, flags);
}

static void cg6_fill(struct fb_info_sbusfb *fb, struct display *p, int s,
int count, unsigned short *boxes)
{
int i;
+ unsigned long flags;
register struct cg6_fbc *fbc = fb->s.cg6.fbc;

+ spin_lock_irqsave(&fb->s.cg6.fbc_lock, flags);
do {
i = fbc->s;
} while (i & 0x10000000);
@@ -290,12 +297,14 @@
i = fbc->draw;
} while (i < 0 && (i & 0x20000000));
}
+ spin_unlock_irqrestore(&fb->s.cg6.fbc_lock, flags);
}

static void cg6_putc(struct vc_data *conp, struct display *p, int c, int yy, int xx)
{
struct fb_info_sbusfb *fb = (struct fb_info_sbusfb *)p->fb_info;
register struct cg6_fbc *fbc = fb->s.cg6.fbc;
+ unsigned long flags;
int i, x, y;
u8 *fd;

@@ -314,6 +323,8 @@
x = fb->x_margin + (xx << fontwidthlog(p));
else
x = fb->x_margin + (xx * fontwidth(p));
+
+ spin_lock_irqsave(&fb->s.cg6.fbc_lock, flags);
do {
i = fbc->s;
} while (i & 0x10000000);
@@ -339,6 +350,7 @@
fd += 2;
}
}
+ spin_unlock_irqrestore(&fb->s.cg6.fbc_lock, flags);
}

static void cg6_putcs(struct vc_data *conp, struct display *p, const unsigned short *s,
@@ -346,9 +358,11 @@
{
struct fb_info_sbusfb *fb = (struct fb_info_sbusfb *)p->fb_info;
register struct cg6_fbc *fbc = fb->s.cg6.fbc;
+ unsigned long flags;
int i, x, y;
u8 *fd1, *fd2, *fd3, *fd4;

+ spin_lock_irqsave(&fb->s.cg6.fbc_lock, flags);
do {
i = fbc->s;
} while (i & 0x10000000);
@@ -443,6 +457,7 @@
}
}
}
+ spin_unlock_irqrestore(&fb->s.cg6.fbc_lock, flags);
}

static void cg6_revc(struct display *p, int xx, int yy)
@@ -535,6 +550,7 @@
struct cg6_tec *tec = fb->s.cg6.tec;
struct cg6_fbc *fbc = fb->s.cg6.fbc;
u32 mode;
+ unsigned long flags;
int i;

/* Turn off stuff in the Transform Engine. */
@@ -557,6 +573,7 @@
/* Set things in the FBC. Bad things appear to happen if we do
* back to back store/loads on the mode register, so copy it
* out instead. */
+ spin_lock_irqsave(&fb->s.cg6.fbc_lock, flags);
mode = fbc->mode;
do {
i = fbc->s;
@@ -578,6 +595,8 @@
fbc->clipminy = 0;
fbc->clipmaxx = fb->type.fb_width - 1;
fbc->clipmaxy = fb->type.fb_height - 1;
+ spin_unlock_irqrestore(&fb->s.cg6.fbc_lock, flags);
+
/* Enable cursor in Brooktree DAC. */
fb->s.cg6.bt->addr = 0x06 << 24;
fb->s.cg6.bt->control |= 0x03 << 24;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/