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 -- w...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
