[U-Boot] [PATCH 0/4] Introduce lcd_console rotation.

[U-Boot] [PATCH 0/4] Introduce lcd_console rotation.

[Hannes Petermaier]

Sometimes, for example if the display is mounted in portrait mode or even if it

mounted landscape but rotated by 180 degree, we need to rotate our content of
the display respectively the framebuffer, so that user can read the messages who
are printed out.

For this we introduce the feature called "CONFIG_LCD_ROTATION", this may be
defined in the board-configuration if needed. After this the lcd_console will
be initialized with a given rotation from "vl_rot" out of "vidinfo_t" which is
provided by the board specific code.

If CONFIG_LCD_ROTATION is not defined, the console will be initialized with
0 degrees rotation - the screen behaves like the days before.

Patch 1-3 make preparations to the code.
Patch 4 implements the new feature


Hannes Petermaier (4):
  common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy
  common/lcd_console: ask only one-time for bg/fg-color per call
  common/lcd_console: move single static variables into common (static)
    structure
  common/lcd_console: introduce display/framebuffer rotation

 README                |   17 +++
 common/lcd.c          |   22 +--
 common/lcd_console.c  |  395 +++++++++++++++++++++++++++++++++++++------------
 include/lcd.h         |    1 +
 include/lcd_console.h |    9 +-
 5 files changed, 334 insertions(+), 110 deletions(-)

-- 
1.7.9.5

[U-Boot] [PATCH 1/4] common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy

[Hannes Petermaier]

From: Hannes Petermaier <hannes.petermaier@br-automation.com>

the capability of drawing some *str with count from lcd_drawchars is unnary.
It is always called from lcd_putc_xy with one character of and count = 1.

So we simply rename lcd_drawchars into lcd_putc_xy and remove the loops inside.

Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
---

 common/lcd_console.c |   23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/common/lcd_console.c b/common/lcd_console.c
index 8bf83b9..243b7c5 100644
--- a/common/lcd_console.c
+++ b/common/lcd_console.c
@@ -55,18 +55,17 @@ int lcd_get_screen_columns(void)
 	return console_cols;
 }
 
-static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
+static void lcd_putc_xy(ushort x, ushort y, char c)
 {
 	uchar *dest;
 	ushort row;
 	int fg_color, bg_color;
+	int i;
 
 	dest = (uchar *)(lcd_console_address +
 			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
 
 	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
-		uchar *s = str;
-		int i;
 #if LCD_BPP == LCD_COLOR16
 		ushort *d = (ushort *)dest;
 #elif LCD_BPP == LCD_COLOR32
@@ -77,25 +76,17 @@ static void lcd_drawchars(ushort x, ushort y, uchar *str, int count)
 
 		fg_color = lcd_getfgcolor();
 		bg_color = lcd_getbgcolor();
-		for (i = 0; i < count; ++i) {
-			uchar c, bits;
 
-			c = *s++;
-			bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
+		uchar bits;
+		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
 
-			for (c = 0; c < 8; ++c) {
-				*d++ = (bits & 0x80) ? fg_color : bg_color;
-				bits <<= 1;
-			}
+		for (i = 0; i < 8; ++i) {
+			*d++ = (bits & 0x80) ? fg_color : bg_color;
+			bits <<= 1;
 		}
 	}
 }
 
-static inline void lcd_putc_xy(ushort x, ushort y, uchar c)
-{
-	lcd_drawchars(x, y, &c, 1);
-}
-
 static void console_scrollup(void)
 {
 	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
-- 
1.7.9.5

[U-Boot] [PATCH 2/4] common/lcd_console: ask only one-time for bg/fg-color per call

[Hannes Petermaier]

From: Hannes Petermaier <hannes.petermaier@br-automation.com>

Don't call the lcd_getfgcolor and lcd_getbgcolor within the "draw-loop", this
only wastes time.

Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
---

 common/lcd_console.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/common/lcd_console.c b/common/lcd_console.c
index 243b7c5..b7dda7a 100644
--- a/common/lcd_console.c
+++ b/common/lcd_console.c
@@ -59,7 +59,8 @@ static void lcd_putc_xy(ushort x, ushort y, char c)
 {
 	uchar *dest;
 	ushort row;
-	int fg_color, bg_color;
+	int fg_color = lcd_getfgcolor();
+	int bg_color = lcd_getbgcolor();
 	int i;
 
 	dest = (uchar *)(lcd_console_address +
@@ -73,10 +74,6 @@ static void lcd_putc_xy(ushort x, ushort y, char c)
 #else
 		uchar *d = dest;
 #endif
-
-		fg_color = lcd_getfgcolor();
-		bg_color = lcd_getbgcolor();
-
 		uchar bits;
 		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
 
-- 
1.7.9.5

[U-Boot] [PATCH 3/4] common/lcd_console: move single static variables into common (static) structure

[Hannes Petermaier]

From: Hannes Petermaier <hannes.petermaier@br-automation.com>

For coming implementation of lcd_console rotation, we will need some more
variables for holding information about framebuffer size, rotation, ...

For better readability we catch all them into a common structure.

Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
---

 common/lcd_console.c |   76 +++++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/common/lcd_console.c b/common/lcd_console.c
index b7dda7a..cac77be 100644
--- a/common/lcd_console.c
+++ b/common/lcd_console.c
@@ -11,48 +11,49 @@
 #include <video_font.h>		/* Get font data, width and height */
 
 #define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
-#define CONSOLE_ROW_FIRST	lcd_console_address
-#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * console_rows)
+#define CONSOLE_ROW_FIRST	cons.lcd_address
+#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * cons.rows)
 
-static short console_curr_col;
-static short console_curr_row;
-static short console_cols;
-static short console_rows;
-static void *lcd_console_address;
+struct console_t {
+	short curr_col, curr_row;
+	short cols, rows;
+	void *lcd_address;
+};
+static struct console_t cons;
 
 void lcd_init_console(void *address, int rows, int cols)
 {
-	console_curr_col = 0;
-	console_curr_row = 0;
-	console_cols = cols;
-	console_rows = rows;
-	lcd_console_address = address;
+	memset(&cons, 0, sizeof(cons));
+	cons.cols = cols;
+	cons.rows = rows;
+	cons.lcd_address = address;
+
 }
 
 void lcd_set_col(short col)
 {
-	console_curr_col = col;
+	cons.curr_col = col;
 }
 
 void lcd_set_row(short row)
 {
-	console_curr_row = row;
+	cons.curr_row = row;
 }
 
 void lcd_position_cursor(unsigned col, unsigned row)
 {
-	console_curr_col = min_t(short, col, console_cols - 1);
-	console_curr_row = min_t(short, row, console_rows - 1);
+	cons.curr_col = min_t(short, col, cons.cols - 1);
+	cons.curr_row = min_t(short, row, cons.rows - 1);
 }
 
 int lcd_get_screen_rows(void)
 {
-	return console_rows;
+	return cons.rows;
 }
 
 int lcd_get_screen_columns(void)
 {
-	return console_cols;
+	return cons.cols;
 }
 
 static void lcd_putc_xy(ushort x, ushort y, char c)
@@ -63,7 +64,7 @@ static void lcd_putc_xy(ushort x, ushort y, char c)
 	int bg_color = lcd_getbgcolor();
 	int i;
 
-	dest = (uchar *)(lcd_console_address +
+	dest = (uchar *)(cons.lcd_address +
 			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
 
 	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
@@ -91,7 +92,7 @@ static void console_scrollup(void)
 
 	/* Copy up rows ignoring those that will be overwritten */
 	memcpy(CONSOLE_ROW_FIRST,
-	       lcd_console_address + CONSOLE_ROW_SIZE * rows,
+	       cons.lcd_address + CONSOLE_ROW_SIZE * rows,
 	       CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows);
 
 	/* Clear the last rows */
@@ -99,7 +100,7 @@ static void console_scrollup(void)
 	memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
 	       bg_color, CONSOLE_ROW_SIZE * rows);
 #else
-	u32 *ppix = lcd_console_address +
+	u32 *ppix = cons.lcd_address +
 		    CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
 	u32 i;
 	for (i = 0;
@@ -109,27 +110,27 @@ static void console_scrollup(void)
 	}
 #endif
 	lcd_sync();
-	console_curr_row -= rows;
+	cons.curr_row -= rows;
 }
 
 static inline void console_back(void)
 {
-	if (--console_curr_col < 0) {
-		console_curr_col = console_cols - 1;
-		if (--console_curr_row < 0)
-			console_curr_row = 0;
+	if (--cons.curr_col < 0) {
+		cons.curr_col = cons.cols - 1;
+		if (--cons.curr_row < 0)
+			cons.curr_row = 0;
 	}
 
-	lcd_putc_xy(console_curr_col * VIDEO_FONT_WIDTH,
-		    console_curr_row * VIDEO_FONT_HEIGHT, ' ');
+	lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
+		    cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
 }
 
 static inline void console_newline(void)
 {
-	console_curr_col = 0;
+	cons.curr_col = 0;
 
 	/* Check if we need to scroll the terminal */
-	if (++console_curr_row >= console_rows)
+	if (++cons.curr_row >= cons.rows)
 		console_scrollup();
 	else
 		lcd_sync();
@@ -145,18 +146,17 @@ void lcd_putc(const char c)
 
 	switch (c) {
 	case '\r':
-		console_curr_col = 0;
-
+		cons.curr_col = 0;
 		return;
 	case '\n':
 		console_newline();
 
 		return;
 	case '\t':	/* Tab (8 chars alignment) */
-		console_curr_col +=  8;
-		console_curr_col &= ~7;
+		cons.curr_col +=  8;
+		cons.curr_col &= ~7;
 
-		if (console_curr_col >= console_cols)
+		if (cons.curr_col >= cons.cols)
 			console_newline();
 
 		return;
@@ -165,9 +165,9 @@ void lcd_putc(const char c)
 
 		return;
 	default:
-		lcd_putc_xy(console_curr_col * VIDEO_FONT_WIDTH,
-			    console_curr_row * VIDEO_FONT_HEIGHT, c);
-		if (++console_curr_col >= console_cols)
+		lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
+			    cons.curr_row * VIDEO_FONT_HEIGHT, c);
+		if (++cons.curr_col >= cons.cols)
 			console_newline();
 	}
 }
-- 
1.7.9.5

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Hannes Petermaier]

From: Hannes Petermaier <hannes.petermaier@br-automation.com>

Sometimes, for example if the display is mounted in portrait mode or even if it
mounted landscape but rotated by 180 degrees, we need to rotate our content of
the display respectively the framebuffer, so that user can read the messages
who are printed out.

For this we introduce the feature called "CONFIG_LCD_ROTATION", this may be
defined in the board-configuration if needed. After this the lcd_console will
be initialized with a given rotation from "vl_rot" out of "vidinfo_t" which is
provided by the board specific code.

If CONFIG_LCD_ROTATION is not defined, the console will be initialized with
0 degrees rotation - the screen behaves like the days before.

Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
---

 README                |   17 +++
 common/lcd.c          |   22 ++--
 common/lcd_console.c  |  333 ++++++++++++++++++++++++++++++++++++++++---------
 include/lcd.h         |    1 +
 include/lcd_console.h |    9 +-
 5 files changed, 309 insertions(+), 73 deletions(-)

diff --git a/README b/README
index 3c4a2e6..a95b1e8 100644
--- a/README
+++ b/README
@@ -1933,6 +1933,23 @@ CBFS (Coreboot Filesystem) support
 		the console jump but can help speed up operation when scrolling
 		is slow.
 
+		CONFIG_LCD_ROTATION
+
+		Sometimes, for example if the display is mounted in portrait
+		mode or even if it mounted landscape but rotated by 180degree,
+		we need to rotate our content of the display respectively the
+		framebuffer, so that user can read the messages who are printed
+		out.
+		For this we introduce the feature called "CONFIG_LCD_ROTATION",
+		this may be defined in the board-configuration if needed. After
+		this the lcd_console will be initialized with a given rotation
+		from "vl_rot" out of "vidinfo_t" which is provided by the board
+		specific code.
+
+		If CONFIG_LCD_ROTATION is not defined, the console will be
+		initialized with 0degree rotation - the screen behaves like the
+		days before.
+
 		CONFIG_LCD_BMP_RLE8
 
 		Support drawing of RLE8-compressed bitmaps on the LCD.
diff --git a/common/lcd.c b/common/lcd.c
index f33942c..dfa4c69 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -167,7 +167,6 @@ int drv_lcd_init(void)
 
 void lcd_clear(void)
 {
-	short console_rows, console_cols;
 	int bg_color;
 	char *s;
 	ulong addr;
@@ -211,16 +210,21 @@ void lcd_clear(void)
 	}
 #endif
 #endif
-	/* Paint the logo and retrieve LCD base address */
-	debug("[LCD] Drawing the logo...\n");
-#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
-	console_rows = (panel_info.vl_row - BMP_LOGO_HEIGHT);
-	console_rows /= VIDEO_FONT_HEIGHT;
+	/* setup text-console */
+	debug("[LCD] setting up console...\n");
+#ifdef CONFIG_LCD_ROTATION
+	lcd_init_console(lcd_base,
+			 panel_info.vl_col,
+			 panel_info.vl_row,
+			 panel_info.vl_rot);
 #else
-	console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
+	lcd_init_console(lcd_base,
+			 panel_info.vl_col,
+			 panel_info.vl_row,
+			 0);
 #endif
-	console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
-	lcd_init_console(lcd_base, console_rows, console_cols);
+	/* Paint the logo and retrieve LCD base address */
+	debug("[LCD] Drawing the logo...\n");
 	if (do_splash) {
 		s = getenv("splashimage");
 		if (s) {
diff --git a/common/lcd_console.c b/common/lcd_console.c
index cac77be..6199c9a 100644
--- a/common/lcd_console.c
+++ b/common/lcd_console.c
@@ -2,6 +2,7 @@
  * (C) Copyright 2001-2014
  * DENX Software Engineering -- wd at denx.de
  * Compulab Ltd - http://compulab.co.il/
+ * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -10,26 +11,27 @@
 #include <lcd.h>
 #include <video_font.h>		/* Get font data, width and height */
 
-#define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
-#define CONSOLE_ROW_FIRST	cons.lcd_address
-#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * cons.rows)
+#define PIXLBYTES		(NBYTES(LCD_BPP))
+
+#if LCD_BPP == LCD_COLOR16
+	#define fbptr_t ushort
+#elif LCD_BPP == LCD_COLOR32
+	#define fbptr_t u32
+#else
+	#define fbptr_t uchar
+#endif
 
 struct console_t {
 	short curr_col, curr_row;
 	short cols, rows;
 	void *lcd_address;
+	u32 lcdsizex, lcdsizey;
+	void (*fp_putc_xy)(ushort x, ushort y, char c);
+	void (*fp_console_moverow)(u32 rowdst, u32 rowsrc);
+	void (*fp_console_setrow)(u32 row, int clr);
 };
 static struct console_t cons;
 
-void lcd_init_console(void *address, int rows, int cols)
-{
-	memset(&cons, 0, sizeof(cons));
-	cons.cols = cols;
-	cons.rows = rows;
-	cons.lcd_address = address;
-
-}
-
 void lcd_set_col(short col)
 {
 	cons.curr_col = col;
@@ -56,63 +58,221 @@ int lcd_get_screen_columns(void)
 	return cons.cols;
 }
 
-static void lcd_putc_xy(ushort x, ushort y, char c)
+static void lcd_putc_xy0(ushort x, ushort y, char c)
 {
-	uchar *dest;
-	ushort row;
 	int fg_color = lcd_getfgcolor();
 	int bg_color = lcd_getbgcolor();
+	int i, row;
+	uchar *dest = (uchar *)(cons.lcd_address +
+				y * cons.lcdsizex * PIXLBYTES +
+				x * PIXLBYTES);
+
+	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
+		fbptr_t *d = (fbptr_t *)dest;
+		uchar bits;
+		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
+		for (i = 0; i < 8; ++i) {
+			*d++ = (bits & 0x80) ? fg_color : bg_color;
+			bits <<= 1;
+		}
+		dest += cons.lcdsizex * PIXLBYTES;
+	}
+}
+
+static inline void console_setrow0(u32 row, int clr)
+{
 	int i;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			       row * VIDEO_FONT_HEIGHT *
+			       cons.lcdsizex * PIXLBYTES);
 
-	dest = (uchar *)(cons.lcd_address +
-			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
+	fbptr_t *d = (fbptr_t *)dst;
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
+		*d++ = clr;
+}
 
-	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
-#if LCD_BPP == LCD_COLOR16
-		ushort *d = (ushort *)dest;
-#elif LCD_BPP == LCD_COLOR32
-		u32 *d = (u32 *)dest;
-#else
-		uchar *d = dest;
-#endif
+static inline void console_moverow0(u32 rowdst, u32 rowsrc)
+{
+	int i;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			       rowdst * VIDEO_FONT_HEIGHT *
+			       cons.lcdsizex * PIXLBYTES);
+
+	uchar *src = (uchar *)(cons.lcd_address +
+			       rowsrc * VIDEO_FONT_HEIGHT *
+			       cons.lcdsizex * PIXLBYTES);
+
+	fbptr_t *pdst = (fbptr_t *)dst;
+	fbptr_t *psrc = (fbptr_t *)src;
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
+		*pdst++ = *psrc++;
+}
+#ifdef CONFIG_LCD_ROTATION
+static void lcd_putc_xy90(ushort x, ushort y, char c)
+{
+	int fg_color = lcd_getfgcolor();
+	int bg_color = lcd_getbgcolor();
+	int i, col;
+	uchar *dest = (uchar *)(cons.lcd_address +
+				cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
+				(x+1) * cons.lcdsizex * PIXLBYTES +
+				y * PIXLBYTES);
+
+	fbptr_t *d = (fbptr_t *)dest;
+	uchar msk = 0x80;
+	uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
+	for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
+		for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
+			*d++ = (*(pfont + i) & msk) ? fg_color : bg_color;
+		msk >>= 1;
+		d -= (cons.lcdsizex + VIDEO_FONT_HEIGHT);
+	}
+}
+
+static inline void console_setrow90(u32 row, int clr)
+{
+	int i, j;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			       row*VIDEO_FONT_HEIGHT * PIXLBYTES);
+
+	for (j = 0; j < cons.lcdsizey; j++) {
+		fbptr_t *pdst = (fbptr_t *)dst;
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*pdst++ = clr;
+		dst += cons.lcdsizex * PIXLBYTES;
+	}
+}
+
+static inline void console_moverow90(u32 rowdst, u32 rowsrc)
+{
+	int i, j;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			      rowdst*VIDEO_FONT_HEIGHT * PIXLBYTES);
+
+	uchar *src = (uchar *)(cons.lcd_address +
+			      rowsrc*VIDEO_FONT_HEIGHT * PIXLBYTES);
+
+	for (j = 0; j < cons.lcdsizey; j++) {
+		fbptr_t *psrc = (fbptr_t *)src;
+		fbptr_t *pdst = (fbptr_t *)dst;
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*pdst++ = *psrc++;
+		src += cons.lcdsizex * PIXLBYTES;
+		dst += cons.lcdsizex * PIXLBYTES;
+	}
+}
+
+static void lcd_putc_xy180(ushort x, ushort y, char c)
+{
+	int fg_color = lcd_getfgcolor();
+	int bg_color = lcd_getbgcolor();
+	int i, row;
+	uchar *dest = (uchar *)(cons.lcd_address +
+				cons.lcdsizex * PIXLBYTES +
+				cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
+				y * cons.lcdsizex * PIXLBYTES -
+				(x+1) * PIXLBYTES);
+
+	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
+		fbptr_t *d = (fbptr_t *)dest;
 		uchar bits;
 		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
 
 		for (i = 0; i < 8; ++i) {
-			*d++ = (bits & 0x80) ? fg_color : bg_color;
+			*d-- = (bits & 0x80) ? fg_color : bg_color;
 			bits <<= 1;
 		}
+		dest -= cons.lcdsizex * PIXLBYTES;
 	}
 }
 
-static void console_scrollup(void)
+static void lcd_putc_xy270(ushort x, ushort y, char c)
 {
-	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
+	int fg_color = lcd_getfgcolor();
 	int bg_color = lcd_getbgcolor();
+	int col, i;
+	uchar *dest = (uchar *)(cons.lcd_address +
+				((x+1) * cons.lcdsizex) * PIXLBYTES -
+				y * PIXLBYTES);
+
+	fbptr_t *d = (fbptr_t *)dest;
+	uchar msk = 0x80;
+	uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
+	for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
+		for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
+			*d-- = (*(pfont + i) & msk) ? fg_color : bg_color;
+		msk >>= 1;
+		d += (cons.lcdsizex + VIDEO_FONT_HEIGHT);
+	}
+}
 
-	/* Copy up rows ignoring those that will be overwritten */
-	memcpy(CONSOLE_ROW_FIRST,
-	       cons.lcd_address + CONSOLE_ROW_SIZE * rows,
-	       CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows);
+static inline void console_setrow270(u32 row, int clr)
+{
+	int i, j;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			       cons.lcdsizex * PIXLBYTES -
+			       (row*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
+
+	for (j = 0; j < cons.lcdsizey; j++) {
+		fbptr_t *pdst = (fbptr_t *)dst;
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*pdst-- = clr;
+		dst += cons.lcdsizex * PIXLBYTES;
+	}
+}
 
-	/* Clear the last rows */
-#if (LCD_BPP != LCD_COLOR32)
-	memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
-	       bg_color, CONSOLE_ROW_SIZE * rows);
-#else
-	u32 *ppix = cons.lcd_address +
-		    CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
-	u32 i;
-	for (i = 0;
-	    i < (CONSOLE_ROW_SIZE * rows) / NBYTES(panel_info.vl_bpix);
-	    i++) {
-		*ppix++ = bg_color;
+static inline void console_moverow270(u32 rowdst, u32 rowsrc)
+{
+	int i, j;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			      cons.lcdsizex * PIXLBYTES -
+			      (rowdst*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
+
+	uchar *src = (uchar *)(cons.lcd_address +
+			      cons.lcdsizex * PIXLBYTES -
+			      (rowsrc*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
+
+	for (j = 0; j < cons.lcdsizey; j++) {
+		fbptr_t *psrc = (fbptr_t *)src;
+		fbptr_t *pdst = (fbptr_t *)dst;
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*pdst-- = *psrc--;
+		src += cons.lcdsizex * PIXLBYTES;
+		dst += cons.lcdsizex * PIXLBYTES;
 	}
-#endif
-	lcd_sync();
-	cons.curr_row -= rows;
 }
 
+static inline void console_setrow180(u32 row, int clr)
+{
+	int i;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			       (cons.rows-row-1) * VIDEO_FONT_HEIGHT *
+			       cons.lcdsizex * PIXLBYTES);
+
+	fbptr_t *d = (fbptr_t *)dst;
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
+		*d++ = clr;
+}
+
+static inline void console_moverow180(u32 rowdst, u32 rowsrc)
+{
+	int i;
+	uchar *dst = (uchar *)(cons.lcd_address +
+			       (cons.rows-rowdst-1) * VIDEO_FONT_HEIGHT *
+			       cons.lcdsizex * PIXLBYTES);
+
+	uchar *src = (uchar *)(cons.lcd_address +
+			       (cons.rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
+			       cons.lcdsizex * PIXLBYTES);
+
+	fbptr_t *pdst = (fbptr_t *)dst;
+	fbptr_t *psrc = (fbptr_t *)src;
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
+		*pdst++ = *psrc++;
+}
+
+#endif /* CONFIG_LCD_ROTATION */
+
 static inline void console_back(void)
 {
 	if (--cons.curr_col < 0) {
@@ -121,26 +281,83 @@ static inline void console_back(void)
 			cons.curr_row = 0;
 	}
 
-	lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
-		    cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
+	cons.fp_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
+			cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
 }
 
 static inline void console_newline(void)
 {
+	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
+	int bg_color = lcd_getbgcolor();
+	int i;
+
 	cons.curr_col = 0;
 
 	/* Check if we need to scroll the terminal */
-	if (++cons.curr_row >= cons.rows)
-		console_scrollup();
-	else
-		lcd_sync();
+	if (++cons.curr_row >= cons.rows) {
+		for (i = 0; i < cons.rows-rows; i++)
+			cons.fp_console_moverow(i, i+rows);
+		for (i = 0; i < rows; i++)
+			cons.fp_console_setrow(cons.rows-i-1, bg_color);
+		cons.curr_row -= rows;
+	}
+	lcd_sync();
+}
+
+static void console_calc_rowcol(int vl_cols, int vl_rows, short *cl, short *rw)
+{
+	*cl = vl_cols / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+	*rw = (vl_rows - BMP_LOGO_HEIGHT);
+	*rw /= VIDEO_FONT_HEIGHT;
+#else
+	*rw = vl_rows / VIDEO_FONT_HEIGHT;
+#endif
+}
+
+void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot)
+{
+	memset(&cons, 0, sizeof(cons));
+	cons.lcd_address = address;
+
+	cons.lcdsizex = vl_cols;
+	cons.lcdsizey = vl_rows;
+
+	if (vl_rot == 0) {
+		cons.fp_putc_xy = &lcd_putc_xy0;
+		cons.fp_console_moverow = &console_moverow0;
+		cons.fp_console_setrow = &console_setrow0;
+		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
+#ifdef CONFIG_LCD_ROTATION
+	} else if (vl_rot == 90) {
+		cons.fp_putc_xy = &lcd_putc_xy90;
+		cons.fp_console_moverow = &console_moverow90;
+		cons.fp_console_setrow = &console_setrow90;
+		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
+	} else if (vl_rot == 180) {
+		cons.fp_putc_xy = &lcd_putc_xy180;
+		cons.fp_console_moverow = &console_moverow180;
+		cons.fp_console_setrow = &console_setrow180;
+		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
+	} else if (vl_rot == 270) {
+		cons.fp_putc_xy = &lcd_putc_xy270;
+		cons.fp_console_moverow = &console_moverow270;
+		cons.fp_console_setrow = &console_setrow270;
+		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
+#endif
+	} else {
+		puts("lcd_init_console: invalid framebuffer rotation!\n");
+	}
+
+	debug("lcd_console: have %d/%d col/rws on scr %dx%d (%d deg rotated)\n",
+	      cons.cols, cons.rows, cons.lcdsizex, cons.lcdsizey, vl_rot);
+
 }
 
 void lcd_putc(const char c)
 {
 	if (!lcd_is_enabled) {
 		serial_putc(c);
-
 		return;
 	}
 
@@ -150,7 +367,6 @@ void lcd_putc(const char c)
 		return;
 	case '\n':
 		console_newline();
-
 		return;
 	case '\t':	/* Tab (8 chars alignment) */
 		cons.curr_col +=  8;
@@ -158,15 +374,13 @@ void lcd_putc(const char c)
 
 		if (cons.curr_col >= cons.cols)
 			console_newline();
-
 		return;
 	case '\b':
 		console_back();
-
 		return;
 	default:
-		lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
-			    cons.curr_row * VIDEO_FONT_HEIGHT, c);
+		cons.fp_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
+				cons.curr_row * VIDEO_FONT_HEIGHT, c);
 		if (++cons.curr_col >= cons.cols)
 			console_newline();
 	}
@@ -176,7 +390,6 @@ void lcd_puts(const char *s)
 {
 	if (!lcd_is_enabled) {
 		serial_puts(s);
-
 		return;
 	}
 
diff --git a/include/lcd.h b/include/lcd.h
index f049fd3..41e0c11 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -51,6 +51,7 @@ void lcd_set_flush_dcache(int flush);
 typedef struct vidinfo {
 	ushort	vl_col;		/* Number of columns (i.e. 160) */
 	ushort	vl_row;		/* Number of rows (i.e. 100) */
+	ushort	vl_rot;		/* Rotation of Display (i.e. 90) */
 	u_char	vl_bpix;	/* Bits per pixel, 0 = 1 */
 	ushort	*cmap;		/* Pointer to the colormap */
 	void	*priv;		/* Pointer to driver-specific data */
diff --git a/include/lcd_console.h b/include/lcd_console.h
index 429214d..2244b3f 100644
--- a/include/lcd_console.h
+++ b/include/lcd_console.h
@@ -16,11 +16,12 @@
  * console has.
  *
  * @address: Console base address
- * @rows: Number of rows in the console
- * @cols: Number of columns in the console
+ * @vl_rows: Number of rows in the console
+ * @vl_cols: Number of columns in the console
+ * @vl_rot: Rotation of display in degree (0 - 90 - 180 - 270) counterlockwise
  */
-void lcd_init_console(void *address, int rows, int cols);
-
+/*void lcd_init_console(void *address, int rows, int cols); */
+void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot);
 /**
  * lcd_set_col() - Set the number of the current lcd console column
  *
-- 
1.7.9.5

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Igor Grinberg]

Hi Hannes,

On 03/11/15 14:57, Hannes Petermaier wrote:
> From: Hannes Petermaier <hannes.petermaier@br-automation.com>
> 
> Sometimes, for example if the display is mounted in portrait mode or even if it
> mounted landscape but rotated by 180 degrees, we need to rotate our content of
> the display respectively the framebuffer, so that user can read the messages
> who are printed out.
> 
> For this we introduce the feature called "CONFIG_LCD_ROTATION", this may be
> defined in the board-configuration if needed. After this the lcd_console will
> be initialized with a given rotation from "vl_rot" out of "vidinfo_t" which is
> provided by the board specific code.
> 
> If CONFIG_LCD_ROTATION is not defined, the console will be initialized with
> 0 degrees rotation - the screen behaves like the days before.
> 
> Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
> Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>

[...]

> diff --git a/common/lcd.c b/common/lcd.c
> index f33942c..dfa4c69 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -167,7 +167,6 @@ int drv_lcd_init(void)
>  
>  void lcd_clear(void)
>  {
> -	short console_rows, console_cols;
>  	int bg_color;
>  	char *s;
>  	ulong addr;
> @@ -211,16 +210,21 @@ void lcd_clear(void)
>  	}
>  #endif
>  #endif
> -	/* Paint the logo and retrieve LCD base address */
> -	debug("[LCD] Drawing the logo...\n");
> -#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
> -	console_rows = (panel_info.vl_row - BMP_LOGO_HEIGHT);
> -	console_rows /= VIDEO_FONT_HEIGHT;
> +	/* setup text-console */
> +	debug("[LCD] setting up console...\n");
> +#ifdef CONFIG_LCD_ROTATION
> +	lcd_init_console(lcd_base,
> +			 panel_info.vl_col,
> +			 panel_info.vl_row,
> +			 panel_info.vl_rot);
>  #else
> -	console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
> +	lcd_init_console(lcd_base,
> +			 panel_info.vl_col,
> +			 panel_info.vl_row,
> +			 0);
>  #endif

Please, don't start the #ifdef mess here...
just always pass the panel_info.vl_rot.

> -	console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
> -	lcd_init_console(lcd_base, console_rows, console_cols);
> +	/* Paint the logo and retrieve LCD base address */
> +	debug("[LCD] Drawing the logo...\n");
>  	if (do_splash) {
>  		s = getenv("splashimage");
>  		if (s) {
> diff --git a/common/lcd_console.c b/common/lcd_console.c
> index cac77be..6199c9a 100644
> --- a/common/lcd_console.c
> +++ b/common/lcd_console.c
> @@ -2,6 +2,7 @@
>   * (C) Copyright 2001-2014
>   * DENX Software Engineering -- wd at denx.de
>   * Compulab Ltd - http://compulab.co.il/
> + * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
>   *
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
> @@ -10,26 +11,27 @@
>  #include <lcd.h>
>  #include <video_font.h>		/* Get font data, width and height */
>  
> -#define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
> -#define CONSOLE_ROW_FIRST	cons.lcd_address
> -#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * cons.rows)
> +#define PIXLBYTES		(NBYTES(LCD_BPP))
> +
> +#if LCD_BPP == LCD_COLOR16
> +	#define fbptr_t ushort
> +#elif LCD_BPP == LCD_COLOR32
> +	#define fbptr_t u32
> +#else
> +	#define fbptr_t uchar
> +#endif
>  
>  struct console_t {
>  	short curr_col, curr_row;
>  	short cols, rows;
>  	void *lcd_address;
> +	u32 lcdsizex, lcdsizey;
> +	void (*fp_putc_xy)(ushort x, ushort y, char c);
> +	void (*fp_console_moverow)(u32 rowdst, u32 rowsrc);
> +	void (*fp_console_setrow)(u32 row, int clr);
>  };
>  static struct console_t cons;
>  
> -void lcd_init_console(void *address, int rows, int cols)
> -{
> -	memset(&cons, 0, sizeof(cons));
> -	cons.cols = cols;
> -	cons.rows = rows;
> -	cons.lcd_address = address;
> -
> -}
> -
>  void lcd_set_col(short col)
>  {
>  	cons.curr_col = col;
> @@ -56,63 +58,221 @@ int lcd_get_screen_columns(void)
>  	return cons.cols;
>  }
>  
> -static void lcd_putc_xy(ushort x, ushort y, char c)
> +static void lcd_putc_xy0(ushort x, ushort y, char c)
>  {
> -	uchar *dest;
> -	ushort row;
>  	int fg_color = lcd_getfgcolor();
>  	int bg_color = lcd_getbgcolor();
> +	int i, row;
> +	uchar *dest = (uchar *)(cons.lcd_address +
> +				y * cons.lcdsizex * PIXLBYTES +
> +				x * PIXLBYTES);
> +
> +	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
> +		fbptr_t *d = (fbptr_t *)dest;
> +		uchar bits;
> +		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
> +		for (i = 0; i < 8; ++i) {
> +			*d++ = (bits & 0x80) ? fg_color : bg_color;
> +			bits <<= 1;
> +		}
> +		dest += cons.lcdsizex * PIXLBYTES;
> +	}
> +}
> +
> +static inline void console_setrow0(u32 row, int clr)
> +{
>  	int i;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       row * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
>  
> -	dest = (uchar *)(cons.lcd_address +
> -			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
> +	fbptr_t *d = (fbptr_t *)dst;
> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> +		*d++ = clr;
> +}
>  
> -	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
> -#if LCD_BPP == LCD_COLOR16
> -		ushort *d = (ushort *)dest;
> -#elif LCD_BPP == LCD_COLOR32
> -		u32 *d = (u32 *)dest;
> -#else
> -		uchar *d = dest;
> -#endif
> +static inline void console_moverow0(u32 rowdst, u32 rowsrc)
> +{
> +	int i;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       rowdst * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
> +
> +	uchar *src = (uchar *)(cons.lcd_address +
> +			       rowsrc * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
> +
> +	fbptr_t *pdst = (fbptr_t *)dst;
> +	fbptr_t *psrc = (fbptr_t *)src;
> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> +		*pdst++ = *psrc++;
> +}
> +#ifdef CONFIG_LCD_ROTATION
> +static void lcd_putc_xy90(ushort x, ushort y, char c)
> +{
> +	int fg_color = lcd_getfgcolor();
> +	int bg_color = lcd_getbgcolor();
> +	int i, col;
> +	uchar *dest = (uchar *)(cons.lcd_address +
> +				cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
> +				(x+1) * cons.lcdsizex * PIXLBYTES +
> +				y * PIXLBYTES);
> +
> +	fbptr_t *d = (fbptr_t *)dest;
> +	uchar msk = 0x80;
> +	uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
> +	for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
> +		for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
> +			*d++ = (*(pfont + i) & msk) ? fg_color : bg_color;
> +		msk >>= 1;
> +		d -= (cons.lcdsizex + VIDEO_FONT_HEIGHT);
> +	}
> +}
> +
> +static inline void console_setrow90(u32 row, int clr)
> +{
> +	int i, j;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       row*VIDEO_FONT_HEIGHT * PIXLBYTES);
> +
> +	for (j = 0; j < cons.lcdsizey; j++) {
> +		fbptr_t *pdst = (fbptr_t *)dst;
> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
> +			*pdst++ = clr;
> +		dst += cons.lcdsizex * PIXLBYTES;
> +	}
> +}
> +
> +static inline void console_moverow90(u32 rowdst, u32 rowsrc)
> +{
> +	int i, j;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			      rowdst*VIDEO_FONT_HEIGHT * PIXLBYTES);
> +
> +	uchar *src = (uchar *)(cons.lcd_address +
> +			      rowsrc*VIDEO_FONT_HEIGHT * PIXLBYTES);
> +
> +	for (j = 0; j < cons.lcdsizey; j++) {
> +		fbptr_t *psrc = (fbptr_t *)src;
> +		fbptr_t *pdst = (fbptr_t *)dst;
> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
> +			*pdst++ = *psrc++;
> +		src += cons.lcdsizex * PIXLBYTES;
> +		dst += cons.lcdsizex * PIXLBYTES;
> +	}
> +}
> +
> +static void lcd_putc_xy180(ushort x, ushort y, char c)
> +{
> +	int fg_color = lcd_getfgcolor();
> +	int bg_color = lcd_getbgcolor();
> +	int i, row;
> +	uchar *dest = (uchar *)(cons.lcd_address +
> +				cons.lcdsizex * PIXLBYTES +
> +				cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
> +				y * cons.lcdsizex * PIXLBYTES -
> +				(x+1) * PIXLBYTES);
> +
> +	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
> +		fbptr_t *d = (fbptr_t *)dest;
>  		uchar bits;
>  		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
>  
>  		for (i = 0; i < 8; ++i) {
> -			*d++ = (bits & 0x80) ? fg_color : bg_color;
> +			*d-- = (bits & 0x80) ? fg_color : bg_color;
>  			bits <<= 1;
>  		}
> +		dest -= cons.lcdsizex * PIXLBYTES;
>  	}
>  }
>  
> -static void console_scrollup(void)
> +static void lcd_putc_xy270(ushort x, ushort y, char c)
>  {
> -	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
> +	int fg_color = lcd_getfgcolor();
>  	int bg_color = lcd_getbgcolor();
> +	int col, i;
> +	uchar *dest = (uchar *)(cons.lcd_address +
> +				((x+1) * cons.lcdsizex) * PIXLBYTES -
> +				y * PIXLBYTES);
> +
> +	fbptr_t *d = (fbptr_t *)dest;
> +	uchar msk = 0x80;
> +	uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
> +	for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
> +		for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
> +			*d-- = (*(pfont + i) & msk) ? fg_color : bg_color;
> +		msk >>= 1;
> +		d += (cons.lcdsizex + VIDEO_FONT_HEIGHT);
> +	}
> +}
>  
> -	/* Copy up rows ignoring those that will be overwritten */
> -	memcpy(CONSOLE_ROW_FIRST,
> -	       cons.lcd_address + CONSOLE_ROW_SIZE * rows,
> -	       CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows);
> +static inline void console_setrow270(u32 row, int clr)
> +{
> +	int i, j;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       cons.lcdsizex * PIXLBYTES -
> +			       (row*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
> +
> +	for (j = 0; j < cons.lcdsizey; j++) {
> +		fbptr_t *pdst = (fbptr_t *)dst;
> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
> +			*pdst-- = clr;
> +		dst += cons.lcdsizex * PIXLBYTES;
> +	}
> +}
>  
> -	/* Clear the last rows */
> -#if (LCD_BPP != LCD_COLOR32)
> -	memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
> -	       bg_color, CONSOLE_ROW_SIZE * rows);
> -#else
> -	u32 *ppix = cons.lcd_address +
> -		    CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
> -	u32 i;
> -	for (i = 0;
> -	    i < (CONSOLE_ROW_SIZE * rows) / NBYTES(panel_info.vl_bpix);
> -	    i++) {
> -		*ppix++ = bg_color;
> +static inline void console_moverow270(u32 rowdst, u32 rowsrc)
> +{
> +	int i, j;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			      cons.lcdsizex * PIXLBYTES -
> +			      (rowdst*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
> +
> +	uchar *src = (uchar *)(cons.lcd_address +
> +			      cons.lcdsizex * PIXLBYTES -
> +			      (rowsrc*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
> +
> +	for (j = 0; j < cons.lcdsizey; j++) {
> +		fbptr_t *psrc = (fbptr_t *)src;
> +		fbptr_t *pdst = (fbptr_t *)dst;
> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
> +			*pdst-- = *psrc--;
> +		src += cons.lcdsizex * PIXLBYTES;
> +		dst += cons.lcdsizex * PIXLBYTES;
>  	}
> -#endif
> -	lcd_sync();
> -	cons.curr_row -= rows;
>  }
>  
> +static inline void console_setrow180(u32 row, int clr)
> +{
> +	int i;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       (cons.rows-row-1) * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
> +
> +	fbptr_t *d = (fbptr_t *)dst;
> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> +		*d++ = clr;
> +}
> +
> +static inline void console_moverow180(u32 rowdst, u32 rowsrc)
> +{
> +	int i;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       (cons.rows-rowdst-1) * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
> +
> +	uchar *src = (uchar *)(cons.lcd_address +
> +			       (cons.rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
> +
> +	fbptr_t *pdst = (fbptr_t *)dst;
> +	fbptr_t *psrc = (fbptr_t *)src;
> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> +		*pdst++ = *psrc++;
> +}
> +
> +#endif /* CONFIG_LCD_ROTATION */

Can't this whole thing go into a separate file?
So, the console stuff will only define weak functions which can be overridden
by the rotation functionality.
This will keep the console code clean (also from ifdefs) and have the rotation
functionality cleanly added by a CONFIG_ symbol, which will control the
compilation for the separate file.

> +
>  static inline void console_back(void)
>  {
>  	if (--cons.curr_col < 0) {
> @@ -121,26 +281,83 @@ static inline void console_back(void)
>  			cons.curr_row = 0;
>  	}
>  
> -	lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
> -		    cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
> +	cons.fp_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
> +			cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
>  }
>  
>  static inline void console_newline(void)
>  {
> +	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
> +	int bg_color = lcd_getbgcolor();
> +	int i;
> +
>  	cons.curr_col = 0;
>  
>  	/* Check if we need to scroll the terminal */
> -	if (++cons.curr_row >= cons.rows)
> -		console_scrollup();
> -	else
> -		lcd_sync();
> +	if (++cons.curr_row >= cons.rows) {
> +		for (i = 0; i < cons.rows-rows; i++)
> +			cons.fp_console_moverow(i, i+rows);
> +		for (i = 0; i < rows; i++)
> +			cons.fp_console_setrow(cons.rows-i-1, bg_color);
> +		cons.curr_row -= rows;
> +	}
> +	lcd_sync();
> +}
> +
> +static void console_calc_rowcol(int vl_cols, int vl_rows, short *cl, short *rw)
> +{
> +	*cl = vl_cols / VIDEO_FONT_WIDTH;
> +#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
> +	*rw = (vl_rows - BMP_LOGO_HEIGHT);
> +	*rw /= VIDEO_FONT_HEIGHT;
> +#else
> +	*rw = vl_rows / VIDEO_FONT_HEIGHT;
> +#endif
> +}
> +
> +void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot)
> +{
> +	memset(&cons, 0, sizeof(cons));
> +	cons.lcd_address = address;
> +
> +	cons.lcdsizex = vl_cols;
> +	cons.lcdsizey = vl_rows;
> +
> +	if (vl_rot == 0) {
> +		cons.fp_putc_xy = &lcd_putc_xy0;
> +		cons.fp_console_moverow = &console_moverow0;
> +		cons.fp_console_setrow = &console_setrow0;
> +		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);

This call can be made after the if else structure and written only once.

> +#ifdef CONFIG_LCD_ROTATION
> +	} else if (vl_rot == 90) {
> +		cons.fp_putc_xy = &lcd_putc_xy90;
> +		cons.fp_console_moverow = &console_moverow90;
> +		cons.fp_console_setrow = &console_setrow90;
> +		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
> +	} else if (vl_rot == 180) {
> +		cons.fp_putc_xy = &lcd_putc_xy180;
> +		cons.fp_console_moverow = &console_moverow180;
> +		cons.fp_console_setrow = &console_setrow180;
> +		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
> +	} else if (vl_rot == 270) {
> +		cons.fp_putc_xy = &lcd_putc_xy270;
> +		cons.fp_console_moverow = &console_moverow270;
> +		cons.fp_console_setrow = &console_setrow270;
> +		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
> +#endif
> +	} else {
> +		puts("lcd_init_console: invalid framebuffer rotation!\n");
> +	}

I would recommend extracting the whole if else ... structure into
a separate function say lcd_setup_console_rot() or something and
make the default one doing only the vl_rot == 0 stuff.

> +
> +	debug("lcd_console: have %d/%d col/rws on scr %dx%d (%d deg rotated)\n",
> +	      cons.cols, cons.rows, cons.lcdsizex, cons.lcdsizey, vl_rot);
> +
>  }
>  
>  void lcd_putc(const char c)
>  {
>  	if (!lcd_is_enabled) {
>  		serial_putc(c);
> -
>  		return;
>  	}
>  

[...]


-- 
Regards,
Igor.

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Hannes Petermaier]

On 2015-03-12 13:26, Igor Grinberg wrote:
> Hi Hannes,
Hi Igor,
thanks for response.
>   #endif
> -	/* Paint the logo and retrieve LCD base address */
> -	debug("[LCD] Drawing the logo...\n");
> -#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
> -	console_rows = (panel_info.vl_row - BMP_LOGO_HEIGHT);
> -	console_rows /= VIDEO_FONT_HEIGHT;
> +	/* setup text-console */
> +	debug("[LCD] setting up console...\n");
> +#ifdef CONFIG_LCD_ROTATION
> +	lcd_init_console(lcd_base,
> +			 panel_info.vl_col,
> +			 panel_info.vl_row,
> +			 panel_info.vl_rot);
>   #else
> -	console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
> +	lcd_init_console(lcd_base,
> +			 panel_info.vl_col,
> +			 panel_info.vl_row,
> +			 0);
>   #endif
> Please, don't start the #ifdef mess here...
> just always pass the panel_info.vl_rot.
This is not possible, because 'vl_rot' does'nt exist if 
CONFIG_LCD_ROTATION is not defined. (have a look into lcd.h). I made 
this to be compatible to all who have allready initialized a panel_info 
without vl_rot.
>
>> -	console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
>> -	lcd_init_console(lcd_base, console_rows, console_cols);
>> +	/* Paint the logo and retrieve LCD base address */
>> +	debug("[LCD] Drawing the logo...\n");
>>   	if (do_splash) {
>>   		s = getenv("splashimage");
>>   		if (s) {
>> diff --git a/common/lcd_console.c b/common/lcd_console.c
>> index cac77be..6199c9a 100644
>> --- a/common/lcd_console.c
>> +++ b/common/lcd_console.c
>> @@ -2,6 +2,7 @@
>>    * (C) Copyright 2001-2014
>>    * DENX Software Engineering -- wd at denx.de
>>    * Compulab Ltd - http://compulab.co.il/
>> + * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
>>    *
>>    * SPDX-License-Identifier:	GPL-2.0+
>>    */
>> @@ -10,26 +11,27 @@
>>   #include <lcd.h>
>>   #include <video_font.h>		/* Get font data, width and height */
>>   
>> -#define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
>> -#define CONSOLE_ROW_FIRST	cons.lcd_address
>> -#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * cons.rows)
>> +#define PIXLBYTES		(NBYTES(LCD_BPP))
>> +
>> +#if LCD_BPP == LCD_COLOR16
>> +	#define fbptr_t ushort
>> +#elif LCD_BPP == LCD_COLOR32
>> +	#define fbptr_t u32
>> +#else
>> +	#define fbptr_t uchar
>> +#endif
>>   
>>   struct console_t {
>>   	short curr_col, curr_row;
>>   	short cols, rows;
>>   	void *lcd_address;
>> +	u32 lcdsizex, lcdsizey;
>> +	void (*fp_putc_xy)(ushort x, ushort y, char c);
>> +	void (*fp_console_moverow)(u32 rowdst, u32 rowsrc);
>> +	void (*fp_console_setrow)(u32 row, int clr);
>>   };
>>   static struct console_t cons;
>>   
>> -void lcd_init_console(void *address, int rows, int cols)
>> -{
>> -	memset(&cons, 0, sizeof(cons));
>> -	cons.cols = cols;
>> -	cons.rows = rows;
>> -	cons.lcd_address = address;
>> -
>> -}
>> -
>>   void lcd_set_col(short col)
>>   {
>>   	cons.curr_col = col;
>> @@ -56,63 +58,221 @@ int lcd_get_screen_columns(void)
>>   	return cons.cols;
>>   }
>>   
>> -static void lcd_putc_xy(ushort x, ushort y, char c)
>> +static void lcd_putc_xy0(ushort x, ushort y, char c)
>>   {
>> -	uchar *dest;
>> -	ushort row;
>>   	int fg_color = lcd_getfgcolor();
>>   	int bg_color = lcd_getbgcolor();
>> +	int i, row;
>> +	uchar *dest = (uchar *)(cons.lcd_address +
>> +				y * cons.lcdsizex * PIXLBYTES +
>> +				x * PIXLBYTES);
>> +
>> +	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
>> +		fbptr_t *d = (fbptr_t *)dest;
>> +		uchar bits;
>> +		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
>> +		for (i = 0; i < 8; ++i) {
>> +			*d++ = (bits & 0x80) ? fg_color : bg_color;
>> +			bits <<= 1;
>> +		}
>> +		dest += cons.lcdsizex * PIXLBYTES;
>> +	}
>> +}
>> +
>> +static inline void console_setrow0(u32 row, int clr)
>> +{
>>   	int i;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			       row * VIDEO_FONT_HEIGHT *
>> +			       cons.lcdsizex * PIXLBYTES);
>>   
>> -	dest = (uchar *)(cons.lcd_address +
>> -			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
>> +	fbptr_t *d = (fbptr_t *)dst;
>> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>> +		*d++ = clr;
>> +}
>>   
>> -	for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
>> -#if LCD_BPP == LCD_COLOR16
>> -		ushort *d = (ushort *)dest;
>> -#elif LCD_BPP == LCD_COLOR32
>> -		u32 *d = (u32 *)dest;
>> -#else
>> -		uchar *d = dest;
>> -#endif
>> +static inline void console_moverow0(u32 rowdst, u32 rowsrc)
>> +{
>> +	int i;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			       rowdst * VIDEO_FONT_HEIGHT *
>> +			       cons.lcdsizex * PIXLBYTES);
>> +
>> +	uchar *src = (uchar *)(cons.lcd_address +
>> +			       rowsrc * VIDEO_FONT_HEIGHT *
>> +			       cons.lcdsizex * PIXLBYTES);
>> +
>> +	fbptr_t *pdst = (fbptr_t *)dst;
>> +	fbptr_t *psrc = (fbptr_t *)src;
>> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>> +		*pdst++ = *psrc++;
>> +}
>> +#ifdef CONFIG_LCD_ROTATION
>> +static void lcd_putc_xy90(ushort x, ushort y, char c)
>> +{
>> +	int fg_color = lcd_getfgcolor();
>> +	int bg_color = lcd_getbgcolor();
>> +	int i, col;
>> +	uchar *dest = (uchar *)(cons.lcd_address +
>> +				cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
>> +				(x+1) * cons.lcdsizex * PIXLBYTES +
>> +				y * PIXLBYTES);
>> +
>> +	fbptr_t *d = (fbptr_t *)dest;
>> +	uchar msk = 0x80;
>> +	uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
>> +	for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
>> +		for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
>> +			*d++ = (*(pfont + i) & msk) ? fg_color : bg_color;
>> +		msk >>= 1;
>> +		d -= (cons.lcdsizex + VIDEO_FONT_HEIGHT);
>> +	}
>> +}
>> +
>> +static inline void console_setrow90(u32 row, int clr)
>> +{
>> +	int i, j;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			       row*VIDEO_FONT_HEIGHT * PIXLBYTES);
>> +
>> +	for (j = 0; j < cons.lcdsizey; j++) {
>> +		fbptr_t *pdst = (fbptr_t *)dst;
>> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>> +			*pdst++ = clr;
>> +		dst += cons.lcdsizex * PIXLBYTES;
>> +	}
>> +}
>> +
>> +static inline void console_moverow90(u32 rowdst, u32 rowsrc)
>> +{
>> +	int i, j;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			      rowdst*VIDEO_FONT_HEIGHT * PIXLBYTES);
>> +
>> +	uchar *src = (uchar *)(cons.lcd_address +
>> +			      rowsrc*VIDEO_FONT_HEIGHT * PIXLBYTES);
>> +
>> +	for (j = 0; j < cons.lcdsizey; j++) {
>> +		fbptr_t *psrc = (fbptr_t *)src;
>> +		fbptr_t *pdst = (fbptr_t *)dst;
>> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>> +			*pdst++ = *psrc++;
>> +		src += cons.lcdsizex * PIXLBYTES;
>> +		dst += cons.lcdsizex * PIXLBYTES;
>> +	}
>> +}
>> +
>> +static void lcd_putc_xy180(ushort x, ushort y, char c)
>> +{
>> +	int fg_color = lcd_getfgcolor();
>> +	int bg_color = lcd_getbgcolor();
>> +	int i, row;
>> +	uchar *dest = (uchar *)(cons.lcd_address +
>> +				cons.lcdsizex * PIXLBYTES +
>> +				cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
>> +				y * cons.lcdsizex * PIXLBYTES -
>> +				(x+1) * PIXLBYTES);
>> +
>> +	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
>> +		fbptr_t *d = (fbptr_t *)dest;
>>   		uchar bits;
>>   		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
>>   
>>   		for (i = 0; i < 8; ++i) {
>> -			*d++ = (bits & 0x80) ? fg_color : bg_color;
>> +			*d-- = (bits & 0x80) ? fg_color : bg_color;
>>   			bits <<= 1;
>>   		}
>> +		dest -= cons.lcdsizex * PIXLBYTES;
>>   	}
>>   }
>>   
>> -static void console_scrollup(void)
>> +static void lcd_putc_xy270(ushort x, ushort y, char c)
>>   {
>> -	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
>> +	int fg_color = lcd_getfgcolor();
>>   	int bg_color = lcd_getbgcolor();
>> +	int col, i;
>> +	uchar *dest = (uchar *)(cons.lcd_address +
>> +				((x+1) * cons.lcdsizex) * PIXLBYTES -
>> +				y * PIXLBYTES);
>> +
>> +	fbptr_t *d = (fbptr_t *)dest;
>> +	uchar msk = 0x80;
>> +	uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
>> +	for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
>> +		for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
>> +			*d-- = (*(pfont + i) & msk) ? fg_color : bg_color;
>> +		msk >>= 1;
>> +		d += (cons.lcdsizex + VIDEO_FONT_HEIGHT);
>> +	}
>> +}
>>   
>> -	/* Copy up rows ignoring those that will be overwritten */
>> -	memcpy(CONSOLE_ROW_FIRST,
>> -	       cons.lcd_address + CONSOLE_ROW_SIZE * rows,
>> -	       CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows);
>> +static inline void console_setrow270(u32 row, int clr)
>> +{
>> +	int i, j;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			       cons.lcdsizex * PIXLBYTES -
>> +			       (row*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
>> +
>> +	for (j = 0; j < cons.lcdsizey; j++) {
>> +		fbptr_t *pdst = (fbptr_t *)dst;
>> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>> +			*pdst-- = clr;
>> +		dst += cons.lcdsizex * PIXLBYTES;
>> +	}
>> +}
>>   
>> -	/* Clear the last rows */
>> -#if (LCD_BPP != LCD_COLOR32)
>> -	memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
>> -	       bg_color, CONSOLE_ROW_SIZE * rows);
>> -#else
>> -	u32 *ppix = cons.lcd_address +
>> -		    CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
>> -	u32 i;
>> -	for (i = 0;
>> -	    i < (CONSOLE_ROW_SIZE * rows) / NBYTES(panel_info.vl_bpix);
>> -	    i++) {
>> -		*ppix++ = bg_color;
>> +static inline void console_moverow270(u32 rowdst, u32 rowsrc)
>> +{
>> +	int i, j;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			      cons.lcdsizex * PIXLBYTES -
>> +			      (rowdst*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
>> +
>> +	uchar *src = (uchar *)(cons.lcd_address +
>> +			      cons.lcdsizex * PIXLBYTES -
>> +			      (rowsrc*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
>> +
>> +	for (j = 0; j < cons.lcdsizey; j++) {
>> +		fbptr_t *psrc = (fbptr_t *)src;
>> +		fbptr_t *pdst = (fbptr_t *)dst;
>> +		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>> +			*pdst-- = *psrc--;
>> +		src += cons.lcdsizex * PIXLBYTES;
>> +		dst += cons.lcdsizex * PIXLBYTES;
>>   	}
>> -#endif
>> -	lcd_sync();
>> -	cons.curr_row -= rows;
>>   }
>>   
>> +static inline void console_setrow180(u32 row, int clr)
>> +{
>> +	int i;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			       (cons.rows-row-1) * VIDEO_FONT_HEIGHT *
>> +			       cons.lcdsizex * PIXLBYTES);
>> +
>> +	fbptr_t *d = (fbptr_t *)dst;
>> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>> +		*d++ = clr;
>> +}
>> +
>> +static inline void console_moverow180(u32 rowdst, u32 rowsrc)
>> +{
>> +	int i;
>> +	uchar *dst = (uchar *)(cons.lcd_address +
>> +			       (cons.rows-rowdst-1) * VIDEO_FONT_HEIGHT *
>> +			       cons.lcdsizex * PIXLBYTES);
>> +
>> +	uchar *src = (uchar *)(cons.lcd_address +
>> +			       (cons.rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
>> +			       cons.lcdsizex * PIXLBYTES);
>> +
>> +	fbptr_t *pdst = (fbptr_t *)dst;
>> +	fbptr_t *psrc = (fbptr_t *)src;
>> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>> +		*pdst++ = *psrc++;
>> +}
>> +
>> +#endif /* CONFIG_LCD_ROTATION */
> Can't this whole thing go into a separate file?
> So, the console stuff will only define weak functions which can be overridden
> by the rotation functionality.
> This will keep the console code clean (also from ifdefs) and have the rotation
> functionality cleanly added by a CONFIG_ symbol, which will control the
> compilation for the separate file.
Might be possible, which name should we give to it ? 
lcd_console_rotation.c ?
But how we deal with the function-pointer initialization ?
>
>> +
>>   static inline void console_back(void)
>>   {
>>   	if (--cons.curr_col < 0) {
>> @@ -121,26 +281,83 @@ static inline void console_back(void)
>>   			cons.curr_row = 0;
>>   	}
>>   
>> -	lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
>> -		    cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
>> +	cons.fp_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
>> +			cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
>>   }
>>   
>>   static inline void console_newline(void)
>>   {
>> +	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
>> +	int bg_color = lcd_getbgcolor();
>> +	int i;
>> +
>>   	cons.curr_col = 0;
>>   
>>   	/* Check if we need to scroll the terminal */
>> -	if (++cons.curr_row >= cons.rows)
>> -		console_scrollup();
>> -	else
>> -		lcd_sync();
>> +	if (++cons.curr_row >= cons.rows) {
>> +		for (i = 0; i < cons.rows-rows; i++)
>> +			cons.fp_console_moverow(i, i+rows);
>> +		for (i = 0; i < rows; i++)
>> +			cons.fp_console_setrow(cons.rows-i-1, bg_color);
>> +		cons.curr_row -= rows;
>> +	}
>> +	lcd_sync();
>> +}
>> +
>> +static void console_calc_rowcol(int vl_cols, int vl_rows, short *cl, short *rw)
>> +{
>> +	*cl = vl_cols / VIDEO_FONT_WIDTH;
>> +#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
>> +	*rw = (vl_rows - BMP_LOGO_HEIGHT);
>> +	*rw /= VIDEO_FONT_HEIGHT;
>> +#else
>> +	*rw = vl_rows / VIDEO_FONT_HEIGHT;
>> +#endif
>> +}
>> +
>> +void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot)
>> +{
>> +	memset(&cons, 0, sizeof(cons));
>> +	cons.lcd_address = address;
>> +
>> +	cons.lcdsizex = vl_cols;
>> +	cons.lcdsizey = vl_rows;
>> +
>> +	if (vl_rot == 0) {
>> +		cons.fp_putc_xy = &lcd_putc_xy0;
>> +		cons.fp_console_moverow = &console_moverow0;
>> +		cons.fp_console_setrow = &console_setrow0;
>> +		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
> This call can be made after the if else structure and written only once.
No. Have a closer look to it. If display is rotated 0?/180? the 
calculation is not the same if display is rotated 90/270?
>
>> +#ifdef CONFIG_LCD_ROTATION
>> +	} else if (vl_rot == 90) {
>> +		cons.fp_putc_xy = &lcd_putc_xy90;
>> +		cons.fp_console_moverow = &console_moverow90;
>> +		cons.fp_console_setrow = &console_setrow90;
>> +		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
>> +	} else if (vl_rot == 180) {
>> +		cons.fp_putc_xy = &lcd_putc_xy180;
>> +		cons.fp_console_moverow = &console_moverow180;
>> +		cons.fp_console_setrow = &console_setrow180;
>> +		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
>> +	} else if (vl_rot == 270) {
>> +		cons.fp_putc_xy = &lcd_putc_xy270;
>> +		cons.fp_console_moverow = &console_moverow270;
>> +		cons.fp_console_setrow = &console_setrow270;
>> +		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
>> +#endif
>> +	} else {
>> +		puts("lcd_init_console: invalid framebuffer rotation!\n");
>> +	}
> I would recommend extracting the whole if else ... structure into
> a separate function say lcd_setup_console_rot() or something and
> make the default one doing only the vl_rot == 0 stuff.
Whats the use of this ?
Should this also be in a separate file?

best regards,
Hannes

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Igor Grinberg]

Hi Hannes,

On 03/12/15 18:46, Hannes Petermaier wrote:
> 
> On 2015-03-12 13:26, Igor Grinberg wrote:
>> Hi Hannes,
> Hi Igor,
> thanks for response.
>>   #endif
>> -    /* Paint the logo and retrieve LCD base address */
>> -    debug("[LCD] Drawing the logo...\n");
>> -#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
>> -    console_rows = (panel_info.vl_row - BMP_LOGO_HEIGHT);
>> -    console_rows /= VIDEO_FONT_HEIGHT;
>> +    /* setup text-console */
>> +    debug("[LCD] setting up console...\n");
>> +#ifdef CONFIG_LCD_ROTATION
>> +    lcd_init_console(lcd_base,
>> +             panel_info.vl_col,
>> +             panel_info.vl_row,
>> +             panel_info.vl_rot);
>>   #else
>> -    console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
>> +    lcd_init_console(lcd_base,
>> +             panel_info.vl_col,
>> +             panel_info.vl_row,
>> +             0);
>>   #endif
>> Please, don't start the #ifdef mess here...
>> just always pass the panel_info.vl_rot.
> This is not possible, because 'vl_rot' does'nt exist if
> CONFIG_LCD_ROTATION is not defined. (have a look into lcd.h).

Of course I did before sending the reply...
What I'm trying to say is - let it exist and default to 0 angle rotation.

> I made this to be compatible to all who have allready initialized a
> panel_info without vl_rot.

This increases the mess and I think is not sensible enough.
Just change the users to initialize the panel_info with vl_rot.
I think also that default initialization of globals is 0, right?
If so, those users that do not initialize the vl_rot explicitly,
should have it initialized to 0 implicitly by compiler.

>>
>>> -    console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
>>> -    lcd_init_console(lcd_base, console_rows, console_cols);
>>> +    /* Paint the logo and retrieve LCD base address */
>>> +    debug("[LCD] Drawing the logo...\n");
>>>       if (do_splash) {
>>>           s = getenv("splashimage");
>>>           if (s) {
>>> diff --git a/common/lcd_console.c b/common/lcd_console.c
>>> index cac77be..6199c9a 100644
>>> --- a/common/lcd_console.c
>>> +++ b/common/lcd_console.c
>>> @@ -2,6 +2,7 @@
>>>    * (C) Copyright 2001-2014
>>>    * DENX Software Engineering -- wd at denx.de
>>>    * Compulab Ltd - http://compulab.co.il/
>>> + * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
>>>    *
>>>    * SPDX-License-Identifier:    GPL-2.0+
>>>    */
>>> @@ -10,26 +11,27 @@
>>>   #include <lcd.h>
>>>   #include <video_font.h>        /* Get font data, width and height */
>>>   -#define CONSOLE_ROW_SIZE    (VIDEO_FONT_HEIGHT * lcd_line_length)
>>> -#define CONSOLE_ROW_FIRST    cons.lcd_address
>>> -#define CONSOLE_SIZE        (CONSOLE_ROW_SIZE * cons.rows)
>>> +#define PIXLBYTES        (NBYTES(LCD_BPP))
>>> +
>>> +#if LCD_BPP == LCD_COLOR16
>>> +    #define fbptr_t ushort
>>> +#elif LCD_BPP == LCD_COLOR32
>>> +    #define fbptr_t u32
>>> +#else
>>> +    #define fbptr_t uchar
>>> +#endif
>>>     struct console_t {
>>>       short curr_col, curr_row;
>>>       short cols, rows;
>>>       void *lcd_address;
>>> +    u32 lcdsizex, lcdsizey;
>>> +    void (*fp_putc_xy)(ushort x, ushort y, char c);
>>> +    void (*fp_console_moverow)(u32 rowdst, u32 rowsrc);
>>> +    void (*fp_console_setrow)(u32 row, int clr);
>>>   };
>>>   static struct console_t cons;
>>>   -void lcd_init_console(void *address, int rows, int cols)
>>> -{
>>> -    memset(&cons, 0, sizeof(cons));
>>> -    cons.cols = cols;
>>> -    cons.rows = rows;
>>> -    cons.lcd_address = address;
>>> -
>>> -}
>>> -
>>>   void lcd_set_col(short col)
>>>   {
>>>       cons.curr_col = col;
>>> @@ -56,63 +58,221 @@ int lcd_get_screen_columns(void)
>>>       return cons.cols;
>>>   }
>>>   -static void lcd_putc_xy(ushort x, ushort y, char c)
>>> +static void lcd_putc_xy0(ushort x, ushort y, char c)
>>>   {
>>> -    uchar *dest;
>>> -    ushort row;
>>>       int fg_color = lcd_getfgcolor();
>>>       int bg_color = lcd_getbgcolor();
>>> +    int i, row;
>>> +    uchar *dest = (uchar *)(cons.lcd_address +
>>> +                y * cons.lcdsizex * PIXLBYTES +
>>> +                x * PIXLBYTES);
>>> +
>>> +    for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
>>> +        fbptr_t *d = (fbptr_t *)dest;
>>> +        uchar bits;
>>> +        bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
>>> +        for (i = 0; i < 8; ++i) {
>>> +            *d++ = (bits & 0x80) ? fg_color : bg_color;
>>> +            bits <<= 1;
>>> +        }
>>> +        dest += cons.lcdsizex * PIXLBYTES;
>>> +    }
>>> +}
>>> +
>>> +static inline void console_setrow0(u32 row, int clr)
>>> +{
>>>       int i;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                   row * VIDEO_FONT_HEIGHT *
>>> +                   cons.lcdsizex * PIXLBYTES);
>>>   -    dest = (uchar *)(cons.lcd_address +
>>> -             y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
>>> +    fbptr_t *d = (fbptr_t *)dst;
>>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>>> +        *d++ = clr;
>>> +}
>>>   -    for (row = 0; row < VIDEO_FONT_HEIGHT; ++row, dest += lcd_line_length) {
>>> -#if LCD_BPP == LCD_COLOR16
>>> -        ushort *d = (ushort *)dest;
>>> -#elif LCD_BPP == LCD_COLOR32
>>> -        u32 *d = (u32 *)dest;
>>> -#else
>>> -        uchar *d = dest;
>>> -#endif
>>> +static inline void console_moverow0(u32 rowdst, u32 rowsrc)
>>> +{
>>> +    int i;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                   rowdst * VIDEO_FONT_HEIGHT *
>>> +                   cons.lcdsizex * PIXLBYTES);
>>> +
>>> +    uchar *src = (uchar *)(cons.lcd_address +
>>> +                   rowsrc * VIDEO_FONT_HEIGHT *
>>> +                   cons.lcdsizex * PIXLBYTES);
>>> +
>>> +    fbptr_t *pdst = (fbptr_t *)dst;
>>> +    fbptr_t *psrc = (fbptr_t *)src;
>>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>>> +        *pdst++ = *psrc++;
>>> +}
>>> +#ifdef CONFIG_LCD_ROTATION
>>> +static void lcd_putc_xy90(ushort x, ushort y, char c)
>>> +{
>>> +    int fg_color = lcd_getfgcolor();
>>> +    int bg_color = lcd_getbgcolor();
>>> +    int i, col;
>>> +    uchar *dest = (uchar *)(cons.lcd_address +
>>> +                cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
>>> +                (x+1) * cons.lcdsizex * PIXLBYTES +
>>> +                y * PIXLBYTES);
>>> +
>>> +    fbptr_t *d = (fbptr_t *)dest;
>>> +    uchar msk = 0x80;
>>> +    uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
>>> +    for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
>>> +        for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
>>> +            *d++ = (*(pfont + i) & msk) ? fg_color : bg_color;
>>> +        msk >>= 1;
>>> +        d -= (cons.lcdsizex + VIDEO_FONT_HEIGHT);
>>> +    }
>>> +}
>>> +
>>> +static inline void console_setrow90(u32 row, int clr)
>>> +{
>>> +    int i, j;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                   row*VIDEO_FONT_HEIGHT * PIXLBYTES);
>>> +
>>> +    for (j = 0; j < cons.lcdsizey; j++) {
>>> +        fbptr_t *pdst = (fbptr_t *)dst;
>>> +        for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>>> +            *pdst++ = clr;
>>> +        dst += cons.lcdsizex * PIXLBYTES;
>>> +    }
>>> +}
>>> +
>>> +static inline void console_moverow90(u32 rowdst, u32 rowsrc)
>>> +{
>>> +    int i, j;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                  rowdst*VIDEO_FONT_HEIGHT * PIXLBYTES);
>>> +
>>> +    uchar *src = (uchar *)(cons.lcd_address +
>>> +                  rowsrc*VIDEO_FONT_HEIGHT * PIXLBYTES);
>>> +
>>> +    for (j = 0; j < cons.lcdsizey; j++) {
>>> +        fbptr_t *psrc = (fbptr_t *)src;
>>> +        fbptr_t *pdst = (fbptr_t *)dst;
>>> +        for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>>> +            *pdst++ = *psrc++;
>>> +        src += cons.lcdsizex * PIXLBYTES;
>>> +        dst += cons.lcdsizex * PIXLBYTES;
>>> +    }
>>> +}
>>> +
>>> +static void lcd_putc_xy180(ushort x, ushort y, char c)
>>> +{
>>> +    int fg_color = lcd_getfgcolor();
>>> +    int bg_color = lcd_getbgcolor();
>>> +    int i, row;
>>> +    uchar *dest = (uchar *)(cons.lcd_address +
>>> +                cons.lcdsizex * PIXLBYTES +
>>> +                cons.lcdsizey * cons.lcdsizex * PIXLBYTES -
>>> +                y * cons.lcdsizex * PIXLBYTES -
>>> +                (x+1) * PIXLBYTES);
>>> +
>>> +    for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
>>> +        fbptr_t *d = (fbptr_t *)dest;
>>>           uchar bits;
>>>           bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
>>>             for (i = 0; i < 8; ++i) {
>>> -            *d++ = (bits & 0x80) ? fg_color : bg_color;
>>> +            *d-- = (bits & 0x80) ? fg_color : bg_color;
>>>               bits <<= 1;
>>>           }
>>> +        dest -= cons.lcdsizex * PIXLBYTES;
>>>       }
>>>   }
>>>   -static void console_scrollup(void)
>>> +static void lcd_putc_xy270(ushort x, ushort y, char c)
>>>   {
>>> -    const int rows = CONFIG_CONSOLE_SCROLL_LINES;
>>> +    int fg_color = lcd_getfgcolor();
>>>       int bg_color = lcd_getbgcolor();
>>> +    int col, i;
>>> +    uchar *dest = (uchar *)(cons.lcd_address +
>>> +                ((x+1) * cons.lcdsizex) * PIXLBYTES -
>>> +                y * PIXLBYTES);
>>> +
>>> +    fbptr_t *d = (fbptr_t *)dest;
>>> +    uchar msk = 0x80;
>>> +    uchar *pfont = video_fontdata + c * VIDEO_FONT_HEIGHT;
>>> +    for (col = 0; col < VIDEO_FONT_WIDTH; ++col) {
>>> +        for (i = 0; i < VIDEO_FONT_HEIGHT; ++i)
>>> +            *d-- = (*(pfont + i) & msk) ? fg_color : bg_color;
>>> +        msk >>= 1;
>>> +        d += (cons.lcdsizex + VIDEO_FONT_HEIGHT);
>>> +    }
>>> +}
>>>   -    /* Copy up rows ignoring those that will be overwritten */
>>> -    memcpy(CONSOLE_ROW_FIRST,
>>> -           cons.lcd_address + CONSOLE_ROW_SIZE * rows,
>>> -           CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows);
>>> +static inline void console_setrow270(u32 row, int clr)
>>> +{
>>> +    int i, j;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                   cons.lcdsizex * PIXLBYTES -
>>> +                   (row*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
>>> +
>>> +    for (j = 0; j < cons.lcdsizey; j++) {
>>> +        fbptr_t *pdst = (fbptr_t *)dst;
>>> +        for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>>> +            *pdst-- = clr;
>>> +        dst += cons.lcdsizex * PIXLBYTES;
>>> +    }
>>> +}
>>>   -    /* Clear the last rows */
>>> -#if (LCD_BPP != LCD_COLOR32)
>>> -    memset(lcd_console_address + CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows,
>>> -           bg_color, CONSOLE_ROW_SIZE * rows);
>>> -#else
>>> -    u32 *ppix = cons.lcd_address +
>>> -            CONSOLE_SIZE - CONSOLE_ROW_SIZE * rows;
>>> -    u32 i;
>>> -    for (i = 0;
>>> -        i < (CONSOLE_ROW_SIZE * rows) / NBYTES(panel_info.vl_bpix);
>>> -        i++) {
>>> -        *ppix++ = bg_color;
>>> +static inline void console_moverow270(u32 rowdst, u32 rowsrc)
>>> +{
>>> +    int i, j;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                  cons.lcdsizex * PIXLBYTES -
>>> +                  (rowdst*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
>>> +
>>> +    uchar *src = (uchar *)(cons.lcd_address +
>>> +                  cons.lcdsizex * PIXLBYTES -
>>> +                  (rowsrc*VIDEO_FONT_HEIGHT+1) * PIXLBYTES);
>>> +
>>> +    for (j = 0; j < cons.lcdsizey; j++) {
>>> +        fbptr_t *psrc = (fbptr_t *)src;
>>> +        fbptr_t *pdst = (fbptr_t *)dst;
>>> +        for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
>>> +            *pdst-- = *psrc--;
>>> +        src += cons.lcdsizex * PIXLBYTES;
>>> +        dst += cons.lcdsizex * PIXLBYTES;
>>>       }
>>> -#endif
>>> -    lcd_sync();
>>> -    cons.curr_row -= rows;
>>>   }
>>>   +static inline void console_setrow180(u32 row, int clr)
>>> +{
>>> +    int i;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                   (cons.rows-row-1) * VIDEO_FONT_HEIGHT *
>>> +                   cons.lcdsizex * PIXLBYTES);
>>> +
>>> +    fbptr_t *d = (fbptr_t *)dst;
>>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>>> +        *d++ = clr;
>>> +}
>>> +
>>> +static inline void console_moverow180(u32 rowdst, u32 rowsrc)
>>> +{
>>> +    int i;
>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>> +                   (cons.rows-rowdst-1) * VIDEO_FONT_HEIGHT *
>>> +                   cons.lcdsizex * PIXLBYTES);
>>> +
>>> +    uchar *src = (uchar *)(cons.lcd_address +
>>> +                   (cons.rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
>>> +                   cons.lcdsizex * PIXLBYTES);
>>> +
>>> +    fbptr_t *pdst = (fbptr_t *)dst;
>>> +    fbptr_t *psrc = (fbptr_t *)src;
>>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>>> +        *pdst++ = *psrc++;
>>> +}
>>> +
>>> +#endif /* CONFIG_LCD_ROTATION */
>> Can't this whole thing go into a separate file?
>> So, the console stuff will only define weak functions which can be overridden
>> by the rotation functionality.
>> This will keep the console code clean (also from ifdefs) and have the rotation
>> functionality cleanly added by a CONFIG_ symbol, which will control the
>> compilation for the separate file.
> Might be possible, which name should we give to it ? lcd_console_rotation.c ?

Sounds good.

> But how we deal with the function-pointer initialization ?

I think the usual method would do...
You call some kind of lcd_console_init_rot() with most of the code
that you currently have in lcd_init_console() that is related to rotation.
If the CONFIG_LCD_ROTATION is not set, then the lcd_init_console() stub
just returns the 0 rotation config.

>>
>>> +
>>>   static inline void console_back(void)
>>>   {
>>>       if (--cons.curr_col < 0) {
>>> @@ -121,26 +281,83 @@ static inline void console_back(void)
>>>               cons.curr_row = 0;
>>>       }
>>>   -    lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
>>> -            cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
>>> +    cons.fp_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
>>> +            cons.curr_row * VIDEO_FONT_HEIGHT, ' ');
>>>   }
>>>     static inline void console_newline(void)
>>>   {
>>> +    const int rows = CONFIG_CONSOLE_SCROLL_LINES;
>>> +    int bg_color = lcd_getbgcolor();
>>> +    int i;
>>> +
>>>       cons.curr_col = 0;
>>>         /* Check if we need to scroll the terminal */
>>> -    if (++cons.curr_row >= cons.rows)
>>> -        console_scrollup();
>>> -    else
>>> -        lcd_sync();
>>> +    if (++cons.curr_row >= cons.rows) {
>>> +        for (i = 0; i < cons.rows-rows; i++)
>>> +            cons.fp_console_moverow(i, i+rows);
>>> +        for (i = 0; i < rows; i++)
>>> +            cons.fp_console_setrow(cons.rows-i-1, bg_color);
>>> +        cons.curr_row -= rows;
>>> +    }
>>> +    lcd_sync();
>>> +}
>>> +
>>> +static void console_calc_rowcol(int vl_cols, int vl_rows, short *cl, short *rw)
>>> +{
>>> +    *cl = vl_cols / VIDEO_FONT_WIDTH;
>>> +#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
>>> +    *rw = (vl_rows - BMP_LOGO_HEIGHT);
>>> +    *rw /= VIDEO_FONT_HEIGHT;
>>> +#else
>>> +    *rw = vl_rows / VIDEO_FONT_HEIGHT;
>>> +#endif
>>> +}
>>> +
>>> +void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot)
>>> +{
>>> +    memset(&cons, 0, sizeof(cons));
>>> +    cons.lcd_address = address;
>>> +
>>> +    cons.lcdsizex = vl_cols;
>>> +    cons.lcdsizey = vl_rows;
>>> +
>>> +    if (vl_rot == 0) {
>>> +        cons.fp_putc_xy = &lcd_putc_xy0;
>>> +        cons.fp_console_moverow = &console_moverow0;
>>> +        cons.fp_console_setrow = &console_setrow0;
>>> +        console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
>> This call can be made after the if else structure and written only once.
> No. Have a closer look to it. If display is rotated 0?/180? the
> calculation is not the same if display is rotated 90/270?

Yep. Sorry, I've missed the switch between cols and rows...

>>
>>> +#ifdef CONFIG_LCD_ROTATION
>>> +    } else if (vl_rot == 90) {
>>> +        cons.fp_putc_xy = &lcd_putc_xy90;
>>> +        cons.fp_console_moverow = &console_moverow90;
>>> +        cons.fp_console_setrow = &console_setrow90;
>>> +        console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
>>> +    } else if (vl_rot == 180) {
>>> +        cons.fp_putc_xy = &lcd_putc_xy180;
>>> +        cons.fp_console_moverow = &console_moverow180;
>>> +        cons.fp_console_setrow = &console_setrow180;
>>> +        console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
>>> +    } else if (vl_rot == 270) {
>>> +        cons.fp_putc_xy = &lcd_putc_xy270;
>>> +        cons.fp_console_moverow = &console_moverow270;
>>> +        cons.fp_console_setrow = &console_setrow270;
>>> +        console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
>>> +#endif
>>> +    } else {
>>> +        puts("lcd_init_console: invalid framebuffer rotation!\n");
>>> +    }
>> I would recommend extracting the whole if else ... structure into
>> a separate function say lcd_setup_console_rot() or something and
>> make the default one doing only the vl_rot == 0 stuff.
> Whats the use of this ?
> Should this also be in a separate file?

Yes, that is how I think it will do better.

Just to explain my points:
1) make the rotation feature an integrative part of the code by always using
   the rotation code: if CONFIG_LCD_ROTATION is *not* set then just rotate it
   to 0 degrees and compile out the rest of the code.
2) make the rotation feature a bit more separate from the console code.
   I believe this way will make it cleaner.

Also, might it be useful to allow specifiying the angle through the
CONFIG_LCD_ROTATION define?
Have you considered using Kconfig for this?

-- 
Regards,
Igor.

[U-Boot] [PATCH 2/4] common/lcd_console: ask only one-time for bg/fg-color per call

[Nikita Kiryanov]

Hi Hannes,

On 03/11/2015 02:57 PM, Hannes Petermaier wrote:
> From: Hannes Petermaier <hannes.petermaier@br-automation.com>
>
> Don't call the lcd_getfgcolor and lcd_getbgcolor within the "draw-loop", this
> only wastes time.
>
> Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
> Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
> ---
>
>   common/lcd_console.c |    7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)

Acked-by: Nikita Kiryanov <nikita@compulab.co.il>

--
Regards,
Nikita Kiryanov

[U-Boot] [PATCH 1/4] common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy

[Nikita Kiryanov]

Hi Hannes,

On 03/11/2015 02:57 PM, Hannes Petermaier wrote:
> From: Hannes Petermaier <hannes.petermaier@br-automation.com>
>
> the capability of drawing some *str with count from lcd_drawchars is unnary.
> It is always called from lcd_putc_xy with one character of and count = 1.
>
> So we simply rename lcd_drawchars into lcd_putc_xy and remove the loops inside.
>
> Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
> Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>

Acked-by: Nikita Kiryanov <nikita@compulab.co.il>

-- 
Regards,
Nikita Kiryanov

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Nikita Kiryanov]

Hi Hannes,

I second Grinberg's suggestion of a separate file and 0 degree default (also as a
fallback for invalid rotation value, see below).
Some additional comments:

On 03/11/2015 02:57 PM, Hannes Petermaier wrote:
> From: Hannes Petermaier <hannes.petermaier@br-automation.com>
>
> Sometimes, for example if the display is mounted in portrait mode or even if it
> mounted landscape but rotated by 180 degrees, we need to rotate our content of
> the display respectively the framebuffer, so that user can read the messages
> who are printed out.
>
> For this we introduce the feature called "CONFIG_LCD_ROTATION", this may be
> defined in the board-configuration if needed. After this the lcd_console will
> be initialized with a given rotation from "vl_rot" out of "vidinfo_t" which is
> provided by the board specific code.
>
> If CONFIG_LCD_ROTATION is not defined, the console will be initialized with
> 0 degrees rotation - the screen behaves like the days before.
>
> Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
> Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
> ---
>
>   README                |   17 +++
>   common/lcd.c          |   22 ++--
>   common/lcd_console.c  |  333 ++++++++++++++++++++++++++++++++++++++++---------
>   include/lcd.h         |    1 +
>   include/lcd_console.h |    9 +-
>   5 files changed, 309 insertions(+), 73 deletions(-)
>
> diff --git a/README b/README
> index 3c4a2e6..a95b1e8 100644
> --- a/README
> +++ b/README
> @@ -1933,6 +1933,23 @@ CBFS (Coreboot Filesystem) support
>   		the console jump but can help speed up operation when scrolling
>   		is slow.
>
> +		CONFIG_LCD_ROTATION
> +
> +		Sometimes, for example if the display is mounted in portrait
> +		mode or even if it mounted landscape but rotated by 180degree,
> +		we need to rotate our content of the display respectively the
> +		framebuffer, so that user can read the messages who are printed
> +		out.
> +		For this we introduce the feature called "CONFIG_LCD_ROTATION",
> +		this may be defined in the board-configuration if needed. After
> +		this the lcd_console will be initialized with a given rotation
> +		from "vl_rot" out of "vidinfo_t" which is provided by the board
> +		specific code.
> +
> +		If CONFIG_LCD_ROTATION is not defined, the console will be
> +		initialized with 0degree rotation

This is enough. "the screen behaves like the days before" is vague and unnecessary
(days before what?)

  - the screen behaves like the
> +		days before.
> +
>   		CONFIG_LCD_BMP_RLE8
>
>   		Support drawing of RLE8-compressed bitmaps on the LCD.

[...]

> +static inline void console_setrow0(u32 row, int clr)
> +{
>   	int i;
> +	uchar *dst = (uchar *)(cons.lcd_address +
> +			       row * VIDEO_FONT_HEIGHT *
> +			       cons.lcdsizex * PIXLBYTES);
>
> -	dest = (uchar *)(cons.lcd_address +
> -			 y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
> +	fbptr_t *d = (fbptr_t *)dst;

Here you can just create the fbptr variable directly. You have a bunch of
function where this recasting is avoidable.

> +	for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> +		*d++ = clr;
> +}

[...]

> +void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot)
> +{
> +	memset(&cons, 0, sizeof(cons));
> +	cons.lcd_address = address;
> +
> +	cons.lcdsizex = vl_cols;
> +	cons.lcdsizey = vl_rows;
> +
> +	if (vl_rot == 0) {
> +		cons.fp_putc_xy = &lcd_putc_xy0;
> +		cons.fp_console_moverow = &console_moverow0;
> +		cons.fp_console_setrow = &console_setrow0;
> +		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
> +#ifdef CONFIG_LCD_ROTATION
> +	} else if (vl_rot == 90) {
> +		cons.fp_putc_xy = &lcd_putc_xy90;
> +		cons.fp_console_moverow = &console_moverow90;
> +		cons.fp_console_setrow = &console_setrow90;
> +		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
> +	} else if (vl_rot == 180) {
> +		cons.fp_putc_xy = &lcd_putc_xy180;
> +		cons.fp_console_moverow = &console_moverow180;
> +		cons.fp_console_setrow = &console_setrow180;
> +		console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
> +	} else if (vl_rot == 270) {
> +		cons.fp_putc_xy = &lcd_putc_xy270;
> +		cons.fp_console_moverow = &console_moverow270;
> +		cons.fp_console_setrow = &console_setrow270;
> +		console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
> +#endif
> +	} else {
> +		puts("lcd_init_console: invalid framebuffer rotation!\n");

This case leaves the function pointers uninitialized, which would crash the system later on.
I suggest you default to 0 degree rotation both for the generic case and for the fallback behavior
(with the warning message where necessary).

> +	}
> +
> +	debug("lcd_console: have %d/%d col/rws on scr %dx%d (%d deg rotated)\n",
> +	      cons.cols, cons.rows, cons.lcdsizex, cons.lcdsizey, vl_rot);
> +
>   }
>
>   void lcd_putc(const char c)
>   {
>   	if (!lcd_is_enabled) {
>   		serial_putc(c);
> -

This is a cleanup. It should not be in this patch.

>   		return;
>   	}
>
> @@ -150,7 +367,6 @@ void lcd_putc(const char c)
>   		return;
>   	case '\n':
>   		console_newline();
> -

Same here, and in other places...


>   		return;
>   	}
>
> diff --git a/include/lcd.h b/include/lcd.h
> index f049fd3..41e0c11 100644
> --- a/include/lcd.h
> +++ b/include/lcd.h
> @@ -51,6 +51,7 @@ void lcd_set_flush_dcache(int flush);
>   typedef struct vidinfo {
>   	ushort	vl_col;		/* Number of columns (i.e. 160) */
>   	ushort	vl_row;		/* Number of rows (i.e. 100) */
> +	ushort	vl_rot;		/* Rotation of Display (i.e. 90) */
>   	u_char	vl_bpix;	/* Bits per pixel, 0 = 1 */
>   	ushort	*cmap;		/* Pointer to the colormap */
>   	void	*priv;		/* Pointer to driver-specific data */
> diff --git a/include/lcd_console.h b/include/lcd_console.h
> index 429214d..2244b3f 100644
> --- a/include/lcd_console.h
> +++ b/include/lcd_console.h
> @@ -16,11 +16,12 @@
>    * console has.
>    *
>    * @address: Console base address
> - * @rows: Number of rows in the console
> - * @cols: Number of columns in the console
> + * @vl_rows: Number of rows in the console
> + * @vl_cols: Number of columns in the console
> + * @vl_rot: Rotation of display in degree (0 - 90 - 180 - 270) counterlockwise
>    */
> -void lcd_init_console(void *address, int rows, int cols);
> -
> +/*void lcd_init_console(void *address, int rows, int cols); */

Please delete this comment

> +void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot);
>   /**
>    * lcd_set_col() - Set the number of the current lcd console column
>    *
>

-- 
Regards,
Nikita Kiryanov

[U-Boot] [PATCH 3/4] common/lcd_console: move single static variables into common (static) structure

[Nikita Kiryanov]

Hi Hannes,

On 03/11/2015 02:57 PM, Hannes Petermaier wrote:
> From: Hannes Petermaier <hannes.petermaier@br-automation.com>
>
> For coming implementation of lcd_console rotation, we will need some more
> variables for holding information about framebuffer size, rotation, ...
>
> For better readability we catch all them into a common structure.
>
> Signed-off-by: Hannes Petermaier <hannes.petermaier@br-automation.com>
> Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
> ---
>
>   common/lcd_console.c |   76 +++++++++++++++++++++++++-------------------------
>   1 file changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/common/lcd_console.c b/common/lcd_console.c
> index b7dda7a..cac77be 100644
> --- a/common/lcd_console.c
> +++ b/common/lcd_console.c
> @@ -11,48 +11,49 @@
>   #include <video_font.h>		/* Get font data, width and height */
>
>   #define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * lcd_line_length)
> -#define CONSOLE_ROW_FIRST	lcd_console_address
> -#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * console_rows)
> +#define CONSOLE_ROW_FIRST	cons.lcd_address
> +#define CONSOLE_SIZE		(CONSOLE_ROW_SIZE * cons.rows)
>
> -static short console_curr_col;
> -static short console_curr_row;
> -static short console_cols;
> -static short console_rows;
> -static void *lcd_console_address;
> +struct console_t {
> +	short curr_col, curr_row;
> +	short cols, rows;
> +	void *lcd_address;

Can this be void *base_address? I think that's a bit more descriptive.

Other than that,
Acked-by: Nikita Kiryanov <nikita@compulab.co.il>

> +};

-- 
Regards,
Nikita Kiryanov

[U-Boot] [PATCH 3/4] common/lcd_console: move single static variables into common (static) structure

[Hannes Petermaier]

Nikita Kiryanov <nikita@compulab.co.il> schrieb am 15.03.2015 19:57:08:

> Hi Hannes,
Hi Nikita,
> 
> > -static void *lcd_console_address;
> > +struct console_t {
> > +   short curr_col, curr_row;
> > +   short cols, rows;
> > +   void *lcd_address;
> 
> Can this be void *base_address? I think that's a bit more descriptive.

Yes, i will do some cleanup after Patch 4/4 is finished and within this 
action i will rename this variable to
fbbase - this should be most descriptive.

> 
> Other than that,
> Acked-by: Nikita Kiryanov <nikita@compulab.co.il>
> 
> > +};
> 
> -- 
> Regards,
> Nikita Kiryanov
many thanks and best regards,
Hannes

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Hannes Petermaier]

Nikita Kiryanov <nikita@compulab.co.il> schrieb am 15.03.2015 19:56:31:

> 
> Hi Hannes,
Hi Nikita,
many thanks for response.
> 
> I second Grinberg's suggestion of a separate file and 0 degree default 
(also as a
> fallback for invalid rotation value, see below).
Okay - i will provide a V2 from this patch where i try to implement as 
sugessted, maybe we will need some V3 :-)
Probably on wednesday.

> Some additional comments:
> 
> > +
> > +      If CONFIG_LCD_ROTATION is not defined, the console will be
> > +      initialized with 0degree rotation
> 
> This is enough. "the screen behaves like the days before" is vague and 
unnecessary
> (days before what?)
Okay - i will do so.

> 
> > +static inline void console_setrow0(u32 row, int clr)
> > +{
> >      int i;
> > +   uchar *dst = (uchar *)(cons.lcd_address +
> > +                row * VIDEO_FONT_HEIGHT *
> > +                cons.lcdsizex * PIXLBYTES);
> >
> > -   dest = (uchar *)(cons.lcd_address +
> > -          y * lcd_line_length + x * NBITS(LCD_BPP) / 8);
> > +   fbptr_t *d = (fbptr_t *)dst;
> 
> Here you can just create the fbptr variable directly. You have a bunch 
of
> function where this recasting is avoidable.
> 
> > +   for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> > +      *d++ = clr;
> > +}

> 
> > +void lcd_init_console(void *address, int vl_cols, int vl_rows, int 
vl_rot)
> > +{
> > +   memset(&cons, 0, sizeof(cons));
> > +   cons.lcd_address = address;
> > +
> > +   cons.lcdsizex = vl_cols;
> > +   cons.lcdsizey = vl_rows;
> > +
> > +   if (vl_rot == 0) {
> > +      cons.fp_putc_xy = &lcd_putc_xy0;
> > +      cons.fp_console_moverow = &console_moverow0;
> > +      cons.fp_console_setrow = &console_setrow0;
> > +      console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
> > +#ifdef CONFIG_LCD_ROTATION
> > +   } else if (vl_rot == 90) {
> > +      cons.fp_putc_xy = &lcd_putc_xy90;
> > +      cons.fp_console_moverow = &console_moverow90;
> > +      cons.fp_console_setrow = &console_setrow90;
> > +      console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
> > +   } else if (vl_rot == 180) {
> > +      cons.fp_putc_xy = &lcd_putc_xy180;
> > +      cons.fp_console_moverow = &console_moverow180;
> > +      cons.fp_console_setrow = &console_setrow180;
> > +      console_calc_rowcol(vl_cols, vl_rows, &cons.cols, &cons.rows);
> > +   } else if (vl_rot == 270) {
> > +      cons.fp_putc_xy = &lcd_putc_xy270;
> > +      cons.fp_console_moverow = &console_moverow270;
> > +      cons.fp_console_setrow = &console_setrow270;
> > +      console_calc_rowcol(vl_rows, vl_cols, &cons.cols, &cons.rows);
> > +#endif
> > +   } else {
> > +      puts("lcd_init_console: invalid framebuffer rotation!\n");
> 
> This case leaves the function pointers uninitialized, which would crash 
the 
> system later on.
> I suggest you default to 0 degree rotation both for the generic case and 
for 
> the fallback behavior
> (with the warning message where necessary).
Oh my god, i haven't seen this mess ... i will remove it during moving 
lcd_rotation stuff
into separate file.

> >   void lcd_putc(const char c)
> >   {
> >      if (!lcd_is_enabled) {
> >         serial_putc(c);
> > -
> 
> This is a cleanup. It should not be in this patch.
I will make this cleanup in a following patch, after this series has been 
merged.


> > --- a/include/lcd_console.h
> > +++ b/include/lcd_console.h
> > @@ -16,11 +16,12 @@
> >    * console has.
> >    *
> >    * @address: Console base address
> > - * @rows: Number of rows in the console
> > - * @cols: Number of columns in the console
> > + * @vl_rows: Number of rows in the console
> > + * @vl_cols: Number of columns in the console
> > + * @vl_rot: Rotation of display in degree (0 - 90 - 180 - 270) 
counterlockwise
> >    */
> > -void lcd_init_console(void *address, int rows, int cols);
> > -
> > +/*void lcd_init_console(void *address, int rows, int cols); */
> 
> Please delete this comment
Okay.

> -- 
> Regards,
> Nikita Kiryanov
best regards,
Hannes

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Hannes Petermaier]

> Hi Hannes,
Hi Igor,

> >> +    /* setup text-console */
> >> +    debug("[LCD] setting up console...\n");
> >> +#ifdef CONFIG_LCD_ROTATION
> >> +    lcd_init_console(lcd_base,
> >> +             panel_info.vl_col,
> >> +             panel_info.vl_row,
> >> +             panel_info.vl_rot);
> >>   #else
> >> -    console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
> >> +    lcd_init_console(lcd_base,
> >> +             panel_info.vl_col,
> >> +             panel_info.vl_row,
> >> +             0);
> >>   #endif
> >> Please, don't start the #ifdef mess here...
> >> just always pass the panel_info.vl_rot.
> > This is not possible, because 'vl_rot' does'nt exist if
> > CONFIG_LCD_ROTATION is not defined. (have a look into lcd.h).
> 
> Of course I did before sending the reply...
> What I'm trying to say is - let it exist and default to 0 angle 
rotation.
> 
> > I made this to be compatible to all who have allready initialized a
> > panel_info without vl_rot.
> 
> This increases the mess and I think is not sensible enough.
> Just change the users to initialize the panel_info with vl_rot.
> I think also that default initialization of globals is 0, right?
> If so, those users that do not initialize the vl_rot explicitly,
> should have it initialized to 0 implicitly by compiler.
Yes, thats a good idea. I will check if the compiler really initializes 
the global
struct panel_info with zero and change this.
 
[...]
> >>>   }
> >>>   +static inline void console_setrow180(u32 row, int clr)
> >>> +{
> >>> +    int i;
> >>> +    uchar *dst = (uchar *)(cons.lcd_address +
> >>> +                   (cons.rows-row-1) * VIDEO_FONT_HEIGHT *
> >>> +                   cons.lcdsizex * PIXLBYTES);
> >>> +
> >>> +    fbptr_t *d = (fbptr_t *)dst;
> >>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> >>> +        *d++ = clr;
> >>> +}
> >>> +
> >>> +static inline void console_moverow180(u32 rowdst, u32 rowsrc)
> >>> +{
> >>> +    int i;
> >>> +    uchar *dst = (uchar *)(cons.lcd_address +
> >>> +                   (cons.rows-rowdst-1) * VIDEO_FONT_HEIGHT *
> >>> +                   cons.lcdsizex * PIXLBYTES);
> >>> +
> >>> +    uchar *src = (uchar *)(cons.lcd_address +
> >>> +                   (cons.rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
> >>> +                   cons.lcdsizex * PIXLBYTES);
> >>> +
> >>> +    fbptr_t *pdst = (fbptr_t *)dst;
> >>> +    fbptr_t *psrc = (fbptr_t *)src;
> >>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
> >>> +        *pdst++ = *psrc++;
> >>> +}
> >>> +
> >>> +#endif /* CONFIG_LCD_ROTATION */
> >> Can't this whole thing go into a separate file?
> >> So, the console stuff will only define weak functions which can be 
overridden
> >> by the rotation functionality.
> >> This will keep the console code clean (also from ifdefs) and have the 
rotation
> >> functionality cleanly added by a CONFIG_ symbol, which will control 
the
> >> compilation for the separate file.
> > Might be possible, which name should we give to it ? 
lcd_console_rotation.c ?
> 
> Sounds good.
> 
> > But how we deal with the function-pointer initialization ?
> 
> I think the usual method would do...
> You call some kind of lcd_console_init_rot() with most of the code
> that you currently have in lcd_init_console() that is related to 
rotation.
> If the CONFIG_LCD_ROTATION is not set, then the lcd_init_console() stub
> just returns the 0 rotation config.

I just started to move rotation specific functions into own file, called 
lcd_console_rotation.c and
ran into some trouble.

1) 
I need during initialization the console_calc_rowcol(...) function, which 
is provided by lcd.c.
A possible solution might be to "un-static" it - but i am not happy with 
this. 
Another way could be to take up vl_rot into console_t structure and pass 
only a pointer to structure to this function and decide inside the 
function.
But this would create a little mix between 0 degree and rotation code.
Yet another idea is to have (also having pointer to console_t in call) in 
lcd_console_rotation also such a calc function which overrides the values 
calculated before.

or maybe you've another solution ?

2)
I need in almost every "paint-function" the framebuffer base 
(cons.lcd_address) and the screen dimension.
This information is stored in the static structure within lcd.c - i don't 
like to make this public.
A possible solution could be to change all painting function to work 
without some global variable and pass
a third argument, pointer to console_t, and take informations from there. 
This will consume one more register
on function call, runtime is equal i think.

Whats your opinion around this ?

> >> I would recommend extracting the whole if else ... structure into
> >> a separate function say lcd_setup_console_rot() or something and
> >> make the default one doing only the vl_rot == 0 stuff.
> > Whats the use of this ?
> > Should this also be in a separate file?
> 
> Yes, that is how I think it will do better.
> 
> Just to explain my points:
> 1) make the rotation feature an integrative part of the code by always 
using
>    the rotation code: if CONFIG_LCD_ROTATION is *not* set then just 
rotate it
>    to 0 degrees and compile out the rest of the code.
> 2) make the rotation feature a bit more separate from the console code.
>    I believe this way will make it cleaner.
Actually (within new code) i do initialize the pointers and things around 
with 0 degree rotation and
call afterwards the new function lcd_init_console_rot which is implemented 
in lcd_console_rotation.c
and "__weak" in lcd_console.c which does re-initialze fps and col/row 
stuff.

> 
> Also, might it be useful to allow specifiying the angle through the
> CONFIG_LCD_ROTATION define?
> Have you considered using Kconfig for this?
First i thought about this to specifiy rotation angle with a value defined 
CONFIG_LCD_ROTATION - but this is only usefull to one of my boards (where 
display is rotated 180degrees). In another board i have one U-Boot for a 
bunch of variants (16 at this time) where all rotation angles are needed. 
I want to take there this information out of device-tree and write it to 
vl_rot.
> 
> -- 
> Regards,
> Igor.
best regards,
Hannes

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Igor Grinberg]

On 03/16/15 10:32, Hannes Petermaier wrote:
>> Hi Hannes,
> Hi Igor,
> 
>>>> +    /* setup text-console */
>>>> +    debug("[LCD] setting up console...\n");
>>>> +#ifdef CONFIG_LCD_ROTATION
>>>> +    lcd_init_console(lcd_base,
>>>> +             panel_info.vl_col,
>>>> +             panel_info.vl_row,
>>>> +             panel_info.vl_rot);
>>>>   #else
>>>> -    console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
>>>> +    lcd_init_console(lcd_base,
>>>> +             panel_info.vl_col,
>>>> +             panel_info.vl_row,
>>>> +             0);
>>>>   #endif
>>>> Please, don't start the #ifdef mess here...
>>>> just always pass the panel_info.vl_rot.
>>> This is not possible, because 'vl_rot' does'nt exist if
>>> CONFIG_LCD_ROTATION is not defined. (have a look into lcd.h).
>>
>> Of course I did before sending the reply...
>> What I'm trying to say is - let it exist and default to 0 angle 
> rotation.
>>
>>> I made this to be compatible to all who have allready initialized a
>>> panel_info without vl_rot.
>>
>> This increases the mess and I think is not sensible enough.
>> Just change the users to initialize the panel_info with vl_rot.
>> I think also that default initialization of globals is 0, right?
>> If so, those users that do not initialize the vl_rot explicitly,
>> should have it initialized to 0 implicitly by compiler.
> Yes, thats a good idea. I will check if the compiler really initializes 
> the global
> struct panel_info with zero and change this.
>  
> [...]
>>>>>   }
>>>>>   +static inline void console_setrow180(u32 row, int clr)
>>>>> +{
>>>>> +    int i;
>>>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>>>> +                   (cons.rows-row-1) * VIDEO_FONT_HEIGHT *
>>>>> +                   cons.lcdsizex * PIXLBYTES);
>>>>> +
>>>>> +    fbptr_t *d = (fbptr_t *)dst;
>>>>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>>>>> +        *d++ = clr;
>>>>> +}
>>>>> +
>>>>> +static inline void console_moverow180(u32 rowdst, u32 rowsrc)
>>>>> +{
>>>>> +    int i;
>>>>> +    uchar *dst = (uchar *)(cons.lcd_address +
>>>>> +                   (cons.rows-rowdst-1) * VIDEO_FONT_HEIGHT *
>>>>> +                   cons.lcdsizex * PIXLBYTES);
>>>>> +
>>>>> +    uchar *src = (uchar *)(cons.lcd_address +
>>>>> +                   (cons.rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
>>>>> +                   cons.lcdsizex * PIXLBYTES);
>>>>> +
>>>>> +    fbptr_t *pdst = (fbptr_t *)dst;
>>>>> +    fbptr_t *psrc = (fbptr_t *)src;
>>>>> +    for (i = 0; i < (VIDEO_FONT_HEIGHT * cons.lcdsizex); i++)
>>>>> +        *pdst++ = *psrc++;
>>>>> +}
>>>>> +
>>>>> +#endif /* CONFIG_LCD_ROTATION */
>>>> Can't this whole thing go into a separate file?
>>>> So, the console stuff will only define weak functions which can be 
> overridden
>>>> by the rotation functionality.
>>>> This will keep the console code clean (also from ifdefs) and have the 
> rotation
>>>> functionality cleanly added by a CONFIG_ symbol, which will control 
> the
>>>> compilation for the separate file.
>>> Might be possible, which name should we give to it ? 
> lcd_console_rotation.c ?
>>
>> Sounds good.
>>
>>> But how we deal with the function-pointer initialization ?
>>
>> I think the usual method would do...
>> You call some kind of lcd_console_init_rot() with most of the code
>> that you currently have in lcd_init_console() that is related to 
> rotation.
>> If the CONFIG_LCD_ROTATION is not set, then the lcd_init_console() stub
>> just returns the 0 rotation config.
> 
> I just started to move rotation specific functions into own file, called 
> lcd_console_rotation.c and
> ran into some trouble.
> 
> 1) 
> I need during initialization the console_calc_rowcol(...) function, which 
> is provided by lcd.c.
> A possible solution might be to "un-static" it - but i am not happy with 
> this. 
> Another way could be to take up vl_rot into console_t structure and pass 
> only a pointer to structure to this function and decide inside the 
> function.
> But this would create a little mix between 0 degree and rotation code.
> Yet another idea is to have (also having pointer to console_t in call) in 
> lcd_console_rotation also such a calc function which overrides the values 
> calculated before.
> 
> or maybe you've another solution ?

Well, you need to perform the rows and columns calculation regardless of
the rotation feature, so the console_calc_rowcol() should be there anyway.
It is a "bonus" that the rotation code can use the same function (and it
looks generic enough) for rows and columns calculation, so IMO, a cleaner
solution would be just make it non static.

> 
> 2)
> I need in almost every "paint-function" the framebuffer base 
> (cons.lcd_address) and the screen dimension.
> This information is stored in the static structure within lcd.c - i don't 
> like to make this public.
> A possible solution could be to change all painting function to work 
> without some global variable and pass
> a third argument, pointer to console_t, and take informations from there. 
> This will consume one more register
> on function call, runtime is equal i think.
> 
> Whats your opinion around this ?

Well, since Nikita is working on refactoring the lcd.c stuff and
already examined several aproaches, I think he can provide a better
opinion on this.
Nikita?

> 
>>>> I would recommend extracting the whole if else ... structure into
>>>> a separate function say lcd_setup_console_rot() or something and
>>>> make the default one doing only the vl_rot == 0 stuff.
>>> Whats the use of this ?
>>> Should this also be in a separate file?
>>
>> Yes, that is how I think it will do better.
>>
>> Just to explain my points:
>> 1) make the rotation feature an integrative part of the code by always 
> using
>>    the rotation code: if CONFIG_LCD_ROTATION is *not* set then just 
> rotate it
>>    to 0 degrees and compile out the rest of the code.
>> 2) make the rotation feature a bit more separate from the console code.
>>    I believe this way will make it cleaner.
> Actually (within new code) i do initialize the pointers and things around 
> with 0 degree rotation and
> call afterwards the new function lcd_init_console_rot which is implemented 
> in lcd_console_rotation.c
> and "__weak" in lcd_console.c which does re-initialze fps and col/row 
> stuff.

Sounds good.

> 
>>
>> Also, might it be useful to allow specifiying the angle through the
>> CONFIG_LCD_ROTATION define?
>> Have you considered using Kconfig for this?
> First i thought about this to specifiy rotation angle with a value defined 
> CONFIG_LCD_ROTATION - but this is only usefull to one of my boards (where 
> display is rotated 180degrees). In another board i have one U-Boot for a 
> bunch of variants (16 at this time) where all rotation angles are needed. 
> I want to take there this information out of device-tree and write it to 
> vl_rot.

Yep. This sounds even better.
Does DT have an already defined property for this?
Have you considered may be using an environment variable for this?


-- 
Regards,
Igor.

[U-Boot] [PATCH 4/4] common/lcd_console: introduce display/framebuffer rotation

[Nikita Kiryanov]

On 03/16/2015 01:35 PM, Igor Grinberg wrote:
> On 03/16/15 10:32, Hannes Petermaier wrote:
>>
>> 2)
>> I need in almost every "paint-function" the framebuffer base
>> (cons.lcd_address) and the screen dimension.
>> This information is stored in the static structure within lcd.c - i don't
>> like to make this public.
>> A possible solution could be to change all painting function to work
>> without some global variable and pass
>> a third argument, pointer to console_t, and take informations from there.
>> This will consume one more register
>> on function call, runtime is equal i think.
>>
>> Whats your opinion around this ?
>
> Well, since Nikita is working on refactoring the lcd.c stuff and
> already examined several aproaches, I think he can provide a better
> opinion on this.
> Nikita?

I think that passing a pointer to console_t is the way to go. You have an
object oriented design here, and that's the object oriented way of accessing
the object's members from its methods.

-- 
Regards,
Nikita Kiryanov