[U-Boot] [PATCH 2/7] common lcd: simplify #ifdefs
Anatolij Gustschin
agust at denx.de
Fri Jun 8 14:52:31 CEST 2012
Hi,
On Thu, 24 May 2012 14:42:39 +0300
Igor Grinberg <grinberg at compulab.co.il> wrote:
> From: Nikita Kiryanov <nikita at compulab.co.il>
>
> Simplify #ifdefs by slightly changing the order of operations
>
> Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg at 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
More information about the U-Boot
mailing list