[U-Boot] [PATCH v3] Add 16bpp BMP support
This patch adds 16bpp BMP support to the common lcd code.
Use CONFIG_BMP_16BPP and set LCD_BPP to LCD_COLOR16 to enable the code.
At the moment it's only been tested on the MIMC200 AVR32 board, but extending this to other platforms should be a simple task !!
Signed-off-by: Mark Jackson mpfj@mimc.co.uk ---
common/lcd.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index ae79051..16d6f2a 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -84,7 +84,7 @@ extern void lcd_enable (void); static void *lcd_logo (void);
-#if LCD_BPP == LCD_COLOR8 +#if (LCD_BPP == LCD_COLOR8) || (LCD_BPP == LCD_COLOR16) extern void lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue); #endif @@ -656,7 +656,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
bpix = NBITS(panel_info.vl_bpix);
- if ((bpix != 1) && (bpix != 8)) { + if ((bpix != 1) && (bpix != 8) && (bpix != 16)) { printf ("Error: %d bit/pixel mode not supported by U-Boot\n", bpix); return 1; @@ -738,17 +738,46 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y) bmap = (uchar *)bmp + le32_to_cpu (bmp->header.data_offset); fb = (uchar *) (lcd_base + (y + height - 1) * lcd_line_length + x); - for (i = 0; i < height; ++i) { - WATCHDOG_RESET(); - for (j = 0; j < width ; j++) + + switch (bpix) { + case 1: /* pass through */ + case 8: + for (i = 0; i < height; ++i) { + WATCHDOG_RESET(); + for (j = 0; j < width ; j++) #if defined(CONFIG_PXA250) || defined(CONFIG_ATMEL_LCD) - *(fb++) = *(bmap++); + *(fb++) = *(bmap++); #elif defined(CONFIG_MPC823) || defined(CONFIG_MCC200) - *(fb++)=255-*(bmap++); + *(fb++)=255-*(bmap++); #endif - bmap += (width - padded_line); - fb -= (width + lcd_line_length); - } + bmap += (width - padded_line); + fb -= (width + lcd_line_length); + } + break; + +#if defined(CONFIG_BMP_16BPP) + case 16: + for (i = 0; i < height; ++i) { + WATCHDOG_RESET(); + for (j = 0; j < width; j++) { +#if defined(CONFIG_ATMEL_LCD_BGR555) + *(fb++) = ((bmap[0] & 0x1f) << 2) | (bmap[1] & 0x03); + *(fb++) = (bmap[0] & 0xe0) | ((bmap[1] & 0x7c) >> 2); + bmap += 2; +#else + *(fb++) = *(bmap++); + *(fb++) = *(bmap++); +#endif + } + bmap += (padded_line - width) * 2; + fb -= (width * 2 + lcd_line_length); + } + break; +#endif /* CONFIG_BMP_16BPP */ + + default: + break; + };
return (0); }
Dear Mark Jackson,
In message 497F1732.6050901@mimc.co.uk you wrote:
This patch adds 16bpp BMP support to the common lcd code.
Use CONFIG_BMP_16BPP and set LCD_BPP to LCD_COLOR16 to enable the code.
At the moment it's only been tested on the MIMC200 AVR32 board, but extending this to other platforms should be a simple task !!
Signed-off-by: Mark Jackson mpfj@mimc.co.uk
common/lcd.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index ae79051..16d6f2a 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -84,7 +84,7 @@ extern void lcd_enable (void); static void *lcd_logo (void);
-#if LCD_BPP == LCD_COLOR8 +#if (LCD_BPP == LCD_COLOR8) || (LCD_BPP == LCD_COLOR16) extern void lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue); #endif @@ -656,7 +656,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
bpix = NBITS(panel_info.vl_bpix);
- if ((bpix != 1) && (bpix != 8)) {
- if ((bpix != 1) && (bpix != 8) && (bpix != 16)) { printf ("Error: %d bit/pixel mode not supported by U-Boot\n", bpix); return 1;
@@ -738,17 +738,46 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y) bmap = (uchar *)bmp + le32_to_cpu (bmp->header.data_offset); fb = (uchar *) (lcd_base + (y + height - 1) * lcd_line_length + x);
- for (i = 0; i < height; ++i) {
WATCHDOG_RESET();for (j = 0; j < width ; j++)
- switch (bpix) {
- case 1: /* pass through */
- case 8:
for (i = 0; i < height; ++i) {WATCHDOG_RESET();for (j = 0; j < width ; j++)
*(fb++) = *(bmap++);
*(fb++) = *(bmap++);
*(fb++)=255-*(bmap++);
*(fb++)=255-*(bmap++);
bmap += (width - padded_line);fb -= (width + lcd_line_length);- }
bmap += (width - padded_line);fb -= (width + lcd_line_length);}break;+#if defined(CONFIG_BMP_16BPP)
- case 16:
for (i = 0; i < height; ++i) {WATCHDOG_RESET();for (j = 0; j < width; j++) {+#if defined(CONFIG_ATMEL_LCD_BGR555)
*(fb++) = ((bmap[0] & 0x1f) << 2) | (bmap[1] & 0x03);*(fb++) = (bmap[0] & 0xe0) | ((bmap[1] & 0x7c) >> 2);bmap += 2;+#else
*(fb++) = *(bmap++);*(fb++) = *(bmap++);+#endif
}bmap += (padded_line - width) * 2;fb -= (width * 2 + lcd_line_length);
Is it intentional that you reverse padded_line and width here, i.e. you are sure it's not
bmap += (width - padded_line) * 2; ?
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Mark Jackson,
In message 497F1732.6050901@mimc.co.uk you wrote:
This patch adds 16bpp BMP support to the common lcd code.
Use CONFIG_BMP_16BPP and set LCD_BPP to LCD_COLOR16 to enable the code.
At the moment it's only been tested on the MIMC200 AVR32 board, but extending this to other platforms should be a simple task !!
Signed-off-by: Mark Jackson mpfj@mimc.co.uk
common/lcd.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index ae79051..16d6f2a 100644 --- a/common/lcd.c +++ b/common/lcd.c
<snip>
bmap += (padded_line - width) * 2;fb -= (width * 2 + lcd_line_length);Is it intentional that you reverse padded_line and width here, i.e. you are sure it's not
bmap += (width - padded_line) * 2;?
The "bmap += ..." line is to step forward to the start of the next line of bmp data, taking into account any padding bytes.
If I read the code correct, padded_line is defined as ...
padded_line = (width&0x3) ? ((width&~0x3)+4) : (width);
... so it will always be >= width. Correct ?
If so, then ...
bmap += (width - padded_line) * 2;
... will be <= 0, and so will actually step bmap back into the data you've just used, whereas ...
bmap += (padded_line - width) * 2;
... will be >= 0, and will step forward to the start of the next line as required.
Or have I misunderstood the bmp format and the existing code ?
Regards Mark
Dear Mark Jackson,
In message 49817E75.7060907@mimc.co.uk you wrote:
bmap += (padded_line - width) * 2;fb -= (width * 2 + lcd_line_length);Is it intentional that you reverse padded_line and width here, i.e. you are sure it's not
bmap += (width - padded_line) * 2;?
The "bmap += ..." line is to step forward to the start of the next line of bmp data, taking into account any padding bytes.
If I read the code correct, padded_line is defined as ...
padded_line = (width&0x3) ? ((width&~0x3)+4) : (width);
... so it will always be >= width. Correct ?
If so, then ...
bmap += (width - padded_line) * 2;
... will be <= 0, and so will actually step bmap back into the data you've just used, whereas ...
bmap += (padded_line - width) * 2;
... will be >= 0, and will step forward to the start of the next line as required.
Or have I misunderstood the bmp format and the existing code ?
I don't know - I'm just asking because the 16 bpp case is different from the 1 and 8 bpp cases where the operands are swapped.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Mark Jackson,
In message 49817E75.7060907@mimc.co.uk you wrote:
<snip>
Or have I misunderstood the bmp format and the existing code ?
I don't know - I'm just asking because the 16 bpp case is different from the 1 and 8 bpp cases where the operands are swapped.
It works for me ... is that enough ?
Mark
On Tue, 27 Jan 2009, Mark Jackson wrote:
This patch adds 16bpp BMP support to the common lcd code.
Use CONFIG_BMP_16BPP and set LCD_BPP to LCD_COLOR16 to enable the code.
At the moment it's only been tested on the MIMC200 AVR32 board, but extending this to other platforms should be a simple task !!
Signed-off-by: Mark Jackson mpfj@mimc.co.uk
It is a pity I didn't notice these patches earlier, and I didn't notice them, because it's only today that I realized that I needed to change this code too. But - for the other case 8 bit bmp on a 16 bit lcd without a colourmap support. I have implemented that, it works now, a patch will follow, but while working on it I noticed how this generic code is difficult to work with due to all the ifdefs and especially platform-specific types and code. So, looking at this your patch - do we really need the one more CONFIG_ define for CONFIG_BMP_16BPP? What are the drawbacks of adding your code unconditionally? extra 100 bytes for all configurations using LCD?
Another question - do you really need 16bpp bmp? I saw a discussion on this list, that other picture formats should not be added to U-Boot - you can easily convert any format to bmp. Are 256 colours really not enough for you? I used a real photo today as a test image, converted to an 8-bit bmp. It looked well enough on my qvga. And normally you use this lcd code to display a splashscreen, which is usually a computer-generated image, so 256 colours should suffice? Although, I am not an expert in graphical desing.
If we really add more bmp formats, we also get more combinations like of bmp / lcd:
BMP LCD 1-bit 1-bit 8-bit 1-bit 16-bit 1-bit 1-bit 8-bit ...
if we really want to go that way, maybe better break this code into several functions for different format conversions?
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0901302206180.4617@axis700.grange you wrote:
platform-specific types and code. So, looking at this your patch - do we really need the one more CONFIG_ define for CONFIG_BMP_16BPP? What are the drawbacks of adding your code unconditionally? extra 100 bytes for all configurations using LCD?
Yes.
Another question - do you really need 16bpp bmp? I saw a discussion on this list, that other picture formats should not be added to U-Boot - you can easily convert any format to bmp. Are 256 colours really not enough for you? I used a real photo today as a test image, converted to an 8-bit bmp. It looked well enough on my qvga. And normally you use this lcd code to display a splashscreen, which is usually a computer-generated image, so 256 colours should suffice? Although, I am not an expert in graphical desing.
I can understand that 8 bpp doe snot satisfy anoybode with more than just basic graphics needs.
If we really add more bmp formats, we also get more combinations like of bmp / lcd:
Not necessarily. We can always request that bitmap images match the "natural" color depth of the display. It makes no sense to send a 16 bpp image to a 1 bpp display, nor does it vice versa.
BMP LCD 1-bit 1-bit 8-bit 1-bit 16-bit 1-bit 1-bit 8-bit ...
if we really want to go that way, maybe better break this code into several functions for different format conversions?
We do NOT want to do everything that is possible, but only what is reasonable.
Best regards,
Wolfgang Denk
On Fri, 30 Jan 2009, Wolfgang Denk wrote:
If we really add more bmp formats, we also get more combinations like of bmp / lcd:
Not necessarily. We can always request that bitmap images match the "natural" color depth of the display. It makes no sense to send a 16 bpp image to a 1 bpp display, nor does it vice versa.
Hm, then I wouldn't be able to present 8bpp BMP on i.MX31 with 16bpp colours?
BMP LCD 1-bit 1-bit 8-bit 1-bit 16-bit 1-bit 1-bit 8-bit ...
if we really want to go that way, maybe better break this code into several functions for different format conversions?
We do NOT want to do everything that is possible, but only what is reasonable.
Isn't sending RGB24 image with 256 colours to a 16-bit display reasonable?
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
Wolfgang Denk wrote:
Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0901302206180.4617@axis700.grange you wrote:
platform-specific types and code. So, looking at this your patch - do we really need the one more CONFIG_ define for CONFIG_BMP_16BPP? What are the drawbacks of adding your code unconditionally? extra 100 bytes for all configurations using LCD?
Yes.
In fact, there's almost a case for adding *even more* #defines to remove the 1bpp and 8bpp code when you've #defined your board to use 16bpp.
Another question - do you really need 16bpp bmp? I saw a discussion on this list, that other picture formats should not be added to U-Boot - you can easily convert any format to bmp. Are 256 colours really not enough for you? I used a real photo today as a test image, converted to an 8-bit bmp. It looked well enough on my qvga. And normally you use this lcd code to display a splashscreen, which is usually a computer-generated image, so 256 colours should suffice? Although, I am not an expert in graphical desing.
I can understand that 8 bpp doe snot satisfy anoybode with more than just basic graphics needs.
Exactly ... in my case, I boot up linux which is also using 16bpp. My aim was to have the bootsplash image displayed by u-boot, and remain *intact* throughtout the linux boot sequence. Switching from 8bpp (in u-boot) to 16bpp (in linux) would cause some nasty screen corruption, and require the image to be re-displayed, which kind of spoils the whole concept of a boot logo.
If we really add more bmp formats, we also get more combinations like of bmp / lcd:
Not necessarily. We can always request that bitmap images match the "natural" color depth of the display. It makes no sense to send a 16 bpp image to a 1 bpp display, nor does it vice versa.
As far as I understand, U-boot was not written be some fully-fledged OS ... rather to just allow a smooth transition from power-on to "real" OS. Thus we only need to support some fairly "simple" combinations of options, but enough to keep the majority happy.
I guess up till now, 1bpp and 8bpp have been sufficient.
BMP LCD 1-bit 1-bit 8-bit 1-bit 16-bit 1-bit 1-bit 8-bit ...
if we really want to go that way, maybe better break this code into several functions for different format conversions?
We do NOT want to do everything that is possible, but only what is reasonable.
Exactly ... otherwise where do you stop ? JPG, GIF, TIFF, PNG, etc ? We're *only* meant to be showing a simply boot up image (not view lots of different sized photos or movies !!), in a very controlled environment (i.e. no "user" options ... just what the designers want / require).
Regards Mark
Mark Jackson wrote:
We do NOT want to do everything that is possible, but only what is reasonable.
Exactly ... otherwise where do you stop ? JPG, GIF, TIFF, PNG, etc ? We're *only* meant to be showing a simply boot up image (not view lots of different sized photos or movies !!), in a very controlled environment (i.e. no "user" options ... just what the designers want / require).
Why not do it even simpler? Drop BMP and generate an image matching the native format of the LCD controller at compile time :-)
Haavard
Haavard Skinnemoen wrote:
Mark Jackson wrote:
We do NOT want to do everything that is possible, but only what is reasonable.
Exactly ... otherwise where do you stop ? JPG, GIF, TIFF, PNG, etc ? We're *only* meant to be showing a simply boot up image (not view lots of different sized photos or movies !!), in a very controlled environment (i.e. no "user" options ... just what the designers want / require).
Why not do it even simpler? Drop BMP and generate an image matching the native format of the LCD controller at compile time :-)
Not sure if you're serious, but that'd reduce some of the functionality I was expecting to use.
My splashimage is stored in a particular, separate flash partition, and I'm intending to allow end-users to change the boot logo (via a userspace app ?) to customise / personalise the unit as they require (e.g. their own company logo).
Hard-coding the image would render this impossible.
Regards Mark
Mark Jackson wrote:
Haavard Skinnemoen wrote:
Mark Jackson wrote:
We do NOT want to do everything that is possible, but only what is reasonable.
Exactly ... otherwise where do you stop ? JPG, GIF, TIFF, PNG, etc ? We're *only* meant to be showing a simply boot up image (not view lots of different sized photos or movies !!), in a very controlled environment (i.e. no "user" options ... just what the designers want / require).
Why not do it even simpler? Drop BMP and generate an image matching the native format of the LCD controller at compile time :-)
Not sure if you're serious, but that'd reduce some of the functionality I was expecting to use.
Well, it was just a thought that struck me, so I'm not going to claim I considered all the pros and cons.
My splashimage is stored in a particular, separate flash partition, and I'm intending to allow end-users to change the boot logo (via a userspace app ?) to customise / personalise the unit as they require (e.g. their own company logo).
You can still do this if the userspace app knows how to generate an image in the correct format -- I'm not arguing against storing the image in a separate flash partition or anything like that. I just think it might be possible to reduce the run-time size and complexity of u-boot by being more strict about the image format.
You could also add support for PNG, JPEG and any format you want to the userspace app -- this will probably be much easier than adding similar support to u-boot itself.
Hard-coding the image would render this impossible.
Of course. But hard-coding the image _format_ isn't the same thing. In a way, we're already using a hard-coded image format, but it's one that is easy to generate for the host, not one that's easy to display by the target.
Haavard
participants (5)
-
Guennadi Liakhovetski -
Haavard Skinnemoen -
Mark Jackson -
Mark Jackson -
Wolfgang Denk