[U-Boot] [PATCH 0/2] video: lcd: splash screen prepare changes
These 2 patches are the result of discussion in these threads: http://lists.denx.de/pipermail/u-boot/2013-May/155463.html http://lists.denx.de/pipermail/u-boot/2013-June/155630.html http://lists.denx.de/pipermail/u-boot/2013-June/155632.html
The upshot is, move splash_screen_prepare to a common location so it can be used in cfb_console.c and (possibly) make it weak.
These 2 patches do that and the first can be accepted without the second if we decide not to make it weak.
One other note, I'm submitting this based on Anatolij's tree since it's video related but board/compulab/cm_t35 does not exist in his tree. If the second patch is accepted, cm_t35 will have to change since it uses the non-weak method. I will submit an optional 3rd patch to Stefano's tree with the necessary changes.
Robert Winkler (2): video: lcd: Add CONFIG_SPLASH_SCREEN_PREPARE support to CONFIG_VIDEO video: lcd: Make splash_screen_prepare weak, remove config macro
README | 8 -------- common/Makefile | 1 + common/lcd.c | 19 ++++++------------- common/splash.c | 31 +++++++++++++++++++++++++++++++ doc/README.splashprepare | 8 ++++++++ drivers/video/cfb_console.c | 8 ++++++-- include/lcd.h | 1 - include/splash.h | 30 ++++++++++++++++++++++++++++++ 8 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 common/splash.c create mode 100644 doc/README.splashprepare create mode 100644 include/splash.h
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 --- 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 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. + */ +#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 + */ + +#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. + */ +#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 + */ + +#ifndef _SPLASH_H_ +#define _SPLASH_H_ + + +int splash_screen_prepare(void); + + +#endif
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();
- 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
Hi Igor,
On Sun, Jun 16, 2013 at 10:34 PM, Igor Grinberg grinberg@compulab.co.il wrote:
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.
No it shouldn't. The function is always called so it always needs to be defined. It's the same behavior we had before, it was always compiled into lcd.h. Whether or not the CONFIG was defined just changed whether it actually called board_splash_screen_prepare or just returned 0.
This is of course true for when it's a weak function as well.
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.
Agreed. I was just trying to match the other superfluous comments.
+#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...
Good point, will do.
- */
+#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.
On 06/17/13 20:08, Robert Winkler wrote:
Hi Igor,
On Sun, Jun 16, 2013 at 10:34 PM, Igor Grinberg grinberg@compulab.co.il wrote:
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.
No it shouldn't. The function is always called so it always needs to be defined. It's the same behavior we had before, it was always compiled into lcd.h. Whether or not the CONFIG was defined just changed whether it actually called board_splash_screen_prepare or just returned 0.
This is of course true for when it's a weak function as well.
Well, what I meant is, once you make it a separate file (named splash.c), it is rather strange that it should be always compiled. It is fine for me now, but once more splash screen specific code moves to this file, it surely should depend on CONFIG_SPLASH_SCREEN.
In general, I think the construct of 1) having the .c file compiled conditionally (meaning depend on CONFIG_... option) 2) having the .h file to define the interface to the .c file and provide a default implementation (when the above CONFIG_... is not set),
like in many cases Linux does, brings the benefit of clear yet robust (in regard to the CONFIG_... defined or not) code has proven itself.
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.
Agreed. I was just trying to match the other superfluous comments.
+#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...
Good point, will do.
- */
+#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.
On Mon, Jun 17, 2013 at 11:20 PM, Igor Grinberg grinberg@compulab.co.il wrote:
On 06/17/13 20:08, Robert Winkler wrote:
Hi Igor,
On Sun, Jun 16, 2013 at 10:34 PM, Igor Grinberg grinberg@compulab.co.il wrote:
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.
No it shouldn't. The function is always called so it always needs to be defined. It's the same behavior we had before, it was always compiled into lcd.h. Whether or not the CONFIG was defined just changed whether it actually called board_splash_screen_prepare or just returned 0.
This is of course true for when it's a weak function as well.
Well, what I meant is, once you make it a separate file (named splash.c), it is rather strange that it should be always compiled. It is fine for me now, but once more splash screen specific code moves to this file, it surely should depend on CONFIG_SPLASH_SCREEN.
In general, I think the construct of
- having the .c file compiled conditionally (meaning depend on CONFIG_... option)
- having the .h file to define the interface to the .c file and provide a default implementation (when the above CONFIG_... is not set),
like in many cases Linux does, brings the benefit of clear yet robust (in regard to the CONFIG_... defined or not) code has proven itself.
Ah, I'm sorry, I misread that as SPLASH_SCREEN_PREPARE, but that makes more sense. However I haven't noticed any function definitions in h files in U-Boot and as it stands it has to be this way. Since a lot of the common code in lcd.c and cfb_console is unrelated to (or at least, not only used) for splash screens, maybe splash.c/h should be called something else and thus always included. Not sure what to call it, but something to do with logos/bmp probably.
The overlap between lcd_init -> lcd_clear -> lcd_logo -> bmp_display and video_init -> video_logo -> video_display_bitmap
is as much related to general bmp/logo display as splash screen functionality and it would probably be more sensible to pull it all out that than try to snip out only the splash screen stuff for splash.c
Just an idea.
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.
Agreed. I was just trying to match the other superfluous comments.
+#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...
Good point, will do.
- */
+#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.
-- Regards, Igor.
Remove CONFIG_SPLASH_SCREEN_PREPARE from README Add doc/README.splashprepare to document functionality
Signed-off-by: Robert Winkler robert.winkler@boundarydevices.com --- README | 8 -------- common/splash.c | 14 ++++---------- doc/README.splashprepare | 8 ++++++++ 3 files changed, 12 insertions(+), 18 deletions(-) create mode 100644 doc/README.splashprepare
diff --git a/README b/README index 0d37d56..fd3d3a5 100644 --- a/README +++ b/README @@ -1604,14 +1604,6 @@ CBFS (Coreboot Filesystem) support => vertically centered image at x = dspWidth - bmpWidth - 9
- CONFIG_SPLASH_SCREEN_PREPARE - - If this option is set then the board_splash_screen_prepare() - function, which must be defined in your code, is called as part - of the splash screen display sequence. It gives the board an - opportunity to prepare the splash image data before it is - processed and sent to the frame buffer by U-Boot. - - Gzip compressed BMP image support: CONFIG_VIDEO_BMP_GZIP
If this option is set, additionally to standard BMP diff --git a/common/splash.c b/common/splash.c index fe13c69..c40861a 100644 --- a/common/splash.c +++ b/common/splash.c @@ -21,17 +21,11 @@ */
#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) +int __splash_screen_prepare(void) { - printf("SPLASH_SCREEN_PREPARE not defined\n"); return 0; } -#endif + +int splash_screen_prepare(void) + __attribute__ ((weak, alias("__splash_screen_prepare"))); diff --git a/doc/README.splashprepare b/doc/README.splashprepare new file mode 100644 index 0000000..8d103bf --- /dev/null +++ b/doc/README.splashprepare @@ -0,0 +1,8 @@ +--------------------------------------------------------------------- +Splash Screen +--------------------------------------------------------------------- +The splash_screen_prepare() function is a weak function defined in +common/splash.c. It is called as part of the splash screen display +sequence. It gives the board an opportunity to prepare the splash +image data before it is processed and sent to the frame buffer by +U-Boot. Define your own version to use this feature.
On 06/14/13 20:00, Robert Winkler wrote:
These 2 patches are the result of discussion in these threads: http://lists.denx.de/pipermail/u-boot/2013-May/155463.html http://lists.denx.de/pipermail/u-boot/2013-June/155630.html http://lists.denx.de/pipermail/u-boot/2013-June/155632.html
The upshot is, move splash_screen_prepare to a common location so it can be used in cfb_console.c and (possibly) make it weak.
These 2 patches do that and the first can be accepted without the second if we decide not to make it weak.
One other note, I'm submitting this based on Anatolij's tree since it's video related but board/compulab/cm_t35 does not exist in his tree.
That's because recently I've moved it from board/cm_t35 to board/compulab/cm_t35 and probably Anatolij has not updated his tree to the latest upstream.
If the second patch is accepted, cm_t35 will have to change since it uses the non-weak method. I will submit an optional 3rd patch to Stefano's tree with the necessary changes.
Robert Winkler (2): video: lcd: Add CONFIG_SPLASH_SCREEN_PREPARE support to CONFIG_VIDEO video: lcd: Make splash_screen_prepare weak, remove config macro
README | 8 -------- common/Makefile | 1 + common/lcd.c | 19 ++++++------------- common/splash.c | 31 +++++++++++++++++++++++++++++++ doc/README.splashprepare | 8 ++++++++ drivers/video/cfb_console.c | 8 ++++++-- include/lcd.h | 1 - include/splash.h | 30 ++++++++++++++++++++++++++++++ 8 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 common/splash.c create mode 100644 doc/README.splashprepare create mode 100644 include/splash.h
participants (2)
-
Igor Grinberg -
Robert Winkler