[U-Boot] [PATCH 13/13] lcd: split splash code into its own function

[Simon Glass]

Hi Nikita,

On 29 January 2015 at 04:21, Nikita Kiryanov <nikita at compulab.co.il> wrote:
> lcd_logo() currently performs tasks well beyond just displaying the logo.
> It has code which displays splash image, it has logic which determines
> when the different display features are displayed, and it is coupled with
> the lcd console because it holds the responsibility of returning the
> lcd console base address.
>
> Make lcd_logo() just about the logo by:
> * Moving splash image display code into a dedicated function
> * Moving the logic regarding when various features are displayed to
>   lcd_clear() (which is arguably not the correct name for housing such
>   code either, but it is currently the most fitting location code wise)
> * Move the responsibility of setting the console base address to
>   lcd_clear() too.
>
> Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> Cc: Bo Shen <voice.shen at atmel.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Anatolij Gustschin <agust at denx.de>
> ---
>  common/lcd.c     | 52 ++++++++++++++++++++++------------------------------
>  common/splash.c  | 16 ++++++++++++++++
>  include/splash.h | 11 ++++++++++-
>  3 files changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/common/lcd.c b/common/lcd.c
> index d0c0aff..f33942c 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -46,7 +46,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>
>  static int lcd_init(void *lcdbase);
> -static void *lcd_logo(void);
> +static void lcd_logo(void);
>  static void lcd_setfgcolor(int color);
>  static void lcd_setbgcolor(int color);
>
> @@ -169,6 +169,9 @@ void lcd_clear(void)
>  {
>         short console_rows, console_cols;
>         int bg_color;
> +       char *s;
> +       ulong addr;
> +       static int do_splash = 1;
>  #if LCD_BPP == LCD_COLOR8
>         /* Setting the palette */
>         lcd_setcolreg(CONSOLE_COLOR_BLACK, 0, 0, 0);
> @@ -218,7 +221,23 @@ void lcd_clear(void)
>  #endif
>         console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
>         lcd_init_console(lcd_base, console_rows, console_cols);
> -       lcd_init_console(lcd_logo(), console_rows, console_cols);
> +       if (do_splash) {
> +               s = getenv("splashimage");
> +               if (s) {
> +                       do_splash = 0;
> +                       addr = simple_strtoul(s, NULL, 16);
> +                       if (lcd_splash(addr) == 0) {
> +                               lcd_sync();
> +                               return;
> +                       }
> +               }
> +       }
> +
> +       lcd_logo();
> +#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
> +       addr = (ulong)lcd_base + BMP_LOGO_HEIGHT * lcd_line_length;
> +       lcd_init_console((void *)addr, console_rows, console_cols);

I'm just a bit unsure about this - before this function was always
called. I have some idea but can you explain why it is now in an
#ifidef?

> +#endif
>         lcd_sync();
>  }
>
> @@ -701,29 +720,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
>  }
>  #endif
>
> -static void *lcd_logo(void)
> +static void lcd_logo(void)
>  {
> -#ifdef CONFIG_SPLASH_SCREEN
> -       char *s;
> -       ulong addr;
> -       static int do_splash = 1;
> -
> -       if (do_splash && (s = getenv("splashimage")) != NULL) {
> -               int x = 0, y = 0;
> -               do_splash = 0;
> -
> -               if (splash_screen_prepare())
> -                       return (void *)lcd_base;
> -
> -               addr = simple_strtoul (s, NULL, 16);
> -
> -               splash_get_pos(&x, &y);
> -
> -               if (bmp_display(addr, x, y) == 0)
> -                       return (void *)lcd_base;
> -       }
> -#endif /* CONFIG_SPLASH_SCREEN */
> -
>         lcd_logo_plot(0, 0);
>
>  #ifdef CONFIG_LCD_INFO
> @@ -731,12 +729,6 @@ static void *lcd_logo(void)
>         lcd_set_row(LCD_INFO_Y / VIDEO_FONT_HEIGHT);
>         lcd_show_board_info();
>  #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);
> -#else
> -       return (void *)lcd_base;
> -#endif /* CONFIG_LCD_LOGO && !defined(CONFIG_LCD_INFO_BELOW_LOGO) */
>  }
>
>  #ifdef CONFIG_SPLASHIMAGE_GUARD
> diff --git a/common/splash.c b/common/splash.c
> index 144fb10..561d35b 100644
> --- a/common/splash.c
> +++ b/common/splash.c
> @@ -22,6 +22,7 @@
>
>  #include <common.h>
>  #include <splash.h>
> +#include <lcd.h>
>
>  __weak int splash_screen_prepare(void)
>  {
> @@ -50,3 +51,18 @@ void splash_get_pos(int *x, int *y)
>         }
>  }
>  #endif /* CONFIG_SPLASH_SCREEN_ALIGN */
> +
> +#if defined(CONFIG_SPLASH_SCREEN) && defined(CONFIG_LCD)
> +int lcd_splash(ulong addr)
> +{
> +       int x = 0, y = 0, ret;
> +
> +       ret = splash_screen_prepare();
> +       if (ret)
> +               return ret;
> +
> +       splash_get_pos(&x, &y);
> +
> +       return bmp_display(addr, x, y);
> +}
> +#endif
> diff --git a/include/splash.h b/include/splash.h
> index 89ee7b2..4dbf754 100644
> --- a/include/splash.h
> +++ b/include/splash.h
> @@ -21,7 +21,7 @@
>
>  #ifndef _SPLASH_H_
>  #define _SPLASH_H_
> -
> +#include <errno.h>
>
>  int splash_screen_prepare(void);
>
> @@ -31,6 +31,15 @@ void splash_get_pos(int *x, int *y);
>  static inline void splash_get_pos(int *x, int *y) { }
>  #endif
>
> +#if defined(CONFIG_SPLASH_SCREEN) && defined(CONFIG_LCD)
> +int lcd_splash(ulong addr);
> +#else
> +static inline int lcd_splash(ulong addr)
> +{
> +       return -ENOSYS;
> +}
> +#endif
> +
>  #define BMP_ALIGN_CENTER       0x7FFF
>
>  #endif
> --
> 1.9.1
>

Regards,
SImon