Hi,

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

> From: Nikita Kiryanov <nik...@compulab.co.il>
> 
> Simplify #ifdefs by slightly changing the order of operations
> 
> Signed-off-by: Nikita Kiryanov <nik...@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinb...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to