[U-Boot] [PATCH V2 0/4] common/lcd cleanup
This patch series attempts to simplify #ifdef complexity in common/lcd.c.
It was compile tested on Arm and PowerPC using MAKEALL
checkpatch reports warnings on some of the patches: 0003: WARNING: use of volatile is usually wrong Since 'volatile' was in the original code I left it in the patch as well.
Changes in V2: - Rebased on u-boot-video - patches 2 and 3 of original patchset dropped because I'm not sure what to do about them - simplify lcd_logo: used bitmap_display() to further simplify code - simplify lcd_display_bitmap: fixed pointer increment error - simplify lcd_display_bitmap: change to simplify lcd_logo breaks MCC200 board because it does not #define CONFIG_CMD_BMP. Added a local implementation of bitmap_display().
Nikita Kiryanov (4): common lcd: simplify lcd_logo common lcd: simplify lcd_display common lcd: simplify core functions common lcd: simplify lcd_display_bitmap
board/mcc200/lcd.c | 20 +++++++ common/lcd.c | 141 +++++++++++++++++++++++++++------------------------- 2 files changed, 94 insertions(+), 67 deletions(-)
Simplify lcd_logo by extracting bmp unzip into its own function.
Signed-off-by: Nikita Kiryanov nikita@compulab.co.il --- Changes in V2: - used bitmap_display() to further simplify code
common/lcd.c | 12 +----------- 1 files changed, 1 insertions(+), 11 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index 506a138..8890635 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -842,17 +842,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) + if (bmp_display(addr, x, y) == 0) return (void *)lcd_base; } #endif /* CONFIG_SPLASH_SCREEN */
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 8890635..4a5c8d5 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) @@ -722,15 +738,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)
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 --- common/lcd.c | 58 ++++++++++++++++++++++++++++------------------------------ 1 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index 4a5c8d5..3c0f1b1 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -498,21 +498,35 @@ static int lcd_getbgcolor(void) /************************************************************************/ /* ** Chipset depending Bitmap / Logo stuff... */ /************************************************************************/ +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 @@ -525,20 +539,17 @@ 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 */ -#if defined(CONFIG_CPU_PXA) - cmap = (ushort *) fbi->palette; -#elif defined(CONFIG_MPC823) + /* Leave room for default color map + * default case: generic system with no cmap (most likely 16bpp) + * cmap was set to the source palette, so no change is done. + * This avoids even more ifdefs in the next stanza + */ +#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 - /* - * 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; + cmap = configuration_get_cmap(); #endif
WATCHDOG_RESET(); @@ -639,12 +650,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 +687,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 */
Dear Nikita Kiryanov,
In message 1340607844-8718-4-git-send-email-nikita@compulab.co.il you wrote:
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
common/lcd.c | 58 ++++++++++++++++++++++++++++------------------------------ 1 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index 4a5c8d5..3c0f1b1 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -498,21 +498,35 @@ static int lcd_getbgcolor(void) /************************************************************************/ /* ** Chipset depending Bitmap / Logo stuff... */ /************************************************************************/ +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 +}
Please fix and use I/O accessors instead of volatile pointers.
Best regards,
Wolfgang Denk
On 06/25/2012 10:23 AM, Wolfgang Denk wrote:
Dear Nikita Kiryanov,
In message 1340607844-8718-4-git-send-email-nikita@compulab.co.il you wrote:
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
common/lcd.c | 58 ++++++++++++++++++++++++++++------------------------------ 1 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index 4a5c8d5..3c0f1b1 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -498,21 +498,35 @@ static int lcd_getbgcolor(void) /************************************************************************/ /* ** Chipset depending Bitmap / Logo stuff... */ /************************************************************************/ +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 +}
Please fix and use I/O accessors instead of volatile pointers.
Best regards,
Wolfgang Denk
The goal of this patch set was to reduce #define complexity in common/lcd. Do you mind if I address the volatile pointers with a follow up patch?
-- Nikita
Move highly platform dependant code into its own functions to reduce the number of #ifdefs in lcd_display_bitmap
To avoid breaking the mcc200 board which does not #define CONFIG_CMD_BMP, this patch also implements bmp_display() for mcc200.
Signed-off-by: Nikita Kiryanov nikita@compulab.co.il --- Changes in V2: - A change in a previous patch broke mcc200 board which does not #define CONFIG_CMD_BMP. Added a local implementation of bitmap_display()
board/mcc200/lcd.c | 20 ++++++++++++++++++++ common/lcd.c | 44 +++++++++++++++++++++++++++----------------- 2 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/board/mcc200/lcd.c b/board/mcc200/lcd.c index d8f754c..893f4b7 100644 --- a/board/mcc200/lcd.c +++ b/board/mcc200/lcd.c @@ -21,6 +21,7 @@ #include <common.h> #include <lcd.h> #include <mpc5xxx.h> +#include <malloc.h>
#ifdef CONFIG_LCD
@@ -210,4 +211,23 @@ void show_progress (int size, int tot) }
#endif + +int bmp_display(ulong addr, int x, int y) +{ + int ret; + bmp_image_t *bmp = (bmp_image_t *)addr; + + if (!bmp) { + printf("There is no valid bmp file at the given address\n"); + return 1; + } + + ret = lcd_display_bitmap((ulong)bmp, x, y); + + if ((unsigned long)bmp != addr) + free(bmp); + + return ret; +} + #endif /* CONFIG_LCD */ diff --git a/common/lcd.c b/common/lcd.c index 3c0f1b1..e55b457 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -636,6 +636,29 @@ static void splash_align_axis(int *axis, unsigned long panel_size, } #endif
+#if defined CONFIG_CPU_PXA || defined(CONFIG_ATMEL_LCD) +#define FB_PUT_BYTE(fb, from) *(fb)++ = *(from)++ +#elif defined(CONFIG_MPC823) || defined(CONFIG_MCC200) +#define FB_PUT_BYTE(fb, from) *(fb)++ = (255 - *(from)++) +#endif + +#if defined(CONFIG_BMP_16BPP) +#if defined(CONFIG_ATMEL_LCD_BGR555) +static inline void fb_put_word(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 fb_put_word(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 + FB_PUT_BYTE(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++) + fb_put_word(&fb, &bmap); + bmap += (padded_line - width) * 2; fb -= (width * 2 + lcd_line_length); }
participants (2)
-
Nikita Kiryanov -
Wolfgang Denk