Dear Himba,
in message 41125EF9.40607@siol.net you wrote:
CHANGELOG: Patch by Hinko Kocevar, 05 Aug 2004: Add support for bmp command and bootsplash feature under pxa Add support for ATAG_VIDEOLFB usage when LCD is used
I'm sorry, but I have to reject this patch.
--- u-boot/lib_arm/armlinux.c~xcep-lcd 2004-08-04 22:40:27.000000000 +0200 +++ u-boot/lib_arm/armlinux.c 2004-08-04 23:10:20.000000000 +0200 ... @@ -365,8 +365,16 @@ params->hdr.size = tag_size (tag_videolfb);
params->u.videolfb.lfb_base = (u32) gd->fb_base; - /* 7168 = 256*4*56/8 - actually 2 pages (8192 bytes) are allocated */ - params->u.videolfb.lfb_size = 7168; + /* 80896 = (320*240) + PAGE_SIZE; for 8bpp and not page aligned + * UPDATE: lcd_setmem is not needed to get fb working, because + * we are not relocating the code properly. 80896b includes + * palette also - see pxafb_init_mem() for (al)location! + * layout of the mem block: + * FB + * DMA descriptors + * palette + */ + params->u.videolfb.lfb_size = 80896;
I cannot accept this. So far, on the TRAB board only 7 kB of memory were allocated for the VFD buffer, and now you hard-wire 79 kB instead - more than 10 times more than before!
I agree that it's bad to have a hardwired value, but it's even worse to simply overwrite it. Your modification breaks U-Boot on the TRAB board. Please fix this.
[I recommend to make the actual value configurable in the board's config file.]
--- u-boot/common/cmd_bmp.c~xcep-lcd 2004-08-04 22:40:55.000000000 +0200 +++ u-boot/common/cmd_bmp.c 2004-08-04 23:35:37.000000000 +0200 @@ -28,6 +28,7 @@ #include <common.h> #include <bmp_layout.h> #include <command.h> +#include <linux/byteorder/little_endian.h>
You should NEVER include linux/byteorder/little_endian.h!!! Always include <asm/byteorder.h> only. Anything else is broken.
--- u-boot/cpu/pxa/pxafb.c~xcep-lcd 2004-08-04 22:40:27.000000000 +0200 +++ u-boot/cpu/pxa/pxafb.c 2004-08-04 23:34:49.000000000 +0200 @@ -34,6 +34,7 @@ #include <linux/types.h> #include <devices.h> #include <asm/arch/pxa-regs.h> +#include <linux/byteorder/little_endian.h>
Same here.
@@ -604,7 +643,7 @@ lcd_line_length = (panel_info.vl_col * NBITS (panel_info.vl_bpix)) / 8;
lcd_init (lcd_base); /* LCD initialization */ - +
PLEASE DO NOT ADD TRAILING WHITE SPACE!!!
+int lcd_display_bitmap(ulong bmp_image, int x, int y) +{ + ushort *cmap; + ushort i, j; + uchar *fb; + bmp_image_t *bmp=(bmp_image_t *)bmp_image; + uchar *bmap; + ushort padded_line; + unsigned long width, height; + unsigned colors,bpix; + unsigned long compression; + struct pxafb_info *fbi = &panel_info.pxa; + + + if (!((bmp->header.signature[0]=='B') && + (bmp->header.signature[1]=='M'))) { + printf ("Error: no valid bmp image at %lx\n", bmp_image); + return 1; + } + + width = le32_to_cpu (bmp->header.width); + height = le32_to_cpu (bmp->header.height); + colors = 1<<le16_to_cpu (bmp->header.bit_count); + compression = le32_to_cpu (bmp->header.compression); ...
This code looks like a 1:1 copy of the same function in "cpu/mpc8xx/lcd.c". Instead of duplicating the code, we should factor it out into a common source file.
Please fix these issues and resubmit.
Best regards,
Wolfgang Denk