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

[Hannes Petermaier <hannes@petermaier.org>]

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

best regards,
Hannes