Hi Bastian,
there is a number of issues with this patch, please see comments below.
On Fri, 10 Aug 2012 09:26:43 +0200 Bastian Ruppert Bastian.Ruppert@Sewerin.de wrote:
Signed-off-by: Bastian Ruppert Bastian.Ruppert@Sewerin.de CC: Anatolij Gustschin agust@denx.de CC: Tom Rini trini@ti.com CC: Stefano Babic sbabic@denx.de
drivers/video/cfb_console.c | 61 ++++++++++++++++++++++++++++--------------- 1 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index 19d061f..21b52bd 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -66,7 +66,11 @@
- CONFIG_CONSOLE_TIME - display time/date in upper right
corner, needs CONFIG_CMD_DATE andCONFIG_CONSOLE_CURSOR
- CONFIG_VIDEO_LOGO - display Linux Logo in upper left corner
- CONFIG_VIDEO_LOGO - display Linux Logo in upper left corner.
Use CONFIG_SPLASH_SCREEN_ALIGN with
environment variable "splashpos" to place
the logo on other position. In this case
no CONSOLE_EXTRA_INFO is possible.- CONFIG_VIDEO_BMP_LOGO - use bmp_logo instead of linux_logo
- CONFIG_CONSOLE_EXTRA_INFO - display additional board information
strings that normaly goes to serial@@ -369,6 +373,8 @@ static void *video_fb_address; /* frame buffer address */ static void *video_console_address; /* console buffer start address */
static int video_logo_height = VIDEO_LOGO_HEIGHT; +static int video_logo_xpos; +static int video_logo_ypos;
static int __maybe_unused cursor_state; static int __maybe_unused old_col; @@ -1594,40 +1600,53 @@ static void *video_logo(void) __maybe_unused int y_off = 0;
#ifdef CONFIG_SPLASH_SCREEN
- char *s; ulong addr;
- s = getenv("splashimage");
+#endif +#if defined(CONFIG_SPLASH_SCREEN) || defined(CONFIG_SPLASH_SCREEN_ALIGN)
- char *s;
+#endif
these ifdefs should be better reduced, I think we can use __maybe_unused here, like for y_off above.
+#ifdef CONFIG_SPLASH_SCREEN_ALIGN
- s = getenv("splashpos"); if (s != NULL) {
int x = 0, y = 0;
if (s[0] == 'm')video_logo_xpos = BMP_ALIGN_CENTER;
The 'm' case will work with splashscreen, but not with the video logo. There is no proper offset calculation in logo_plot() if xpos or ypos are set to BMP_ALIGN_CENTER. As a result the logo offset will be wrong and an access to wrong offset can even brick the board (on boards with small frame buffers).
...
if (video_display_bitmap(addr, \video_logo_xpos, \
no need to use "" here.
...
- logo_plot(video_fb_address, \
VIDEO_COLS, \video_logo_xpos, \
ditto.
...
+#ifdef CONFIG_SPLASH_SCREEN_ALIGN
- /* when using splashpos for video_logo, no console output */
- return (video_fb_address + video_logo_height * VIDEO_LINE_LEN);
The returned address is used as text console offset, so if the logo is moved down, the video_logo_height should be increased by video_logo_ypos. Otherwise the text and cursor positions will be wrong in the video console.
I've fixed these issues and submitted a patch v2 3/6. Please test.
Thanks,
Anatolij