VGA console endian bug
Geert Uytterhoeven
geert at linux-m68k.org
Sat Aug 11 18:33:46 EST 2001
On Fri, 10 Aug 2001, Hollis Blanchard wrote:
> On Fri, Aug 10, 2001 at 10:41:39AM +0200, Geert Uytterhoeven wrote:
> > On Thu, 9 Aug 2001, Hollis Blanchard wrote:
> > > These two patches came from Daniel Berlin on July 9. They completely resolve
> > > the VGA console backwards-endian problem for me on PPC. If they have Geert's
> > > seal of approval ;) can they be committed?
> > >
> > > -Hollis
> > >
> > > --- linuxppc_2_4_devel/include/asm-ppc/vga.h.old Thu Aug 9 11:49:23 2001
> > > +++ linuxppc_2_4_devel/include/asm-ppc/vga.h Thu Aug 9 11:49:42 2001
> > > @@ -37,6 +37,9 @@
> > >
> > > #define VT_BUF_HAVE_MEMCPYW
> > > #define scr_memcpyw memcpy
> > > +#define VT_BUF_HAVE_MEMCPYF
> > > +#define scr_memcpyw_to memcpy
> > > +#define scr_memcpyw_from memcpy
> > >
> > > #endif /* !CONFIG_VGA_CONSOLE && !CONFIG_MDA_CONSOLE */
> >
> > The first I cannot approve: if scr_{write,read}w() swap bytes, how can it work
> > if scr_memcpyw_{to,from}() don't swap bytes?
>
> I have no idea. Perhaps those functions weren't used, and that's why it
> appeared to work. I have not tried with only the second patch; perhaps that
> will be enough...
>
> > And where's the 3rd patch? I once posted a patch to fix a similar bug in
> > drivers/char/console.c.
>
> I can't find it in the linuxppc-dev archives. :(
It indeed wasn't sent to that list. I attached the mail I sent to Ben H, which
includes the mail I sent before.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-------------- next part --------------
From geert at linux-m68k.org Sat Aug 11 10:31:13 2001
Date: Mon, 9 Jul 2001 18:45:14 +0200 (CEST)
From: Geert Uytterhoeven <geert at linux-m68k.org>
To: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Geert Uytterhoeven <geert at linux-m68k.org>
Subject: Re: Fwd: Re: gpm selection vs VGA console & fbcon
Hi Ben,
> Geert, does that patch sounds the right thing to you ? If yes,
> I'll push it all over the bk's and to Linus, I'm tired of
> getting this bug reported ;)
> *** vga.h Wed Jul 4 10:58:40 2001
> --- /root/linux/include/asm-ppc/vga.h Thu Jun 28 14:22:02 2001
> *************** extern inline u16 scr_readw(volatile con
> *** 37,42 ****
> --- 37,45 ----
>
> #define VT_BUF_HAVE_MEMCPYW
> #define scr_memcpyw memcpy
> + #define VT_BUF_HAVE_MEMCPYF
> + #define scr_memcpyw_to memcpy
> + #define scr_memcpyw_from memcpy
>
> #endif /* !CONFIG_VGA_CONSOLE && !CONFIG_MDA_CONSOLE */
I fail to see how the first part can solve a problem: scr_memcpyw_{to,from}
copy between the VGA text buffer on the graphics card (little endian) and
native endianness, so setting them to memcpy() cannot be right. I suspect
there's another bug somewhere else...
Note that I've been having a fight with Olaf Hering about this as well :-)
BTW, the comment after the #endif is wrong, it should be
#endif /* CONFIG_VGA_CONSOLE || CONFIG_MDA_CONSOLE */
instead of
#endif /* !CONFIG_VGA_CONSOLE && !CONFIG_MDA_CONSOLE */
> *** fbcon.c Tue Jul 3 17:58:41 2001
> --- /root/linux/drivers/video/fbcon.c Wed Jun 27 14:52:59 2001
> *************** static void fbcon_invert_region(struct v
> *** 1954,1966 ****
> if (!conp->vc_can_do_color)
> *p++ ^= 0x0800;
> else if (conp->vc_hi_font_mask == 0x100) {
> ! u16 a = *p;
> a = ((a) & 0x11ff) | (((a) & 0xe000) >> 4) | (((a) & 0x0e00) << 4);
> ! *p++ = a;
> } else {
> ! u16 a = *p;
> a = ((a) & 0x88ff) | (((a) & 0x7000) >> 4) | (((a) & 0x0700) << 4);
> ! *p++ = a;
> }
> if (p == (u16 *)softback_end)
> p = (u16 *)softback_buf;
> --- 1954,1966 ----
> if (!conp->vc_can_do_color)
> *p++ ^= 0x0800;
> else if (conp->vc_hi_font_mask == 0x100) {
> ! u16 a = scr_readw(p);
> a = ((a) & 0x11ff) | (((a) & 0xe000) >> 4) | (((a) & 0x0e00) << 4);
> ! scr_writew(a, p++);
> } else {
> ! u16 a = scr_readw(p);
> a = ((a) & 0x88ff) | (((a) & 0x7000) >> 4) | (((a) & 0x0700) << 4);
> ! scr_writew(a, p++);
> }
> if (p == (u16 *)softback_end)
> p = (u16 *)softback_buf;
The second part is OK for me.
There's a similar bug in console.c: always use scr_{read,write}w() when
accessing the text/attribute buffer. See the patch (still untested by me) in
the mail below.
--snip--------------------------------------------------------------------------
>From geert at linux-m68k.org Mon Jul 9 18:38:17 2001
Date: Sat, 2 Jun 2001 21:23:32 +0200 (CEST)
From: Geert Uytterhoeven <geert at linux-m68k.org>
To: Daniel Berlin <dan at cgsoftware.com>
Cc: Linux Frame Buffer Device Development
<linux-fbdev-devel at lists.sourceforge.net>,
Alan Cox <alan at lxorguk.ukuu.org.uk>, Olaf Hering <olh at suse.de>
Subject: Re: FBCon invert_region is endian unsafe,
same with generic invert_region
Hi Daniel,
> I'm not quite sure who the maintainer is, fbcon.c lists 5 people, and
> no one is listed in the MAINTAINERS file (i searched on fbcon).
>
> This one has been bothering me for a long time.
> It causes text selection to fail on fbcon devices for big endian
> platforms (Because, obviously, it gets big endian data on big endian
> machines, sinceit neglects to use scr_readw/scr_writew. Dumb. You end
> up inverting, pasting each byte reversed without fixing this, so where
> you would normally have pasted/see a background inverted 'u' (0x75),
> you get a 'W' (0x57)).
[...]
> However, it's important to note that console.c has the same damn bug
> in it's generic inversion routine.
>
> Changing the code to the right way makes text selection work perfectly
> again, which it hasn't since 2.2.x, at least for me.
So this patch (untested) fixes it?
Olaf: is this the missing piece for you as well?
--- console-2.4.5/drivers/char/console.c.orig Mon Feb 19 10:36:47 2001
+++ console-2.4.5/drivers/char/console.c Sat Jun 2 21:15:10 2001
@@ -390,20 +390,28 @@
else {
u16 *q = p;
int cnt = count;
+ u16 a;
if (!can_do_color) {
- while (cnt--) *q++ ^= 0x0800;
+ while (cnt--) {
+ a = scr_readw(q);
+ a ^= 0x0800;
+ scr_writew(a, q);
+ q++;
+ }
} else if (hi_font_mask == 0x100) {
while (cnt--) {
- u16 a = *q;
+ a = scr_readw(q);
a = ((a) & 0x11ff) | (((a) & 0xe000) >> 4) | (((a) & 0x0e00) << 4);
- *q++ = a;
+ scr_writew(a, q);
+ q++;
}
} else {
while (cnt--) {
- u16 a = *q;
+ a = scr_readw(q);
a = ((a) & 0x88ff) | (((a) & 0x7000) >> 4) | (((a) & 0x0700) << 4);
- *q++ = a;
+ scr_writew(a, q);
+ q++
}
}
}
--- console-2.4.5/drivers/video/fbcon.c.orig Mon Feb 19 10:37:00 2001
+++ console-2.4.5/drivers/video/fbcon.c Sat Jun 2 21:07:38 2001
@@ -1934,17 +1934,15 @@
static void fbcon_invert_region(struct vc_data *conp, u16 *p, int cnt)
{
while (cnt--) {
+ u16 a = scr_readw(p);
if (!conp->vc_can_do_color)
- *p++ ^= 0x0800;
- else if (conp->vc_hi_font_mask == 0x100) {
- u16 a = *p;
+ a ^= 0x0800;
+ else if (conp->vc_hi_font_mask == 0x100)
a = ((a) & 0x11ff) | (((a) & 0xe000) >> 4) | (((a) & 0x0e00) << 4);
- *p++ = a;
- } else {
- u16 a = *p;
+ else
a = ((a) & 0x88ff) | (((a) & 0x7000) >> 4) | (((a) & 0x0700) << 4);
- *p++ = a;
- }
+ scr_writew(a, p);
+ p++;
if (p == (u16 *)softback_end)
p = (u16 *)softback_buf;
if (p == (u16 *)softback_in)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--snip--------------------------------------------------------------------------
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
More information about the Linuxppc-dev
mailing list