Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-02-23 Thread Haavard Skinnemoen
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-02-02 Thread Haavard Skinnemoen
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-02-01 Thread Mark Jackson
Wolfgang Denk wrote:
 Dear Guennadi Liakhovetski,
 
 In message pine.lnx.4.64.0901302206180.4...@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-bit1-bit
 8-bit1-bit
 16-bit   1-bit
 1-bit8-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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-01-30 Thread Mark Jackson
Wolfgang Denk wrote:
 Dear Mark Jackson,
 
 In message 49817e75.7060...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-01-30 Thread Wolfgang Denk
Dear Guennadi Liakhovetski,

In message pine.lnx.4.64.0901302206180.4...@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-bit1-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

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
A Freudian slip is when you say one thing but mean your mother.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-01-30 Thread Guennadi Liakhovetski
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: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-01-29 Thread Mark Jackson
Wolfgang Denk wrote:
 Dear Mark Jackson,
 
 In message 497f1732.6050...@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 m...@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 = (width0x3) ? ((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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-01-28 Thread Wolfgang Denk
Dear Mark Jackson,

In message 497f1732.6050...@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 m...@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);

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

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
If you're not part of the solution, you're part of the problem.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot