[U-Boot] [PATCH 0/7] common/lcd cleanup

[U-Boot] [PATCH 0/7] common/lcd cleanup

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

This patch series attempts to simplify #ifdef complexity in common/lcd.c. It 
preceeds my future work of adding splash screen support for omap.

It was compile tested on Arm and PowerPC using MAKEALL, and is dependant on the
following patches:
http://patchwork.ozlabs.org/patch/155491/
http://patchwork.ozlabs.org/patch/155492/
http://patchwork.ozlabs.org/patch/155493/
http://patchwork.ozlabs.org/patch/158179/

checkpatch reports warnings on some of the patches:
0001: line over 80 chars
	Since the original line was over 80 characters already, and there's no
	good way of shortening it, I left it over 80 chars.
0006: WARNING: use of volatile is usually wrong
	Since 'volatile' was in the original code I left it in the patch as
	well.

Nikita Kiryanov (7):
  common lcd: minor coding style changes
  common lcd: simplify #ifdefs
  common lcd: simplify bitmap_plot
  common lcd: simplify lcd_logo
  common lcd: simplify lcd_display
  common lcd: simplify core functions
  common lcd: simplify lcd_display_bitmap

 common/lcd.c |  422 +++++++++++++++++++++++++++++++---------------------------
 1 files changed, 226 insertions(+), 196 deletions(-)

[U-Boot] [PATCH 1/7] common lcd: minor coding style changes

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

No functional changes

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
checkpatch reports a warning:
0001: line over 80 chars
	Since the original line was over 80 characters already, and there's no
	good way of shortening it, I left it over 80 chars.

 common/lcd.c |  245 +++++++++++++++++++++++++++++-----------------------------
 1 files changed, 124 insertions(+), 121 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 85c6cf4..506a138 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -76,42 +76,42 @@ DECLARE_GLOBAL_DATA_PTR;
 
 ulong lcd_setmem (ulong addr);
 
-static void lcd_drawchars (ushort x, ushort y, uchar *str, int count);
-static inline void lcd_puts_xy (ushort x, ushort y, uchar *s);
-static inline void lcd_putc_xy (ushort x, ushort y, uchar  c);
+static void lcd_drawchars(ushort x, ushort y, uchar *str, int count);
+static inline void lcd_puts_xy(ushort x, ushort y, uchar *s);
+static inline void lcd_putc_xy(ushort x, ushort y, uchar  c);
 
-static int lcd_init (void *lcdbase);
+static int lcd_init(void *lcdbase);
 
 static void *lcd_logo (void);
 
-static int lcd_getbgcolor (void);
-static void lcd_setfgcolor (int color);
-static void lcd_setbgcolor (int color);
+static int lcd_getbgcolor(void);
+static void lcd_setfgcolor(int color);
+static void lcd_setbgcolor(int color);
 
 char lcd_is_enabled = 0;
 
 #ifdef	NOT_USED_SO_FAR
-static void lcd_getcolreg (ushort regno,
+static void lcd_getcolreg(ushort regno,
 				ushort *red, ushort *green, ushort *blue);
-static int lcd_getfgcolor (void);
+static int lcd_getfgcolor(void);
 #endif	/* NOT_USED_SO_FAR */
 
 /************************************************************************/
 
 /*----------------------------------------------------------------------*/
 
-static void console_scrollup (void)
+static void console_scrollup(void)
 {
 	/* Copy up rows ignoring the first one */
-	memcpy (CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, CONSOLE_SCROLL_SIZE);
+	memcpy(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND, CONSOLE_SCROLL_SIZE);
 
 	/* Clear the last one */
-	memset (CONSOLE_ROW_LAST, COLOR_MASK(lcd_color_bg), CONSOLE_ROW_SIZE);
+	memset(CONSOLE_ROW_LAST, COLOR_MASK(lcd_color_bg), CONSOLE_ROW_SIZE);
 }
 
 /*----------------------------------------------------------------------*/
 
-static inline void console_back (void)
+static inline void console_back(void)
 {
 	if (--console_col < 0) {
 		console_col = CONSOLE_COLS-1 ;
@@ -120,14 +120,13 @@ static inline void console_back (void)
 		}
 	}
 
-	lcd_putc_xy (console_col * VIDEO_FONT_WIDTH,
-		     console_row * VIDEO_FONT_HEIGHT,
-		     ' ');
+	lcd_putc_xy(console_col * VIDEO_FONT_WIDTH,
+		console_row * VIDEO_FONT_HEIGHT, ' ');
 }
 
 /*----------------------------------------------------------------------*/
 
-static inline void console_newline (void)
+static inline void console_newline(void)
 {
 	++console_row;
 	console_col = 0;
@@ -135,61 +134,62 @@ static inline void console_newline (void)
 	/* Check if we need to scroll the terminal */
 	if (console_row >= CONSOLE_ROWS) {
 		/* Scroll everything up */
-		console_scrollup () ;
+		console_scrollup();
 		--console_row;
 	}
 }
 
 /*----------------------------------------------------------------------*/
 
-void lcd_putc (const char c)
+void lcd_putc(const char c)
 {
 	if (!lcd_is_enabled) {
 		serial_putc(c);
+
 		return;
 	}
 
 	switch (c) {
-	case '\r':	console_col = 0;
-			return;
+	case '\r':
+		console_col = 0;
 
-	case '\n':	console_newline();
-			return;
+		return;
+	case '\n':
+		console_newline();
 
+		return;
 	case '\t':	/* Tab (8 chars alignment) */
-			console_col +=  8;
-			console_col &= ~7;
+		console_col +=  8;
+		console_col &= ~7;
 
-			if (console_col >= CONSOLE_COLS) {
-				console_newline();
-			}
-			return;
+		if (console_col >= CONSOLE_COLS)
+			console_newline();
 
-	case '\b':	console_back();
-			return;
+		return;
+	case '\b':
+		console_back();
 
-	default:	lcd_putc_xy (console_col * VIDEO_FONT_WIDTH,
-				     console_row * VIDEO_FONT_HEIGHT,
-				     c);
-			if (++console_col >= CONSOLE_COLS) {
-				console_newline();
-			}
-			return;
+		return;
+	default:
+		lcd_putc_xy(console_col * VIDEO_FONT_WIDTH,
+			console_row * VIDEO_FONT_HEIGHT, c);
+		if (++console_col >= CONSOLE_COLS)
+			console_newline();
 	}
-	/* NOTREACHED */
 }
 
 /*----------------------------------------------------------------------*/
 
-void lcd_puts (const char *s)
+void lcd_puts(const char *s)
 {
 	if (!lcd_is_enabled) {
-		serial_puts (s);
+		serial_puts(s);
+
 		return;
 	}
 
 	while (*s) {
-		lcd_putc (*s++);
+		lcd_putc(*s++);
 	}
 }
 
@@ -211,7 +211,7 @@ void lcd_printf(const char *fmt, ...)
 /* ** Low-Level Graphics Routines					*/
 /************************************************************************/
 
-static void lcd_drawchars (ushort x, ushort y, uchar *str, int count)
+static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
 {
 	uchar *dest;
 	ushort row;
@@ -226,7 +226,7 @@ static void lcd_drawchars (ushort x, ushort y, uchar *str, int count)
 
 	dest = (uchar *)(lcd_base + y * lcd_line_length + x * (1 << LCD_BPP) / 8);
 
-	for (row=0;  row < VIDEO_FONT_HEIGHT;  ++row, dest += lcd_line_length)  {
+	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
 		uchar *s = str;
 		int i;
 #if LCD_BPP == LCD_COLOR16
@@ -239,7 +239,7 @@ static void lcd_drawchars (ushort x, ushort y, uchar *str, int count)
 		uchar rest = *d & -(1 << (8-off));
 		uchar sym;
 #endif
-		for (i=0; i<count; ++i) {
+		for (i = 0; i < count; ++i) {
 			uchar c, bits;
 
 			c = *s++;
@@ -247,18 +247,18 @@ static void lcd_drawchars (ushort x, ushort y, uchar *str, int count)
 
 #if LCD_BPP == LCD_MONOCHROME
 			sym  = (COLOR_MASK(lcd_color_fg) & bits) |
-			       (COLOR_MASK(lcd_color_bg) & ~bits);
+				(COLOR_MASK(lcd_color_bg) & ~bits);
 
 			*d++ = rest | (sym >> off);
 			rest = sym << (8-off);
 #elif LCD_BPP == LCD_COLOR8
-			for (c=0; c<8; ++c) {
+			for (c = 0; c < 8; ++c) {
 				*d++ = (bits & 0x80) ?
 						lcd_color_fg : lcd_color_bg;
 				bits <<= 1;
 			}
 #elif LCD_BPP == LCD_COLOR16
-			for (c=0; c<8; ++c) {
+			for (c = 0; c < 8; ++c) {
 				*d++ = (bits & 0x80) ?
 						lcd_color_fg : lcd_color_bg;
 				bits <<= 1;
@@ -273,14 +273,14 @@ static void lcd_drawchars (ushort x, ushort y, uchar *str, int count)
 
 /*----------------------------------------------------------------------*/
 
-static inline void lcd_puts_xy (ushort x, ushort y, uchar *s)
+static inline void lcd_puts_xy(ushort x, ushort y, uchar *s)
 {
 	lcd_drawchars(x, y, s, strlen((char *)s));
 }
 
 /*----------------------------------------------------------------------*/
 
-static inline void lcd_putc_xy (ushort x, ushort y, uchar c)
+static inline void lcd_putc_xy(ushort x, ushort y, uchar c)
 {
 	lcd_drawchars(x, y, &c, 1);
 }
@@ -298,7 +298,7 @@ static int test_colors[N_BLK_HOR*N_BLK_VERT] = {
 	CONSOLE_COLOR_BLUE,	CONSOLE_COLOR_MAGENTA,	CONSOLE_COLOR_CYAN,
 };
 
-static void test_pattern (void)
+static void test_pattern(void)
 {
 	ushort v_max  = panel_info.vl_row;
 	ushort h_max  = panel_info.vl_col;
@@ -307,13 +307,13 @@ static void test_pattern (void)
 	ushort v, h;
 	uchar *pix = (uchar *)lcd_base;
 
-	printf ("[LCD] Test Pattern: %d x %d [%d x %d]\n",
+	printf("[LCD] Test Pattern: %d x %d [%d x %d]\n",
 		h_max, v_max, h_step, v_step);
 
 	/* WARNING: Code silently assumes 8bit/pixel */
-	for (v=0; v<v_max; ++v) {
+	for (v = 0; v < v_max; ++v) {
 		uchar iy = v / v_step;
-		for (h=0; h<h_max; ++h) {
+		for (h = 0; h < h_max; ++h) {
 			uchar ix = N_BLK_HOR * iy + (h/h_step);
 			*pix++ = test_colors[ix];
 		}
@@ -335,12 +335,12 @@ int drv_lcd_init (void)
 
 	lcd_line_length = (panel_info.vl_col * NBITS (panel_info.vl_bpix)) / 8;
 
-	lcd_init (lcd_base);		/* LCD initialization */
+	lcd_init(lcd_base);		/* LCD initialization */
 
 	/* Device initialization */
-	memset (&lcddev, 0, sizeof (lcddev));
+	memset(&lcddev, 0, sizeof(lcddev));
 
-	strcpy (lcddev.name, "lcd");
+	strcpy(lcddev.name, "lcd");
 	lcddev.ext   = 0;			/* No extensions */
 	lcddev.flags = DEV_FLAGS_OUTPUT;	/* Output only */
 	lcddev.putc  = lcd_putc;		/* 'putc' function */
@@ -367,35 +367,35 @@ void lcd_clear(void)
 
 #elif LCD_BPP == LCD_COLOR8
 	/* Setting the palette */
-	lcd_setcolreg  (CONSOLE_COLOR_BLACK,       0,    0,    0);
-	lcd_setcolreg  (CONSOLE_COLOR_RED,	0xFF,    0,    0);
-	lcd_setcolreg  (CONSOLE_COLOR_GREEN,       0, 0xFF,    0);
-	lcd_setcolreg  (CONSOLE_COLOR_YELLOW,	0xFF, 0xFF,    0);
-	lcd_setcolreg  (CONSOLE_COLOR_BLUE,        0,    0, 0xFF);
-	lcd_setcolreg  (CONSOLE_COLOR_MAGENTA,	0xFF,    0, 0xFF);
-	lcd_setcolreg  (CONSOLE_COLOR_CYAN,	   0, 0xFF, 0xFF);
-	lcd_setcolreg  (CONSOLE_COLOR_GREY,	0xAA, 0xAA, 0xAA);
-	lcd_setcolreg  (CONSOLE_COLOR_WHITE,	0xFF, 0xFF, 0xFF);
+	lcd_setcolreg(CONSOLE_COLOR_BLACK, 0, 0, 0);
+	lcd_setcolreg(CONSOLE_COLOR_RED, 0xFF, 0, 0);
+	lcd_setcolreg(CONSOLE_COLOR_GREEN, 0, 0xFF, 0);
+	lcd_setcolreg(CONSOLE_COLOR_YELLOW, 0xFF, 0xFF, 0);
+	lcd_setcolreg(CONSOLE_COLOR_BLUE, 0, 0, 0xFF);
+	lcd_setcolreg(CONSOLE_COLOR_MAGENTA, 0xFF, 0, 0xFF);
+	lcd_setcolreg(CONSOLE_COLOR_CYAN, 0, 0xFF, 0xFF);
+	lcd_setcolreg(CONSOLE_COLOR_GREY, 0xAA, 0xAA, 0xAA);
+	lcd_setcolreg(CONSOLE_COLOR_WHITE, 0xFF, 0xFF, 0xFF);
 #endif
 
 #ifndef CONFIG_SYS_WHITE_ON_BLACK
-	lcd_setfgcolor (CONSOLE_COLOR_BLACK);
-	lcd_setbgcolor (CONSOLE_COLOR_WHITE);
+	lcd_setfgcolor(CONSOLE_COLOR_BLACK);
+	lcd_setbgcolor(CONSOLE_COLOR_WHITE);
 #else
-	lcd_setfgcolor (CONSOLE_COLOR_WHITE);
-	lcd_setbgcolor (CONSOLE_COLOR_BLACK);
+	lcd_setfgcolor(CONSOLE_COLOR_WHITE);
+	lcd_setbgcolor(CONSOLE_COLOR_BLACK);
 #endif	/* CONFIG_SYS_WHITE_ON_BLACK */
 
 #ifdef	LCD_TEST_PATTERN
 	test_pattern();
 #else
 	/* set framebuffer to background color */
-	memset ((char *)lcd_base,
+	memset((char *)lcd_base,
 		COLOR_MASK(lcd_getbgcolor()),
 		lcd_line_length*panel_info.vl_row);
 #endif
 	/* Paint the logo and retrieve LCD base address */
-	debug ("[LCD] Drawing the logo...\n");
+	debug("[LCD] Drawing the logo...\n");
 	lcd_console_address = lcd_logo ();
 
 	console_col = 0;
@@ -410,12 +410,12 @@ U_BOOT_CMD(
 
 /*----------------------------------------------------------------------*/
 
-static int lcd_init (void *lcdbase)
+static int lcd_init(void *lcdbase)
 {
 	/* Initialize the lcd controller */
-	debug ("[LCD] Initializing LCD frambuffer at %p\n", lcdbase);
+	debug("[LCD] Initializing LCD frambuffer@%p\n", lcdbase);
 
-	lcd_ctrl_init (lcdbase);
+	lcd_ctrl_init(lcdbase);
 	lcd_is_enabled = 1;
 	lcd_clear();
 	lcd_enable ();
@@ -442,13 +442,13 @@ static int lcd_init (void *lcdbase)
  *
  * Note that this is running from ROM, so no write access to global data.
  */
-ulong lcd_setmem (ulong addr)
+ulong lcd_setmem(ulong addr)
 {
 	ulong size;
-	int line_length = (panel_info.vl_col * NBITS (panel_info.vl_bpix)) / 8;
+	int line_length = (panel_info.vl_col * NBITS(panel_info.vl_bpix)) / 8;
 
-	debug ("LCD panel info: %d x %d, %d bit/pix\n",
-		panel_info.vl_col, panel_info.vl_row, NBITS (panel_info.vl_bpix) );
+	debug("LCD panel info: %d x %d, %d bit/pix\n", panel_info.vl_col,
+		panel_info.vl_row, NBITS(panel_info.vl_bpix));
 
 	size = line_length * panel_info.vl_row;
 
@@ -458,21 +458,21 @@ ulong lcd_setmem (ulong addr)
 	/* Allocate pages for the frame buffer. */
 	addr -= size;
 
-	debug ("Reserving %ldk for LCD Framebuffer at: %08lx\n", size>>10, addr);
+	debug("Reserving %ldk for LCD Framebuffer at: %08lx\n", size>>10, addr);
 
-	return (addr);
+	return addr;
 }
 
 /*----------------------------------------------------------------------*/
 
-static void lcd_setfgcolor (int color)
+static void lcd_setfgcolor(int color)
 {
 	lcd_color_fg = color;
 }
 
 /*----------------------------------------------------------------------*/
 
-static void lcd_setbgcolor (int color)
+static void lcd_setbgcolor(int color)
 {
 	lcd_color_bg = color;
 }
@@ -480,7 +480,7 @@ static void lcd_setbgcolor (int color)
 /*----------------------------------------------------------------------*/
 
 #ifdef	NOT_USED_SO_FAR
-static int lcd_getfgcolor (void)
+static int lcd_getfgcolor(void)
 {
 	return lcd_color_fg;
 }
@@ -488,7 +488,7 @@ static int lcd_getfgcolor (void)
 
 /*----------------------------------------------------------------------*/
 
-static int lcd_getbgcolor (void)
+static int lcd_getbgcolor(void)
 {
 	return lcd_color_bg;
 }
@@ -499,7 +499,7 @@ static int lcd_getbgcolor (void)
 /* ** Chipset depending Bitmap / Logo stuff...                          */
 /************************************************************************/
 #ifdef CONFIG_LCD_LOGO
-void bitmap_plot (int x, int y)
+void bitmap_plot(int x, int y)
 {
 #ifdef CONFIG_ATMEL_LCD
 	uint *cmap;
@@ -517,7 +517,7 @@ void bitmap_plot (int x, int y)
 	volatile cpm8xx_t *cp = &(immr->im_cpm);
 #endif
 
-	debug ("Logo: width %d  height %d  colors %d  cmap %d\n",
+	debug("Logo: width %d  height %d  colors %d  cmap %d\n",
 		BMP_LOGO_WIDTH, BMP_LOGO_HEIGHT, BMP_LOGO_COLORS,
 		ARRAY_SIZE(bmp_logo_palette));
 
@@ -527,9 +527,9 @@ void bitmap_plot (int x, int y)
 	if (NBITS(panel_info.vl_bpix) < 12) {
 		/* Leave room for default color map */
 #if defined(CONFIG_CPU_PXA)
-		cmap = (ushort *)fbi->palette;
+		cmap = (ushort *) fbi->palette;
 #elif defined(CONFIG_MPC823)
-		cmap = (ushort *)&(cp->lcd_cmap[BMP_LOGO_OFFSET*sizeof(ushort)]);
+		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
 #elif defined(CONFIG_ATMEL_LCD)
 		cmap = (uint *) (panel_info.mmio + ATMEL_LCDC_LUT(0));
 #else
@@ -550,12 +550,12 @@ void bitmap_plot (int x, int y)
 			uint lut_entry;
 #ifdef CONFIG_ATMEL_LCD_BGR555
 			lut_entry = ((colreg & 0x000F) << 11) |
-				    ((colreg & 0x00F0) <<  2) |
-				    ((colreg & 0x0F00) >>  7);
+					((colreg & 0x00F0) <<  2) |
+					((colreg & 0x0F00) >>  7);
 #else /* CONFIG_ATMEL_LCD_RGB565 */
 			lut_entry = ((colreg & 0x000F) << 1) |
-				    ((colreg & 0x00F0) << 3) |
-				    ((colreg & 0x0F00) << 4);
+					((colreg & 0x00F0) << 3) |
+					((colreg & 0x0F00) << 4);
 #endif
 			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
 			cmap++;
@@ -570,8 +570,8 @@ void bitmap_plot (int x, int y)
 
 		WATCHDOG_RESET();
 
-		for (i=0; i<BMP_LOGO_HEIGHT; ++i) {
-			memcpy (fb, bmap, BMP_LOGO_WIDTH);
+		for (i = 0; i < BMP_LOGO_HEIGHT; ++i) {
+			memcpy(fb, bmap, BMP_LOGO_WIDTH);
 			bmap += BMP_LOGO_WIDTH;
 			fb   += panel_info.vl_col;
 		}
@@ -579,8 +579,8 @@ void bitmap_plot (int x, int y)
 	else { /* true color mode */
 		u16 col16;
 		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++) {
+		for (i = 0; i < BMP_LOGO_HEIGHT; ++i) {
+			for (j = 0; j < BMP_LOGO_WIDTH; j++) {
 				col16 = bmp_logo_palette[(bmap[j]-16)];
 				fb16[j] =
 					((col16 & 0x000F) << 1) |
@@ -630,14 +630,15 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	volatile cpm8xx_t *cp = &(immr->im_cpm);
 #endif
 
-	if (!((bmp->header.signature[0]=='B') &&
-		(bmp->header.signature[1]=='M'))) {
-		printf ("Error: no valid bmp image at %lx\n", bmp_image);
+	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);
+	width = le32_to_cpu(bmp->header.width);
+	height = le32_to_cpu(bmp->header.height);
 	bmp_bpix = le16_to_cpu(bmp->header.bit_count);
 	colors = 1 << bmp_bpix;
 
@@ -646,6 +647,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	if ((bpix != 1) && (bpix != 8) && (bpix != 16) && (bpix != 32)) {
 		printf ("Error: %d bit/pixel mode, but BMP has %d bit/pixel\n",
 			bpix, bmp_bpix);
+
 		return 1;
 	}
 
@@ -654,10 +656,11 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		printf ("Error: %d bit/pixel mode, but BMP has %d bit/pixel\n",
 			bpix,
 			le16_to_cpu(bmp->header.bit_count));
+
 		return 1;
 	}
 
-	debug ("Display-bmp: %d x %d  with %d colors\n",
+	debug("Display-bmp: %d x %d  with %d colors\n",
 		(int)width, (int)height, (int)colors);
 
 #if !defined(CONFIG_MCC200)
@@ -674,7 +677,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		cmap_base = cmap;
 
 		/* Set color map */
-		for (i=0; i<colors; ++i) {
+		for (i = 0; i < colors; ++i) {
 			bmp_color_table_entry_t cte = bmp->color_table[i];
 #if !defined(CONFIG_ATMEL_LCD)
 			ushort colreg =
@@ -709,8 +712,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	 * specific.
 	 */
 #if defined(CONFIG_MCC200)
-	if (bpix==1)
-	{
+	if (bpix == 1) {
 		width = ((width + 7) & ~7) >> 3;
 		x     = ((x + 7) & ~7) >> 3;
 		pwidth= ((pwidth + 7) & ~7) >> 3;
@@ -731,12 +733,12 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		y = max(0, panel_info.vl_row - height + y + 1);
 #endif /* CONFIG_SPLASH_SCREEN_ALIGN */
 
-	if ((x + width)>pwidth)
+	if ((x + width) > pwidth)
 		width = pwidth - x;
-	if ((y + height)>panel_info.vl_row)
+	if ((y + height) > panel_info.vl_row)
 		height = panel_info.vl_row - y;
 
-	bmap = (uchar *)bmp + le32_to_cpu (bmp->header.data_offset);
+	bmap = (uchar *)bmp + le32_to_cpu(bmp->header.data_offset);
 	fb   = (uchar *) (lcd_base +
 		(y + height - 1) * lcd_line_length + x * bpix / 8);
 
@@ -806,11 +808,11 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		break;
 	};
 
-	return (0);
+	return 0;
 }
 #endif
 
-static void *lcd_logo (void)
+static void *lcd_logo(void)
 {
 #ifdef CONFIG_SPLASH_SCREEN
 	char *s;
@@ -823,13 +825,15 @@ static void *lcd_logo (void)
 
 		addr = simple_strtoul (s, NULL, 16);
 #ifdef CONFIG_SPLASH_SCREEN_ALIGN
-		if ((s = getenv ("splashpos")) != NULL) {
+		s = getenv("splashpos");
+		if (s != NULL) {
 			if (s[0] == 'm')
 				x = BMP_ALIGN_CENTER;
 			else
-				x = simple_strtol (s, NULL, 0);
+				x = simple_strtol(s, NULL, 0);
 
-			if ((s = strchr (s + 1, ',')) != NULL) {
+			s = strchr(s + 1, ',');
+			if (s != NULL) {
 				if (s[1] == 'm')
 					y = BMP_ALIGN_CENTER;
 				else
@@ -842,15 +846,14 @@ static void *lcd_logo (void)
 		bmp_image_t *bmp = (bmp_image_t *)addr;
 		unsigned long len;
 
-		if (!((bmp->header.signature[0]=='B') &&
-		      (bmp->header.signature[1]=='M'))) {
+		if (!((bmp->header.signature[0] == 'B') &&
+			(bmp->header.signature[1] == 'M'))) {
 			addr = (ulong)gunzip_bmp(addr, &len);
 		}
 #endif
 
-		if (lcd_display_bitmap (addr, x, y) == 0) {
-			return ((void *)lcd_base);
-		}
+		if (lcd_display_bitmap(addr, x, y) == 0)
+			return (void *)lcd_base;
 	}
 #endif /* CONFIG_SPLASH_SCREEN */
 
@@ -863,9 +866,9 @@ static void *lcd_logo (void)
 #endif /* CONFIG_LCD_INFO */
 
 #if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
-	return ((void *)((ulong)lcd_base + BMP_LOGO_HEIGHT * lcd_line_length));
+	return (void *)((ulong)lcd_base + BMP_LOGO_HEIGHT * lcd_line_length);
 #else
-	return ((void *)lcd_base);
+	return (void *)lcd_base;
 #endif /* CONFIG_LCD_LOGO && !CONFIG_LCD_INFO_BELOW_LOGO */
 }
 
-- 
1.7.1

[U-Boot] [PATCH 2/7] common lcd: simplify #ifdefs

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

Simplify #ifdefs by slightly changing the order of operations

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 common/lcd.c |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 506a138..3b2f25f 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -525,20 +525,18 @@ void bitmap_plot(int x, int y)
 	fb   = (uchar *)(lcd_base + y * lcd_line_length + x);
 
 	if (NBITS(panel_info.vl_bpix) < 12) {
-		/* Leave room for default color map */
+		/* Leave room for default color map
+		 * default case: generic system with no cmap (most likely 16bpp)
+		 * We set cmap to the source palette, so no change is done.
+		 * This avoids even more ifdefs in the next stanza
+		 */
+		cmap = bmp_logo_palette;
 #if defined(CONFIG_CPU_PXA)
 		cmap = (ushort *) fbi->palette;
 #elif defined(CONFIG_MPC823)
 		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
 #elif defined(CONFIG_ATMEL_LCD)
 		cmap = (uint *) (panel_info.mmio + ATMEL_LCDC_LUT(0));
-#else
-		/*
-		 * default case: generic system with no cmap (most likely 16bpp)
-		 * We set cmap to the source palette, so no change is done.
-		 * This avoids even more ifdef in the next stanza
-		 */
-		cmap = bmp_logo_palette;
 #endif
 
 		WATCHDOG_RESET();
@@ -680,14 +678,12 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		for (i = 0; i < colors; ++i) {
 			bmp_color_table_entry_t cte = bmp->color_table[i];
 #if !defined(CONFIG_ATMEL_LCD)
-			ushort colreg =
+			*cmap =
 				( ((cte.red)   << 8) & 0xf800) |
 				( ((cte.green) << 3) & 0x07e0) |
 				( ((cte.blue)  >> 3) & 0x001f) ;
 #ifdef CONFIG_SYS_INVERT_COLORS
-			*cmap = 0xffff - colreg;
-#else
-			*cmap = colreg;
+			*cmap = 0xffff - *cmap;
 #endif
 #if defined(CONFIG_MPC823)
 			cmap--;
-- 
1.7.1

[U-Boot] [PATCH 3/7] common lcd: simplify bitmap_plot

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

Simplify bitmap_plot in terms of number of #ifdefs by making some of the
code into an external macro

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 common/lcd.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 3b2f25f..448e0f0 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -498,6 +498,18 @@ static int lcd_getbgcolor(void)
 /************************************************************************/
 /* ** Chipset depending Bitmap / Logo stuff...                          */
 /************************************************************************/
+#ifdef CONFIG_ATMEL_LCD
+#ifdef CONFIG_ATMEL_LCD_BGR555
+#define LUT_ENTRY(colreg) (((colreg) & 0x000F) << 11) | \
+			(((colreg) & 0x00F0) <<  2) | \
+			(((colreg) & 0x0F00) >>  7);
+#else /* CONFIG_ATMEL_LCD_RGB565 */
+#define LUT_ENTRY(colreg) (((colreg) & 0x000F) << 1) | \
+			(((colreg) & 0x00F0) <<  3) | \
+			(((colreg) & 0x0F00) >>  4);
+#endif
+#endif
+
 #ifdef CONFIG_LCD_LOGO
 void bitmap_plot(int x, int y)
 {
@@ -545,19 +557,9 @@ void bitmap_plot(int x, int y)
 		for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
 			ushort colreg = bmp_logo_palette[i];
 #ifdef CONFIG_ATMEL_LCD
-			uint lut_entry;
-#ifdef CONFIG_ATMEL_LCD_BGR555
-			lut_entry = ((colreg & 0x000F) << 11) |
-					((colreg & 0x00F0) <<  2) |
-					((colreg & 0x0F00) >>  7);
-#else /* CONFIG_ATMEL_LCD_RGB565 */
-			lut_entry = ((colreg & 0x000F) << 1) |
-					((colreg & 0x00F0) << 3) |
-					((colreg & 0x0F00) << 4);
-#endif
-			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
+			*(cmap + BMP_LOGO_OFFSET) = LUT_ENTRY(colreg);
 			cmap++;
-#else /* !CONFIG_ATMEL_LCD */
+#else
 #ifdef  CONFIG_SYS_INVERT_COLORS
 			*cmap++ = 0xffff - colreg;
 #else
-- 
1.7.1

[U-Boot] [PATCH 4/7] common lcd: simplify lcd_logo

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

Simplify lcd_logo by extracting bmp unzip into its own function.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 common/lcd.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 448e0f0..855d3df 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -810,6 +810,25 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 }
 #endif
 
+#ifdef CONFIG_VIDEO_BMP_GZIP
+static inline ulong __gunzip_bmp(ulong addr)
+{
+	bmp_image_t *bmp = (bmp_image_t *)addr;
+	unsigned long len;
+
+	if (!((bmp->header.signature[0] == 'B') &&
+			(bmp->header.signature[1] == 'M')))
+		addr = (ulong)gunzip_bmp(addr, &len);
+
+	return addr;
+}
+#else
+static inline ulong __gunzip_bmp(ulong addr)
+{
+	return addr;
+}
+#endif
+
 static void *lcd_logo(void)
 {
 #ifdef CONFIG_SPLASH_SCREEN
@@ -840,16 +859,7 @@ static void *lcd_logo(void)
 		}
 #endif /* CONFIG_SPLASH_SCREEN_ALIGN */
 
-#ifdef CONFIG_VIDEO_BMP_GZIP
-		bmp_image_t *bmp = (bmp_image_t *)addr;
-		unsigned long len;
-
-		if (!((bmp->header.signature[0] == 'B') &&
-			(bmp->header.signature[1] == 'M'))) {
-			addr = (ulong)gunzip_bmp(addr, &len);
-		}
-#endif
-
+		addr = __gunzip_bmp(addr);
 		if (lcd_display_bitmap(addr, x, y) == 0)
 			return (void *)lcd_base;
 	}
-- 
1.7.1

[U-Boot] [PATCH 5/7] common lcd: simplify lcd_display

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

Simplify lcd_display by centralizing code into a funciton

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 common/lcd.c |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 855d3df..3292e42 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -607,6 +607,22 @@ static inline void bitmap_plot(int x, int y) {}
 
 #ifdef CONFIG_SPLASH_SCREEN_ALIGN
 #define BMP_ALIGN_CENTER	0x7FFF
+
+static void splash_align_axis(int *axis, unsigned long panel_size,
+					unsigned long picture_size)
+{
+	unsigned long panel_picture_delta = panel_size - picture_size;
+	unsigned long axis_alignment;
+
+	if (*axis == BMP_ALIGN_CENTER)
+		axis_alignment = panel_picture_delta / 2;
+	else if (*axis < 0)
+		axis_alignment = panel_picture_delta + *axis + 1;
+	else
+		return;
+
+	*axis = max(0, axis_alignment);
+}
 #endif
 
 int lcd_display_bitmap(ulong bmp_image, int x, int y)
@@ -720,15 +736,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	padded_line = (width&0x3) ? ((width&~0x3)+4) : (width);
 
 #ifdef CONFIG_SPLASH_SCREEN_ALIGN
-	if (x == BMP_ALIGN_CENTER)
-		x = max(0, (pwidth - width) / 2);
-	else if (x < 0)
-		x = max(0, pwidth - width + x + 1);
-
-	if (y == BMP_ALIGN_CENTER)
-		y = max(0, (panel_info.vl_row - height) / 2);
-	else if (y < 0)
-		y = max(0, panel_info.vl_row - height + y + 1);
+	splash_align_axis(&x, pwidth, width);
+	splash_align_axis(&y, panel_info.vl_row, height);
 #endif /* CONFIG_SPLASH_SCREEN_ALIGN */
 
 	if ((x + width) > pwidth)
-- 
1.7.1

[U-Boot] [PATCH 6/7] common lcd: simplify core functions

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

Move highly platform dependant code into its own function to reduce the
number of #ifdefs in the bigger functions

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
checkpatch reports a warning:
0006: WARNING: use of volatile is usually wrong
	Since 'volatile' was in the original code I left it in the patch as
	well.

 common/lcd.c |   50 +++++++++++++++++++++++++-------------------------
 1 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 3292e42..199a8c2 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -510,21 +510,35 @@ static int lcd_getbgcolor(void)
 #endif
 #endif
 
+static inline ushort *configuration_get_cmap(void)
+{
+#if defined CONFIG_CPU_PXA
+	struct pxafb_info *fbi = &panel_info.pxa;
+	return (ushort *)fbi->palette;
+#elif defined(CONFIG_MPC823)
+	volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
+	volatile cpm8xx_t *cp = &(immr->im_cpm);
+	return (ushort *)&(cp->lcd_cmap[255 * sizeof(ushort)]);
+#elif defined(CONFIG_ATMEL_LCD)
+	return (ushort *)(panel_info.mmio + ATMEL_LCDC_LUT(0));
+#else
+	return (ushort *)panel_info.cmap;
+#endif
+}
+
 #ifdef CONFIG_LCD_LOGO
 void bitmap_plot(int x, int y)
 {
 #ifdef CONFIG_ATMEL_LCD
-	uint *cmap;
+	uint *cmap = (uint *)bmp_logo_palette;
 #else
-	ushort *cmap;
+	ushort *cmap = (ushort *)bmp_logo_palette;
 #endif
 	ushort i, j;
 	uchar *bmap;
 	uchar *fb;
 	ushort *fb16;
-#if defined(CONFIG_CPU_PXA)
-	struct pxafb_info *fbi = &panel_info.pxa;
-#elif defined(CONFIG_MPC823)
+#if defined(CONFIG_MPC823)
 	volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
 	volatile cpm8xx_t *cp = &(immr->im_cpm);
 #endif
@@ -539,16 +553,15 @@ void bitmap_plot(int x, int y)
 	if (NBITS(panel_info.vl_bpix) < 12) {
 		/* Leave room for default color map
 		 * default case: generic system with no cmap (most likely 16bpp)
-		 * We set cmap to the source palette, so no change is done.
+		 * cmap was set to the source palette, so no change is done.
 		 * This avoids even more ifdefs in the next stanza
 		 */
-		cmap = bmp_logo_palette;
-#if defined(CONFIG_CPU_PXA)
-		cmap = (ushort *) fbi->palette;
-#elif defined(CONFIG_MPC823)
+#if defined(CONFIG_MPC823)
 		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
 #elif defined(CONFIG_ATMEL_LCD)
-		cmap = (uint *) (panel_info.mmio + ATMEL_LCDC_LUT(0));
+		cmap = (uint *)configuration_get_cmap();
+#else
+		cmap = configuration_get_cmap();
 #endif
 
 		WATCHDOG_RESET();
@@ -639,12 +652,6 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	unsigned long width, height, byte_width;
 	unsigned long pwidth = panel_info.vl_col;
 	unsigned colors, bpix, bmp_bpix;
-#if defined(CONFIG_CPU_PXA)
-	struct pxafb_info *fbi = &panel_info.pxa;
-#elif defined(CONFIG_MPC823)
-	volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
-	volatile cpm8xx_t *cp = &(immr->im_cpm);
-#endif
 
 	if (!((bmp->header.signature[0] == 'B') &&
 		(bmp->header.signature[1] == 'M'))) {
@@ -682,14 +689,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 #if !defined(CONFIG_MCC200)
 	/* MCC200 LCD doesn't need CMAP, supports 1bpp b&w only */
 	if (bmp_bpix == 8) {
-#if defined(CONFIG_CPU_PXA)
-		cmap = (ushort *)fbi->palette;
-#elif defined(CONFIG_MPC823)
-		cmap = (ushort *)&(cp->lcd_cmap[255*sizeof(ushort)]);
-#elif !defined(CONFIG_ATMEL_LCD) && !defined(CONFIG_EXYNOS_FB)
-		cmap = panel_info.cmap;
-#endif
-
+		cmap = configuration_get_cmap();
 		cmap_base = cmap;
 
 		/* Set color map */
-- 
1.7.1

[U-Boot] [PATCH 7/7] common lcd: simplify lcd_display_bitmap

[Igor Grinberg]

From: Nikita Kiryanov <nikita@compulab.co.il>

Move highly platform dependant code into its own functions to reduce the
number of #ifdefs in lcd_display_bitmap

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 common/lcd.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index 199a8c2..a55ee58 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -638,6 +638,29 @@ static void splash_align_axis(int *axis, unsigned long panel_size,
 }
 #endif
 
+#if defined CONFIG_CPU_PXA || defined(CONFIG_ATMEL_LCD)
+#define CONFIGURATION_FB_PUTB(fb, from) *(fb)++ = *(from)++
+#elif defined(CONFIG_MPC823) || defined(CONFIG_MCC200)
+#define CONFIGURATION_FB_PUTB(fb, from) *(fb)++ = (255 - *(from)++)
+#endif
+
+#if defined(CONFIG_BMP_16BPP)
+#if defined(CONFIG_ATMEL_LCD_BGR555)
+static inline void configuration_fb_puts(uchar *fb, uchar *from)
+{
+	*(fb++) = ((from[0] & 0x1f) << 2) | (from[1] & 0x03);
+	*(fb++) = (from[0] & 0xe0) | ((from[1] & 0x7c) >> 2);
+	from += 2;
+}
+#else
+static inline void configuration_fb_puts(uchar *fb, uchar *from)
+{
+	*(fb++) = *(from++);
+	*(fb++) = *(from++);
+}
+#endif
+#endif /* CONFIG_BMP_16BPP */
+
 int lcd_display_bitmap(ulong bmp_image, int x, int y)
 {
 #if !defined(CONFIG_MCC200)
@@ -761,11 +784,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 			WATCHDOG_RESET();
 			for (j = 0; j < width; j++) {
 				if (bpix != 16) {
-#if defined(CONFIG_CPU_PXA) || defined(CONFIG_ATMEL_LCD)
-					*(fb++) = *(bmap++);
-#elif defined(CONFIG_MPC823) || defined(CONFIG_MCC200)
-					*(fb++) = 255 - *(bmap++);
-#endif
+					CONFIGURATION_FB_PUTB(fb, bmap);
 				} else {
 					*(uint16_t *)fb = cmap_base[*(bmap++)];
 					fb += sizeof(uint16_t) / sizeof(*fb);
@@ -780,18 +799,9 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	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
-			}
+			for (j = 0; j < width; j++)
+				configuration_fb_puts(fb, bmap);
+
 			bmap += (padded_line - width) * 2;
 			fb   -= (width * 2 + lcd_line_length);
 		}
-- 
1.7.1

[U-Boot] [PATCH 0/7] common/lcd cleanup

[Igor Grinberg]

gentle ping...

On 05/24/12 14:42, Igor Grinberg wrote:
> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> This patch series attempts to simplify #ifdef complexity in common/lcd.c. It 
> preceeds my future work of adding splash screen support for omap.
> 
> It was compile tested on Arm and PowerPC using MAKEALL, and is dependant on the
> following patches:
> http://patchwork.ozlabs.org/patch/155491/
> http://patchwork.ozlabs.org/patch/155492/
> http://patchwork.ozlabs.org/patch/155493/
> http://patchwork.ozlabs.org/patch/158179/
> 
> checkpatch reports warnings on some of the patches:
> 0001: line over 80 chars
> 	Since the original line was over 80 characters already, and there's no
> 	good way of shortening it, I left it over 80 chars.
> 0006: WARNING: use of volatile is usually wrong
> 	Since 'volatile' was in the original code I left it in the patch as
> 	well.
> 
> Nikita Kiryanov (7):
>   common lcd: minor coding style changes
>   common lcd: simplify #ifdefs
>   common lcd: simplify bitmap_plot
>   common lcd: simplify lcd_logo
>   common lcd: simplify lcd_display
>   common lcd: simplify core functions
>   common lcd: simplify lcd_display_bitmap
> 
>  common/lcd.c |  422 +++++++++++++++++++++++++++++++---------------------------
>  1 files changed, 226 insertions(+), 196 deletions(-)
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

-- 
Regards,
Igor.

[U-Boot] [PATCH 2/7] common lcd: simplify #ifdefs

[Anatolij Gustschin]

Hi,

On Thu, 24 May 2012 14:42:39 +0300
Igor Grinberg <grinberg@compulab.co.il> wrote:

> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> Simplify #ifdefs by slightly changing the order of operations
> 
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  common/lcd.c |   20 ++++++++------------
>  1 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index 506a138..3b2f25f 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -525,20 +525,18 @@ void bitmap_plot(int x, int y)
>  	fb   = (uchar *)(lcd_base + y * lcd_line_length + x);
>  
>  	if (NBITS(panel_info.vl_bpix) < 12) {
> -		/* Leave room for default color map */
> +		/* Leave room for default color map
> +		 * default case: generic system with no cmap (most likely 16bpp)
> +		 * We set cmap to the source palette, so no change is done.
> +		 * This avoids even more ifdefs in the next stanza
> +		 */
> +		cmap = bmp_logo_palette;
>  #if defined(CONFIG_CPU_PXA)
>  		cmap = (ushort *) fbi->palette;
>  #elif defined(CONFIG_MPC823)
>  		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
>  #elif defined(CONFIG_ATMEL_LCD)
>  		cmap = (uint *) (panel_info.mmio + ATMEL_LCDC_LUT(0));
> -#else
> -		/*
> -		 * default case: generic system with no cmap (most likely 16bpp)
> -		 * We set cmap to the source palette, so no change is done.
> -		 * This avoids even more ifdef in the next stanza
> -		 */
> -		cmap = bmp_logo_palette;
>  #endif              
>  
>  		WATCHDOG_RESET();
> @@ -680,14 +678,12 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
>  		for (i = 0; i < colors; ++i) {
>  			bmp_color_table_entry_t cte = bmp->color_table[i];
>  #if !defined(CONFIG_ATMEL_LCD)
> -			ushort colreg =
> +			*cmap =
>  				( ((cte.red)   << 8) & 0xf800) |
>  				( ((cte.green) << 3) & 0x07e0) |
>  				( ((cte.blue)  >> 3) & 0x001f) ;
>  #ifdef CONFIG_SYS_INVERT_COLORS
> -			*cmap = 0xffff - colreg;
> -#else
> -			*cmap = colreg;
> +			*cmap = 0xffff - *cmap;
>  #endif
>  #if defined(CONFIG_MPC823)
>  			cmap--;

Sorry, but I do not like this change. We should not try to simplify it
this way. Better would be to just reduce all this color map setting code
to something like:

        for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
                bmp_color_table_entry_t cte = bmp->color_table[i];
                lcd_setcolreg(i, cte.red, cte.green, cte.blue);
        }

and fix lcd_setcolreg() functions of the drivers if/where needed.

Thanks,

Anatolij

[U-Boot] [PATCH 3/7] common lcd: simplify bitmap_plot

[Anatolij Gustschin]

Hi,

On Thu, 24 May 2012 14:42:40 +0300
Igor Grinberg <grinberg@compulab.co.il> wrote:

> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> Simplify bitmap_plot in terms of number of #ifdefs by making some of the
> code into an external macro
> 
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  common/lcd.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index 3b2f25f..448e0f0 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -498,6 +498,18 @@ static int lcd_getbgcolor(void)
>  /************************************************************************/
>  /* ** Chipset depending Bitmap / Logo stuff...                          */
>  /************************************************************************/
> +#ifdef CONFIG_ATMEL_LCD
> +#ifdef CONFIG_ATMEL_LCD_BGR555
> +#define LUT_ENTRY(colreg) (((colreg) & 0x000F) << 11) | \
> +			(((colreg) & 0x00F0) <<  2) | \
> +			(((colreg) & 0x0F00) >>  7);
> +#else /* CONFIG_ATMEL_LCD_RGB565 */
> +#define LUT_ENTRY(colreg) (((colreg) & 0x000F) << 1) | \
> +			(((colreg) & 0x00F0) <<  3) | \
> +			(((colreg) & 0x0F00) >>  4);
> +#endif
> +#endif
> +
>  #ifdef CONFIG_LCD_LOGO
>  void bitmap_plot(int x, int y)
>  {
> @@ -545,19 +557,9 @@ void bitmap_plot(int x, int y)
>  		for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
>  			ushort colreg = bmp_logo_palette[i];
>  #ifdef CONFIG_ATMEL_LCD
> -			uint lut_entry;
> -#ifdef CONFIG_ATMEL_LCD_BGR555
> -			lut_entry = ((colreg & 0x000F) << 11) |
> -					((colreg & 0x00F0) <<  2) |
> -					((colreg & 0x0F00) >>  7);
> -#else /* CONFIG_ATMEL_LCD_RGB565 */
> -			lut_entry = ((colreg & 0x000F) << 1) |
> -					((colreg & 0x00F0) << 3) |
> -					((colreg & 0x0F00) << 4);
> -#endif
> -			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
> +			*(cmap + BMP_LOGO_OFFSET) = LUT_ENTRY(colreg);
>  			cmap++;
> -#else /* !CONFIG_ATMEL_LCD */
> +#else
>  #ifdef  CONFIG_SYS_INVERT_COLORS
>  			*cmap++ = 0xffff - colreg;
>  #else

Here it should be possible to use lcd_setcolreg() in the for loop,
too. I would prefer this approach instead of adding LUT_ENTRY macro.

Thanks,

Anatolij

[U-Boot] [PATCH 7/7] common lcd: simplify lcd_display_bitmap

[Anatolij Gustschin]

Hi,

On Thu, 24 May 2012 14:42:44 +0300
Igor Grinberg <grinberg@compulab.co.il> wrote:

> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> Move highly platform dependant code into its own functions to reduce the
> number of #ifdefs in lcd_display_bitmap
> 
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  common/lcd.c |   44 +++++++++++++++++++++++++++-----------------
>  1 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index 199a8c2..a55ee58 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -638,6 +638,29 @@ static void splash_align_axis(int *axis, unsigned long panel_size,
...
> +#if defined(CONFIG_BMP_16BPP)
> +#if defined(CONFIG_ATMEL_LCD_BGR555)
> +static inline void configuration_fb_puts(uchar *fb, uchar *from)
> +{
> +	*(fb++) = ((from[0] & 0x1f) << 2) | (from[1] & 0x03);
> +	*(fb++) = (from[0] & 0xe0) | ((from[1] & 0x7c) >> 2);
> +	from += 2;
> +}
> +#else
> +static inline void configuration_fb_puts(uchar *fb, uchar *from)
> +{
> +	*(fb++) = *(from++);
> +	*(fb++) = *(from++);
> +}
> +#endif
> +#endif /* CONFIG_BMP_16BPP */

This won't work. The original code increments 'fb' and 'bmap' pointers
in the inner for loop. Using this function in the inner loop won't
increment the pointers as needed, as these will only be incremented in
the function itself (as local variables).

Also please use a different name for the macro, CONFIGURATION_FB_PUTB
isn't a descriptive name. FB_PUT_PIXEL or similar perhaps?

Thanks,

Anatolij

[U-Boot] [PATCH 1/7] common lcd: minor coding style changes

[Anatolij Gustschin]

Hi,

On Thu, 24 May 2012 14:42:38 +0300
Igor Grinberg <grinberg@compulab.co.il> wrote:

> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> No functional changes
> 
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> checkpatch reports a warning:
> 0001: line over 80 chars
> 	Since the original line was over 80 characters already, and there's no
> 	good way of shortening it, I left it over 80 chars.
> 
>  common/lcd.c |  245 +++++++++++++++++++++++++++++-----------------------------
>  1 files changed, 124 insertions(+), 121 deletions(-)

Applied to u-boot-video/master. Thanks!

Anatolij

[U-Boot] [PATCH 4/7] common lcd: simplify lcd_logo

[Anatolij Gustschin]

Hi,

On Thu, 24 May 2012 14:42:41 +0300
Igor Grinberg <grinberg@compulab.co.il> wrote:

> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> Simplify lcd_logo by extracting bmp unzip into its own function.
> 
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  common/lcd.c |   30 ++++++++++++++++++++----------
>  1 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index 448e0f0..855d3df 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -810,6 +810,25 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
>  }
>  #endif
>  
> +#ifdef CONFIG_VIDEO_BMP_GZIP
> +static inline ulong __gunzip_bmp(ulong addr)
> +{
> +	bmp_image_t *bmp = (bmp_image_t *)addr;
> +	unsigned long len;
> +
> +	if (!((bmp->header.signature[0] == 'B') &&
> +			(bmp->header.signature[1] == 'M')))
> +		addr = (ulong)gunzip_bmp(addr, &len);
> +
> +	return addr;
> +}
> +#else
> +static inline ulong __gunzip_bmp(ulong addr)
> +{
> +	return addr;
> +}
> +#endif
> +
>  static void *lcd_logo(void)
>  {
>  #ifdef CONFIG_SPLASH_SCREEN
> @@ -840,16 +859,7 @@ static void *lcd_logo(void)
>  		}
>  #endif /* CONFIG_SPLASH_SCREEN_ALIGN */
>  
> -#ifdef CONFIG_VIDEO_BMP_GZIP
> -		bmp_image_t *bmp = (bmp_image_t *)addr;
> -		unsigned long len;
> -
> -		if (!((bmp->header.signature[0] == 'B') &&
> -			(bmp->header.signature[1] == 'M'))) {
> -			addr = (ulong)gunzip_bmp(addr, &len);
> -		}
> -#endif
> -
> +		addr = __gunzip_bmp(addr);
>  		if (lcd_display_bitmap(addr, x, y) == 0)
>  			return (void *)lcd_base;

We can simplify it even more by using existing code here:

                if (bmp_display(addr, x, y) == 0)
                        return (void *)lcd_base;

bmp_display() will check and gunzip if needed.

Thanks,

Anatolij

[U-Boot] [PATCH 2/7] common lcd: simplify #ifdefs

[Nikita Kiryanov]

On 06/08/2012 03:52 PM, Anatolij Gustschin wrote:
 > Hi,
 >
 > On Thu, 24 May 2012 14:42:39 +0300
 > Igor Grinberg<grinberg@compulab.co.il>  wrote:
 >
 >> From: Nikita Kiryanov<nikita@compulab.co.il>
 >>
 >> Simplify #ifdefs by slightly changing the order of operations
 >>
 >> Signed-off-by: Nikita Kiryanov<nikita@compulab.co.il>
 >> Signed-off-by: Igor Grinberg<grinberg@compulab.co.il>
 >> ---
 >>   common/lcd.c |   20 ++++++++------------
 >>   1 files changed, 8 insertions(+), 12 deletions(-)
 >>
 >> diff --git a/common/lcd.c b/common/lcd.c
 >> index 506a138..3b2f25f 100644
 >> --- a/common/lcd.c
 >> +++ b/common/lcd.c
 >> @@ -525,20 +525,18 @@ void bitmap_plot(int x, int y)
 >>   	fb   = (uchar *)(lcd_base + y * lcd_line_length + x);
 >>
 >>   	if (NBITS(panel_info.vl_bpix)<  12) {
 >> -		/* Leave room for default color map */
 >> +		/* Leave room for default color map
 >> +		 * default case: generic system with no cmap (most likely 16bpp)
 >> +		 * We set cmap to the source palette, so no change is done.
 >> +		 * This avoids even more ifdefs in the next stanza
 >> +		 */
 >> +		cmap = bmp_logo_palette;
 >>   #if defined(CONFIG_CPU_PXA)
 >>   		cmap = (ushort *) fbi->palette;
 >>   #elif defined(CONFIG_MPC823)
 >>   		cmap = (ushort *)&(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
 >>   #elif defined(CONFIG_ATMEL_LCD)
 >>   		cmap = (uint *) (panel_info.mmio + ATMEL_LCDC_LUT(0));
 >> -#else
 >> -		/*
 >> -		 * default case: generic system with no cmap (most likely 16bpp)
 >> -		 * We set cmap to the source palette, so no change is done.
 >> -		 * This avoids even more ifdef in the next stanza
 >> -		 */
 >> -		cmap = bmp_logo_palette;
 >>   #endif
 >>
 >>   		WATCHDOG_RESET();
 >> @@ -680,14 +678,12 @@ int lcd_display_bitmap(ulong bmp_image, int x, 
int y)
 >>   		for (i = 0; i<  colors; ++i) {
 >>   			bmp_color_table_entry_t cte = bmp->color_table[i];
 >>   #if !defined(CONFIG_ATMEL_LCD)
 >> -			ushort colreg =
 >> +			*cmap =
 >>   				( ((cte.red)<<  8)&  0xf800) |
 >>   				( ((cte.green)<<  3)&  0x07e0) |
 >>   				( ((cte.blue)>>  3)&  0x001f) ;
 >>   #ifdef CONFIG_SYS_INVERT_COLORS
 >> -			*cmap = 0xffff - colreg;
 >> -#else
 >> -			*cmap = colreg;
 >> +			*cmap = 0xffff - *cmap;
 >>   #endif
 >>   #if defined(CONFIG_MPC823)
 >>   			cmap--;
 >
 > Sorry, but I do not like this change. We should not try to simplify it
 > this way. Better would be to just reduce all this color map setting code
 > to something like:
 >
 >          for (i = 0; i<  ARRAY_SIZE(bmp_logo_palette); ++i) {
 >                  bmp_color_table_entry_t cte = bmp->color_table[i];
 >                  lcd_setcolreg(i, cte.red, cte.green, cte.blue);
 >          }
 >
 > and fix lcd_setcolreg() functions of the drivers if/where needed.
 >
 > Thanks,
 >
 > Anatolij

This does sound like the way to go, but having looked at the different
versions of lcd_setcolreg I am unsure of how to consolidate the code in
lcd_display_bitmap (and bitmap_plot in patch 3) with the
implementations I see in the drivers.

For one, they shift the color values in a different way than the code
in common/lcd, and I'm not sure who I should listen to, what's in the
drivers or common/lcd...
Also, I see that lcd_setcolreg is used in lcd_clear, where it sets
console colors. This raises the question can/should the treatment of
bmp color map values be similar to that of console color values?

I'd appreciate some guidance on these issues.

Thanks,
Nikita