From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikita Kiryanov Date: Wed, 25 Mar 2015 18:24:26 +0200 Subject: [U-Boot] [PATCH v3 4/4] common/lcd_console: introduce display/framebuffer rotation In-Reply-To: <1426754263-29073-5-git-send-email-oe5hpm@oevsv.at> References: <1426664243-30998-2-git-send-email-oe5hpm@oevsv.at> <1426754263-29073-1-git-send-email-oe5hpm@oevsv.at> <1426754263-29073-5-git-send-email-oe5hpm@oevsv.at> Message-ID: <5512E13A.8000902@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > 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 > Signed-off-by: Hannes Petermaier > --- > > 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: 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: 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", > " " > ); > - 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