Hi Hannes,

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

On 03/19/2015 10:37 AM, Hannes Petermaier wrote:
From: Hannes Petermaier <hannes.peterma...@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.

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

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

diff --git a/README b/README
index b0124d6..c649de1 100644
--- a/README
+++ b/README
@@ -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/

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

s/respectively the/relative to the/

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

s/who are printed/which 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

"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..."

+               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.

[...]

+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
+}

[...]

@@ -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...

diff --git a/common/lcd_console_rotation.c b/common/lcd_console_rotation.c
new file mode 100644
index 0000000..dd9dadf
--- /dev/null
+++ b/common/lcd_console_rotation.c

[...]

+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

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.

--
Regards,
Nikita Kiryanov
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to