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.rupp...@sewerin.de> wrote:
> Signed-off-by: Bastian Ruppert <bastian.rupp...@sewerin.de>
> CC: Anatolij Gustschin <ag...@denx.de>
> CC: Tom Rini <tr...@ti.com>
> CC: Stefano Babic <sba...@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 and
> * CONFIG_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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
