From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anatolij Gustschin Date: Sun, 2 Sep 2012 00:50:45 +0200 Subject: [U-Boot] [PATCH 3/6] video: cfb_console: logo can be positioned via the splashpos variable In-Reply-To: <1344583606-12778-3-git-send-email-Bastian.Ruppert@Sewerin.de> References: <1344583606-12778-1-git-send-email-Bastian.Ruppert@Sewerin.de> <1344583606-12778-3-git-send-email-Bastian.Ruppert@Sewerin.de> Message-ID: <20120902005045.46f464fc@wker> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: > Signed-off-by: Bastian Ruppert > CC: Anatolij Gustschin > CC: Tom Rini > CC: Stefano Babic > --- > 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