diff mbox

[U-Boot,v3] video: cfb_console: Make the software cursor non-destructive

Message ID 1322697050-27844-1-git-send-email-agust@denx.de
State Accepted
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Anatolij Gustschin Nov. 30, 2011, 11:50 p.m. UTC 
From: Gabe Black <gabeblack@chromium.org>

When printing the string "\r\n" to the framebuffer console, the first
character of the current line was being replaced with a space. The "boot"
prompt would become the "oot" prompt. This change makes the cursor
non-destructive so that no matter where it goes on its way to where it's
supposed to be, the end result is that the cursor is where it's supposed to
be with the other text preserved.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
Acked-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Changes in v3:
- fixed checkpatch errors
- slightly modified subject line
- fixed cursor position if video logo is used

Changes in v2:
- Tidy up commit message wording.
- Undo change to documenting comment.
- Undo minor whitespace tweak.
- Consolidate CONFIG_CONSOLE_CURSOR and CONFIG_VIDEO_SW_CURSOR.
- Get rid of a space between video_invertchar and its (.

 drivers/video/cfb_console.c |  109 +++++++++++++++++++++----------------------
 1 files changed, 53 insertions(+), 56 deletions(-)

Comments

Anatolij Gustschin Dec. 5, 2011, 9:07 p.m. UTC | #1 
On Thu,  1 Dec 2011 00:50:50 +0100
Anatolij Gustschin <agust@denx.de> wrote:

> From: Gabe Black <gabeblack@chromium.org>
> 
> When printing the string "\r\n" to the framebuffer console, the first
> character of the current line was being replaced with a space. The "boot"
> prompt would become the "oot" prompt. This change makes the cursor
> non-destructive so that no matter where it goes on its way to where it's
> supposed to be, the end result is that the cursor is where it's supposed to
> be with the other text preserved.
> 
> Signed-off-by: Gabe Black <gabeblack@chromium.org>
> Acked-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> Changes in v3:
> - fixed checkpatch errors
> - slightly modified subject line
> - fixed cursor position if video logo is used
> 
> Changes in v2:
> - Tidy up commit message wording.
> - Undo change to documenting comment.
> - Undo minor whitespace tweak.
> - Consolidate CONFIG_CONSOLE_CURSOR and CONFIG_VIDEO_SW_CURSOR.
> - Get rid of a space between video_invertchar and its (.
> 
>  drivers/video/cfb_console.c |  109 +++++++++++++++++++++----------------------
>  1 files changed, 53 insertions(+), 56 deletions(-)

Applied to u-boot-video/master. Thanks!

Anatolij
Wolfgang Denk March 18, 2012, 6:38 p.m. UTC | #2 
Dear Anatolij Gustschin,

In message <20111205220742.18ed6e2f@wker> you wrote:
> On Thu,  1 Dec 2011 00:50:50 +0100
> Anatolij Gustschin <agust@denx.de> wrote:
> 
> > From: Gabe Black <gabeblack@chromium.org>
> > 
> > When printing the string "\r\n" to the framebuffer console, the first
> > character of the current line was being replaced with a space. The "boot"
> > prompt would become the "oot" prompt. This change makes the cursor
> > non-destructive so that no matter where it goes on its way to where it's
> > supposed to be, the end result is that the cursor is where it's supposed to
> > be with the other text preserved.
> > 
> > Signed-off-by: Gabe Black <gabeblack@chromium.org>
> > Acked-by: Mike Frysinger <vapier@gentoo.org>
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > ---
> > Changes in v3:
> > - fixed checkpatch errors
> > - slightly modified subject line
> > - fixed cursor position if video logo is used
> > 
> > Changes in v2:
> > - Tidy up commit message wording.
> > - Undo change to documenting comment.
> > - Undo minor whitespace tweak.
> > - Consolidate CONFIG_CONSOLE_CURSOR and CONFIG_VIDEO_SW_CURSOR.
> > - Get rid of a space between video_invertchar and its (.
> > 
> >  drivers/video/cfb_console.c |  109 +++++++++++++++++++++----------------------
> >  1 files changed, 53 insertions(+), 56 deletions(-)
> 
> Applied to u-boot-video/master. Thanks!

It appears this has not made it into mainline yet.

Is it still queued in your repo?  Do you intend to send a pull request
any time soon?

Best regards,

Wolfgang Denk
Anatolij Gustschin March 18, 2012, 7:01 p.m. UTC | #3 
Hello Wolfgang,

On Sun, 18 Mar 2012 19:38:57 +0100
Wolfgang Denk <wd@denx.de> wrote:
...
> > >  drivers/video/cfb_console.c |  109 +++++++++++++++++++++----------------------
> > >  1 files changed, 53 insertions(+), 56 deletions(-)
> > 
> > Applied to u-boot-video/master. Thanks!
> 
> It appears this has not made it into mainline yet.
> 
> Is it still queued in your repo?  Do you intend to send a pull request
> any time soon?

This patch was included in my last pull request and is already in
mainline [1]. Unfortunately it caused some issues on N900 as reported
by Pali Rohár. He already tested my another testing patch for fixing
these issues and I'm working on a final patch for mainline and intend
to submit this final patch for inclusion into upcoming release. 

Best regards,
Anatolij

[1] http://git.denx.de/?p=u-boot.git;a=commit;h=03d31fcf4c37d90a00e66f06b38742960139f090
Wolfgang Denk March 18, 2012, 7:31 p.m. UTC | #4 
Dear Anatolij Gustschin,

In message <20120318200142.5b4d5dba@wker> you wrote:
> 
> This patch was included in my last pull request and is already in
> mainline [1]. Unfortunately it caused some issues on N900 as reported

Strange.  I don;t know how I missed it.  Thanks.

> by Pali Rohár. He already tested my another testing patch for fixing
> these issues and I'm working on a final patch for mainline and intend
> to submit this final patch for inclusion into upcoming release. 

Thanks.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
index 480df64..9be6166 100644
--- a/drivers/video/cfb_console.c
+++ b/drivers/video/cfb_console.c
@@ -242,8 +242,9 @@ 
 #define CURSOR_SET
 #endif
 
-#ifdef	CONFIG_CONSOLE_CURSOR
-#ifdef	CURSOR_ON
+#if defined(CONFIG_CONSOLE_CURSOR) || defined(CONFIG_VIDEO_SW_CURSOR)
+#if defined(CURSOR_ON) || \
+	(defined(CONFIG_CONSOLE_CURSOR) && defined(CONFIG_VIDEO_SW_CURSOR))
 #error	only one of CONFIG_CONSOLE_CURSOR, CONFIG_VIDEO_SW_CURSOR, \
 	or CONFIG_VIDEO_HW_CURSOR can be defined
 #endif
@@ -251,27 +252,18 @@  void console_cursor(int state);
 
 #define CURSOR_ON  console_cursor(1)
 #define CURSOR_OFF console_cursor(0)
-#define CURSOR_SET
+#define CURSOR_SET video_set_cursor()
+#endif /* CONFIG_CONSOLE_CURSOR || CONFIG_VIDEO_SW_CURSOR */
+
+#ifdef	CONFIG_CONSOLE_CURSOR
+#ifndef	CONFIG_CONSOLE_TIME
+#error	CONFIG_CONSOLE_CURSOR must be defined for CONFIG_CONSOLE_TIME
+#endif
 #ifndef CONFIG_I8042_KBD
 #warning Cursor drawing on/off needs timer function s.a. drivers/input/i8042.c
 #endif
-#else
-#ifdef	CONFIG_CONSOLE_TIME
-#error	CONFIG_CONSOLE_CURSOR must be defined for CONFIG_CONSOLE_TIME
-#endif
 #endif /* CONFIG_CONSOLE_CURSOR */
 
-#ifdef	CONFIG_VIDEO_SW_CURSOR
-#ifdef	CURSOR_ON
-#error	only one of CONFIG_CONSOLE_CURSOR, CONFIG_VIDEO_SW_CURSOR, \
-	or CONFIG_VIDEO_HW_CURSOR can be defined
-#endif
-#define CURSOR_ON
-#define CURSOR_OFF video_putchar(console_col * VIDEO_FONT_WIDTH,\
-				 console_row * VIDEO_FONT_HEIGHT, ' ')
-#define CURSOR_SET video_set_cursor()
-#endif /* CONFIG_VIDEO_SW_CURSOR */
-
 
 #ifdef CONFIG_VIDEO_HW_CURSOR
 #ifdef	CURSOR_ON
@@ -376,6 +368,10 @@  static void *video_console_address;	/* console buffer start address */
 
 static int video_logo_height = VIDEO_LOGO_HEIGHT;
 
+static int cursor_state;
+static int old_col;
+static int old_row;
+
 static int console_col;		/* cursor col */
 static int console_row;		/* cursor row */
 
@@ -435,6 +431,22 @@  static const int video_font_draw_table32[16][4] = {
 };
 
 
+static void video_invertchar(int xx, int yy)
+{
+	int firstx = xx * VIDEO_PIXEL_SIZE;
+	int lastx = (xx + VIDEO_FONT_WIDTH) * VIDEO_PIXEL_SIZE;
+	int firsty = yy * VIDEO_LINE_LEN;
+	int lasty = (yy + VIDEO_FONT_HEIGHT) * VIDEO_LINE_LEN;
+	int x, y;
+	for (y = firsty; y < lasty; y += VIDEO_LINE_LEN) {
+		for (x = firstx; x < lastx; x++) {
+			u8 *dest = (u8 *)(video_fb_address) + x + y;
+			*dest = ~*dest;
+		}
+	}
+}
+
+
 static void video_drawchars(int xx, int yy, unsigned char *s, int count)
 {
 	u8 *cdat, *dest, *dest0;
@@ -610,27 +622,15 @@  static void video_putchar(int xx, int yy, unsigned char c)
 #if defined(CONFIG_CONSOLE_CURSOR) || defined(CONFIG_VIDEO_SW_CURSOR)
 static void video_set_cursor(void)
 {
-	/* swap drawing colors */
-	eorx = fgx;
-	fgx = bgx;
-	bgx = eorx;
-	eorx = fgx ^ bgx;
-	/* draw cursor */
-	video_putchar(console_col * VIDEO_FONT_WIDTH,
-		      console_row * VIDEO_FONT_HEIGHT, ' ');
-	/* restore drawing colors */
-	eorx = fgx;
-	fgx = bgx;
-	bgx = eorx;
-	eorx = fgx ^ bgx;
+	if (cursor_state)
+		console_cursor(0);
+	console_cursor(1);
 }
-#endif
 
-#ifdef CONFIG_CONSOLE_CURSOR
+
+
 void console_cursor(int state)
 {
-	static int last_state = 0;
-
 #ifdef CONFIG_CONSOLE_TIME
 	struct rtc_time tm;
 	char info[16];
@@ -652,17 +652,22 @@  void console_cursor(int state)
 	}
 #endif
 
-	if (state && (last_state != state)) {
-		video_set_cursor();
-	}
-
-	if (!state && (last_state != state)) {
-		/* clear cursor */
-		video_putchar(console_col * VIDEO_FONT_WIDTH,
-			      console_row * VIDEO_FONT_HEIGHT, ' ');
+	if (cursor_state != state) {
+		if (cursor_state) {
+			/* turn off the cursor */
+			video_invertchar(old_col * VIDEO_FONT_WIDTH,
+					 old_row * VIDEO_FONT_HEIGHT +
+					 video_logo_height);
+		} else {
+			/* turn off the cursor and record where it is */
+			video_invertchar(console_col * VIDEO_FONT_WIDTH,
+					 console_row * VIDEO_FONT_HEIGHT +
+					 video_logo_height);
+			old_col = console_col;
+			old_row = console_row;
+		}
+		cursor_state = state;
 	}
-
-	last_state = state;
 }
 #endif
 
@@ -729,19 +734,11 @@  static void console_back(void)
 		if (console_row < 0)
 			console_row = 0;
 	}
-	video_putchar(console_col * VIDEO_FONT_WIDTH,
-		      console_row * VIDEO_FONT_HEIGHT, ' ');
+	CURSOR_SET;
 }
 
 static void console_newline(void)
 {
-	/* Check if last character in the line was just drawn. If so, cursor was
-	   overwriten and need not to be cleared. Cursor clearing without this
-	   check causes overwriting the 1st character of the line if line lenght
-	   is >= CONSOLE_COLS
-	 */
-	if (console_col < CONSOLE_COLS)
-		CURSOR_OFF;
 	console_row++;
 	console_col = 0;
 
@@ -757,7 +754,6 @@  static void console_newline(void)
 
 static void console_cr(void)
 {
-	CURSOR_OFF;
 	console_col = 0;
 }
 
@@ -765,6 +761,8 @@  void video_putc(const char c)
 {
 	static int nl = 1;
 
+	CURSOR_OFF;
+
 	switch (c) {
 	case 13:		/* back to first column */
 		console_cr();
@@ -777,7 +775,6 @@  void video_putc(const char c)
 		break;
 
 	case 9:		/* tab 8 */
-		CURSOR_OFF;
 		console_col |= 0x0008;
 		console_col &= ~0x0007;