I'm trying to display the bootup logo on a monochrome LCD (1 bit per
pixel). I had to hack fbmem.c in a couple of place to make it
works. I'm wondering now if these changes are correct since I'm not
familiar with this code. Could anybody take a look and tell me ?
Thanks
Franck
-- >8 --
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 17961e3..8c9d51f 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -247,6 +247,7 @@ static void fb_set_logo(struct fb_info *
const struct linux_logo *logo, u8 *dst,
int depth)
{
+ u32 bpp = info->var.bits_per_pixel;
int i, j, k;
const u8 *src = logo->data;
u8 xor = (info->fix.visual == FB_VISUAL_MONO01) ? 0xff : 0;
@@ -275,9 +276,14 @@ static void fb_set_logo(struct fb_info *
for (i = 0; i < logo->height; i++) {
for (j = 0; j < logo->width; src++) {
d = *src ^ xor;
- for (k = 7; k >= 0; k--) {
- *dst++ = ((d >> k) & 1) ? fg : 0;
+ if (bpp == 1) {
+ *dst++ = d;
j++;
+ } else {
+ for (k = 7; k >= 0; k--) {
+ *dst++ = ((d >> k) & 1) ? fg : 0;
+ j++;
+ }
}
}
}
@@ -487,7 +493,6 @@ int fb_show_logo(struct fb_info *info, i
if (fb_logo.logo == NULL || info->state != FBINFO_STATE_RUNNING)
return 0;
- image.depth = 8;
image.data = fb_logo.logo->data;
if (fb_logo.needs_cmapreset)
@@ -506,6 +511,9 @@ int fb_show_logo(struct fb_info *info, i
saved_pseudo_palette = info->pseudo_palette;
info->pseudo_palette = palette;
+ } else {
+ image.fg_color = 1;
+ image.bg_color = 0;
}
if (fb_logo.depth <= 4) {
@@ -525,6 +533,7 @@ int fb_show_logo(struct fb_info *info, i
image.dy = 0;
image.width = fb_logo.logo->width;
image.height = fb_logo.logo->height;
+ image.depth = fb_logo.depth;
if (rotate) {
logo_rotate = kmalloc(fb_logo.logo->width *
> I'm trying to display the bootup logo on a monochrome LCD (1 bit per
> pixel). I had to hack fbmem.c in a couple of place to make it
> works. I'm wondering now if these changes are correct since I'm not
> familiar with this code. Could anybody take a look and tell me ?
There are quite a few bugs in the code. I have a patch I have been working
on for some time. The patch does the following:
I.
Merge slow_imageblit and color_imageblit into one function.
II.
The same code works on both big endian and little endian machines
III.
No need to pad the 4 bpp color images anymore.
IV.
fast_imageblit can work with 32,16,8 bit writes to the
framebuffer. Just change FB_WRITE/FB_READ and access_align.
BUGS:
For some reason fast_imageblits for 16,8 writes think it is
16 bpp,monochrome color maps instead of 256 colors.
slow_imageblit doesn't work with different FB_WRITE/FB_READ
and smaller access_align
diff -urN -X linus-2.6/Documentation/dontdiff linus-2.6/drivers/video/cfbimgblt.c fbdev-2.6/drivers/video/cfbimgblt.c
--- linus-2.6/drivers/video/cfbimgblt.c 2006-11-07 05:38:36.000000000 -0500
+++ fbdev-2.6/drivers/video/cfbimgblt.c 2006-11-12 10:29:49.000000000 -0500
@@ -1,7 +1,7 @@
/*
* Generic BitBLT function for frame buffer with packed pixels of any depth.
*
- * Copyright (C) June 1999 James Simmons
+ * Copyright (C) June 1999 - 2006 James Simmons
*
* This file is subject to the terms and conditions of the GNU General Public
* License. See the file COPYING in the main directory of this archive for
@@ -17,7 +17,7 @@
* their are cards with hardware that coverts images of various depths to the
* framebuffer depth. But not every card has this. All images must be rounded
* up to the nearest byte. For example a bitmap 12 bits wide must be two
- * bytes width.
+ * bytes width.
*
* Tony:
* Incorporate mask tables similar to fbcon-cfb*.c in 2.4 API. This speeds
@@ -32,6 +32,7 @@
#include <linux/module.h>
#include <linux/string.h>
#include <linux/fb.h>
+#include <asm/byteorder.h>
#include <asm/types.h>
#define DEBUG
@@ -43,161 +44,99 @@
#endif
static u32 cfb_tab8[] = {
-#if defined(__BIG_ENDIAN)
0x00000000,0x000000ff,0x0000ff00,0x0000ffff,
0x00ff0000,0x00ff00ff,0x00ffff00,0x00ffffff,
0xff000000,0xff0000ff,0xff00ff00,0xff00ffff,
0xffff0000,0xffff00ff,0xffffff00,0xffffffff
-#elif defined(__LITTLE_ENDIAN)
- 0x00000000,0xff000000,0x00ff0000,0xffff0000,
- 0x0000ff00,0xff00ff00,0x00ffff00,0xffffff00,
- 0x000000ff,0xff0000ff,0x00ff00ff,0xffff00ff,
- 0x0000ffff,0xff00ffff,0x00ffffff,0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
};
static u32 cfb_tab16[] = {
-#if defined(__BIG_ENDIAN)
0x00000000, 0x0000ffff, 0xffff0000, 0xffffffff
-#elif defined(__LITTLE_ENDIAN)
- 0x00000000, 0xffff0000, 0x0000ffff, 0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
};
static u32 cfb_tab32[] = {
0x00000000, 0xffffffff
};
-#define FB_WRITEL fb_writel
-#define FB_READL fb_readl
+#define FB_WRITEL fb_writel
+#define FB_READL fb_readl
-static inline void color_imageblit(const struct fb_image *image,
- struct fb_info *p, u8 __iomem *dst1,
- u32 start_index,
- u32 pitch_index)
+static inline void slow_imageblit(const struct fb_image *image,
+ struct fb_info *p, u8 __iomem *dst,
+ u32 start_index, u32 pitch_index)
{
/* Draw the penguin */
- u32 __iomem *dst, *dst2;
- u32 color = 0, val, shift;
- int i, n, bpp = p->var.bits_per_pixel;
- u32 null_bits = 32 - bpp;
+ int spitch = (image->width * image->depth + 7) >> 3;
+ const u32 *src = (const u32 *) image->data;
+ int scan_align = p->pixmap.scan_align - 1;
u32 *palette = (u32 *) p->pseudo_palette;
- const u8 *src = image->data;
+ int bpp = p->var.bits_per_pixel, i, n;
+ int mask = (1 << image->depth) - 1;
+ int bits = p->pixmap.access_align;
+ int bpw = bits >> 3, s = 32;
+ int null_bits = bits - bpp;
+ u32 color = 0, val, shift;
+ u32 __iomem *dst2;
+ u8 __iomem *dst1;
- dst2 = (u32 __iomem *) dst1;
+ spitch = (spitch + scan_align) & ~scan_align;
+ dst2 = (u32 __iomem *) dst;
for (i = image->height; i--; ) {
n = image->width;
- dst = (u32 __iomem *) dst1;
- shift = 0;
- val = 0;
-
+ shift = val = 0;
+ dst1 = dst;
+
+ /* write leading bits */
if (start_index) {
- u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0, start_index));
- val = FB_READL(dst) & start_mask;
+ u32 start_mask = (~(u32)0 << start_index);
+ val = FB_READL(dst1);
+ val &= start_mask;
shift = start_index;
}
- while (n--) {
- if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
- p->fix.visual == FB_VISUAL_DIRECTCOLOR )
- color = palette[*src];
- else
- color = *src;
- color <<= FB_LEFT_POS(bpp);
- val |= FB_SHIFT_HIGH(color, shift);
- if (shift >= null_bits) {
- FB_WRITEL(val, dst++);
-
- val = (shift == null_bits) ? 0 :
- FB_SHIFT_LOW(color, 32 - shift);
- }
- shift += bpp;
- shift &= (32 - 1);
- src++;
- }
- if (shift) {
- u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift);
-
- FB_WRITEL((FB_READL(dst) & end_mask) | val, dst);
- }
- dst1 += p->fix.line_length;
- if (pitch_index) {
- dst2 += p->fix.line_length;
- dst1 = (u8 __iomem *)((long __force)dst2 & ~(sizeof(u32) - 1));
- start_index += pitch_index;
- start_index &= 32 - 1;
- }
- }
-}
+ while (n--) {
+ if (!s) { src++; s = 32; }
+ s -= image->depth;
-static inline void slow_imageblit(const struct fb_image *image, struct fb_info *p,
- u8 __iomem *dst1, u32 fgcolor,
- u32 bgcolor,
- u32 start_index,
- u32 pitch_index)
-{
- u32 shift, color = 0, bpp = p->var.bits_per_pixel;
- u32 __iomem *dst, *dst2;
- u32 val, pitch = p->fix.line_length;
- u32 null_bits = 32 - bpp;
- u32 spitch = (image->width+7)/8;
- const u8 *src = image->data, *s;
- u32 i, j, l;
-
- dst2 = (u32 __iomem *) dst1;
- fgcolor <<= FB_LEFT_POS(bpp);
- bgcolor <<= FB_LEFT_POS(bpp);
+ color = (swab32p(src) & (mask << s));
+ if (image->depth == 1)
+ color = color ? image->fg_color : image->bg_color;
+ else
+ color >>= s;
- for (i = image->height; i--; ) {
- shift = val = 0;
- l = 8;
- j = image->width;
- dst = (u32 __iomem *) dst1;
- s = src;
+ if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
+ p->fix.visual == FB_VISUAL_DIRECTCOLOR)
+ color = palette[color];
- /* write leading bits */
- if (start_index) {
- u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0,start_index));
- val = FB_READL(dst) & start_mask;
- shift = start_index;
- }
+ val |= (color << shift);
- while (j--) {
- l--;
- color = (*s & (1 << l)) ? fgcolor : bgcolor;
- val |= FB_SHIFT_HIGH(color, shift);
-
- /* Did the bitshift spill bits to the next long? */
+ /* Did the bitshift spill bits into the next long? */
if (shift >= null_bits) {
- FB_WRITEL(val, dst++);
- val = (shift == null_bits) ? 0 :
- FB_SHIFT_LOW(color,32 - shift);
+ FB_WRITEL(val, dst1);
+ dst1 += bpw;
+ val = (shift == null_bits) ? 0 : (color >> (bits - shift));
}
shift += bpp;
- shift &= (32 - 1);
- if (!l) { l = 8; s++; };
+ shift &= (bits - 1);
}
+ s -= (spitch << 3) - image->width * image->depth;
/* write trailing bits */
- if (shift) {
- u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift);
+ if (shift) {
+ u32 end_mask = (~(u32)0 << shift);
- FB_WRITEL((FB_READL(dst) & end_mask) | val, dst);
+ val = FB_READL(dst1);
+ val &= end_mask;
+ FB_WRITEL(val, dst1);
}
-
- dst1 += pitch;
- src += spitch;
+
+ dst += p->fix.line_length;
if (pitch_index) {
- dst2 += pitch;
- dst1 = (u8 __iomem *)((long __force)dst2 & ~(sizeof(u32) - 1));
+ dst2 += p->fix.line_length;
+ dst = (u8 __iomem *)((long __force)dst2 & ~(bpw - 1));
start_index += pitch_index;
- start_index &= 32 - 1;
+ start_index &= bits - 1;
}
-
}
}
@@ -210,101 +149,105 @@
* beginning and end of a scanline is dword aligned
*/
static inline void fast_imageblit(const struct fb_image *image, struct fb_info *p,
- u8 __iomem *dst1, u32 fgcolor,
- u32 bgcolor)
+ u8 __iomem *dst)
{
- u32 fgx = fgcolor, bgx = bgcolor, bpp = p->var.bits_per_pixel;
- u32 ppw = 32/bpp, spitch = (image->width + 7)/8;
- u32 bit_mask, end_mask, eorx, shift;
+ int scan_align = p->pixmap.scan_align - 1, spitch = (image->width + 7) >> 3;
+ u32 bit_mask, end_mask = 0, eorx, fgx, fgcolor, bgx, bgcolor, val;
+ int access = p->pixmap.access_align, bpw = access >> 3, bits;
+ int bpp = p->var.bits_per_pixel, ppw, shift, i, j;
const char *s = image->data, *src;
- u32 __iomem *dst;
+ u8 __iomem *dst1;
u32 *tab = NULL;
- int i, j, k;
-
+
+ spitch = (spitch + scan_align) & ~scan_align;
+
+ if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
+ p->fix.visual == FB_VISUAL_DIRECTCOLOR) {
+ fgx = fgcolor = ((u32*)(p->pseudo_palette))[image->fg_color];
+ bgx = bgcolor = ((u32*)(p->pseudo_palette))[image->bg_color];
+ } else {
+ fgx = fgcolor = image->fg_color;
+ bgx = bgcolor = image->bg_color;
+ }
+
switch (bpp) {
case 8:
tab = cfb_tab8;
+ ppw = 4;
break;
case 16:
tab = cfb_tab16;
+ ppw = 2;
break;
case 32:
default:
tab = cfb_tab32;
+ ppw = 1;
break;
}
- for (i = ppw-1; i--; ) {
+ for (i = 0; i < 32; i += bpp) {
fgx <<= bpp;
bgx <<= bpp;
fgx |= fgcolor;
bgx |= bgcolor;
}
-
+
bit_mask = (1 << ppw) - 1;
eorx = fgx ^ bgx;
- k = image->width/ppw;
for (i = image->height; i--; ) {
- dst = (u32 __iomem *) dst1, shift = 8; src = s;
-
- for (j = k; j--; ) {
- shift -= ppw;
- end_mask = tab[(*src >> shift) & bit_mask];
- FB_WRITEL((end_mask & eorx)^bgx, dst++);
- if (!shift) { shift = 8; src++; }
+ dst1 = dst, shift = 8, bits = 32; src = s;
+
+ for (j = image->width*bpp; j > 0; j -= access) {
+ bits += access;
+ if (bits >= 32) {
+ shift -= ppw;
+ end_mask = swab32(tab[(*src >> shift) & bit_mask]);
+ if (!shift) { shift = 8; src++; }
+ bits = 0;
+ }
+ val = (end_mask & eorx)^bgx;
+ FB_WRITEL(val, dst1);
+ dst1 += bpw;
}
- dst1 += p->fix.line_length;
+ dst += p->fix.line_length;
s += spitch;
}
-}
-
+}
+
void cfb_imageblit(struct fb_info *p, const struct fb_image *image)
{
- u32 fgcolor, bgcolor, start_index, bitstart, pitch_index = 0;
- u32 bpl = sizeof(u32), bpp = p->var.bits_per_pixel;
- u32 width = image->width;
- u32 dx = image->dx, dy = image->dy;
+ u32 bits = p->pixmap.access_align, bpp = p->var.bits_per_pixel;
+ u32 width = image->width, dx = image->dx, dy = image->dy;
+ u32 start_index, bitstart, pitch_index = 0;
+ int bpl = bits >> 3;
u8 __iomem *dst1;
if (p->state != FBINFO_STATE_RUNNING)
return;
- bitstart = (dy * p->fix.line_length * 8) + (dx * bpp);
- start_index = bitstart & (32 - 1);
- pitch_index = (p->fix.line_length & (bpl - 1)) * 8;
+ bitstart = ((dy * p->fix.line_length) << 3) + (dx * bpp);
+ start_index = bitstart & (bits - 1);
+ pitch_index = (p->fix.line_length & (bpl - 1)) << 3;
- bitstart /= 8;
+ bitstart >>= 3;
bitstart &= ~(bpl - 1);
dst1 = p->screen_base + bitstart;
if (p->fbops->fb_sync)
p->fbops->fb_sync(p);
- if (image->depth == 1) {
- if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
- p->fix.visual == FB_VISUAL_DIRECTCOLOR) {
- fgcolor = ((u32*)(p->pseudo_palette))[image->fg_color];
- bgcolor = ((u32*)(p->pseudo_palette))[image->bg_color];
- } else {
- fgcolor = image->fg_color;
- bgcolor = image->bg_color;
- }
-
- if (32 % bpp == 0 && !start_index && !pitch_index &&
- ((width & (32/bpp-1)) == 0) &&
- bpp >= 8 && bpp <= 32)
- fast_imageblit(image, p, dst1, fgcolor, bgcolor);
- else
- slow_imageblit(image, p, dst1, fgcolor, bgcolor,
- start_index, pitch_index);
- } else
- color_imageblit(image, p, dst1, start_index, pitch_index);
+ if (bits % bpp == 0 && image->depth == 1 && !start_index && !pitch_index && bpp >= 8 && bpp <= 32
+ && ((width & (bits/bpp-1)) == 0))
+ fast_imageblit(image, p, dst1);
+ else
+ slow_imageblit(image, p, dst1, start_index, pitch_index);
}
EXPORT_SYMBOL(cfb_imageblit);
-MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_AUTHOR("James Simmons <[email protected]>");
MODULE_DESCRIPTION("Generic software accelerated imaging drawing");
MODULE_LICENSE("GPL");
diff -urN -X linus-2.6/Documentation/dontdiff linus-2.6/drivers/video/fbmem.c fbdev-2.6/drivers/video/fbmem.c
--- linus-2.6/drivers/video/fbmem.c 2006-11-07 05:38:36.000000000 -0500
+++ fbdev-2.6/drivers/video/fbmem.c 2006-11-11 10:00:32.000000000 -0500
@@ -243,48 +243,6 @@
palette[i] = i << redshift | i << greenshift | i << blueshift;
}
-static void fb_set_logo(struct fb_info *info,
- const struct linux_logo *logo, u8 *dst,
- int depth)
-{
- int i, j, k;
- const u8 *src = logo->data;
- u8 xor = (info->fix.visual == FB_VISUAL_MONO01) ? 0xff : 0;
- u8 fg = 1, d;
-
- if (fb_get_color_depth(&info->var, &info->fix) == 3)
- fg = 7;
-
- if (info->fix.visual == FB_VISUAL_MONO01 ||
- info->fix.visual == FB_VISUAL_MONO10)
- fg = ~((u8) (0xfff << info->var.green.length));
-
- switch (depth) {
- case 4:
- for (i = 0; i < logo->height; i++)
- for (j = 0; j < logo->width; src++) {
- *dst++ = *src >> 4;
- j++;
- if (j < logo->width) {
- *dst++ = *src & 0x0f;
- j++;
- }
- }
- break;
- case 1:
- for (i = 0; i < logo->height; i++) {
- for (j = 0; j < logo->width; src++) {
- d = *src ^ xor;
- for (k = 7; k >= 0; k--) {
- *dst++ = ((d >> k) & 1) ? fg : 0;
- j++;
- }
- }
- }
- break;
- }
-}
-
/*
* Three (3) kinds of logo maps exist. linux_logo_clut224 (>16 colors),
* linux_logo_vga16 (16 colors) and linux_logo_mono (2 colors). Depending on
@@ -452,11 +410,9 @@
/* Return if no suitable logo was found */
fb_logo.logo = fb_find_logo(depth);
-
- if (!fb_logo.logo) {
+ if (!fb_logo.logo)
return 0;
- }
-
+
if (rotate == FB_ROTATE_UR || rotate == FB_ROTATE_UD)
yres = info->var.yres;
else
@@ -480,14 +436,13 @@
int fb_show_logo(struct fb_info *info, int rotate)
{
u32 *palette = NULL, *saved_pseudo_palette = NULL;
- unsigned char *logo_new = NULL, *logo_rotate = NULL;
struct fb_image image;
/* Return if the frame buffer is not mapped or suspended */
if (fb_logo.logo == NULL || info->state != FBINFO_STATE_RUNNING)
return 0;
- image.depth = 8;
+ image.depth = fb_logo.depth;
image.data = fb_logo.logo->data;
if (fb_logo.needs_cmapreset)
@@ -508,17 +463,13 @@
info->pseudo_palette = palette;
}
- if (fb_logo.depth <= 4) {
- logo_new = kmalloc(fb_logo.logo->width * fb_logo.logo->height,
- GFP_KERNEL);
- if (logo_new == NULL) {
- kfree(palette);
- if (saved_pseudo_palette)
- info->pseudo_palette = saved_pseudo_palette;
- return 0;
+ if (fb_logo.depth == 1) {
+ if (info->fix.visual == FB_VISUAL_MONO01) {
+ u32 fg = image.fg_color;
+
+ image.fg_color = image.bg_color;
+ image.bg_color = fg;
}
- image.data = logo_new;
- fb_set_logo(info, fb_logo.logo, logo_new, fb_logo.depth);
}
image.dx = 0;
@@ -527,19 +478,17 @@
image.height = fb_logo.logo->height;
if (rotate) {
- logo_rotate = kmalloc(fb_logo.logo->width *
- fb_logo.logo->height, GFP_KERNEL);
+ unsigned char *logo_rotate = kmalloc(fb_logo.logo->width *
+ fb_logo.logo->height, GFP_KERNEL);
if (logo_rotate)
fb_rotate_logo(info, logo_rotate, &image, rotate);
+ kfree(logo_rotate);
}
-
fb_do_show_logo(info, &image, rotate);
kfree(palette);
if (saved_pseudo_palette != NULL)
info->pseudo_palette = saved_pseudo_palette;
- kfree(logo_new);
- kfree(logo_rotate);
return fb_logo.logo->height;
}
#else
On 11/12/06, James Simmons <[email protected]> wrote:
>
> > I'm trying to display the bootup logo on a monochrome LCD (1 bit per
> > pixel). I had to hack fbmem.c in a couple of place to make it
> > works. I'm wondering now if these changes are correct since I'm not
> > familiar with this code. Could anybody take a look and tell me ?
>
> There are quite a few bugs in the code. I have a patch I have been working
> on for some time. The patch does the following:
>
I'd like to give your patch a try but have some trouble to apply it
cleanly. Care to resend it ?
> I.
> Merge slow_imageblit and color_imageblit into one function.
> II.
> The same code works on both big endian and little endian machines
Does this suppose to fix this issue I encountered:
http://marc.theaimsgroup.com/?l=linux-kernel&m=116315548626875&w=2
Thanks
--
Franck
>> There are quite a few bugs in the code. I have a patch I have been working
>> on for some time. The patch does the following:
>>
>
> I'd like to give your patch a try but have some trouble to apply it
> cleanly. Care to resend it ?
Which tree are you working off ?> The patch is against linus git tree.
>> I.
>> Merge slow_imageblit and color_imageblit into one function.
>> II.
>> The same code works on both big endian and little endian machines
>
> Does this suppose to fix this issue I encountered:
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=116315548626875&w=2
This should fix the problems you reported. I tested this patch on a big
endian and little endian framebuffer on a little endian machine.
On 11/13/06, James Simmons <[email protected]> wrote:
>
> >> There are quite a few bugs in the code. I have a patch I have been working
> >> on for some time. The patch does the following:
> >>
> >
> > I'd like to give your patch a try but have some trouble to apply it
> > cleanly. Care to resend it ?
>
> Which tree are you working off ?> The patch is against linus git tree.
>
I use the same tree. It seems that the patch get corrupted when it
reached me. Care to attach it next time ?
> >> I.
> >> Merge slow_imageblit and color_imageblit into one function.
> >> II.
> >> The same code works on both big endian and little endian machines
> >
> > Does this suppose to fix this issue I encountered:
> >
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=116315548626875&w=2
>
> This should fix the problems you reported. I tested this patch on a big
> endian and little endian framebuffer on a little endian machine.
>
Something I'm still missing hope you can shed some light there. Why
does the current code work on Rafael's machine (little endian one
using vesa driver) but not on mine which is a little endian one as
well ?
Thanks
--
Franck
On 11/13/06, James Simmons <[email protected]> wrote:
>
> >> There are quite a few bugs in the code. I have a patch I have been working
> >> on for some time. The patch does the following:
> >>
> >
> > I'd like to give your patch a try but have some trouble to apply it
> > cleanly. Care to resend it ?
>
> Which tree are you working off ?> The patch is against linus git tree.
>
It seems that you use "format=flowed" with your mailer. Can you try to
disable it ?
Even if I save the message to a file, the patch is still corrupted...
Thanks
--
Franck
Let me know if it works now.
On Mon, 13 Nov 2006, Franck Bui-Huu wrote:
> On 11/13/06, James Simmons <[email protected]> wrote:
> >
> > >> There are quite a few bugs in the code. I have a patch I have been
> > working
> > >> on for some time. The patch does the following:
> > >>
> > >
> > > I'd like to give your patch a try but have some trouble to apply it
> > > cleanly. Care to resend it ?
> >
> > Which tree are you working off ?> The patch is against linus git tree.
> >
>
> It seems that you use "format=flowed" with your mailer. Can you try to
> disable it ?
>
> Even if I save the message to a file, the patch is still corrupted...
>
> Thanks
>
diff -urN -X linus-2.6/Documentation/dontdiff linus-2.6/drivers/video/cfbimgblt.c fbdev-2.6/drivers/video/cfbimgblt.c
--- linus-2.6/drivers/video/cfbimgblt.c 2006-11-07 05:38:36.000000000 -0500
+++ fbdev-2.6/drivers/video/cfbimgblt.c 2006-11-12 10:29:49.000000000 -0500
@@ -1,7 +1,7 @@
/*
* Generic BitBLT function for frame buffer with packed pixels of any depth.
*
- * Copyright (C) June 1999 James Simmons
+ * Copyright (C) June 1999 - 2006 James Simmons
*
* This file is subject to the terms and conditions of the GNU General Public
* License. See the file COPYING in the main directory of this archive for
@@ -17,7 +17,7 @@
* their are cards with hardware that coverts images of various depths to the
* framebuffer depth. But not every card has this. All images must be rounded
* up to the nearest byte. For example a bitmap 12 bits wide must be two
- * bytes width.
+ * bytes width.
*
* Tony:
* Incorporate mask tables similar to fbcon-cfb*.c in 2.4 API. This speeds
@@ -32,6 +32,7 @@
#include <linux/module.h>
#include <linux/string.h>
#include <linux/fb.h>
+#include <asm/byteorder.h>
#include <asm/types.h>
#define DEBUG
@@ -43,161 +44,99 @@
#endif
static u32 cfb_tab8[] = {
-#if defined(__BIG_ENDIAN)
0x00000000,0x000000ff,0x0000ff00,0x0000ffff,
0x00ff0000,0x00ff00ff,0x00ffff00,0x00ffffff,
0xff000000,0xff0000ff,0xff00ff00,0xff00ffff,
0xffff0000,0xffff00ff,0xffffff00,0xffffffff
-#elif defined(__LITTLE_ENDIAN)
- 0x00000000,0xff000000,0x00ff0000,0xffff0000,
- 0x0000ff00,0xff00ff00,0x00ffff00,0xffffff00,
- 0x000000ff,0xff0000ff,0x00ff00ff,0xffff00ff,
- 0x0000ffff,0xff00ffff,0x00ffffff,0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
};
static u32 cfb_tab16[] = {
-#if defined(__BIG_ENDIAN)
0x00000000, 0x0000ffff, 0xffff0000, 0xffffffff
-#elif defined(__LITTLE_ENDIAN)
- 0x00000000, 0xffff0000, 0x0000ffff, 0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
};
static u32 cfb_tab32[] = {
0x00000000, 0xffffffff
};
-#define FB_WRITEL fb_writel
-#define FB_READL fb_readl
+#define FB_WRITEL fb_writel
+#define FB_READL fb_readl
-static inline void color_imageblit(const struct fb_image *image,
- struct fb_info *p, u8 __iomem *dst1,
- u32 start_index,
- u32 pitch_index)
+static inline void slow_imageblit(const struct fb_image *image,
+ struct fb_info *p, u8 __iomem *dst,
+ u32 start_index, u32 pitch_index)
{
/* Draw the penguin */
- u32 __iomem *dst, *dst2;
- u32 color = 0, val, shift;
- int i, n, bpp = p->var.bits_per_pixel;
- u32 null_bits = 32 - bpp;
+ int spitch = (image->width * image->depth + 7) >> 3;
+ const u32 *src = (const u32 *) image->data;
+ int scan_align = p->pixmap.scan_align - 1;
u32 *palette = (u32 *) p->pseudo_palette;
- const u8 *src = image->data;
+ int bpp = p->var.bits_per_pixel, i, n;
+ int mask = (1 << image->depth) - 1;
+ int bits = p->pixmap.access_align;
+ int bpw = bits >> 3, s = 32;
+ int null_bits = bits - bpp;
+ u32 color = 0, val, shift;
+ u32 __iomem *dst2;
+ u8 __iomem *dst1;
- dst2 = (u32 __iomem *) dst1;
+ spitch = (spitch + scan_align) & ~scan_align;
+ dst2 = (u32 __iomem *) dst;
for (i = image->height; i--; ) {
n = image->width;
- dst = (u32 __iomem *) dst1;
- shift = 0;
- val = 0;
-
+ shift = val = 0;
+ dst1 = dst;
+
+ /* write leading bits */
if (start_index) {
- u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0, start_index));
- val = FB_READL(dst) & start_mask;
+ u32 start_mask = (~(u32)0 << start_index);
+ val = FB_READL(dst1);
+ val &= start_mask;
shift = start_index;
}
- while (n--) {
- if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
- p->fix.visual == FB_VISUAL_DIRECTCOLOR )
- color = palette[*src];
- else
- color = *src;
- color <<= FB_LEFT_POS(bpp);
- val |= FB_SHIFT_HIGH(color, shift);
- if (shift >= null_bits) {
- FB_WRITEL(val, dst++);
-
- val = (shift == null_bits) ? 0 :
- FB_SHIFT_LOW(color, 32 - shift);
- }
- shift += bpp;
- shift &= (32 - 1);
- src++;
- }
- if (shift) {
- u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift);
-
- FB_WRITEL((FB_READL(dst) & end_mask) | val, dst);
- }
- dst1 += p->fix.line_length;
- if (pitch_index) {
- dst2 += p->fix.line_length;
- dst1 = (u8 __iomem *)((long __force)dst2 & ~(sizeof(u32) - 1));
- start_index += pitch_index;
- start_index &= 32 - 1;
- }
- }
-}
+ while (n--) {
+ if (!s) { src++; s = 32; }
+ s -= image->depth;
-static inline void slow_imageblit(const struct fb_image *image, struct fb_info *p,
- u8 __iomem *dst1, u32 fgcolor,
- u32 bgcolor,
- u32 start_index,
- u32 pitch_index)
-{
- u32 shift, color = 0, bpp = p->var.bits_per_pixel;
- u32 __iomem *dst, *dst2;
- u32 val, pitch = p->fix.line_length;
- u32 null_bits = 32 - bpp;
- u32 spitch = (image->width+7)/8;
- const u8 *src = image->data, *s;
- u32 i, j, l;
-
- dst2 = (u32 __iomem *) dst1;
- fgcolor <<= FB_LEFT_POS(bpp);
- bgcolor <<= FB_LEFT_POS(bpp);
+ color = (swab32p(src) & (mask << s));
+ if (image->depth == 1)
+ color = color ? image->fg_color : image->bg_color;
+ else
+ color >>= s;
- for (i = image->height; i--; ) {
- shift = val = 0;
- l = 8;
- j = image->width;
- dst = (u32 __iomem *) dst1;
- s = src;
+ if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
+ p->fix.visual == FB_VISUAL_DIRECTCOLOR)
+ color = palette[color];
- /* write leading bits */
- if (start_index) {
- u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0,start_index));
- val = FB_READL(dst) & start_mask;
- shift = start_index;
- }
+ val |= (color << shift);
- while (j--) {
- l--;
- color = (*s & (1 << l)) ? fgcolor : bgcolor;
- val |= FB_SHIFT_HIGH(color, shift);
-
- /* Did the bitshift spill bits to the next long? */
+ /* Did the bitshift spill bits into the next long? */
if (shift >= null_bits) {
- FB_WRITEL(val, dst++);
- val = (shift == null_bits) ? 0 :
- FB_SHIFT_LOW(color,32 - shift);
+ FB_WRITEL(val, dst1);
+ dst1 += bpw;
+ val = (shift == null_bits) ? 0 : (color >> (bits - shift));
}
shift += bpp;
- shift &= (32 - 1);
- if (!l) { l = 8; s++; };
+ shift &= (bits - 1);
}
+ s -= (spitch << 3) - image->width * image->depth;
/* write trailing bits */
- if (shift) {
- u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift);
+ if (shift) {
+ u32 end_mask = (~(u32)0 << shift);
- FB_WRITEL((FB_READL(dst) & end_mask) | val, dst);
+ val = FB_READL(dst1);
+ val &= end_mask;
+ FB_WRITEL(val, dst1);
}
-
- dst1 += pitch;
- src += spitch;
+
+ dst += p->fix.line_length;
if (pitch_index) {
- dst2 += pitch;
- dst1 = (u8 __iomem *)((long __force)dst2 & ~(sizeof(u32) - 1));
+ dst2 += p->fix.line_length;
+ dst = (u8 __iomem *)((long __force)dst2 & ~(bpw - 1));
start_index += pitch_index;
- start_index &= 32 - 1;
+ start_index &= bits - 1;
}
-
}
}
@@ -210,101 +149,105 @@
* beginning and end of a scanline is dword aligned
*/
static inline void fast_imageblit(const struct fb_image *image, struct fb_info *p,
- u8 __iomem *dst1, u32 fgcolor,
- u32 bgcolor)
+ u8 __iomem *dst)
{
- u32 fgx = fgcolor, bgx = bgcolor, bpp = p->var.bits_per_pixel;
- u32 ppw = 32/bpp, spitch = (image->width + 7)/8;
- u32 bit_mask, end_mask, eorx, shift;
+ int scan_align = p->pixmap.scan_align - 1, spitch = (image->width + 7) >> 3;
+ u32 bit_mask, end_mask = 0, eorx, fgx, fgcolor, bgx, bgcolor, val;
+ int access = p->pixmap.access_align, bpw = access >> 3, bits;
+ int bpp = p->var.bits_per_pixel, ppw, shift, i, j;
const char *s = image->data, *src;
- u32 __iomem *dst;
+ u8 __iomem *dst1;
u32 *tab = NULL;
- int i, j, k;
-
+
+ spitch = (spitch + scan_align) & ~scan_align;
+
+ if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
+ p->fix.visual == FB_VISUAL_DIRECTCOLOR) {
+ fgx = fgcolor = ((u32*)(p->pseudo_palette))[image->fg_color];
+ bgx = bgcolor = ((u32*)(p->pseudo_palette))[image->bg_color];
+ } else {
+ fgx = fgcolor = image->fg_color;
+ bgx = bgcolor = image->bg_color;
+ }
+
switch (bpp) {
case 8:
tab = cfb_tab8;
+ ppw = 4;
break;
case 16:
tab = cfb_tab16;
+ ppw = 2;
break;
case 32:
default:
tab = cfb_tab32;
+ ppw = 1;
break;
}
- for (i = ppw-1; i--; ) {
+ for (i = 0; i < 32; i += bpp) {
fgx <<= bpp;
bgx <<= bpp;
fgx |= fgcolor;
bgx |= bgcolor;
}
-
+
bit_mask = (1 << ppw) - 1;
eorx = fgx ^ bgx;
- k = image->width/ppw;
for (i = image->height; i--; ) {
- dst = (u32 __iomem *) dst1, shift = 8; src = s;
-
- for (j = k; j--; ) {
- shift -= ppw;
- end_mask = tab[(*src >> shift) & bit_mask];
- FB_WRITEL((end_mask & eorx)^bgx, dst++);
- if (!shift) { shift = 8; src++; }
+ dst1 = dst, shift = 8, bits = 32; src = s;
+
+ for (j = image->width*bpp; j > 0; j -= access) {
+ bits += access;
+ if (bits >= 32) {
+ shift -= ppw;
+ end_mask = swab32(tab[(*src >> shift) & bit_mask]);
+ if (!shift) { shift = 8; src++; }
+ bits = 0;
+ }
+ val = (end_mask & eorx)^bgx;
+ FB_WRITEL(val, dst1);
+ dst1 += bpw;
}
- dst1 += p->fix.line_length;
+ dst += p->fix.line_length;
s += spitch;
}
-}
-
+}
+
void cfb_imageblit(struct fb_info *p, const struct fb_image *image)
{
- u32 fgcolor, bgcolor, start_index, bitstart, pitch_index = 0;
- u32 bpl = sizeof(u32), bpp = p->var.bits_per_pixel;
- u32 width = image->width;
- u32 dx = image->dx, dy = image->dy;
+ u32 bits = p->pixmap.access_align, bpp = p->var.bits_per_pixel;
+ u32 width = image->width, dx = image->dx, dy = image->dy;
+ u32 start_index, bitstart, pitch_index = 0;
+ int bpl = bits >> 3;
u8 __iomem *dst1;
if (p->state != FBINFO_STATE_RUNNING)
return;
- bitstart = (dy * p->fix.line_length * 8) + (dx * bpp);
- start_index = bitstart & (32 - 1);
- pitch_index = (p->fix.line_length & (bpl - 1)) * 8;
+ bitstart = ((dy * p->fix.line_length) << 3) + (dx * bpp);
+ start_index = bitstart & (bits - 1);
+ pitch_index = (p->fix.line_length & (bpl - 1)) << 3;
- bitstart /= 8;
+ bitstart >>= 3;
bitstart &= ~(bpl - 1);
dst1 = p->screen_base + bitstart;
if (p->fbops->fb_sync)
p->fbops->fb_sync(p);
- if (image->depth == 1) {
- if (p->fix.visual == FB_VISUAL_TRUECOLOR ||
- p->fix.visual == FB_VISUAL_DIRECTCOLOR) {
- fgcolor = ((u32*)(p->pseudo_palette))[image->fg_color];
- bgcolor = ((u32*)(p->pseudo_palette))[image->bg_color];
- } else {
- fgcolor = image->fg_color;
- bgcolor = image->bg_color;
- }
-
- if (32 % bpp == 0 && !start_index && !pitch_index &&
- ((width & (32/bpp-1)) == 0) &&
- bpp >= 8 && bpp <= 32)
- fast_imageblit(image, p, dst1, fgcolor, bgcolor);
- else
- slow_imageblit(image, p, dst1, fgcolor, bgcolor,
- start_index, pitch_index);
- } else
- color_imageblit(image, p, dst1, start_index, pitch_index);
+ if (bits % bpp == 0 && image->depth == 1 && !start_index && !pitch_index && bpp >= 8 && bpp <= 32
+ && ((width & (bits/bpp-1)) == 0))
+ fast_imageblit(image, p, dst1);
+ else
+ slow_imageblit(image, p, dst1, start_index, pitch_index);
}
EXPORT_SYMBOL(cfb_imageblit);
-MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_AUTHOR("James Simmons <[email protected]>");
MODULE_DESCRIPTION("Generic software accelerated imaging drawing");
MODULE_LICENSE("GPL");
diff -urN -X linus-2.6/Documentation/dontdiff linus-2.6/drivers/video/fbmem.c fbdev-2.6/drivers/video/fbmem.c
--- linus-2.6/drivers/video/fbmem.c 2006-11-07 05:38:36.000000000 -0500
+++ fbdev-2.6/drivers/video/fbmem.c 2006-11-11 10:00:32.000000000 -0500
@@ -243,48 +243,6 @@
palette[i] = i << redshift | i << greenshift | i << blueshift;
}
-static void fb_set_logo(struct fb_info *info,
- const struct linux_logo *logo, u8 *dst,
- int depth)
-{
- int i, j, k;
- const u8 *src = logo->data;
- u8 xor = (info->fix.visual == FB_VISUAL_MONO01) ? 0xff : 0;
- u8 fg = 1, d;
-
- if (fb_get_color_depth(&info->var, &info->fix) == 3)
- fg = 7;
-
- if (info->fix.visual == FB_VISUAL_MONO01 ||
- info->fix.visual == FB_VISUAL_MONO10)
- fg = ~((u8) (0xfff << info->var.green.length));
-
- switch (depth) {
- case 4:
- for (i = 0; i < logo->height; i++)
- for (j = 0; j < logo->width; src++) {
- *dst++ = *src >> 4;
- j++;
- if (j < logo->width) {
- *dst++ = *src & 0x0f;
- j++;
- }
- }
- break;
- case 1:
- for (i = 0; i < logo->height; i++) {
- for (j = 0; j < logo->width; src++) {
- d = *src ^ xor;
- for (k = 7; k >= 0; k--) {
- *dst++ = ((d >> k) & 1) ? fg : 0;
- j++;
- }
- }
- }
- break;
- }
-}
-
/*
* Three (3) kinds of logo maps exist. linux_logo_clut224 (>16 colors),
* linux_logo_vga16 (16 colors) and linux_logo_mono (2 colors). Depending on
@@ -452,11 +410,9 @@
/* Return if no suitable logo was found */
fb_logo.logo = fb_find_logo(depth);
-
- if (!fb_logo.logo) {
+ if (!fb_logo.logo)
return 0;
- }
-
+
if (rotate == FB_ROTATE_UR || rotate == FB_ROTATE_UD)
yres = info->var.yres;
else
@@ -480,14 +436,13 @@
int fb_show_logo(struct fb_info *info, int rotate)
{
u32 *palette = NULL, *saved_pseudo_palette = NULL;
- unsigned char *logo_new = NULL, *logo_rotate = NULL;
struct fb_image image;
/* Return if the frame buffer is not mapped or suspended */
if (fb_logo.logo == NULL || info->state != FBINFO_STATE_RUNNING)
return 0;
- image.depth = 8;
+ image.depth = fb_logo.depth;
image.data = fb_logo.logo->data;
if (fb_logo.needs_cmapreset)
@@ -508,17 +463,13 @@
info->pseudo_palette = palette;
}
- if (fb_logo.depth <= 4) {
- logo_new = kmalloc(fb_logo.logo->width * fb_logo.logo->height,
- GFP_KERNEL);
- if (logo_new == NULL) {
- kfree(palette);
- if (saved_pseudo_palette)
- info->pseudo_palette = saved_pseudo_palette;
- return 0;
+ if (fb_logo.depth == 1) {
+ if (info->fix.visual == FB_VISUAL_MONO01) {
+ u32 fg = image.fg_color;
+
+ image.fg_color = image.bg_color;
+ image.bg_color = fg;
}
- image.data = logo_new;
- fb_set_logo(info, fb_logo.logo, logo_new, fb_logo.depth);
}
image.dx = 0;
@@ -527,19 +478,17 @@
image.height = fb_logo.logo->height;
if (rotate) {
- logo_rotate = kmalloc(fb_logo.logo->width *
- fb_logo.logo->height, GFP_KERNEL);
+ unsigned char *logo_rotate = kmalloc(fb_logo.logo->width *
+ fb_logo.logo->height, GFP_KERNEL);
if (logo_rotate)
fb_rotate_logo(info, logo_rotate, &image, rotate);
+ kfree(logo_rotate);
}
-
fb_do_show_logo(info, &image, rotate);
kfree(palette);
if (saved_pseudo_palette != NULL)
info->pseudo_palette = saved_pseudo_palette;
- kfree(logo_new);
- kfree(logo_rotate);
return fb_logo.logo->height;
}
#else
> > > Does this suppose to fix this issue I encountered:
> > >
> > > http://marc.theaimsgroup.com/?l=linux-kernel&m=116315548626875&w=2
> >
> > This should fix the problems you reported. I tested this patch on a big
> > endian and little endian framebuffer on a little endian machine.
> >
>
> Something I'm still missing hope you can shed some light there. Why
> does the current code work on Rafael's machine (little endian one
> using vesa driver) but not on mine which is a little endian one as
> well ?
Both of you are using vesafb?
On 11/13/06, James Simmons <[email protected]> wrote:
>
> Let me know if it works now.
>
OK I can apply it now.
>
> diff -urN -X linus-2.6/Documentation/dontdiff linus-2.6/drivers/video/cfbimgblt.c fbdev-2.6/drivers/video/cfbimgblt.c
> --- linus-2.6/drivers/video/cfbimgblt.c 2006-11-07 05:38:36.000000000 -0500
> +++ fbdev-2.6/drivers/video/cfbimgblt.c 2006-11-12 10:29:49.000000000 -0500
[snip]
> +static inline void slow_imageblit(const struct fb_image *image,
> + struct fb_info *p, u8 __iomem *dst,
> + u32 start_index, u32 pitch_index)
I still have my problem there: for example if image data are
0, 0, 0x54, 0, ...
then slow_imageblit() will write into the frame buffer, the following bytes:
0, 0, 0x2a, 0, ...
instead of the initial ones:
0, 0, 0x54, 0, ...
Bits of each bytes are reversed. I already tried to explain my
problem, please look at
http://marc.theaimsgroup.com/?l=linux-kernel&m=116315548626875&w=2
>
> diff -urN -X linus-2.6/Documentation/dontdiff linus-2.6/drivers/video/fbmem.c fbdev-2.6/drivers/video/fbmem.c
[snip]
> @@ -480,14 +436,13 @@
> int fb_show_logo(struct fb_info *info, int rotate)
> {
> u32 *palette = NULL, *saved_pseudo_palette = NULL;
> - unsigned char *logo_new = NULL, *logo_rotate = NULL;
> struct fb_image image;
>
> /* Return if the frame buffer is not mapped or suspended */
> if (fb_logo.logo == NULL || info->state != FBINFO_STATE_RUNNING)
> return 0;
>
> - image.depth = 8;
> + image.depth = fb_logo.depth;
> image.data = fb_logo.logo->data;
>
> if (fb_logo.needs_cmapreset)
> @@ -508,17 +463,13 @@
> info->pseudo_palette = palette;
> }
>
> - if (fb_logo.depth <= 4) {
> - logo_new = kmalloc(fb_logo.logo->width * fb_logo.logo->height,
> - GFP_KERNEL);
> - if (logo_new == NULL) {
> - kfree(palette);
> - if (saved_pseudo_palette)
> - info->pseudo_palette = saved_pseudo_palette;
> - return 0;
> + if (fb_logo.depth == 1) {
> + if (info->fix.visual == FB_VISUAL_MONO01) {
> + u32 fg = image.fg_color;
> +
> + image.fg_color = image.bg_color;
> + image.bg_color = fg;
I had to fix this part to make the bootup logo worked. image.fg_color
is not uninitialised at this point. I had to change this part as
follow:
if (fb_logo.depth == 1) {
image.fg_color = (info->fix.visual == FB_VISUAL_MONO01) ? 1: 0;
image.bg_color = !image.fg_color;
}
Otherwise this part of your patch (the one which fix the logo display)
seems ok for my naive look. It would be nice if this part could be
sent in a single patch to Andrew. I already tried to fix it but your
patch looks better.
Thanks
--
Franck
> > +static inline void slow_imageblit(const struct fb_image *image,
> > + struct fb_info *p, u8 __iomem *dst,
> > + u32 start_index, u32 pitch_index)
>
> I still have my problem there: for example if image data are
> 0, 0, 0x54, 0, ...
>
> then slow_imageblit() will write into the frame buffer, the following bytes:
> 0, 0, 0x2a, 0, ...
>
> instead of the initial ones:
> 0, 0, 0x54, 0, ...
>
> Bits of each bytes are reversed. I already tried to explain my
> problem, please look at
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=116315548626875&w=2
Are those actually numbers? If they are the problem isn't byte reversal
but bit shifting.
1010100 = 54
0101010 = 2A
> > + if (fb_logo.depth == 1) {
> > + if (info->fix.visual == FB_VISUAL_MONO01) {
> > + u32 fg = image.fg_color;
> > +
> > + image.fg_color = image.bg_color;
> > + image.bg_color = fg;
>
> I had to fix this part to make the bootup logo worked. image.fg_color
> is not uninitialised at this point. I had to change this part as
> follow:
>
> if (fb_logo.depth == 1) {
> image.fg_color = (info->fix.visual == FB_VISUAL_MONO01) ? 1:
> 0;
> image.bg_color = !image.fg_color;
> }
>
> Otherwise this part of your patch (the one which fix the logo display)
> seems ok for my naive look. It would be nice if this part could be
> sent in a single patch to Andrew. I already tried to fix it but your
> patch looks better.
Your right about the colors not being set for mono logo. Your above fix is
not correct but it needs to be fixed. I really don't understand why
fbmem.c has its own routines to handle the logo for the color map. I can
set creating a fbcmap and calling fb_set_cmap instead. That will be a
separte patch.
On 11/17/06, James Simmons <[email protected]> wrote:
>
> Are those actually numbers? If they are the problem isn't byte reversal
> but bit shifting.
>
> 1010100 = 54
> 0101010 = 2A
It's not byte reversal, but _bits_ of each bytes have been inversed
(bit7->bit0, bit6->bit1, bit5->bit2, bit4->bit3, bit3->bit4, ...)
after calling slow_imageblit(). Is it something expected ?
> I really don't understand why fbmem.c has its own routines to handle the logo for the color > map. I can set creating a fbcmap and calling fb_set_cmap instead.
Unfortunately I cannot help you on this point...
> That will be a separte patch.
>
Thanks
--
Franck
> On 11/17/06, James Simmons <[email protected]> wrote:
> >
> > Are those actually numbers? If they are the problem isn't byte reversal
> > but bit shifting.
> >
> > 1010100 = 54
> > 0101010 = 2A
>
> It's not byte reversal, but _bits_ of each bytes have been inversed
> (bit7->bit0, bit6->bit1, bit5->bit2, bit4->bit3, bit3->bit4, ...)
> after calling slow_imageblit(). Is it something expected ?
Yipes!! Bit reversal. I have never seen that before. Is only the logo
messed up? Slow_imageblit can be called if there is no dword alignment
for the font bitmaps. So the question is do most if not all our fonts
look okay?
> > I really don't understand why fbmem.c has its own routines to handle the
> > logo for the color > map. I can set creating a fbcmap and calling
> > fb_set_cmap instead.
>
> Unfortunately I cannot help you on this point...
>
> > That will be a separte patch.
> >
>
> Thanks
>
On 11/20/06, James Simmons <[email protected]> wrote:
>
> > On 11/17/06, James Simmons <[email protected]> wrote:
> > >
> > > Are those actually numbers? If they are the problem isn't byte reversal
> > > but bit shifting.
> > >
> > > 1010100 = 54
> > > 0101010 = 2A
> >
> > It's not byte reversal, but _bits_ of each bytes have been inversed
> > (bit7->bit0, bit6->bit1, bit5->bit2, bit4->bit3, bit3->bit4, ...)
> > after calling slow_imageblit(). Is it something expected ?
>
> Yipes!! Bit reversal. I have never seen that before. Is only the logo
> messed up? Slow_imageblit can be called if there is no dword alignment
> for the font bitmaps. So the question is do most if not all our fonts
> look okay?
>
No, it's not an only logo issue. Bit reversals happen for all images
which are passed to slow_imageblit() including all fonts.
Can it be a 'bit_per_pixel = 1' issue ? It seems that this config has
not been widely tested.
If you look at slow_imageblit() current implementation and for example
let's say that at the begining of the function we have:
- __LITTLE_ENDIAN is defined
- bpp = 1
- fgcolor = 1
- bgcolor = 0
- start_index = 0
The function core can be simplified into:
for (i = image->height; i--; ) {
shift = val = 0;
l = 8;
j = image->width;
dst = (u32 __iomem *) dst1;
s = src;
while (j--) {
l--;
color = (*s & (1 << l)) ? 1 : 0;
val |= color << shift;
/* Did the bitshift spill bits to the next long? */
if (shift >= null_bits) {
FB_WRITEL(val, dst++);
val = (shift == null_bits) ? 0 :
FB_SHIFT_LOW(color,32 - shift);
}
shift += 1;
shift &= (32 - 1);
if (!l) { l = 8; s++; };
}
[ ...]
Doesn't this bit of code do a bit reversal ? Specially these 2
following lines of code:
color = (*s & (1 << l)) ? 1 : 0;
val |= color << shift;
with 'l' taking values from 7 to 0, and 'shift' taking values from 0
to 31.
Thanks
Franck
> > Yipes!! Bit reversal. I have never seen that before. Is only the logo
> > messed up? Slow_imageblit can be called if there is no dword alignment
> > for the font bitmaps. So the question is do most if not all our fonts
> > look okay?
> >
>
> No, it's not an only logo issue. Bit reversals happen for all images
> which are passed to slow_imageblit() including all fonts.
I thought so.
> Can it be a 'bit_per_pixel = 1' issue ? It seems that this config has
> not been widely tested.
Nope. I have tested it on 4bpp and above. No monochrome.
> If you look at slow_imageblit() current implementation and for example
> let's say that at the begining of the function we have:
>
> - __LITTLE_ENDIAN is defined
> - bpp = 1
> - fgcolor = 1
> - bgcolor = 0
> - start_index = 0
>
> The function core can be simplified into:
>
> for (i = image->height; i--; ) {
> shift = val = 0;
> l = 8;
> j = image->width;
> dst = (u32 __iomem *) dst1;
> s = src;
>
> while (j--) {
> l--;
> color = (*s & (1 << l)) ? 1 : 0;
> val |= color << shift;
> /* Did the bitshift spill bits
> to the next long? */
> if (shift >= null_bits) {
> FB_WRITEL(val, dst++);
> val = (shift == null_bits) ? 0 :
> FB_SHIFT_LOW(color,32 - shift);
> }
> shift += 1;
> shift &= (32 - 1);
> if (!l) { l = 8; s++; };
> }
>
> [ ...]
>
> Doesn't this bit of code do a bit reversal ? Specially these 2
> following lines of code:
>
> color = (*s & (1 << l)) ? 1 : 0;
> val |= color << shift;
>
>
> with 'l' taking values from 7 to 0, and 'shift' taking values from 0
> to 31.
Lets look at the new code that I have done with your above parameters.
for (i = image->height; i--; ) {
shift = val = 0;
n = image->width;
dst = (u32 __iomem *) dst1;
while (n--) {
if (!s) { src++; s = 32; }
s -= 1;
color = (swapb32p(src) & (1 << s)) ? 1 : 0;
val |= color << shift;
/* Did the bitshift spill bits to the next long? */
if (shift >= 31) {
FB_WRITEL(val, dst++);
val = (shift == 31) ? 0 :(color >> (32 - shift));
}
shift += 1;
shift &= (32 - 1);
}
[ ...]
with 's' taking values from 31 to 0, and 'shift' taking values from 0 to
31. In the case of bits_per_pixel = 1 we have
s -= 1;
color = (swapb32p(src) & (1 << s)) ? 1 : 0;
val |= color << shift;
which reduces to val = color;
I'm I seeing it wrong? BTW what is your visual?
James Simmons wrote:
> Lets look at the new code that I have done with your above parameters.
>
> for (i = image->height; i--; ) {
> shift = val = 0;
> n = image->width;
> dst = (u32 __iomem *) dst1;
>
> while (n--) {
> if (!s) { src++; s = 32; }
> s -= 1;
> color = (swapb32p(src) & (1 << s)) ? 1 : 0;
> val |= color << shift;
>
> /* Did the bitshift spill bits to the next long? */
> if (shift >= 31) {
> FB_WRITEL(val, dst++);
> val = (shift == 31) ? 0 :(color >> (32 - shift));
> }
> shift += 1;
> shift &= (32 - 1);
> }
>
> [ ...]
>
> with 's' taking values from 31 to 0, and 'shift' taking values from 0 to
> 31. In the case of bits_per_pixel = 1 we have
>
> s -= 1;
> color = (swapb32p(src) & (1 << s)) ? 1 : 0;
> val |= color << shift;
I suppose here that you meant 'swab32p' instead of 'swapb32p'. I can't
find any definition of 'swapb32p' and in your last patch you sent you
is using 'swab32p' which converts a 32-bits little endian word into a
32-bits big endian one.
>
> which reduces to val = color;
>
> I'm I seeing it wrong?
Well, I would say yes you are. If src = { 0xa3, 0x30, 0xef, 0x72 ...}
swab32p(src) -> 0x72ef30a3 -> 01110010 11101111 00110000 10100011
during loop #1 (s=31, shift=0):
color = 0x72ef30a3 & (1<<31) ? 1 : 0; color is 0
val |= 0 << 0; val is 0
during loop #2 (s=30, shift=1):
color = 0x72ef30a3 & (1<<30) ? 1 : 0; color is 1
val |= 1 << 1; val is 2
during loop #3 (s=29, shift=2):
color = 0x72ef30a3 & (1<<29) ? 1 : 0; color is 1
val |= 1 << 2; val is 6
during loop #4 (s=28, shift=3):
color = 0x72ef30a3 & (1<<28) ? 1 : 0; color is 1
val |= 1 << 3; val is 0xe
during loop #5 (s=27, shift=4):
color = 0x72ef30a3 & (1<<27) ? 1 : 0; color is 0
val |= 0 << 4; val is 0xe
during loop #6 (s=26, shift=5):
color = 0x72ef30a3 & (1<<26) ? 1 : 0; color is 0
val |= 0 << 5; val is 0xe
during loop #7 (s=25, shift=6):
color = 0x72ef30a3 & (1<<25) ? 1 : 0; color is 1
val |= 1 << 6; val is 0x4e
during loop #8 (s=24, shift=7):
color = 0x72ef30a3 & (1<<24) ? 1 : 0; color is 0
val |= 0 << 7; val is 0x4e
and so on...
Finally val -> 11000101 00001100 11110111 01001110 -> 0xc50cf74e
and FB_WRITEL(val, dst++) will write { 0x4e, 0xf7, 0x0c, 0xc5} into
the frame buffer.
Am I seeing it wrong ?
> BTW what is your visual?
>
FYI, I give you all screen info, maybe something is miss-initialized...
static struct fb_fix_screeninfo t6963c_fb_fix __initdata = {
.id = DRIVER_NAME,
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_MONO01,
.accel = FB_ACCEL_NONE,
};
static struct fb_var_screeninfo t6963c_fb_var __initdata = {
.bits_per_pixel = 1,
.red = {0, 1, 0},
.green = {0, 1, 0},
.blue = {0, 1, 0},
.transp = {0, 0, 0},
.activate = FB_ACTIVATE_NOW,
.height = -1, /* height of picture in mm */
.width = -1, /* width of picture in mm */
.accel_flags = 0,
.vmode = FB_VMODE_NONINTERLACED,
};
Franck
I'm going away for the weekend so you wouldn't here from me but I will
look over the code and test it out.
On Wed, 22 Nov 2006, Franck Bui-Huu wrote:
> James Simmons wrote:
> > Lets look at the new code that I have done with your above parameters.
> >
> > for (i = image->height; i--; ) {
> > shift = val = 0;
> > n = image->width;
> > dst = (u32 __iomem *) dst1;
> >
> > while (n--) {
> > if (!s) { src++; s = 32; }
> > s -= 1;
> > color = (swapb32p(src) & (1 << s)) ? 1 : 0;
> > val |= color << shift;
> >
> > /* Did the bitshift spill bits to the next long? */
> > if (shift >= 31) {
> > FB_WRITEL(val, dst++);
> > val = (shift == 31) ? 0 :(color >> (32 - shift));
> > }
> > shift += 1;
> > shift &= (32 - 1);
> > }
> >
> > [ ...]
> >
> > with 's' taking values from 31 to 0, and 'shift' taking values from 0 to
> > 31. In the case of bits_per_pixel = 1 we have
> >
> > s -= 1;
> > color = (swapb32p(src) & (1 << s)) ? 1 : 0;
> > val |= color << shift;
>
> I suppose here that you meant 'swab32p' instead of 'swapb32p'. I can't
> find any definition of 'swapb32p' and in your last patch you sent you
> is using 'swab32p' which converts a 32-bits little endian word into a
> 32-bits big endian one.
>
> >
> > which reduces to val = color;
> >
> > I'm I seeing it wrong?
>
> Well, I would say yes you are. If src = { 0xa3, 0x30, 0xef, 0x72 ...}
>
> swab32p(src) -> 0x72ef30a3 -> 01110010 11101111 00110000 10100011
>
>
> during loop #1 (s=31, shift=0):
>
> color = 0x72ef30a3 & (1<<31) ? 1 : 0; color is 0
> val |= 0 << 0; val is 0
>
> during loop #2 (s=30, shift=1):
>
> color = 0x72ef30a3 & (1<<30) ? 1 : 0; color is 1
> val |= 1 << 1; val is 2
>
> during loop #3 (s=29, shift=2):
>
> color = 0x72ef30a3 & (1<<29) ? 1 : 0; color is 1
> val |= 1 << 2; val is 6
>
> during loop #4 (s=28, shift=3):
>
> color = 0x72ef30a3 & (1<<28) ? 1 : 0; color is 1
> val |= 1 << 3; val is 0xe
>
> during loop #5 (s=27, shift=4):
>
> color = 0x72ef30a3 & (1<<27) ? 1 : 0; color is 0
> val |= 0 << 4; val is 0xe
>
> during loop #6 (s=26, shift=5):
>
> color = 0x72ef30a3 & (1<<26) ? 1 : 0; color is 0
> val |= 0 << 5; val is 0xe
>
> during loop #7 (s=25, shift=6):
>
> color = 0x72ef30a3 & (1<<25) ? 1 : 0; color is 1
> val |= 1 << 6; val is 0x4e
>
> during loop #8 (s=24, shift=7):
>
> color = 0x72ef30a3 & (1<<24) ? 1 : 0; color is 0
> val |= 0 << 7; val is 0x4e
>
> and so on...
>
> Finally val -> 11000101 00001100 11110111 01001110 -> 0xc50cf74e
>
> and FB_WRITEL(val, dst++) will write { 0x4e, 0xf7, 0x0c, 0xc5} into
> the frame buffer.
>
> Am I seeing it wrong ?
>
> > BTW what is your visual?
> >
>
> FYI, I give you all screen info, maybe something is miss-initialized...
>
> static struct fb_fix_screeninfo t6963c_fb_fix __initdata = {
> .id = DRIVER_NAME,
> .type = FB_TYPE_PACKED_PIXELS,
> .visual = FB_VISUAL_MONO01,
> .accel = FB_ACCEL_NONE,
> };
>
> static struct fb_var_screeninfo t6963c_fb_var __initdata = {
> .bits_per_pixel = 1,
> .red = {0, 1, 0},
> .green = {0, 1, 0},
> .blue = {0, 1, 0},
> .transp = {0, 0, 0},
> .activate = FB_ACTIVATE_NOW,
> .height = -1, /* height of picture in mm */
> .width = -1, /* width of picture in mm */
> .accel_flags = 0,
> .vmode = FB_VMODE_NONINTERLACED,
> };
>
> Franck
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys - and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> _______________________________________________
> Linux-fbdev-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
>
> James Simmons wrote:
> > Lets look at the new code that I have done with your above parameters.
> >
> > for (i = image->height; i--; ) {
> > shift = val = 0;
> > n = image->width;
> > dst = (u32 __iomem *) dst1;
> >
> > while (n--) {
> > if (!s) { src++; s = 32; }
> > s -= 1;
> > color = (swapb32p(src) & (1 << s)) ? 1 : 0;
Replace the below line in my patch I sent
> > val |= color << shift;
with
val <<= shift;
val |= color;
> > /* Did the bitshift spill bits to the next long? */
> > if (shift >= 31) {
> > FB_WRITEL(val, dst++);
> > val = (shift == 31) ? 0 :(color >> (32 - shift));
> > }
> > shift += 1;
> > shift &= (32 - 1);
> > }
> >
> > [ ...]
Let me know if that works.
On 11/22/06, James Simmons <[email protected]> wrote:
> Replace the below line in my patch I sent
>
> > > val |= color << shift;
>
> with
> val <<= shift;
> val |= color;
I think it can't work since shift is 0 to 31, you'll end up with 'val
<<= 31' which I don't think is what you want.
doing
val <<= 1;
make it works but it's still very fragile. Code which deals with
trailing bit seems bogus since new value of 'val' is simply discarded
here.
/* write trailing bits */
if (shift) {
u32 end_mask = (~(u32)0 << shift);
val = FB_READL(dst1);
val &= end_mask;
FB_WRITEL(val, dst1);
}
Another thing is that I don't see how very small images (for example
when image->width = 4) will be handled.
> > > /* Did the bitshift spill bits to the next long? */
> > > if (shift >= 31) {
> > > FB_WRITEL(val, dst++);
> > > val = (shift == 31) ? 0 :(color >> (32 - shift));
> > > }
> > > shift += 1;
> > > shift &= (32 - 1);
> > > }
> > >
> > > [ ...]
>
> Let me know if that works.
I'm wondering if working with 32 bits words really worth... I mean the
code is quite hard to follow because it needs to deal with endianess,
heading bits, trailings bits whereas working with 8 bits would be so
much easier, wouldn't it ? Are writings in video RAM very long ?
--
Franck
> > Replace the below line in my patch I sent
> >
> > > > val |= color << shift;
> >
> > with
> > val <<= shift;
> > val |= color;
>
> I think it can't work since shift is 0 to 31, you'll end up with 'val
> <<= 31' which I don't think is what you want.
> doing
> val <<= 1;
>
> make it works but it's still very fragile. Code which deals with
> trailing bit seems bogus since new value of 'val' is simply discarded
> here.
I'm going to test the code in depth over the next few days. I managed to
fix most of the problems with fast_imageblit. Now to fix the slow image
blit code.
> I'm wondering if working with 32 bits words really worth... I mean the
> code is quite hard to follow because it needs to deal with endianess,
> heading bits, trailings bits whereas working with 8 bits would be so
> much easier, wouldn't it ? Are writings in video RAM very long ?
Yes. We need to minimize the writes over the PCI bus. Its really really
slow.