[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Rodolfo Giometti]

Signed-off-by: Rodolfo Giometti <giometti@linux.it>
---
 README           |    4 +++
 common/lcd.c     |   41 ++++++++++++++++++++++++++++++++++----
 tools/bmp_logo.c |   56 +++++++++++++++++++++++++++++++++++------------------
 3 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/README b/README
index b78ea61..207c881 100644
--- a/README
+++ b/README
@@ -2209,6 +2209,10 @@ Low Level (hardware related) configuration options:
   CFG_POCMR2_MASK_ATTRIB: (MPC826x only)
 		Overrides the default PCI memory map in cpu/mpc8260/pci.c if set.
 
+- CFG_LOGO_CMAP_MODE
+		Define the current LCD color map mode. Default value is 444 but
+		565 can also be used.
+
 - CONFIG_ETHER_ON_FEC[12]
 		Define to enable FEC[12] on a 8xx series processor.
 
diff --git a/common/lcd.c b/common/lcd.c
index eaed2ab..dfbc4d0 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -52,6 +52,10 @@
 
 #ifdef CONFIG_LCD
 
+#ifndef CFG_LOGO_CMAP_MODE
+#define CFG_LOGO_CMAP_MODE	444	/* the default */
+#endif
+
 /************************************************************************/
 /* ** FONT DATA								*/
 /************************************************************************/
@@ -62,7 +66,7 @@
 /************************************************************************/
 #ifdef CONFIG_LCD_LOGO
 # include <bmp_logo.h>		/* Get logo data, width and height	*/
-# if (CONSOLE_COLOR_WHITE >= BMP_LOGO_OFFSET)
+# if (LCD_BPP < LCD_COLOR16) && (CONSOLE_COLOR_WHITE >= BMP_LOGO_OFFSET)
 #  error Default Color Map overlaps with Logo Color Map
 # endif
 #endif
@@ -317,7 +321,11 @@ static void test_pattern (void)
 	ushort v_step = (v_max + N_BLK_VERT - 1) / N_BLK_VERT;
 	ushort h_step = (h_max + N_BLK_HOR  - 1) / N_BLK_HOR;
 	ushort v, h;
+#if LCD_BPP == LCD_COLOR16
+	ushort *pix = (ushort *)lcd_base;
+#else
 	uchar *pix = (uchar *)lcd_base;
+#endif
 
 	printf ("[LCD] Test Pattern: %d x %d [%d x %d]\n",
 		h_max, v_max, h_step, v_step);
@@ -519,10 +527,11 @@ void bitmap_plot (int x, int y)
 	volatile immap_t *immr = (immap_t *) CFG_IMMR;
 	volatile cpm8xx_t *cp = &(immr->im_cpm);
 #endif
+	uint palette;
 
 	debug ("Logo: width %d  height %d  colors %d  cmap %d\n",
 		BMP_LOGO_WIDTH, BMP_LOGO_HEIGHT, BMP_LOGO_COLORS,
-		sizeof(bmp_logo_palette)/(sizeof(ushort)));
+		sizeof(bmp_logo_palette)/(sizeof(uint)));
 
 	bmap = &bmp_logo_bitmap[0];
 	fb   = (uchar *)(lcd_base + y * lcd_line_length + x);
@@ -538,8 +547,20 @@ void bitmap_plot (int x, int y)
 		WATCHDOG_RESET();
 
 		/* Set color map */
-		for (i=0; i<(sizeof(bmp_logo_palette)/(sizeof(ushort))); ++i) {
-			ushort colreg = bmp_logo_palette[i];
+		for (i=0; i<(sizeof(bmp_logo_palette)/(sizeof(uint))); ++i) {
+			palette = bmp_logo_palette[i];
+#if CFG_LOGO_CMAP_MODE == 444
+			ushort colreg = ((palette & 0xf00000)>>12) | \
+					((palette & 0x00f000)>>8)  | \
+					((palette & 0x0000f0)>>4);
+#elif CFG_LOGO_CMAP_MODE == 565
+			ushort colreg = ((palette & 0xf80000)>>8) | \
+					((palette & 0x00fc00)>>5) | \
+					((palette & 0x0000f8)>>3);
+#else
+# error "Unsupported CMAP mode"
+#endif
+
 #ifdef  CFG_INVERT_COLORS
 			*cmap++ = 0xffff - colreg;
 #else
@@ -559,8 +580,18 @@ void bitmap_plot (int x, int y)
 		fb16 = (ushort *)(lcd_base + y * lcd_line_length + x);
 		for (i=0; i<BMP_LOGO_HEIGHT; ++i) {
 			for (j=0; j<BMP_LOGO_WIDTH; j++) {
-				fb16[j] = bmp_logo_palette[(bmap[j])];
+				palette = bmp_logo_palette[bmap[j]-BMP_LOGO_OFFSET];
+#if CFG_LOGO_CMAP_MODE == 444
+				fb16[j] = ((palette & 0xf00000)>>12) | \
+					  ((palette & 0x00f000)>>8)  | \
+					  ((palette & 0x0000f0)>>4);
+#elif CFG_LOGO_CMAP_MODE == 565
+				fb16[j] = ((palette & 0xf80000)>>8) | \
+					  ((palette & 0x00fc00)>>5) | \
+					  ((palette & 0x0000f8)>>3);
+#endif
 				}
+
 			bmap += BMP_LOGO_WIDTH;
 			fb16 += panel_info.vl_col;
 		}
diff --git a/tools/bmp_logo.c b/tools/bmp_logo.c
index 98be617..cc1663a 100644
--- a/tools/bmp_logo.c
+++ b/tools/bmp_logo.c
@@ -14,7 +14,6 @@
 typedef struct bitmap_s {		/* bitmap description */
 	uint16_t width;
 	uint16_t height;
-	uint8_t	palette[256*3];
 	uint8_t	*data;
 } bitmap_t;
 
@@ -42,11 +41,12 @@ void skip_bytes (FILE *fp, int n)
 
 int main (int argc, char *argv[])
 {
-	int	i, x;
+	int	i, x, d;
 	FILE	*fp;
 	bitmap_t bmp;
 	bitmap_t *b = &bmp;
-	uint16_t data_offset, n_colors;
+	uint16_t data_offset, bit_count, n_colors;
+	uint32_t red, green, blue;
 
 	if (argc < 2) {
 		fprintf (stderr, "Usage: %s file\n", argv[0]);
@@ -73,7 +73,9 @@ int main (int argc, char *argv[])
 	fread (&b->width,   sizeof (uint16_t), 1, fp);
 	skip_bytes (fp, 2);
 	fread (&b->height,  sizeof (uint16_t), 1, fp);
-	skip_bytes (fp, 22);
+	skip_bytes (fp, 4);
+	fread (&bit_count,  sizeof (uint16_t), 1, fp);
+	skip_bytes (fp, 16);
 	fread (&n_colors, sizeof (uint16_t), 1, fp);
 	skip_bytes (fp, 6);
 
@@ -83,10 +85,20 @@ int main (int argc, char *argv[])
 	data_offset = le_short(data_offset);
 	b->width = le_short(b->width);
 	b->height = le_short(b->height);
+	bit_count = le_short(bit_count);
 	n_colors = le_short(n_colors);
 
-	/* assume we are working with an 8-bit file */
-	if ((n_colors == 0) || (n_colors > 256 - DEFAULT_CMAP_SIZE)) {
+	/*
+	 * Sanity checks.
+	 */
+	if (bit_count != 8) {
+		fprintf (stderr, "%s is not a 8bpp image.\n", argv[1]);
+		exit (EXIT_FAILURE);
+	}
+
+	if (n_colors == 0)
+		n_colors = 1 << bit_count;
+	if (n_colors > 256 - DEFAULT_CMAP_SIZE) {
 		/* reserve DEFAULT_CMAP_SIZE color map entries for default map */
 		n_colors = 256 - DEFAULT_CMAP_SIZE;
 	}
@@ -115,25 +127,26 @@ int main (int argc, char *argv[])
 	}
 
 	/* read and print the palette information */
-	printf ("unsigned short bmp_logo_palette[] = {\n");
+	printf ("unsigned int bmp_logo_palette[] = {\n");
 
 	for (i=0; i<n_colors; ++i) {
-		b->palette[(int)(i*3+2)] = fgetc(fp);
-		b->palette[(int)(i*3+1)] = fgetc(fp);
-		b->palette[(int)(i*3+0)] = fgetc(fp);
-		x=fgetc(fp);
+		blue  = fgetc(fp);
+		green = fgetc(fp);
+		red   = fgetc(fp);
+		x     = fgetc(fp);
 
-		printf ("%s0x0%X%X%X,%s",
+		printf ("%s0x%06X,%s",
 			((i%8) == 0) ? "\t" : "  ",
-			(b->palette[(int)(i*3+0)] >> 4) & 0x0F,
-			(b->palette[(int)(i*3+1)] >> 4) & 0x0F,
-			(b->palette[(int)(i*3+2)] >> 4) & 0x0F,
+			(red<<16) | (green<<8) | blue,
 			((i%8) == 7) ? "\n" : ""
 		);
 	}
 
 	/* seek to offset indicated by file header */
-	fseek(fp, (long)data_offset, SEEK_SET);
+	if (fseek(fp, (long)data_offset, SEEK_SET) < 0) {
+		fprintf (stderr, "cannot fseek!\n");
+		exit (EXIT_FAILURE);
+	}
 
 	/* read the bitmap; leave room for default color map */
 	printf ("\n");
@@ -142,8 +155,13 @@ int main (int argc, char *argv[])
 	printf ("unsigned char bmp_logo_bitmap[] = {\n");
 	for (i=(b->height-1)*b->width; i>=0; i-=b->width) {
 		for (x = 0; x < b->width; x++) {
-			b->data[(uint16_t) i + x] = (uint8_t) fgetc (fp) \
-						+ DEFAULT_CMAP_SIZE;
+			d = fgetc (fp);
+			if (feof(fp)) {
+				fprintf (stderr, "unaspected end-of-file!\n");
+				exit (EXIT_FAILURE);
+			}
+
+			*(b->data + i + x) = d + DEFAULT_CMAP_SIZE;
 		}
 	}
 	fclose (fp);
@@ -152,7 +170,7 @@ int main (int argc, char *argv[])
 		if ((i%8) == 0)
 			putchar ('\t');
 		printf ("0x%02X,%c",
-			b->data[i],
+			*(b->data + i),
 			((i%8) == 7) ? '\n' : ' '
 		);
 	}
-- 
1.5.1.1

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Wolfgang Denk]

In message <20070525153644.GA2253@enneenne.com> you wrote:
>
> +- CFG_LOGO_CMAP_MODE
> +		Define the current LCD color map mode. Default value is 444 but
> +		565 can also be used.
> +

Is the color map code really only relevant for the  logo  display?  I
tend to believe thatit has an overall effect and will - for example -
have impact on other bitmap files, too ?

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: wd at denx.de
Uncertain fortune is thoroughly mastered by the equity of the  calcu-
lation.                                               - Blaise Pascal

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Rodolfo Giometti]

On Fri, May 25, 2007 at 05:47:19PM +0200, Wolfgang Denk wrote:
> In message <20070525153644.GA2253@enneenne.com> you wrote:
> >
> > +- CFG_LOGO_CMAP_MODE
> > +		Define the current LCD color map mode. Default value is 444 but
> > +		565 can also be used.
> > +
> 
> Is the color map code really only relevant for the  logo  display?  I
> tend to believe thatit has an overall effect and will - for example -
> have impact on other bitmap files, too ?

This modification affects only function bitmap_plot() which is called
by lcd_logo() only if CONFIG_LCD_LOGO is defined.

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Wolfgang Denk]

In message <20070525162154.GK21180@enneenne.com> you wrote:
>
> > > +- CFG_LOGO_CMAP_MODE
> > > +		Define the current LCD color map mode. Default value is 444 but
> > > +		565 can also be used.
> > > +
> > 
> > Is the color map code really only relevant for the  logo  display?  I
> > tend to believe thatit has an overall effect and will - for example -
> > have impact on other bitmap files, too ?
> 
> This modification affects only function bitmap_plot() which is called
> by lcd_logo() only if CONFIG_LCD_LOGO is defined.

What about lcd_display_bitmap() and the color map it uses?

Why do we have two functions lcd_display_bitmap() and  bitmap_plot(*)
in  the  first  place? I feel both do very similar things? Maybe only
one function is needed?

[Please negotiate this also with Wolfgang Grandegger!]

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: wd at denx.de
"We don't have to protect the environment -- the Second Coming is  at
hand."                                                   - James Watt

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Rodolfo Giometti]

On Fri, May 25, 2007 at 06:53:52PM +0200, Wolfgang Denk wrote:
> 
> What about lcd_display_bitmap() and the color map it uses?

This is used for the splash screen... to be honest I don't know which
is the difference against the logo but so it is...

> Why do we have two functions lcd_display_bitmap() and  bitmap_plot(*)
> in  the  first  place? I feel both do very similar things? Maybe only
> one function is needed?

I think so...

> [Please negotiate this also with Wolfgang Grandegger!]

Ok. Currently I only used bitmap_plot() since, in my opinion, is more
easy to manage...

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Wolfgang Grandegger]

Hi Rodolfo,

sorry for the late answer.

Rodolfo Giometti wrote:
> On Fri, May 25, 2007 at 06:53:52PM +0200, Wolfgang Denk wrote:
>> What about lcd_display_bitmap() and the color map it uses?
> 
> This is used for the splash screen... to be honest I don't know which
> is the difference against the logo but so it is...
> 
>> Why do we have two functions lcd_display_bitmap() and  bitmap_plot(*)
>> in  the  first  place? I feel both do very similar things? Maybe only
>> one function is needed?
> 
> I think so...

Yes, it should be possible handle both with one drawing function.

>> [Please negotiate this also with Wolfgang Grandegger!]
> 
> Ok. Currently I only used bitmap_plot() since, in my opinion, is more
> easy to manage...

One limitation of bitmap_plot, and also display_bitmap is, that it does 
not work for back&white displays, even if the latter function is used 
somehow for black&white display, e.g. with ugly hacks for MCC200. I 
would like to have generic drawing functions for 1, 8 and 16 bpp.

Wolfgang.

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Rodolfo Giometti]

On Wed, May 30, 2007 at 09:50:55AM +0200, Wolfgang Grandegger wrote:
> Hi Rodolfo,
> 
> sorry for the late answer.

Don't worry about it I'm a little busy too. :)

> One limitation of bitmap_plot, and also display_bitmap is, that it does 
> not work for back&white displays, even if the latter function is used 
> somehow for black&white display, e.g. with ugly hacks for MCC200. I 
> would like to have generic drawing functions for 1, 8 and 16 bpp.

My suggestion is to remove completely the logo support and rewrite the
function:

   int lcd_display_bitmap(ulong bmp_image, int x, int y)

in order to support, in a easy configurable way, all bpp LCD
resolutions.

On my side I can write and test the code for 16bpp LCD on a PXA2xx but
I need help and support for other resolutions and CPUs! It could be
acceptable to write a first version of such function for my hardware
and then adding the other support even if it could break current board
logo/splash support?

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Wolfgang Grandegger]

Hi Rodolfo,

some more comments.

Rodolfo Giometti wrote:
> On Wed, May 30, 2007 at 09:50:55AM +0200, Wolfgang Grandegger wrote:
>> Hi Rodolfo,
>>
>> sorry for the late answer.
> 
> Don't worry about it I'm a little busy too. :)
> 
>> One limitation of bitmap_plot, and also display_bitmap is, that it does 
>> not work for back&white displays, even if the latter function is used 
>> somehow for black&white display, e.g. with ugly hacks for MCC200. I 
>> would like to have generic drawing functions for 1, 8 and 16 bpp.
> 
> My suggestion is to remove completely the logo support and rewrite the
> function:
> 
>    int lcd_display_bitmap(ulong bmp_image, int x, int y)
> 
> in order to support, in a easy configurable way, all bpp LCD
> resolutions.
> 
> On my side I can write and test the code for 16bpp LCD on a PXA2xx but
> I need help and support for other resolutions and CPUs! It could be
> acceptable to write a first version of such function for my hardware
> and then adding the other support even if it could break current board
> logo/splash support?

Well, LCD support is broken partly for various boards anyhow. Actually 
it's peppered with hacks and errors :-(. Here some examples:

- lcd_display_bitmap() does not work for black&white.

- lcd_setcolreg() is wrong for MPC823 and PXA. The color arguments are 8
   bit values (0..ff) and must be converted properly for the color table
   of the device. e.g the MPC823 has only 16 values per color.

- #ifdef mess.

- The common code should be hardware _independent_.

- CFG_INVERT_COLORS should be removed like in the Linux driver.

I also have just a MPC823 with various panels for testing but we should 
at least fix the obvious things. I'm currently working on various 
issues, especially to support dynamic LCD controller configuration in 
U-Boot, requiring substantial modifications anyhow. Nevertheless, I'm 
not willing to spend too much time on this driver, because it's for old 
hardware. It's not interesting for new projects.

Wolfgang.

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Rodolfo Giometti]

On Wed, May 30, 2007 at 02:28:06PM +0200, Wolfgang Grandegger wrote:

> I also have just a MPC823 with various panels for testing but we should 
> at least fix the obvious things. I'm currently working on various 
> issues, especially to support dynamic LCD controller configuration in 
> U-Boot, requiring substantial modifications anyhow. Nevertheless, I'm 
> not willing to spend too much time on this driver, because it's for old 
> hardware. It's not interesting for new projects.

So what do you suggest to do in order to fix up and reorder this part?

If I start removing logo support and fixing up the splash management
it could be useful for someone? :)

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Wolfgang Grandegger]

Rodolfo Giometti wrote:
> On Wed, May 30, 2007 at 02:28:06PM +0200, Wolfgang Grandegger wrote:
> 
>> I also have just a MPC823 with various panels for testing but we should 
>> at least fix the obvious things. I'm currently working on various 
>> issues, especially to support dynamic LCD controller configuration in 
>> U-Boot, requiring substantial modifications anyhow. Nevertheless, I'm 
>> not willing to spend too much time on this driver, because it's for old 
>> hardware. It's not interesting for new projects.
> 
> So what do you suggest to do in order to fix up and reorder this part?
> 
> If I start removing logo support and fixing up the splash management
> it could be useful for someone? :)

I suggest, that you wait for my first patch as a base of discussion and 
further improvement.

Thanks.

Wolfgang.

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Rodolfo Giometti]

On Wed, May 30, 2007 at 04:35:37PM +0200, Wolfgang Grandegger wrote:
> 
> I suggest, that you wait for my first patch as a base of discussion and 
> further improvement.

Ok! :)

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127

[U-Boot-Users] [PATCH] LOGO: Adds LCD color map 565 support.

[Wolfgang Grandegger]

Rodolfo Giometti wrote:
> On Wed, May 30, 2007 at 04:35:37PM +0200, Wolfgang Grandegger wrote:
>> I suggest, that you wait for my first patch as a base of discussion and 
>> further improvement.

I realized that there is already for 565 support in 
drivers/cfb_console.c. I think it makes sense to enhance this driver 
with support for 1 bit per pixel (black&white) and add drivers for 
MPC823 and PXA with a clean interface to the framebuffer hardware.

What do you think.

Wolfgang.