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

[Nikita Kiryanov]

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().

-- 
Regards,
Nikita Kiryanov