[U-Boot] [PATCH 1/2] video: lcd: Add CONFIG_SPLASH_SCREEN_PREPARE support to CONFIG_VIDEO

[Igor Grinberg <grinberg@compulab.co.il>]

Hi Robert,

On 06/14/13 20:00, Robert Winkler wrote:
> Create splash.c/h to put the function and any future common
> splash screen code in.
> 
> Signed-off-by: Robert Winkler <robert.winkler@boundarydevices.com>

Thanks for the effort!
Several comments below...

> ---
>  common/Makefile             |  1 +
>  common/lcd.c                | 19 ++++++-------------
>  common/splash.c             | 37 +++++++++++++++++++++++++++++++++++++
>  drivers/video/cfb_console.c |  8 ++++++--
>  include/lcd.h               |  1 -
>  include/splash.h            | 30 ++++++++++++++++++++++++++++++
>  6 files changed, 80 insertions(+), 16 deletions(-)
>  create mode 100644 common/splash.c
>  create mode 100644 include/splash.h
> 
> diff --git a/common/Makefile b/common/Makefile
> index 0e0fff1..1d70584 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -203,6 +203,7 @@ COBJS-y += flash.o
>  COBJS-$(CONFIG_CMD_KGDB) += kgdb.o kgdb_stubs.o
>  COBJS-$(CONFIG_I2C_EDID) += edid.o
>  COBJS-$(CONFIG_KALLSYMS) += kallsyms.o
> +COBJS-y += splash.o

I think this should depend on CONFIG_SPLASH_SCREEN.

>  COBJS-$(CONFIG_LCD) += lcd.o
>  COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
>  COBJS-$(CONFIG_MENU) += menu.o
> diff --git a/common/lcd.c b/common/lcd.c
> index 3a60484..4a85ebb 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -43,6 +43,11 @@
>  #include <lcd.h>
>  #include <watchdog.h>
>  
> +/*
> + * Include splash.h for splash_screen_prepare() etc.
> + */

I think this comment is meaningless, the below include is self explanatory.

> +#include <splash.h>
> +
>  #if defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \
>  	defined(CONFIG_CPU_MONAHANS)
>  #define CONFIG_CPU_PXA
> @@ -1072,18 +1077,6 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
>  }
>  #endif
>  
> -#ifdef CONFIG_SPLASH_SCREEN_PREPARE
> -static inline int splash_screen_prepare(void)
> -{
> -	return board_splash_screen_prepare();
> -}
> -#else
> -static inline int splash_screen_prepare(void)
> -{
> -	return 0;
> -}
> -#endif
> -
>  static void *lcd_logo(void)
>  {
>  #ifdef CONFIG_SPLASH_SCREEN
> @@ -1096,7 +1089,7 @@ static void *lcd_logo(void)
>  		do_splash = 0;
>  
>  		if (splash_screen_prepare())
> -			return (void *)gd->fb_base;
> +			return (void *)lcd_base;
>  
>  		addr = simple_strtoul (s, NULL, 16);
>  #ifdef CONFIG_SPLASH_SCREEN_ALIGN
> diff --git a/common/splash.c b/common/splash.c
> new file mode 100644
> index 0000000..fe13c69
> --- /dev/null
> +++ b/common/splash.c
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2013, Boundary Devices <info@boundarydevices.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA

I would drop the postal address, as it changed in the past and
probably will change in the future and then we will have a bunch of
files with wrong address...

> + */
> +
> +#include <splash.h>
> +#include <config.h>
> +
> +#ifdef CONFIG_SPLASH_SCREEN_PREPARE
> +int splash_screen_prepare(void)
> +{
> +	return board_splash_screen_prepare();
> +}
> +#else
> +int splash_screen_prepare(void)
> +{
> +	printf("SPLASH_SCREEN_PREPARE not defined\n");
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
> index b769222..4543730 100644
> --- a/drivers/video/cfb_console.c
> +++ b/drivers/video/cfb_console.c
> @@ -178,6 +178,11 @@
>  #include <video_fb.h>
>  
>  /*
> + * Include splash.h for splash_screen_prepare() etc.
> + */

Same here, the below include does not need any comment.

> +#include <splash.h>
> +
> +/*
>   * some Macros
>   */
>  #define VIDEO_VISIBLE_COLS	(pGD->winSizeX)
> @@ -1991,10 +1996,9 @@ static void *video_logo(void)
>  #ifdef CONFIG_SPLASH_SCREEN
>  	s = getenv("splashimage");
>  	if (s != NULL) {
> -
> +		splash_screen_prepare();
>  		addr = simple_strtoul(s, NULL, 16);
>  
> -
>  		if (video_display_bitmap(addr,
>  					video_logo_xpos,
>  					video_logo_ypos) == 0) {
> diff --git a/include/lcd.h b/include/lcd.h
> index 30225ed..79bf13e 100644
> --- a/include/lcd.h
> +++ b/include/lcd.h
> @@ -37,7 +37,6 @@ extern struct vidinfo panel_info;
>  
>  void lcd_ctrl_init(void *lcdbase);
>  void lcd_enable(void);
> -int board_splash_screen_prepare(void);
>  
>  /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>  void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue);
> diff --git a/include/splash.h b/include/splash.h
> new file mode 100644
> index 0000000..478f864
> --- /dev/null
> +++ b/include/splash.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2013, Boundary Devices <info@boundarydevices.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA

If you decide to drop the postal address, then please do it also here.

> + */
> +
> +#ifndef _SPLASH_H_
> +#define _SPLASH_H_
> +
> +
> +int splash_screen_prepare(void);
> +
> +
> +#endif
> 

-- 
Regards,
Igor.