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

[Simon Glass]

Hi NIkita,

On 1 February 2015 at 06:59, Nikita Kiryanov <nikita at compulab.co.il> wrote:
> Hi Simon,
>
>
> On 01/31/2015 02:25 AM, Simon Glass wrote:
>>
>> 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?
>
>
> The only time when the base address of the console is defined as anything
> other
> than lcd_base, is when we get to the very end of the original lcd_logo() and
> the above #defines apply. In all other cases, the address of lcd_base is
> used,
> and that is set as default at the start of lcd_clear().

OK I see, thanks.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon