common/lcd_console: introduce display/framebuffer rotation

PATCH Superseded Hannes Petermaier 5 participants 21 messages Mar 18, 2015 – Mar 26, 2015

Share & tools

Hannes Petermaier

author

2015-03-18 07:37 UTC #0 lore raw

v2 cover common/lcd_console: introduce display/framebuffer rotation

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.
The value for vl_rot is coded as following (matching to fbcon=rotate:<n> linux- kernel commandline):
0 = no rotation respectively 0 degree
1 = 90 degree rotation
2 = 180 degree rotation
3 = 270 degree rotation

If CONFIG_LCD_ROTATION is not defined, the console will be initialized with 0 degrees rotation.

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

Changes in v2:
- cleanup README text for feature
- don't make code cleanups (lcd_console.c) within this patch
- remove (unnary) comment in lcd_console.h
- update year to 2015 within copyright in lcd_console.c
- move rotation related code into separate file lcd_console_rotation.c
- rework rotation code
- change meaning of vl_rot to match fbcon=rotate:<n> from the linux-kernel

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                        |   22 +++++
 common/Makefile               |    1 +
 common/lcd.c                  |   15 ++-
 common/lcd_console.c          |  208 +++++++++++++++++++++------------------
 common/lcd_console_rotation.c |  216 +++++++++++++++++++++++++++++++++++++++++
 include/lcd.h                 |    1 +
 include/lcd_console.h         |   18 +++-
 7 files changed, 371 insertions(+), 110 deletions(-)
 create mode 100644 common/lcd_console_rotation.c

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-18 07:37 UTC #1 lore raw

PATCH v2 1/4 common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy

From: Hannes Petermaier <[email protected]>

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 <[email protected]>

---

Changes in v2: None

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

common/lcd_console.c +7 ~~-16~~

index 8bf83b9..243b7c5 100644
@@ -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;

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-19 08:37 UTC #2 lore raw

v3 cover common/lcd_console: introduce display/framebuffer rotation

If CONFIG_LCD_ROTATION is not defined, the console will be initialized with 0 degrees rotation.

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

Changes in v3:
- rename lcd_address to fbbase for better readability.
- remove empty line lcd_console.c
- use printf instead puts to inform about invalid-fb rotation.
- avoid code-duplication (move define of fbptr_t into lcd.h)

 README                        |   22 +++++
 common/Makefile               |    1 +
 common/lcd.c                  |   15 ++-
 common/lcd_console.c          |  201 ++++++++++++++++++++-------------------
 common/lcd_console_rotation.c |  208 +++++++++++++++++++++++++++++++++++++++++
 include/lcd.h                 |    9 ++
 include/lcd_console.h         |   18 +++-
 7 files changed, 363 insertions(+), 111 deletions(-)
 create mode 100644 common/lcd_console_rotation.c

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-19 08:37 UTC #3 lore raw

PATCH v3 1/4 common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy

From: Hannes Petermaier <[email protected]>

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 <[email protected]>

---

Changes in v3: None
Changes in v2: None

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

common/lcd_console.c +7 ~~-16~~

index 8bf83b9..243b7c5 100644
@@ -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;

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-19 08:37 UTC #4 lore raw

PATCH v3 2/4 common/lcd_console: ask only one-time for bg/fg-color per call

In reply to Hannes Petermaier

From: Hannes Petermaier <[email protected]>

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

Signed-off-by Hannes Petermaier <[email protected]>

---

Changes in v3: None
Changes in v2: None

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

common/lcd_console.c +2 -5

index 243b7c5..b7dda7a 100644
@@ -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];

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-19 08:37 UTC #5 lore raw

PATCH v3 3/4 common/lcd_console: move single static variables into common (static) structure

In reply to Hannes Petermaier

From: Hannes Petermaier <[email protected]>

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 <[email protected]>

---

Changes in v3: None
Changes in v2: None

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

common/lcd_console.c +38 ~~-38~~

index b7dda7a..cac77be 100644
@@ -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();
 	}
 }

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-19 08:37 UTC #6 lore raw

PATCH v3 4/4 common/lcd_console: introduce display/framebuffer rotation

In reply to Hannes Petermaier

From: Hannes Petermaier <[email protected]>

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.

If CONFIG_LCD_ROTATION is not defined, the console will be initialized with 0 degrees rotation.

Signed-off-by Hannes Petermaier <[email protected]>

---

 README                        |   22 +++++
 common/Makefile               |    1 +
 common/lcd.c                  |   15 ++-
 common/lcd_console.c          |  161 +++++++++++++++++--------------
 common/lcd_console_rotation.c |  208 +++++++++++++++++++++++++++++++++++++++++
 include/lcd.h                 |    9 ++
 include/lcd_console.h         |   18 +++-
 7 files changed, 349 insertions(+), 85 deletions(-)
 create mode 100644 common/lcd_console_rotation.c

README +22 -0

index b0124d6..c649de1 100644
@@ -1947,6 +1947,28 @@ 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.
+		The value for vl_rot is coded as following (matching to
+		fbcon=rotate:<n> linux-kernel commandline):
+		0 = no rotation respectively 0 degree
+		1 = 90 degree rotation
+		2 = 180 degree rotation
+		3 = 270 degree rotation
+
+		If CONFIG_LCD_ROTATION is not defined, the console will be
+		initialized with 0degree rotation.
+
 		CONFIG_LCD_BMP_RLE8
 
 		Support drawing of RLE8-compressed bitmaps on the LCD.

common/Makefile +1 -0

index 7216a13..a1b3c09 100644
@@ -200,6 +200,7 @@ obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-y += splash.o
 obj-$(CONFIG_SPLASH_SOURCE) += splash_source.o
 obj-$(CONFIG_LCD) += lcd.o lcd_console.o
+obj-$(CONFIG_LCD_ROTATION) += lcd_console_rotation.o
 obj-$(CONFIG_LCD_DT_SIMPLEFB) += lcd_simplefb.o
 obj-$(CONFIG_LYNXKDI) += lynxkdi.o
 obj-$(CONFIG_MENU) += menu.o

common/lcd.c +6 -9

index f33942c..d1b4ca7 100644
@@ -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,14 @@ void lcd_clear(void)
 	}
 #endif
 #endif
+	/* setup text-console */
+	debug("[LCD] setting up console...\n");
+	lcd_init_console(lcd_base,
+			 panel_info.vl_col,
+			 panel_info.vl_row,
+			 panel_info.vl_rot);
 	/* 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;
-#else
-	console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
-#endif
-	console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
-	lcd_init_console(lcd_base, console_rows, console_cols);
 	if (do_splash) {
 		s = getenv("splashimage");
 		if (s) {

common/lcd_console.c +89 ~~-72~~

214 lines not on this page · raw message

common/lcd_console_rotation.c new file +208 -0

208 lines not on this page · raw message

include/lcd.h +9 -0

index f049fd3..59202b7 100644
@@ -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 (0, 1, 2, 3) */
 	u_char	vl_bpix;	/* Bits per pixel, 0 = 1 */
 	ushort	*cmap;		/* Pointer to the colormap */
 	void	*priv;		/* Pointer to driver-specific data */

@@ -196,6 +197,14 @@ void lcd_sync(void);
 #define CONSOLE_COLOR_WHITE	0xffff		/* Must remain last / highest */
 #endif /* color definitions */
 
+#if LCD_BPP == LCD_COLOR16
+#define fbptr_t ushort
+#elif LCD_BPP == LCD_COLOR32
+#define fbptr_t u32
+#else
+#define fbptr_t uchar
+#endif
+
 #ifndef PAGE_SIZE
 #define PAGE_SIZE	4096
 #endif

include/lcd_console.h +14 -4

index 429214d..c52b29a 100644
@@ -9,6 +9,16 @@
 #define CONFIG_CONSOLE_SCROLL_LINES 1
 #endif
 
+struct console_t {
+	short curr_col, curr_row;
+	short cols, rows;
+	void *fbbase;
+	u32 lcdsizex, lcdsizey, lcdrot;
+	void (*fp_putc_xy)(struct console_t *pcons, ushort x, ushort y, char c);
+	void (*fp_console_moverow)(struct console_t *pcons,
+				   u32 rowdst, u32 rowsrc);
+	void (*fp_console_setrow)(struct console_t *pcons, u32 row, int clr);
+};
 /**
  * lcd_init_console() - Initialize lcd console parameters
  *

@@ -16,11 +26,11 @@
  * 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 vl_cols, int vl_rows, int vl_rot);
 /**
  * lcd_set_col() - Set the number of the current lcd console column
  *

Signature · 1 line

1.7.9.5

Nikita Kiryanov

2015-03-25 16:24 UTC #7 lore raw

PATCH v3 4/4 common/lcd_console: introduce display/framebuffer rotation

Hi Hannes,

This is almost an Acked-By from me, just a few final comments:

Quoted 49 lines · Hannes Petermaier

From: Hannes Petermaier <[email protected]>
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.
Signed-off-by Hannes Petermaier <[email protected]>
Signed-off-by Hannes Petermaier <[email protected]>
---
Changes in v3:
- rename lcd_address to fbbase for better readability.
- remove empty line lcd_console.c
- use printf instead puts to inform about invalid-fb rotation.
- avoid code-duplication (move define of fbptr_t into lcd.h)
Changes in v2:
- cleanup README text for feature
- don't make code cleanups (lcd_console.c) within this patch
- remove (unnary) comment in lcd_console.h
- update year to 2015 within copyright in lcd_console.c
- move rotation related code into separate file lcd_console_rotation.c
- rework rotation code
- change meaning of vl_rot to match fbcon=rotate:<n> from the linux-kernel
  README                        |   22 +++++
  common/Makefile               |    1 +
  common/lcd.c                  |   15 ++-
  common/lcd_console.c          |  161 +++++++++++++++++--------------
  common/lcd_console_rotation.c |  208 +++++++++++++++++++++++++++++++++++++++++
  include/lcd.h                 |    9 ++
  include/lcd_console.h         |   18 +++-
  7 files changed, 349 insertions(+), 85 deletions(-)
create mode 100644 common/lcd_console_rotation.c
README +0 -0
index b0124d6..c649de1 100644

README CBFS (Coreboot Filesystem) support · line 1953

Quoted 7 lines

README +4 -0

@@ -1947,6 +1947,28 @@ 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,

s/if it/if it's/

README CBFS (Coreboot Filesystem) support · line 1954

Quoted 1 line

README +1 -0

+		we need to rotate our content of the display respectively the

s/respectively the/relative to the/

README CBFS (Coreboot Filesystem) support · line 1955

Quoted 1 line

README +1 -0

+		framebuffer, so that user can read the messages who are printed

s/who are printed/which are printed/

README CBFS (Coreboot Filesystem) support · line 1959

Quoted 4 lines

README +4 -0

+		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

"this may be defined in the board-configuration if needed"
This is true for all config options in general, no need to mention this. Also, "For this we introduce" is good for a commit message, but doesn't look good once committed.
How about just "Once CONFIG_LCD_ROTATION is defined, the lcd_console will be..."

Quoted 15 lines

README +12 -0

+		from "vl_rot" out of "vidinfo_t" which is provided by the board
+		specific code.
+		The value for vl_rot is coded as following (matching to
+		fbcon=rotate:<n> linux-kernel commandline):
+		0 = no rotation respectively 0 degree
+		1 = 90 degree rotation
+		2 = 180 degree rotation
+		3 = 270 degree rotation
+
+		If CONFIG_LCD_ROTATION is not defined, the console will be
+		initialized with 0degree rotation.
+
  		CONFIG_LCD_BMP_RLE8
 
  		Support drawing of RLE8-compressed bitmaps on the LCD.

[...]

Quoted 10 lines

README +10 -0

+static void console_calc_rowcol(struct console_t *pcons)
+{
+	pcons->cols = pcons->lcdsizex / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+	pcons->rows = (pcons->lcdsizey - BMP_LOGO_HEIGHT);
+	pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+	pcons->rows = pcons->lcdsizey / VIDEO_FONT_HEIGHT;
+#endif
+}

[...]

README U_BOOT_CMD( · line 238

Quoted 5 lines

README +0 -1

@@ -235,4 +253,3 @@ U_BOOT_CMD(
  	"print string on lcd-framebuffer",
  	"    <string>"
  );
-

Looks like part of the cleanup from the previous series slipped through...

Quoted 5 lines

common/lcd_console_rotation.c new file +0 -0
new file mode 100644
index 0000000..dd9dadf

[...]

Quoted 9 lines

common/lcd_console_rotation.c new file +9 -0

+static void console_calc_rowcol_rot(struct console_t *pcons)
+{
+	u32 cols, rows;
+
+	if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
+		cols = pcons->lcdsizey;
+		rows = pcons->lcdsizex;
+	} else {
+		cols = pcons->lcdsizex;

common/lcd_console_rotation.c line 183

Quoted 10 lines

common/lcd_console_rotation.c new file +10 -0

+		rows = pcons->lcdsizey;
+	}
+
+	pcons->cols = cols / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+	pcons->rows = (rows - BMP_LOGO_HEIGHT);
+	pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+	pcons->rows = rows / VIDEO_FONT_HEIGHT;
+#endif

This duplication with console_calc_rowcol() exists because the lcdsizey and lcdsizex data is expected by the functions to be already in pcons. If you change console_calc_rowcol() to accept both variables as additional arguments, then console_calc_rowcol() could be reused in console_calc_rowcol_rot() and we'll get rid of the code duplication.

Signature · 2 lines

Regards,
Nikita Kiryanov

Hannes Petermaier

2015-03-25 21:24 UTC #8 lore raw

PATCH v3 4/4 common/lcd_console: introduce display/framebuffer rotation

Quoted 1 line · Nikita Kiryanov

Hi Hannes,

Hi Nikita,

Quoted 1 line

This is almost an Acked-By from me, just a few final comments:

Perfect, i think with v4 we can finish the thing :-)

Quoted 59 lines · Hannes Petermaier

Quoted 10 lines
README +0 -0
index b0124d6..c649de1 100644
@@ -1947,6 +1947,28 @@ CBFS (Coreboot Filesystem) support
          the console jump but can help speed up operation when 
scrolling
is slow.
README CBFS (Coreboot Filesystem) support · line 1953
Quoted 4 lines
README +4 -0
+        CONFIG_LCD_ROTATION
+
+        Sometimes, for example if the display is mounted in portrait
+        mode or even if it mounted landscape but rotated by 180degree,
s/if it/if it's/
Quoted 1 line
+ we need to rotate our content of the display respectively the
s/respectively the/relative to the/
Quoted 1 line
+ framebuffer, so that user can read the messages who are printed
s/who are printed/which are printed/
Quoted 5 lines
README +3 -0
+        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
"this may be defined in the board-configuration if needed" This is true for all config options in general, no need to mention this. Also, "For this we introduce" is good for a commit message, but doesn't look good
once committed.
How about just "Once CONFIG_LCD_ROTATION is defined, the lcd_console will be..."
Quoted 15 lines
README +12 -0
+        from "vl_rot" out of "vidinfo_t" which is provided by the board
+        specific code.
+        The value for vl_rot is coded as following (matching to
+        fbcon=rotate:<n> linux-kernel commandline):
+        0 = no rotation respectively 0 degree
+        1 = 90 degree rotation
+        2 = 180 degree rotation
+        3 = 270 degree rotation
+
+        If CONFIG_LCD_ROTATION is not defined, the console will be
+        initialized with 0degree rotation.
+
          CONFIG_LCD_BMP_RLE8
 
          Support drawing of RLE8-compressed bitmaps on the LCD.
[...]

Quoted 10 lines

README

Quoted 10 lines

README +10 -0

+static void console_calc_rowcol(struct console_t *pcons)
+{
+    pcons->cols = pcons->lcdsizex / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+    pcons->rows = (pcons->lcdsizey - BMP_LOGO_HEIGHT);
+    pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+    pcons->rows = pcons->lcdsizey / VIDEO_FONT_HEIGHT;
+#endif
+}

Okay, i will fixup the description in v4 ... maybe these troubles are coming from, lets say, sub-optimal english-language practise :-) In original i speak german.

Quoted 11 lines

[...]
README U_BOOT_CMD( · line 238
Quoted 5 lines
README +0 -1
@@ -235,4 +253,3 @@ U_BOOT_CMD(
      "print string on lcd-framebuffer",
      "    <string>"
  );
-
Looks like part of the cleanup from the previous series slipped through...

Okay, i will remove it.

Quoted 19 lines

README +18 -0

+static void console_calc_rowcol_rot(struct console_t *pcons)
+{
+    u32 cols, rows;
+
+    if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
+        cols = pcons->lcdsizey;
+        rows = pcons->lcdsizex;
+    } else {
+        cols = pcons->lcdsizex;
+        rows = pcons->lcdsizey;
+    }
+
+    pcons->cols = cols / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+    pcons->rows = (rows - BMP_LOGO_HEIGHT);
+    pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+    pcons->rows = rows / VIDEO_FONT_HEIGHT;

Quoted 8 lines

Quoted 1 line
+#endif
This duplication with console_calc_rowcol() exists because the lcdsizey and
lcdsizex data is expected by the functions to be already in pcons. If you change console_calc_rowcol() to accept both variables as additional arguments, then console_calc_rowcol() could be reused in console_calc_rowcol_rot() and we'll get rid of the code duplication.

I'm not sure about what is more uggly or better.
To avoid this duplication and use one function i have to make this function non-static and make a mix of rotation-code into lcd_console.c - i wouldn't prefer this.
Maybe the actual way of this (little) duplication is the beautiful one and gives most readability of the code.

what do you mean about?

best regards,
Hannes

Nikita Kiryanov

2015-03-26 16:32 UTC #9 lore raw

PATCH v3 4/4 common/lcd_console: introduce display/framebuffer rotation

Hi Hannes,

Quoted 4 lines · Hannes Petermaier

Quoted 4 lines

common/lcd_console.c +4 -0

+static void console_calc_rowcol(struct console_t *pcons)
+{
+    pcons->cols = pcons->lcdsizex / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)

Quoted 8 lines

Quoted 6 lines
common/lcd_console.c static inline void console_back(void) · line 129
Quoted 6 lines
common/lcd_console.c +6 -0
+    pcons->rows = (pcons->lcdsizey - BMP_LOGO_HEIGHT);
+    pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+    pcons->rows = pcons->lcdsizey / VIDEO_FONT_HEIGHT;
+#endif
+}
Okay, i will fixup the description in v4 ... maybe these troubles are coming from, lets say, sub-optimal english-language practise :-) In original i speak german.

That's perfectly understandable, don't worry about it :)

Quoted 25 lines

Quoted 17 lines
Quoted 17 lines
common/lcd_console_rotation.c new file +17 -0
+static void console_calc_rowcol_rot(struct console_t *pcons)
+{
+    u32 cols, rows;
+
+    if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
+        cols = pcons->lcdsizey;
+        rows = pcons->lcdsizex;
+    } else {
+        cols = pcons->lcdsizex;
+        rows = pcons->lcdsizey;
+    }
+
+    pcons->cols = cols / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+    pcons->rows = (rows - BMP_LOGO_HEIGHT);
+    pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
Quoted 8 lines
common/lcd_console_rotation.c line 183
Quoted 2 lines
common/lcd_console_rotation.c new file +2 -0
+    pcons->rows = rows / VIDEO_FONT_HEIGHT;
+#endif
This duplication with console_calc_rowcol() exists because the lcdsizey and lcdsizex data is expected by the functions to be already in pcons. If you change console_calc_rowcol() to accept both variables as additional arguments, then console_calc_rowcol() could be reused in console_calc_rowcol_rot() and we'll get rid of the code duplication.

Quoted 6 lines

I'm not sure about what is more uggly or better.
To avoid this duplication and use one function i have to make this function non-static and make a mix of rotation-code into lcd_console.c - i wouldn't prefer this. Maybe the actual way of this (little) duplication is the beautiful one and gives most readability of the code.
what do you mean about?

Actually I was thinking about keeping the two functions, but have console_calc_rowcol_rot() use console_calc_rowcol(), like so:

void console_calc_rowcol(struct console_t *pcons, u32 lcdsizex, lcdsizey)

{
	pcons->cols = lcdsizex / VIDEO_FONT_WIDTH;
#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
	pcons->rows = (lcdsizey - BMP_LOGO_HEIGHT);
	pcons->rows /= VIDEO_FONT_HEIGHT;
#else
	pcons->rows = lcdsizey / VIDEO_FONT_HEIGHT;

#endif
}

static void console_calc_rowcol_rot(struct console_t *pcons)

{
	u32 cols, rows;

	if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
		cols = pcons->lcdsizey;
		rows = pcons->lcdsizex;
	} else {
		cols = pcons->lcdsizex;
		rows = pcons->lcdsizey;
	}

	console_calc_rowcol(&cons, cols, rows);
}

Signature · 2 lines

Regards,
Nikita Kiryanov

Hannes Petermaier

author

2015-03-18 07:37 UTC #10 lore raw

PATCH v2 2/4 common/lcd_console: ask only one-time for bg/fg-color per call

In reply to Hannes Petermaier

From: Hannes Petermaier <[email protected]>

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

Signed-off-by Hannes Petermaier <[email protected]>

---

Changes in v2: None

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

common/lcd_console.c +2 -5

index 243b7c5..b7dda7a 100644
@@ -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];

Signature · 1 line

1.7.9.5

Hannes Petermaier

author

2015-03-18 07:37 UTC #11 lore raw

PATCH v2 3/4 common/lcd_console: move single static variables into common (static) structure

In reply to Hannes Petermaier

From: Hannes Petermaier <[email protected]>

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 <[email protected]>

---

Changes in v2: None

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

common/lcd_console.c +38 ~~-38~~

index b7dda7a..cac77be 100644
@@ -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();
 	}
 }

Signature · 1 line

1.7.9.5

Igor Grinberg

2015-03-18 12:11 UTC #12 lore raw

PATCH v2 3/4 common/lcd_console: move single static variables into common (static) structure

Quoted 20 lines · Hannes Petermaier

From: Hannes Petermaier <[email protected]>
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 <[email protected]>
Signed-off-by Hannes Petermaier <[email protected]>
---
Changes in v2: None
 common/lcd_console.c |   76 +++++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)
common/lcd_console.c +0 -0
index b7dda7a..cac77be 100644

[...]

common/lcd_console.c line 20

Quoted 4 lines

common/lcd_console.c +4 -0

+struct console_t {
+	short curr_col, curr_row;
+	short cols, rows;
+	void *lcd_address;

Shouldn't this be fbbase?

Quoted 2 lines

common/lcd_console.c +2 -0
+};
+static struct console_t cons;

[...]

Signature · 2 lines

Regards,
Igor.

Hannes Petermaier

2015-03-18 12:14 UTC #13 lore raw

PATCH v2 3/4 common/lcd_console: move single static variables into common (static) structure

Quoted 11 lines · Igor Grinberg

[...]
common/lcd_console.c line 20
Quoted 4 lines
common/lcd_console.c +4 -0
+struct console_t {
+	short curr_col, curr_row;
+	short cols, rows;
+	void *lcd_address;
Shouldn't this be fbbase?
Quoted 2 lines
common/lcd_console.c +2 -0
+};
+static struct console_t cons;
[...]

Hi Igor,
my plan was/is to make a cleanup patch after this story is closed.
- remove empty lines in code
- rename some variables

would it be better to do that immediately ?

best regards,
Hannes

Igor Grinberg

2015-03-18 12:47 UTC #14 lore raw

PATCH v2 3/4 common/lcd_console: move single static variables into common (static) structure

Quoted 8 lines · Hannes Petermaier

Quoted 7 lines · Igor Grinberg
[...]
common/lcd_console.c line 20
Quoted 4 lines
common/lcd_console.c +4 -0
+struct console_t {
+    short curr_col, curr_row;
+    short cols, rows;
+    void *lcd_address;
Shouldn't this be fbbase?

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
common/lcd_console.c +2 -0
+};
+static struct console_t cons;
[...]
Hi Igor,
my plan was/is to make a cleanup patch after this story is closed.
- remove empty lines in code

Yes indeed, but...

Quoted 1 line

- rename some variables

This depends. For example the lcd_address, you are introducing a lot of new places where this variable is used.
So, IMO, it is better to rename it before you are using it, otherwise the renaming patch will be much bigger and that is called needless churn...

Quoted 2 lines

would it be better to do that immediately ?

Well, I think that there are certain levels of cleanup...
In case of the lcd_address, I think it is better to rename it before use.

I think the rule of thumb (meaning this is not cast in stone) is first do a clean up and then add a functionality to already clean code.

Signature · 2 lines

Regards,
Igor.

Hannes Petermaier

2015-03-18 15:07 UTC #15 lore raw

PATCH v2 3/4 common/lcd_console: move single static variables into common (static) structure

Quoted 24 lines · Igor Grinberg

Quoted 8 lines · Hannes Petermaier
Quoted 7 lines · Igor Grinberg
[...]
common/lcd_console.c line 20
Quoted 4 lines
common/lcd_console.c +4 -0
+struct console_t {
+    short curr_col, curr_row;
+    short cols, rows;
+    void *lcd_address;
Shouldn't this be fbbase?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
common/lcd_console.c +2 -0
+};
+static struct console_t cons;
[...]
Hi Igor,
my plan was/is to make a cleanup patch after this story is closed.
- remove empty lines in code
Yes indeed, but...
Quoted 1 line
- rename some variables
This depends. For example the lcd_address, you are introducing a lot of new places where this variable is used.
So, IMO, it is better to rename it before you are using it, otherwise the renaming patch will be much bigger and that is called needless churn...

Quoted 6 lines

Quoted 1 line
would it be better to do that immediately ?
Well, I think that there are certain levels of cleanup...
In case of the lcd_address, I think it is better to rename it before use.
I think the rule of thumb (meaning this is not cast in stone) is first do a clean up and then add a functionality to already clean code.

Hi Igor,

Okay,
thanks - i will do the renanaming within my implementation of the rotation feature.

best regards,
Hannes

Igor Grinberg

2015-03-18 15:23 UTC #16 lore raw

PATCH v2 3/4 common/lcd_console: move single static variables into common (static) structure

Quoted 29 lines · Hannes Petermaier

Quoted 28 lines · Igor Grinberg
Quoted 8 lines · Hannes Petermaier
Quoted 7 lines · Igor Grinberg
[...]
common/lcd_console.c line 20
Quoted 4 lines
common/lcd_console.c +4 -0
+struct console_t {
+    short curr_col, curr_row;
+    short cols, rows;
+    void *lcd_address;
Shouldn't this be fbbase?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
common/lcd_console.c +2 -0
+};
+static struct console_t cons;
[...]
Hi Igor,
my plan was/is to make a cleanup patch after this story is closed.
- remove empty lines in code
Yes indeed, but...
Quoted 1 line
- rename some variables
This depends. For example the lcd_address, you are introducing a lot of new places where this variable is used.
So, IMO, it is better to rename it before you are using it, otherwise the renaming patch will be much bigger and that is called needless churn...
Quoted 1 line
would it be better to do that immediately ?
Well, I think that there are certain levels of cleanup...
In case of the lcd_address, I think it is better to rename it before use.

Quoted 6 lines

Quoted 2 lines
I think the rule of thumb (meaning this is not cast in stone) is first do a clean up and then add a functionality to already clean code.
Hi Igor,
Okay,
thanks - i will do the renanaming within my implementation of the rotation feature.

Yep. Looks like a good place is this patch as you rename/move it here already...

Signature · 2 lines

Regards,
Igor.

Hannes Petermaier

author

2015-03-18 07:37 UTC #17 lore raw

PATCH v2 4/4 common/lcd_console: introduce display/framebuffer rotation

In reply to Hannes Petermaier

From: Hannes Petermaier <[email protected]>

If CONFIG_LCD_ROTATION is not defined, the console will be initialized with 0 degrees rotation.

Signed-off-by Hannes Petermaier <[email protected]>

---

 README                        |   22 +++++
 common/Makefile               |    1 +
 common/lcd.c                  |   15 ++-
 common/lcd_console.c          |  168 ++++++++++++++++++--------------
 common/lcd_console_rotation.c |  216 +++++++++++++++++++++++++++++++++++++++++
 include/lcd.h                 |    1 +
 include/lcd_console.h         |   18 +++-
 7 files changed, 357 insertions(+), 84 deletions(-)
 create mode 100644 common/lcd_console_rotation.c

README +22 -0

index b0124d6..c649de1 100644
@@ -1947,6 +1947,28 @@ 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.
+		The value for vl_rot is coded as following (matching to
+		fbcon=rotate:<n> linux-kernel commandline):
+		0 = no rotation respectively 0 degree
+		1 = 90 degree rotation
+		2 = 180 degree rotation
+		3 = 270 degree rotation
+
+		If CONFIG_LCD_ROTATION is not defined, the console will be
+		initialized with 0degree rotation.
+
 		CONFIG_LCD_BMP_RLE8
 
 		Support drawing of RLE8-compressed bitmaps on the LCD.

common/Makefile +1 -0

index 7216a13..a1b3c09 100644
@@ -200,6 +200,7 @@ obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-y += splash.o
 obj-$(CONFIG_SPLASH_SOURCE) += splash_source.o
 obj-$(CONFIG_LCD) += lcd.o lcd_console.o
+obj-$(CONFIG_LCD_ROTATION) += lcd_console_rotation.o
 obj-$(CONFIG_LCD_DT_SIMPLEFB) += lcd_simplefb.o
 obj-$(CONFIG_LYNXKDI) += lynxkdi.o
 obj-$(CONFIG_MENU) += menu.o

common/lcd.c +6 -9

index f33942c..d1b4ca7 100644
@@ -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,14 @@ void lcd_clear(void)
 	}
 #endif
 #endif
+	/* setup text-console */
+	debug("[LCD] setting up console...\n");
+	lcd_init_console(lcd_base,
+			 panel_info.vl_col,
+			 panel_info.vl_row,
+			 panel_info.vl_rot);
 	/* 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;
-#else
-	console_rows = panel_info.vl_row / VIDEO_FONT_HEIGHT;
-#endif
-	console_cols = panel_info.vl_col / VIDEO_FONT_WIDTH;
-	lcd_init_console(lcd_base, console_rows, console_cols);
 	if (do_splash) {
 		s = getenv("splashimage");
 		if (s) {

common/lcd_console.c +97 ~~-71~~

222 lines not on this page · raw message

common/lcd_console_rotation.c new file +216 -0

216 lines not on this page · raw message

include/lcd.h +1 -0

index f049fd3..1329280 100644
@@ -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 (0, 1, 2, 3) */
 	u_char	vl_bpix;	/* Bits per pixel, 0 = 1 */
 	ushort	*cmap;		/* Pointer to the colormap */
 	void	*priv;		/* Pointer to driver-specific data */

include/lcd_console.h +14 -4

index 429214d..6612a38 100644
@@ -9,6 +9,16 @@
 #define CONFIG_CONSOLE_SCROLL_LINES 1
 #endif
 
+struct console_t {
+	short curr_col, curr_row;
+	short cols, rows;
+	void *lcd_address;
+	u32 lcdsizex, lcdsizey, lcdrot;
+	void (*fp_putc_xy)(struct console_t *pcons, ushort x, ushort y, char c);
+	void (*fp_console_moverow)(struct console_t *pcons,
+				   u32 rowdst, u32 rowsrc);
+	void (*fp_console_setrow)(struct console_t *pcons, u32 row, int clr);
+};
 /**
  * lcd_init_console() - Initialize lcd console parameters
  *

@@ -16,11 +26,11 @@
  * 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 vl_cols, int vl_rows, int vl_rot);
 /**
  * lcd_set_col() - Set the number of the current lcd console column
  *

Signature · 1 line

1.7.9.5

Igor Grinberg

2015-03-18 12:56 UTC #18 lore raw

PATCH v2 4/4 common/lcd_console: introduce display/framebuffer rotation

Hi Hannes,

Quoted 17 lines · Hannes Petermaier

From: Hannes Petermaier <[email protected]>
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.
Signed-off-by Hannes Petermaier <[email protected]>
Signed-off-by Hannes Petermaier <[email protected]>

[...]

Quoted 4 lines

common/lcd_console.c +0 -0
index cac77be..14fd890 100644

[...]

Quoted 182 lines

common/lcd_console.c +76 ~~-67~~

@@ -10,26 +11,16 @@
 #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)
+#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;
-};
 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,61 +47,50 @@ 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(struct console_t *pcons, ushort x, ushort y, char c)
 {
-	uchar *dest;
-	ushort row;
 	int fg_color = lcd_getfgcolor();
 	int bg_color = lcd_getbgcolor();
-	int i;
-
-	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) {
-#if LCD_BPP == LCD_COLOR16
-		ushort *d = (ushort *)dest;
-#elif LCD_BPP == LCD_COLOR32
-		u32 *d = (u32 *)dest;
-#else
-		uchar *d = dest;
-#endif
-		uchar bits;
-		bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
-
-		for (i = 0; i < 8; ++i) {
-			*d++ = (bits & 0x80) ? fg_color : bg_color;
+	int i, row;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  y * pcons->lcdsizex +
+				  x;
+
+	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
+		uchar bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
+		for (i = 0; i < VIDEO_FONT_WIDTH; ++i) {
+			*dst++ = (bits & 0x80) ? fg_color : bg_color;
 			bits <<= 1;
 		}
+		dst += (pcons->lcdsizex - VIDEO_FONT_WIDTH);
 	}
 }
 
-static void console_scrollup(void)
+static inline void console_setrow0(struct console_t *pcons, u32 row, int clr)
 {
-	const int rows = CONFIG_CONSOLE_SCROLL_LINES;
-	int bg_color = lcd_getbgcolor();
+	int i;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  row * VIDEO_FONT_HEIGHT *
+				  pcons->lcdsizex;
 
-	/* 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);
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * pcons->lcdsizex); i++)
+		*dst++ = clr;
+}
 
-	/* 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;
-	}
-#endif
-	lcd_sync();
-	cons.curr_row -= rows;
+static inline void console_moverow0(struct console_t *pcons,
+				    u32 rowdst, u32 rowsrc)
+{
+	int i;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  rowdst * VIDEO_FONT_HEIGHT *
+				  pcons->lcdsizex;
+
+	fbptr_t *src = (fbptr_t *)pcons->lcd_address +
+				  rowsrc * VIDEO_FONT_HEIGHT *
+				  pcons->lcdsizex;
+
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * pcons->lcdsizex); i++)
+		*dst++ = *src++;
 }
 
 static inline void console_back(void)

@@ -121,19 +101,65 @@ 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,
+			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(&cons, i, i+rows);
+		for (i = 0; i < rows; i++)
+			cons.fp_console_setrow(&cons, cons.rows-i-1, bg_color);
+		cons.curr_row -= rows;
+	}
+	lcd_sync();
+}
+
+static void console_calc_rowcol(struct console_t *pcons)
+{
+	pcons->cols = pcons->lcdsizex / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+	pcons->rows = (pcons->lcdsizey - BMP_LOGO_HEIGHT);
+	pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+	pcons->rows = pcons->lcdsizey / VIDEO_FONT_HEIGHT;
+#endif
+}
+
+void __weak lcd_init_console_rot(struct console_t *pcons)
+{
+	return;
+}
+
+void lcd_init_console(void *address, int vl_cols, int vl_rows, int vl_rot)
+{
+	memset(&cons, 0, sizeof(cons));

common/lcd_console.c static inline void console_back(void) · line 156

Quoted 10 lines

common/lcd_console.c +10 -0

+	cons.lcd_address = address;
+
+	cons.lcdsizex = vl_cols;
+	cons.lcdsizey = vl_rows;
+	cons.lcdrot = vl_rot;
+
+	cons.fp_putc_xy = &lcd_putc_xy0;
+	cons.fp_console_moverow = &console_moverow0;
+	cons.fp_console_setrow = &console_setrow0;
+	console_calc_rowcol(&cons);

I think the above four lines is exactly what should be placed in the __weak variant of lcd_init_console_rot() function (the one just above this one).

common/lcd_console.c static inline void console_back(void) · line 162

Quoted 6 lines

common/lcd_console.c +6 -0

+
+	lcd_init_console_rot(&cons);
+
+	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);
+

no need for the empty line here.

Quoted 34 lines

common/lcd_console.c +3 -3

 }
 
 void lcd_putc(const char c)

@@ -165,8 +191,9 @@ void lcd_putc(const char c)
 
 		return;
 	default:
-		lcd_putc_xy(cons.curr_col * VIDEO_FONT_WIDTH,
-			    cons.curr_row * VIDEO_FONT_HEIGHT, c);
+		cons.fp_putc_xy(&cons,
+				cons.curr_col * VIDEO_FONT_WIDTH,
+				cons.curr_row * VIDEO_FONT_HEIGHT, c);
 		if (++cons.curr_col >= cons.cols)
 			console_newline();
 	}

@@ -235,4 +262,3 @@ U_BOOT_CMD(
 	"print string on lcd-framebuffer",
 	"    <string>"
 );
-

common/lcd_console_rotation.c new file +8 -0

new file mode 100644
index 0000000..3b6bb4f
@@ -0,0 +1,216 @@
+/*
+ * (C) Copyright 2015
+ * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>

common/lcd_console_rotation.c line 18

Quoted 10 lines

common/lcd_console_rotation.c new file +10 -0

+#include <lcd.h>
+#include <video_font.h>		/* Get font data, width and height */
+
+#if LCD_BPP == LCD_COLOR16
+	#define fbptr_t ushort
+#elif LCD_BPP == LCD_COLOR32
+	#define fbptr_t u32
+#else
+	#define fbptr_t uchar
+#endif

That is a duplication of the code in lcd_console.c.
If we can get rid of these size games, probably we should have in the lcd.h, or lcd_console.h, or ...

Quoted 183 lines

common/lcd_console_rotation.c new file +183 -0

+
+static void lcd_putc_xy90(struct console_t *pcons, ushort x, ushort y, char c)
+{
+	int fg_color = lcd_getfgcolor();
+	int bg_color = lcd_getbgcolor();
+	int col, i;
+
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  (x+1) * pcons->lcdsizex -
+				  y;
+
+	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)
+			*dst-- = (*(pfont + i) & msk) ? fg_color : bg_color;
+		msk >>= 1;
+		dst += (pcons->lcdsizex + VIDEO_FONT_HEIGHT);
+	}
+}
+
+static inline void console_setrow90(struct console_t *pcons, u32 row, int clr)
+{
+	int i, j;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  pcons->lcdsizex -
+				  row*VIDEO_FONT_HEIGHT+1;
+
+	for (j = 0; j < pcons->lcdsizey; j++) {
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*dst-- = clr;
+		dst += (pcons->lcdsizex + VIDEO_FONT_HEIGHT);
+	}
+}
+
+static inline void console_moverow90(struct console_t *pcons,
+				      u32 rowdst, u32 rowsrc)
+{
+	int i, j;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  pcons->lcdsizex -
+				  (rowdst*VIDEO_FONT_HEIGHT+1);
+
+	fbptr_t *src = (fbptr_t *)pcons->lcd_address +
+				  pcons->lcdsizex -
+				  (rowsrc*VIDEO_FONT_HEIGHT+1);
+
+	for (j = 0; j < pcons->lcdsizey; j++) {
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*dst-- = *src--;
+		src += (pcons->lcdsizex + VIDEO_FONT_HEIGHT);
+		dst += (pcons->lcdsizex + VIDEO_FONT_HEIGHT);
+	}
+}
+static void lcd_putc_xy180(struct console_t *pcons, ushort x, ushort y, char c)
+{
+	int fg_color = lcd_getfgcolor();
+	int bg_color = lcd_getbgcolor();
+	int i, row;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  pcons->lcdsizex +
+				  pcons->lcdsizey * pcons->lcdsizex -
+				  y * pcons->lcdsizex -
+				  (x+1);
+
+	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
+		uchar bits = video_fontdata[c * VIDEO_FONT_HEIGHT + row];
+
+		for (i = 0; i < VIDEO_FONT_WIDTH; ++i) {
+			*dst-- = (bits & 0x80) ? fg_color : bg_color;
+			bits <<= 1;
+		}
+		dst -= (pcons->lcdsizex - VIDEO_FONT_WIDTH);
+	}
+}
+
+static inline void console_setrow180(struct console_t *pcons, u32 row, int clr)
+{
+	int i;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  (pcons->rows-row-1) * VIDEO_FONT_HEIGHT *
+				  pcons->lcdsizex;
+
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * pcons->lcdsizex); i++)
+		*dst++ = clr;
+}
+
+static inline void console_moverow180(struct console_t *pcons,
+				      u32 rowdst, u32 rowsrc)
+{
+	int i;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  (pcons->rows-rowdst-1) * VIDEO_FONT_HEIGHT *
+				  pcons->lcdsizex;
+
+	fbptr_t *src = (fbptr_t *)pcons->lcd_address +
+				  (pcons->rows-rowsrc-1) * VIDEO_FONT_HEIGHT *
+				  pcons->lcdsizex;
+
+	for (i = 0; i < (VIDEO_FONT_HEIGHT * pcons->lcdsizex); i++)
+		*dst++ = *src++;
+}
+
+static void lcd_putc_xy270(struct console_t *pcons, ushort x, ushort y, char c)
+{
+	int fg_color = lcd_getfgcolor();
+	int bg_color = lcd_getbgcolor();
+	int i, col;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  pcons->lcdsizey * pcons->lcdsizex -
+				  (x+1) * pcons->lcdsizex +
+				  y;
+
+	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)
+			*dst++ = (*(pfont + i) & msk) ? fg_color : bg_color;
+		msk >>= 1;
+		dst -= (pcons->lcdsizex + VIDEO_FONT_HEIGHT);
+	}
+}
+
+static inline void console_setrow270(struct console_t *pcons, u32 row, int clr)
+{
+	int i, j;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  row*VIDEO_FONT_HEIGHT;
+
+	for (j = 0; j < pcons->lcdsizey; j++) {
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*dst++ = clr;
+		dst += (pcons->lcdsizex - VIDEO_FONT_HEIGHT);
+	}
+}
+
+static inline void console_moverow270(struct console_t *pcons,
+				     u32 rowdst, u32 rowsrc)
+{
+	int i, j;
+	fbptr_t *dst = (fbptr_t *)pcons->lcd_address +
+				  rowdst*VIDEO_FONT_HEIGHT;
+
+	fbptr_t *src = (fbptr_t *)pcons->lcd_address +
+				  rowsrc*VIDEO_FONT_HEIGHT;
+
+	for (j = 0; j < pcons->lcdsizey; j++) {
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			*dst++ = *src++;
+		src += (pcons->lcdsizex - VIDEO_FONT_HEIGHT);
+		dst += (pcons->lcdsizex - VIDEO_FONT_HEIGHT);
+	}
+}
+
+static void console_calc_rowcol_rot(struct console_t *pcons)
+{
+	u32 cols, rows;
+
+	if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
+		cols = pcons->lcdsizey;
+		rows = pcons->lcdsizex;
+	} else {
+		cols = pcons->lcdsizex;
+		rows = pcons->lcdsizey;
+	}
+
+	pcons->cols = cols / VIDEO_FONT_WIDTH;
+#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
+	pcons->rows = (rows - BMP_LOGO_HEIGHT);
+	pcons->rows /= VIDEO_FONT_HEIGHT;
+#else
+	pcons->rows = rows / VIDEO_FONT_HEIGHT;
+#endif
+}
+
+void lcd_init_console_rot(struct console_t *pcons)
+{
+	if (pcons->lcdrot == 0) {
+		return;
+	} else if (pcons->lcdrot == 1) {
+		pcons->fp_putc_xy = &lcd_putc_xy90;
+		pcons->fp_console_moverow = &console_moverow90;
+		pcons->fp_console_setrow = &console_setrow90;

common/lcd_console_rotation.c line 211

Quoted 10 lines

common/lcd_console_rotation.c new file +10 -0

+	} else if (pcons->lcdrot == 2) {
+		pcons->fp_putc_xy = &lcd_putc_xy180;
+		pcons->fp_console_moverow = &console_moverow180;
+		pcons->fp_console_setrow = &console_setrow180;
+	} else if (pcons->lcdrot == 3) {
+		pcons->fp_putc_xy = &lcd_putc_xy270;
+		pcons->fp_console_moverow = &console_moverow270;
+		pcons->fp_console_setrow = &console_setrow270;
+	} else {
+		puts("lcd_init_console_rot: invalid framebuffer rotation!\n");

How about
printf("%s: invalid framebuffer rotation!\n", __func__); ?

Quoted 55 lines

common/lcd_console_rotation.c new file +5 -0

+		puts("                      using def. 0 degree rotation.\n");
+		return;
+	}
+	console_calc_rowcol_rot(pcons);
+}

include/lcd.h +1 -0

index f049fd3..1329280 100644
@@ -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 (0, 1, 2, 3) */
 	u_char	vl_bpix;	/* Bits per pixel, 0 = 1 */
 	ushort	*cmap;		/* Pointer to the colormap */
 	void	*priv;		/* Pointer to driver-specific data */

include/lcd_console.h +14 -4

index 429214d..6612a38 100644
@@ -9,6 +9,16 @@
 #define CONFIG_CONSOLE_SCROLL_LINES 1
 #endif
 
+struct console_t {
+	short curr_col, curr_row;
+	short cols, rows;
+	void *lcd_address;
+	u32 lcdsizex, lcdsizey, lcdrot;
+	void (*fp_putc_xy)(struct console_t *pcons, ushort x, ushort y, char c);
+	void (*fp_console_moverow)(struct console_t *pcons,
+				   u32 rowdst, u32 rowsrc);
+	void (*fp_console_setrow)(struct console_t *pcons, u32 row, int clr);
+};
 /**
  * lcd_init_console() - Initialize lcd console parameters
  *

@@ -16,11 +26,11 @@
  * 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 vl_cols, int vl_rows, int vl_rot);
 /**
  * lcd_set_col() - Set the number of the current lcd console column
  *

Signature · 2 lines

Regards,
Igor.

Hannes Petermaier

2015-03-18 14:59 UTC #19 lore raw

PATCH v2 4/4 common/lcd_console: introduce display/framebuffer rotation

"U-Boot" <[email protected]> schrieb am 18.03.2015 13:56:49:

Quoted 2 lines

Hi Hannes,

Hi Igor,
thanks for response - we come closer to the final solution :-)

Quoted 9 lines

common/lcd_console.c static inline void console_back(void)
Quoted 9 lines
common/lcd_console.c +8 -0
+}
+
+void __weak lcd_init_console_rot(struct console_t *pcons)
+{
+   return;
+}
+
+void lcd_init_console(void *address, int vl_cols, int vl_rows, int 

vl_rot)

Quoted 8 lines

common/lcd_console.c +8 -0

+{
+   memset(&cons, 0, sizeof(cons));
+   cons.lcd_address = address;
+
+   cons.lcdsizex = vl_cols;
+   cons.lcdsizey = vl_rows;
+   cons.lcdrot = vl_rot;
+

Quoted 8 lines

common/lcd_console.c static inline void console_back(void) · line 156
Quoted 4 lines
common/lcd_console.c +4 -0
+   cons.fp_putc_xy = &lcd_putc_xy0;
+   cons.fp_console_moverow = &console_moverow0;
+   cons.fp_console_setrow = &console_setrow0;
+   console_calc_rowcol(&cons);
I think the above four lines is exactly what should be placed in the __weak variant of lcd_init_console_rot() function (the one just above this one).

I think not so.
If the lcd_console_rotation.c is compiled in, the __weak function isn't called anymore.
And if user wants to have 0 degree rotation, the init-function from lcd_console_rotation.c doesn't anything.
Therefore it is necessary to initialize these pointers here.

Quoted 4 lines

common/lcd_console.c static inline void console_back(void)
Quoted 4 lines
common/lcd_console.c +4 -0
+
+   lcd_init_console_rot(&cons);
+
+   debug("lcd_console: have %d/%d col/rws on scr %dx%d (%d deg 

rotated)\n",

Quoted 4 lines

common/lcd_console.c static inline void console_back(void) · line 162
Quoted 2 lines
common/lcd_console.c +2 -0
+         cons.cols, cons.rows, cons.lcdsizex, cons.lcdsizey, vl_rot);
+
no need for the empty line here.

Will be changed in v3.

Quoted 7 lines

common/lcd_console_rotation.c new file +7 -0

+
+#include <common.h>
+#include <lcd.h>
+#include <video_font.h>      /* Get font data, width and height */
+
+#if LCD_BPP == LCD_COLOR16
+   #define fbptr_t ushort

Quoted 8 lines

common/lcd_console.c line 20
Quoted 5 lines
common/lcd_console.c +5 -0
+#elif LCD_BPP == LCD_COLOR32
+   #define fbptr_t u32
+#else
+   #define fbptr_t uchar
+#endif
That is a duplication of the code in lcd_console.c.
If we can get rid of these size games, probably we should have in the

lcd.h,

Quoted 1 line

or lcd_console.h, or ...

It might be possible to move this into lcd.h, in every case it is necesarry that common.h
is included before - from here the information about LCD_BPP is coming. I will have a 2nd look to that to avoid this duplication.

Quoted 15 lines

common/lcd_console_rotation.c new file +15 -0

+
+void lcd_init_console_rot(struct console_t *pcons)
+{
+   if (pcons->lcdrot == 0) {
+      return;
+   } else if (pcons->lcdrot == 1) {
+      pcons->fp_putc_xy = &lcd_putc_xy90;
+      pcons->fp_console_moverow = &console_moverow90;
+      pcons->fp_console_setrow = &console_setrow90;
+   } else if (pcons->lcdrot == 2) {
+      pcons->fp_putc_xy = &lcd_putc_xy180;
+      pcons->fp_console_moverow = &console_moverow180;
+      pcons->fp_console_setrow = &console_setrow180;
+   } else if (pcons->lcdrot == 3) {
+      pcons->fp_putc_xy = &lcd_putc_xy270;

Quoted 8 lines

common/lcd_console_rotation.c line 211
Quoted 4 lines
common/lcd_console_rotation.c new file +4 -0
+      pcons->fp_console_moverow = &console_moverow270;
+      pcons->fp_console_setrow = &console_setrow270;
+   } else {
+      puts("lcd_init_console_rot: invalid framebuffer rotation!\n");
How about
printf("%s: invalid framebuffer rotation!\n", __func__); ?

Okay, i will change that.
Sometime ago, somebody told me on the mailing list that i should prefer the puts function if i don't have to print out some values. If we want to use the printf we can also printout the given rotation pcons->lcdrot.

Quoted 3 lines

Signature · 2 lines
Regards,
Igor.

best regards,
Hannes

Igor Grinberg

2015-03-18 15:27 UTC #20 lore raw

PATCH v2 4/4 common/lcd_console: introduce display/framebuffer rotation

Quoted 6 lines · Hannes Petermaier

"U-Boot" <[email protected]> schrieb am 18.03.2015 13:56:49:
Quoted 2 lines
Hi Hannes,
Hi Igor,
thanks for response - we come closer to the final solution :-)

Indeed :-)

Quoted 27 lines

Quoted 10 lines
common/lcd_console.c static inline void console_back(void)
Quoted 10 lines
common/lcd_console.c +8 -0
+}
+
+void __weak lcd_init_console_rot(struct console_t *pcons)
+{
+   return;
+}
+
+void lcd_init_console(void *address, int vl_cols, int vl_rows, int 
vl_rot)
Quoted 8 lines
Quoted 8 lines
common/lcd_console.c +8 -0
+{
+   memset(&cons, 0, sizeof(cons));
+   cons.lcd_address = address;
+
+   cons.lcdsizex = vl_cols;
+   cons.lcdsizey = vl_rows;
+   cons.lcdrot = vl_rot;
+
Quoted 8 lines
common/lcd_console.c static inline void console_back(void) · line 156
Quoted 4 lines
common/lcd_console.c +4 -0
+   cons.fp_putc_xy = &lcd_putc_xy0;
+   cons.fp_console_moverow = &console_moverow0;
+   cons.fp_console_setrow = &console_setrow0;
+   console_calc_rowcol(&cons);
I think the above four lines is exactly what should be placed in the __weak variant of lcd_init_console_rot() function (the one just above this one).

Quoted 6 lines

I think not so.
If the lcd_console_rotation.c is compiled in, the __weak function isn't called anymore.
And if user wants to have 0 degree rotation, the init-function from lcd_console_rotation.c doesn't anything.
Therefore it is necessary to initialize these pointers here.

Right. Forgot about the 0 case...

Quoted 57 lines

Quoted 5 lines
common/lcd_console.c static inline void console_back(void)
Quoted 5 lines
common/lcd_console.c +4 -0
+
+   lcd_init_console_rot(&cons);
+
+   debug("lcd_console: have %d/%d col/rws on scr %dx%d (%d deg 
rotated)\n",
Quoted 4 lines
common/lcd_console.c static inline void console_back(void) · line 162
Quoted 2 lines
common/lcd_console.c +2 -0
+         cons.cols, cons.rows, cons.lcdsizex, cons.lcdsizey, vl_rot);
+
no need for the empty line here.
Will be changed in v3.
Quoted 7 lines
Quoted 7 lines
common/lcd_console_rotation.c new file +7 -0
+
+#include <common.h>
+#include <lcd.h>
+#include <video_font.h>      /* Get font data, width and height */
+
+#if LCD_BPP == LCD_COLOR16
+   #define fbptr_t ushort
Quoted 8 lines
common/lcd_console.c line 20
Quoted 5 lines
common/lcd_console.c +5 -0
+#elif LCD_BPP == LCD_COLOR32
+   #define fbptr_t u32
+#else
+   #define fbptr_t uchar
+#endif
That is a duplication of the code in lcd_console.c.
If we can get rid of these size games, probably we should have in the
lcd.h,
Quoted 1 line
or lcd_console.h, or ...
It might be possible to move this into lcd.h, in every case it is necesarry that common.h
is included before - from here the information about LCD_BPP is coming. I will have a 2nd look to that to avoid this duplication.
Quoted 15 lines
Quoted 15 lines
common/lcd_console_rotation.c new file +15 -0
+
+void lcd_init_console_rot(struct console_t *pcons)
+{
+   if (pcons->lcdrot == 0) {
+      return;
+   } else if (pcons->lcdrot == 1) {
+      pcons->fp_putc_xy = &lcd_putc_xy90;
+      pcons->fp_console_moverow = &console_moverow90;
+      pcons->fp_console_setrow = &console_setrow90;
+   } else if (pcons->lcdrot == 2) {
+      pcons->fp_putc_xy = &lcd_putc_xy180;
+      pcons->fp_console_moverow = &console_moverow180;
+      pcons->fp_console_setrow = &console_setrow180;
+   } else if (pcons->lcdrot == 3) {
+      pcons->fp_putc_xy = &lcd_putc_xy270;
Quoted 8 lines
common/lcd_console_rotation.c line 211
Quoted 4 lines
common/lcd_console_rotation.c new file +4 -0
+      pcons->fp_console_moverow = &console_moverow270;
+      pcons->fp_console_setrow = &console_setrow270;
+   } else {
+      puts("lcd_init_console_rot: invalid framebuffer rotation!\n");
How about
printf("%s: invalid framebuffer rotation!\n", __func__); ?

Quoted 3 lines

Okay, i will change that.
Sometime ago, somebody told me on the mailing list that i should prefer the puts function if i don't have to print out some values.

Well, this is no longer true as we have discussed this during the last U-Boot summit.

Quoted 2 lines

If we want to use the printf we can also printout the given rotation pcons->lcdrot.

Good.

Signature · 2 lines

Regards,
Igor.