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

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(-)

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 at %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 */ }

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

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--;

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

On 06/08/2012 03:52 PM, Anatolij Gustschin wrote:
Hi,
On Thu, 24 May 2012 14:42:39 +0300 Igor Grinberggrinberg@compulab.co.il wrote:
From: Nikita Kiryanovnikita@compulab.co.il
Simplify #ifdefs by slightly changing the order of operations
Signed-off-by: Nikita Kiryanovnikita@compulab.co.il Signed-off-by: Igor Grinberggrinberg@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
*/
#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));cmap = bmp_logo_palette;
-#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 =
#ifdef CONFIG_SYS_INVERT_COLORS*cmap = ( ((cte.red)<< 8)& 0xf800) | ( ((cte.green)<< 3)& 0x07e0) | ( ((cte.blue)>> 3)& 0x001f) ;
*cmap = 0xffff - colreg;
-#else
*cmap = colreg;
#endif #if defined(CONFIG_MPC823) cmap--;*cmap = 0xffff - *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

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

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

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; }

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
if (lcd_display_bitmap(addr, x, y) == 0) return (void *)lcd_base;addr = __gunzip_bmp(addr);
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

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)

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 */

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); }

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

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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (3)
-
Anatolij Gustschin
-
Igor Grinberg
-
Nikita Kiryanov