* [PATCH] fix endian problem in color_imageblit
@ 2002-12-12 3:41 Paul Mackerras
2002-12-15 1:05 ` [Linux-fbdev-devel] " Antonino Daplas
0 siblings, 1 reply; 8+ messages in thread
From: Paul Mackerras @ 2002-12-12 3:41 UTC (permalink / raw
To: James Simmons; +Cc: Linux Kernel Mailing List, Linux Fbdev development list
This patch fixes the endian problems in color_imageblit(). With this
patch, I get the penguin drawn properly on boot.
The main change is that on big-endian systems, when we load a pixel
from the source, we now shift it to the left-hand (most significant)
end of the word. With this change the rest of the logic is correct on
big-endian systems. This may not be the most efficient way to do
things but it is a simple change that works and avoids disturbing the
rest of the code.
Paul.
diff -urN linux-2.5/drivers/video/cfbimgblt.c pmac-2.5/drivers/video/cfbimgblt.c
--- linux-2.5/drivers/video/cfbimgblt.c 2002-12-10 15:20:32.000000000 +1100
+++ pmac-2.5/drivers/video/cfbimgblt.c 2002-12-12 09:14:47.000000000 +1100
@@ -103,10 +103,10 @@
{
/* Draw the penguin */
int i, n;
- unsigned long bitmask = SHIFT_LOW(~0UL, BITS_PER_LONG - p->var.bits_per_pixel);
+ int bpp = p->var.bits_per_pixel;
unsigned long *palette = (unsigned long *) p->pseudo_palette;
unsigned long *dst, *dst2, color = 0, val, shift;
- unsigned long null_bits = BITS_PER_LONG - p->var.bits_per_pixel;
+ unsigned long null_bits = BITS_PER_LONG - bpp;
u8 *src = image->data;
dst2 = (unsigned long *) dst1;
@@ -125,9 +125,10 @@
while (n--) {
if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
p->fix.visual == FB_VISUAL_DIRECTCOLOR )
- color = palette[*src] & bitmask;
+ color = palette[*src];
else
- color = *src & bitmask;
+ color = *src;
+ color <<= LEFT_POS(bpp);
val |= SHIFT_HIGH(color, shift);
if (shift >= null_bits) {
FB_WRITEL(val, dst++);
@@ -136,7 +137,7 @@
else
val = SHIFT_LOW(color, BITS_PER_LONG - shift);
}
- shift += p->var.bits_per_pixel;
+ shift += bpp;
shift &= (BITS_PER_LONG - 1);
src++;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH] fix endian problem in color_imageblit
2002-12-12 3:41 [PATCH] fix endian problem in color_imageblit Paul Mackerras
@ 2002-12-15 1:05 ` Antonino Daplas
2002-12-20 19:54 ` James Simmons
0 siblings, 1 reply; 8+ messages in thread
From: Antonino Daplas @ 2002-12-15 1:05 UTC (permalink / raw
To: Paul Mackerras
Cc: James Simmons, Linux Kernel Mailing List,
Linux Fbdev development list
On Thu, 2002-12-12 at 08:41, Paul Mackerras wrote:
> This patch fixes the endian problems in color_imageblit(). With this
> patch, I get the penguin drawn properly on boot.
>
> The main change is that on big-endian systems, when we load a pixel
> from the source, we now shift it to the left-hand (most significant)
> end of the word. With this change the rest of the logic is correct on
> big-endian systems. This may not be the most efficient way to do
> things but it is a simple change that works and avoids disturbing the
> rest of the code.
>
Nice catch :-) We also need a similar fix for slow_imageblit(), so
James can you apply the attached patch also:
Also, I noticed that some drivers load the pseudo_palette with entries
whose length matches the length of the pixel. The cfb_* functions
always assume that each pseudo_palette entry is an "unsigned long", so
bpp16 will segfault, and so will bpp24/32 for 64-bit machines.
Tony
diff -Naur linux-2.5.51/drivers/video/cfbimgblt.c linux/drivers/video/cfbimgblt.c
--- linux-2.5.51/drivers/video/cfbimgblt.c 2002-12-15 00:54:04.000000000 +0000
+++ linux/drivers/video/cfbimgblt.c 2002-12-15 00:54:21.000000000 +0000
@@ -189,6 +189,7 @@
color = fgcolor;
else
color = bgcolor;
+ color <<= LEFT_POS(bpp);
val |= SHIFT_HIGH(color, shift);
/* Did the bitshift spill bits to the next long? */
Tony
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH] fix endian problem in color_imageblit
2002-12-15 1:05 ` [Linux-fbdev-devel] " Antonino Daplas
@ 2002-12-20 19:54 ` James Simmons
2002-12-21 3:45 ` Antonino Daplas
0 siblings, 1 reply; 8+ messages in thread
From: James Simmons @ 2002-12-20 19:54 UTC (permalink / raw
To: Antonino Daplas
Cc: Paul Mackerras, Linux Kernel Mailing List,
Linux Fbdev development list
> Nice catch :-) We also need a similar fix for slow_imageblit(), so
> James can you apply the attached patch also:
Applied.
> Also, I noticed that some drivers load the pseudo_palette with entries
> whose length matches the length of the pixel. The cfb_* functions
> always assume that each pseudo_palette entry is an "unsigned long", so
> bpp16 will segfault, and so will bpp24/32 for 64-bit machines.
I just noticed that as well. Russell King pointed to it too. I fixed
the unsigned long problem in color_imageblit. All the pseudo_palette
in cfb_* are assumed u32. Is this safe for 16bpp or 8 bpp modes? I will
have test to see. The u16 problem might explain why some people have
trouble with the VESA framebuffer. To all the people having trouble
booting to certain modes for the VESA fraembuffer can you try this patch.
Tell me if it fixes your problems.
--- /usr/src/linus-2.5/drivers/video/cfbimgblt.c Mon Dec 9 22:23:32 2002
+++ cfbimgblt.c Fri Dec 20 12:35:25 2002
@@ -102,11 +102,10 @@
unsigned long start_index, unsigned long pitch_index)
{
/* Draw the penguin */
- int i, n;
- unsigned long bitmask = SHIFT_LOW(~0UL, BITS_PER_LONG - p->var.bits_per_pixel);
- unsigned long *palette = (unsigned long *) p->pseudo_palette;
unsigned long *dst, *dst2, color = 0, val, shift;
- unsigned long null_bits = BITS_PER_LONG - p->var.bits_per_pixel;
+ int i, n, bpp = p->var.bits_per_pixel;
+ unsigned long null_bits = BITS_PER_LONG - bpp;
+ u32 *palette = (u32 *) p->pseudo_palette;
u8 *src = image->data;
dst2 = (unsigned long *) dst1;
@@ -125,9 +124,10 @@
while (n--) {
if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
p->fix.visual == FB_VISUAL_DIRECTCOLOR )
- color = palette[*src] & bitmask;
+ color = palette[*src];
else
- color = *src & bitmask;
+ color = *src;
+ color <<= LEFT_POS(bpp);
val |= SHIFT_HIGH(color, shift);
if (shift >= null_bits) {
FB_WRITEL(val, dst++);
@@ -136,7 +136,7 @@
else
val = SHIFT_LOW(color, BITS_PER_LONG - shift);
}
- shift += p->var.bits_per_pixel;
+ shift += bpp;
shift &= (BITS_PER_LONG - 1);
src++;
}
@@ -188,6 +188,7 @@
color = fgcolor;
else
color = bgcolor;
+ color <<= LEFT_POS(bpp);
val |= SHIFT_HIGH(color, shift);
/* Did the bitshift spill bits to the next long? */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH] fix endian problem in color_imageblit
2002-12-20 19:54 ` James Simmons
@ 2002-12-21 3:45 ` Antonino Daplas
0 siblings, 0 replies; 8+ messages in thread
From: Antonino Daplas @ 2002-12-21 3:45 UTC (permalink / raw
To: James Simmons
Cc: Paul Mackerras, Linux Kernel Mailing List,
Linux Fbdev development list
On Sat, 2002-12-21 at 03:54, James Simmons wrote:
>
> > Nice catch :-) We also need a similar fix for slow_imageblit(), so
> > James can you apply the attached patch also:
>
> Applied.
>
> > Also, I noticed that some drivers load the pseudo_palette with entries
> > whose length matches the length of the pixel. The cfb_* functions
> > always assume that each pseudo_palette entry is an "unsigned long", so
> > bpp16 will segfault, and so will bpp24/32 for 64-bit machines.
>
> I just noticed that as well. Russell King pointed to it too. I fixed
> the unsigned long problem in color_imageblit. All the pseudo_palette
> in cfb_* are assumed u32. Is this safe for 16bpp or 8 bpp modes? I will
The pseudopalette will only matter for truecolor and directcolor as the
rest of the visuals bypasses the pseudopalette. Typecasting to
"unsigned long" rather than "u32" is also safer (even for bpp16) since
in 64-bit machines, the drawing functions use fb_writeq instead of
fb_writel. So, all drivers using the cfb_* functions should change from
"u32" to "unsigned long" _whatever_ the bit depth when loading the
pseudopalette.
Of course, drivers with their own drawing functions can use whatever
they want.
Tony
PS: cfb_fillrect is still limited to u32 though which can hopefully be
fixed in the future.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fix endian problem in color_imageblit
@ 2002-12-21 3:45 ` Antonino Daplas
0 siblings, 0 replies; 8+ messages in thread
From: Antonino Daplas @ 2002-12-21 3:45 UTC (permalink / raw
To: James Simmons
Cc: Paul Mackerras, Linux Kernel Mailing List,
Linux Fbdev development list
On Sat, 2002-12-21 at 03:54, James Simmons wrote:
>
> > Nice catch :-) We also need a similar fix for slow_imageblit(), so
> > James can you apply the attached patch also:
>
> Applied.
>
> > Also, I noticed that some drivers load the pseudo_palette with entries
> > whose length matches the length of the pixel. The cfb_* functions
> > always assume that each pseudo_palette entry is an "unsigned long", so
> > bpp16 will segfault, and so will bpp24/32 for 64-bit machines.
>
> I just noticed that as well. Russell King pointed to it too. I fixed
> the unsigned long problem in color_imageblit. All the pseudo_palette
> in cfb_* are assumed u32. Is this safe for 16bpp or 8 bpp modes? I will
The pseudopalette will only matter for truecolor and directcolor as the
rest of the visuals bypasses the pseudopalette. Typecasting to
"unsigned long" rather than "u32" is also safer (even for bpp16) since
in 64-bit machines, the drawing functions use fb_writeq instead of
fb_writel. So, all drivers using the cfb_* functions should change from
"u32" to "unsigned long" _whatever_ the bit depth when loading the
pseudopalette.
Of course, drivers with their own drawing functions can use whatever
they want.
Tony
PS: cfb_fillrect is still limited to u32 though which can hopefully be
fixed in the future.
-------------------------------------------------------
This SF.NET email is sponsored by: The Best Geek Holiday Gifts!
Time is running out! Thinkgeek.com has the coolest gifts for
your favorite geek. Let your fingers do the typing. Visit Now.
T H I N K G E E K . C O M http://www.thinkgeek.com/sf/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH] fix endian problem in color_imageblit
2002-12-21 3:45 ` Antonino Daplas
(?)
@ 2002-12-21 9:27 ` Russell King
2002-12-23 9:07 ` Geert Uytterhoeven
-1 siblings, 1 reply; 8+ messages in thread
From: Russell King @ 2002-12-21 9:27 UTC (permalink / raw
To: Antonino Daplas
Cc: James Simmons, Paul Mackerras, Linux Kernel Mailing List,
Linux Fbdev development list
On Sat, Dec 21, 2002 at 11:45:44AM +0800, Antonino Daplas wrote:
> The pseudopalette will only matter for truecolor and directcolor as the
> rest of the visuals bypasses the pseudopalette. Typecasting to
> "unsigned long" rather than "u32" is also safer (even for bpp16) since
> in 64-bit machines, the drawing functions use fb_writeq instead of
> fb_writel.
Erm, what does the size of the drawing functions have to do with the
size of the pseudo palette. The pseudo palette is only there to encode
the pixel values for the 16 console colours.
This means that a 64-bit pseudo palette would only be useful if you have
a graphics card that supports > 32bpp, otherwise a 32-bit pseudo palette
is perfectly adequate, even if you are using fb_writeq.
(note that fbcon doesn't support > 32bpp, so we will only ever look at
the first 32 bits, and having it be 64-bit would just be a needless
waste of space imho.)
--
Russell King (rmk@arm.linux.org.uk) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH] fix endian problem in color_imageblit
2002-12-21 9:27 ` [Linux-fbdev-devel] " Russell King
@ 2002-12-23 9:07 ` Geert Uytterhoeven
2002-12-23 11:18 ` Russell King
0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2002-12-23 9:07 UTC (permalink / raw
To: Russell King
Cc: Antonino Daplas, James Simmons, Paul Mackerras,
Linux Kernel Mailing List, Linux Fbdev development list
On Sat, 21 Dec 2002, Russell King wrote:
> On Sat, Dec 21, 2002 at 11:45:44AM +0800, Antonino Daplas wrote:
> > The pseudopalette will only matter for truecolor and directcolor as the
> > rest of the visuals bypasses the pseudopalette. Typecasting to
> > "unsigned long" rather than "u32" is also safer (even for bpp16) since
> > in 64-bit machines, the drawing functions use fb_writeq instead of
> > fb_writel.
>
> Erm, what does the size of the drawing functions have to do with the
> size of the pseudo palette. The pseudo palette is only there to encode
> the pixel values for the 16 console colours.
Indeed.
> This means that a 64-bit pseudo palette would only be useful if you have
> a graphics card that supports > 32bpp, otherwise a 32-bit pseudo palette
> is perfectly adequate, even if you are using fb_writeq.
Yep. Cards with bpp > 32 are rumoured to exist, but there is no fbdev support
for them right now.
> (note that fbcon doesn't support > 32bpp, so we will only ever look at
> the first 32 bits, and having it be 64-bit would just be a needless
> waste of space imho.)
And this can be fixed, once we have some fbdev driver that uses bpp > 32.
Note that the size of the entries in the pseudo palette used to depend on the
mode, i.e. u16 for bpp = 16, u32 for bpp = 24 or 32.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH] fix endian problem in color_imageblit
2002-12-23 9:07 ` Geert Uytterhoeven
@ 2002-12-23 11:18 ` Russell King
0 siblings, 0 replies; 8+ messages in thread
From: Russell King @ 2002-12-23 11:18 UTC (permalink / raw
To: Geert Uytterhoeven
Cc: Antonino Daplas, James Simmons, Paul Mackerras,
Linux Kernel Mailing List, Linux Fbdev development list
On Mon, Dec 23, 2002 at 10:07:35AM +0100, Geert Uytterhoeven wrote:
> Note that the size of the entries in the pseudo palette used to depend on the
> mode, i.e. u16 for bpp = 16, u32 for bpp = 24 or 32.
They used to. However, reading the code in 2.5 today, this appears to be
no longer the case - they're all one size (u32).
--
Russell King (rmk@arm.linux.org.uk) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2002-12-23 11:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-12-12 3:41 [PATCH] fix endian problem in color_imageblit Paul Mackerras
2002-12-15 1:05 ` [Linux-fbdev-devel] " Antonino Daplas
2002-12-20 19:54 ` James Simmons
2002-12-21 3:45 ` Antonino Daplas
2002-12-21 3:45 ` Antonino Daplas
2002-12-21 9:27 ` [Linux-fbdev-devel] " Russell King
2002-12-23 9:07 ` Geert Uytterhoeven
2002-12-23 11:18 ` Russell King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.