[PATCH v5 0/1] vidconsole: refactoring and support for wider fonts

[PATCH v5 0/1] vidconsole: refactoring and support for wider fonts

[Dzmitry Sankouski]

Version 5 contain minor changes:
- move common functions to console-core.c file
- remove static keyword from shared functions

In version 4, only first patch sent, because review fixes to this would add
large rebase & patch formatting overhead. When it'll receive reviewed tag,
I'll resent entire rebased series.

Modern mobile phones typically have high pixel density.
Bootmenu is hardly readable on those with 8x16 font.

This patch series aims to add wider fonts for devices with high ppi.

Add 16x32, 12x22 fonts from linux, and allow font size configuration.

There was significant changes in version 2:
- fix video tests failures
- add runtime font size configuration
- add test for 12x22 font

In version 3,
'video console: add select font logic to vidconsole uclass driver'
patch was removed in favor of already merged patch
'video: Add font functions to the vidconsole API'

Dzmitry Sankouski (1):
  video console: refactoring and optimization

 drivers/video/Makefile              |   6 +
 drivers/video/console_core.c        | 141 +++++++++++++
 drivers/video/console_normal.c      | 150 ++++----------
 drivers/video/console_rotate.c      | 308 ++++------------------------
 drivers/video/vidconsole_internal.h |  95 +++++++++
 5 files changed, 324 insertions(+), 376 deletions(-)
 create mode 100644 drivers/video/console_core.c
 create mode 100644 drivers/video/vidconsole_internal.h

-- 
2.30.2

[PATCH v5 1/1] video console: refactoring and optimization

[Dzmitry Sankouski]

- move common code to vidconsole_internal.h and console_core.c
- unite probe functions
- get rid of code duplications in switch across bpp values
- extract common pixel fill logic in two functions one per
horizontal and vertical filling
- rearrange statements in put_xy* methods in unified way
- replace types - uint*_t to u*

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
Changes for v2: none
Changes for v3: none
Changes for v4:
- move common code to vidconsole_internal.h
- unite probe functions
Changes for v5:
- move common functions to console-core.c file
- remove static keyword from shared functions

 drivers/video/Makefile              |   6 +
 drivers/video/console_core.c        | 141 +++++++++++++
 drivers/video/console_normal.c      | 150 ++++----------
 drivers/video/console_rotate.c      | 308 ++++------------------------
 drivers/video/vidconsole_internal.h |  95 +++++++++
 5 files changed, 324 insertions(+), 376 deletions(-)
 create mode 100644 drivers/video/console_core.c
 create mode 100644 drivers/video/vidconsole_internal.h

diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index cdb7d9a54d..cb3f373645 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -9,6 +9,12 @@ obj-$(CONFIG_BACKLIGHT_GPIO) += backlight_gpio.o
 obj-$(CONFIG_BACKLIGHT_PWM) += pwm_backlight.o
 obj-$(CONFIG_CONSOLE_NORMAL) += console_normal.o
 obj-$(CONFIG_CONSOLE_ROTATION) += console_rotate.o
+ifdef CONFIG_CONSOLE_NORMAL
+obj-y += console_core.o
+else ifdef CONFIG_CONSOLE_ROTATION
+obj-y += console_core.o
+endif
+obj-$(CONFIG_CONSOLE_ROTATION) += console_core.o
 obj-$(CONFIG_CONSOLE_TRUETYPE) += console_truetype.o fonts/
 obj-$(CONFIG_DISPLAY) += display-uclass.o
 obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi-host-uclass.o
diff --git a/drivers/video/console_core.c b/drivers/video/console_core.c
new file mode 100644
index 0000000000..9c2e4cb4ea
--- /dev/null
+++ b/drivers/video/console_core.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2015 Google, Inc
+ * (C) Copyright 2015
+ * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
+ * (C) Copyright 2023 Dzmitry Sankouski <dsankouski@gmail.com>
+ */
+
+#include <video.h>
+#include <video_console.h>
+#include <dm.h>
+#include "vidconsole_internal.h"
+
+int check_bpix_support(int bpix)
+{
+	if (bpix == VIDEO_BPP8 && IS_ENABLED(CONFIG_VIDEO_BPP8))
+		return 0;
+	else if (bpix == VIDEO_BPP16 && IS_ENABLED(CONFIG_VIDEO_BPP16))
+		return 0;
+	else if (bpix == VIDEO_BPP32 && IS_ENABLED(CONFIG_VIDEO_BPP32))
+		return 0;
+	else
+		return -ENOSYS;
+}
+
+inline void fill_pixel_and_goto_next(void **dstp, u32 value, int pbytes, int step)
+{
+	u8 *dst_byte = *dstp;
+
+	if (pbytes == 4) {
+		u32 *dst = *dstp;
+		*dst = value;
+	}
+	if (pbytes == 2) {
+		u16 *dst = *dstp;
+		*dst = value;
+	}
+	if (pbytes == 1) {
+		u8 *dst = *dstp;
+		*dst = value;
+	}
+	*dstp = dst_byte + step;
+}
+
+int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vid_priv,
+			 bool direction)
+{
+	int step, line_step, pbytes, ret;
+	void *dst;
+
+	ret = check_bpix_support(vid_priv->bpix);
+	if (ret)
+		return ret;
+
+	pbytes = VNBYTES(vid_priv->bpix);
+	if (direction) {
+		step = -pbytes;
+		line_step = -vid_priv->line_length;
+	} else {
+		step = pbytes;
+		line_step = vid_priv->line_length;
+	}
+
+	for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) {
+		dst = *line;
+		uchar bits = pfont[row];
+
+		for (int i = 0; i < VIDEO_FONT_WIDTH; i++) {
+			u32 value = (bits & 0x80) ?
+				vid_priv->colour_fg :
+				vid_priv->colour_bg;
+
+			fill_pixel_and_goto_next(&dst,
+						 value,
+						 pbytes,
+						 step
+			);
+			bits <<= 1;
+		}
+		*line += line_step;
+	}
+	return ret;
+}
+
+int fill_char_horizontally(uchar *pfont, void **line, struct video_priv *vid_priv,
+			   bool direction)
+{
+	int step, line_step, pbytes, ret;
+	void *dst;
+	u8 mask = 0x80;
+
+	ret = check_bpix_support(vid_priv->bpix);
+	if (ret)
+		return ret;
+
+	pbytes = VNBYTES(vid_priv->bpix);
+	if (direction) {
+		step = -pbytes;
+		line_step = vid_priv->line_length;
+	} else {
+		step = pbytes;
+		line_step = -vid_priv->line_length;
+	}
+	for (int col = 0; col < VIDEO_FONT_WIDTH; col++) {
+		dst = *line;
+		for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) {
+			u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH] & mask) ?
+						vid_priv->colour_fg :
+						vid_priv->colour_bg;
+
+			fill_pixel_and_goto_next(&dst,
+						 value,
+						 pbytes,
+						 step
+			);
+		}
+		*line += line_step;
+		mask >>= 1;
+	}
+	return ret;
+}
+
+int console_probe(struct udevice *dev)
+{
+	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
+	struct udevice *vid_dev = dev->parent;
+	struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
+
+	vc_priv->x_charsize = VIDEO_FONT_WIDTH;
+	vc_priv->y_charsize = VIDEO_FONT_HEIGHT;
+	if (vid_priv->rot % 2) {
+		vc_priv->cols = vid_priv->ysize / VIDEO_FONT_WIDTH;
+		vc_priv->rows = vid_priv->xsize / VIDEO_FONT_HEIGHT;
+		vc_priv->xsize_frac = VID_TO_POS(vid_priv->ysize);
+	} else {
+		vc_priv->cols = vid_priv->xsize / VIDEO_FONT_WIDTH;
+		vc_priv->rows = vid_priv->ysize / VIDEO_FONT_HEIGHT;
+	}
+
+	return 0;
+}
diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
index 04f022491e..57186bedd8 100644
--- a/drivers/video/console_normal.c
+++ b/drivers/video/console_normal.c
@@ -1,10 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
  * Copyright (c) 2015 Google, Inc
- * (C) Copyright 2001-2015
- * DENX Software Engineering -- wd@denx.de
- * Compulab Ltd - http://compulab.co.il/
+ * (C) Copyright 2015
  * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
+ * (C) Copyright 2023 Dzmitry Sankouski <dsankouski@gmail.com>
  */
 
 #include <common.h>
@@ -12,47 +11,28 @@
 #include <video.h>
 #include <video_console.h>
 #include <video_font.h>		/* Get font data, width and height */
+#include "vidconsole_internal.h"
 
-static int console_normal_set_row(struct udevice *dev, uint row, int clr)
+static int console_set_row(struct udevice *dev, uint row, int clr)
 {
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
-	void *line, *end;
+	void *line, *dst, *end;
 	int pixels = VIDEO_FONT_HEIGHT * vid_priv->xsize;
 	int ret;
 	int i;
+	int pbytes;
+
+	ret = check_bpix_support(vid_priv->bpix);
+	if (ret)
+		return ret;
 
 	line = vid_priv->fb + row * VIDEO_FONT_HEIGHT * vid_priv->line_length;
-	switch (vid_priv->bpix) {
-	case VIDEO_BPP8:
-		if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-			uint8_t *dst = line;
-
-			for (i = 0; i < pixels; i++)
-				*dst++ = clr;
-			end = dst;
-			break;
-		}
-	case VIDEO_BPP16:
-		if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-			uint16_t *dst = line;
-
-			for (i = 0; i < pixels; i++)
-				*dst++ = clr;
-			end = dst;
-			break;
-		}
-	case VIDEO_BPP32:
-		if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-			uint32_t *dst = line;
-
-			for (i = 0; i < pixels; i++)
-				*dst++ = clr;
-			end = dst;
-			break;
-		}
-	default:
-		return -ENOSYS;
-	}
+	dst = line;
+	pbytes = VNBYTES(vid_priv->bpix);
+	for (i = 0; i < pixels; i++)
+		fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes);
+	end = dst;
+
 	ret = vidconsole_sync_copy(dev, line, end);
 	if (ret)
 		return ret;
@@ -60,8 +40,8 @@ static int console_normal_set_row(struct udevice *dev, uint row, int clr)
 	return 0;
 }
 
-static int console_normal_move_rows(struct udevice *dev, uint rowdst,
-				     uint rowsrc, uint count)
+static int console_move_rows(struct udevice *dev, uint rowdst,
+			     uint rowsrc, uint count)
 {
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
 	void *dst;
@@ -79,70 +59,30 @@ static int console_normal_move_rows(struct udevice *dev, uint rowdst,
 	return 0;
 }
 
-static int console_normal_putc_xy(struct udevice *dev, uint x_frac, uint y,
-				  char ch)
+static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch)
 {
 	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
 	struct udevice *vid = dev->parent;
 	struct video_priv *vid_priv = dev_get_uclass_priv(vid);
-	int i, row;
-	void *start;
-	void *line;
-	int ret;
+	int pbytes = VNBYTES(vid_priv->bpix);
+	int x, linenum, ret;
+	void *start, *line;
+	uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT;
 
-	start = vid_priv->fb + y * vid_priv->line_length +
-		VID_TO_PIXEL(x_frac) * VNBYTES(vid_priv->bpix);
+	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
+		return -EAGAIN;
+	linenum = y;
+	x = VID_TO_PIXEL(x_frac);
+	start = vid_priv->fb + linenum * vid_priv->line_length + x * pbytes;
 	line = start;
 
 	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
 		return -EAGAIN;
 
-	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
-		unsigned int idx = (u8)ch * VIDEO_FONT_HEIGHT + row;
-		uchar bits = video_fontdata[idx];
-
-		switch (vid_priv->bpix) {
-		case VIDEO_BPP8:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-				uint8_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_WIDTH; i++) {
-					*dst++ = (bits & 0x80) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-					bits <<= 1;
-				}
-				break;
-			}
-		case VIDEO_BPP16:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-				uint16_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_WIDTH; i++) {
-					*dst++ = (bits & 0x80) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-					bits <<= 1;
-				}
-				break;
-			}
-		case VIDEO_BPP32:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-				uint32_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_WIDTH; i++) {
-					*dst++ = (bits & 0x80) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-					bits <<= 1;
-				}
-				break;
-			}
-		default:
-			return -ENOSYS;
-		}
-		line += vid_priv->line_length;
-	}
+	ret = fill_char_vertically(pfont, &line, vid_priv, NORMAL_DIRECTION);
+	if (ret)
+		return ret;
+
 	ret = vidconsole_sync_copy(dev, start, line);
 	if (ret)
 		return ret;
@@ -150,29 +90,15 @@ static int console_normal_putc_xy(struct udevice *dev, uint x_frac, uint y,
 	return VID_TO_POS(VIDEO_FONT_WIDTH);
 }
 
-static int console_normal_probe(struct udevice *dev)
-{
-	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
-	struct udevice *vid_dev = dev->parent;
-	struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
-
-	vc_priv->x_charsize = VIDEO_FONT_WIDTH;
-	vc_priv->y_charsize = VIDEO_FONT_HEIGHT;
-	vc_priv->cols = vid_priv->xsize / VIDEO_FONT_WIDTH;
-	vc_priv->rows = vid_priv->ysize / VIDEO_FONT_HEIGHT;
-
-	return 0;
-}
-
-struct vidconsole_ops console_normal_ops = {
-	.putc_xy	= console_normal_putc_xy,
-	.move_rows	= console_normal_move_rows,
-	.set_row	= console_normal_set_row,
+struct vidconsole_ops console_ops = {
+	.putc_xy	= console_putc_xy,
+	.move_rows	= console_move_rows,
+	.set_row	= console_set_row,
 };
 
 U_BOOT_DRIVER(vidconsole_normal) = {
 	.name	= "vidconsole0",
 	.id	= UCLASS_VIDEO_CONSOLE,
-	.ops	= &console_normal_ops,
-	.probe	= console_normal_probe,
+	.ops	= &console_ops,
+	.probe	= console_probe,
 };
diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c
index 36c8d0609d..70cc62d178 100644
--- a/drivers/video/console_rotate.c
+++ b/drivers/video/console_rotate.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2015 Google, Inc
  * (C) Copyright 2015
  * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
+ * (C) Copyright 2023 Dzmitry Sankouski <dsankouski@gmail.com>
  */
 
 #include <common.h>
@@ -10,12 +11,13 @@
 #include <video.h>
 #include <video_console.h>
 #include <video_font.h>		/* Get font data, width and height */
+#include "vidconsole_internal.h"
 
 static int console_set_row_1(struct udevice *dev, uint row, int clr)
 {
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
 	int pbytes = VNBYTES(vid_priv->bpix);
-	void *start, *line;
+	void *start, *dst, *line;
 	int i, j;
 	int ret;
 
@@ -23,34 +25,9 @@ static int console_set_row_1(struct udevice *dev, uint row, int clr)
 		(row + 1) * VIDEO_FONT_HEIGHT * pbytes;
 	line = start;
 	for (j = 0; j < vid_priv->ysize; j++) {
-		switch (vid_priv->bpix) {
-		case VIDEO_BPP8:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-				uint8_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
-					*dst++ = clr;
-				break;
-			}
-		case VIDEO_BPP16:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-				uint16_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
-					*dst++ = clr;
-				break;
-			}
-		case VIDEO_BPP32:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-				uint32_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
-					*dst++ = clr;
-				break;
-			}
-		default:
-			return -ENOSYS;
-		}
+		dst = line;
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes);
 		line += vid_priv->line_length;
 	}
 	ret = vidconsole_sync_copy(dev, start, line);
@@ -61,7 +38,7 @@ static int console_set_row_1(struct udevice *dev, uint row, int clr)
 }
 
 static int console_move_rows_1(struct udevice *dev, uint rowdst, uint rowsrc,
-			       uint count)
+				   uint count)
 {
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
 	int pbytes = VNBYTES(vid_priv->bpix);
@@ -76,7 +53,7 @@ static int console_move_rows_1(struct udevice *dev, uint rowdst, uint rowsrc,
 
 	for (j = 0; j < vid_priv->ysize; j++) {
 		ret = vidconsole_memmove(dev, dst, src,
-					 VIDEO_FONT_HEIGHT * pbytes * count);
+					VIDEO_FONT_HEIGHT * pbytes * count);
 		if (ret)
 			return ret;
 		src += vid_priv->line_length;
@@ -91,60 +68,22 @@ static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch)
 	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
 	struct udevice *vid = dev->parent;
 	struct video_priv *vid_priv = dev_get_uclass_priv(vid);
-	uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT;
 	int pbytes = VNBYTES(vid_priv->bpix);
-	int i, col, x, linenum, ret;
-	int mask = 0x80;
+	int x, linenum, ret;
 	void *start, *line;
+	uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT;
 
+	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
+		return -EAGAIN;
 	linenum = VID_TO_PIXEL(x_frac) + 1;
 	x = y + 1;
 	start = vid_priv->fb + linenum * vid_priv->line_length - x * pbytes;
 	line = start;
-	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
-		return -EAGAIN;
 
-	for (col = 0; col < VIDEO_FONT_HEIGHT; col++) {
-		switch (vid_priv->bpix) {
-		case VIDEO_BPP8:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-				uint8_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++) {
-					*dst-- = (pfont[i] & mask) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-				}
-				break;
-			}
-		case VIDEO_BPP16:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-				uint16_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++) {
-					*dst-- = (pfont[i] & mask) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-				}
-				break;
-			}
-		case VIDEO_BPP32:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-				uint32_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++) {
-					*dst-- = (pfont[i] & mask) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-				}
-				break;
-			}
-		default:
-			return -ENOSYS;
-		}
-		line += vid_priv->line_length;
-		mask >>= 1;
-	}
+	ret = fill_char_horizontally(pfont, &line, vid_priv, FLIPPED_DIRECTION);
+	if (ret)
+		return ret;
+
 	/* We draw backwards from 'start, so account for the first line */
 	ret = vidconsole_sync_copy(dev, start - vid_priv->line_length, line);
 	if (ret)
@@ -157,44 +96,18 @@ static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch)
 static int console_set_row_2(struct udevice *dev, uint row, int clr)
 {
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
-	void *start, *line, *end;
+	void *start, *line, *dst, *end;
 	int pixels = VIDEO_FONT_HEIGHT * vid_priv->xsize;
 	int i, ret;
+	int pbytes = VNBYTES(vid_priv->bpix);
 
 	start = vid_priv->fb + vid_priv->ysize * vid_priv->line_length -
 		(row + 1) * VIDEO_FONT_HEIGHT * vid_priv->line_length;
 	line = start;
-	switch (vid_priv->bpix) {
-	case VIDEO_BPP8:
-		if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-			uint8_t *dst = line;
-
-			for (i = 0; i < pixels; i++)
-				*dst++ = clr;
-			end = dst;
-			break;
-		}
-	case VIDEO_BPP16:
-		if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-			uint16_t *dst = line;
-
-			for (i = 0; i < pixels; i++)
-				*dst++ = clr;
-			end = dst;
-			break;
-		}
-	case VIDEO_BPP32:
-		if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-			uint32_t *dst = line;
-
-			for (i = 0; i < pixels; i++)
-				*dst++ = clr;
-			end = dst;
-			break;
-		}
-	default:
-		return -ENOSYS;
-	}
+	dst = line;
+	for (i = 0; i < pixels; i++)
+		fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes);
+	end = dst;
 	ret = vidconsole_sync_copy(dev, start, end);
 	if (ret)
 		return ret;
@@ -227,8 +140,9 @@ static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch)
 	struct udevice *vid = dev->parent;
 	struct video_priv *vid_priv = dev_get_uclass_priv(vid);
 	int pbytes = VNBYTES(vid_priv->bpix);
-	int i, row, x, linenum, ret;
+	int linenum, x, ret;
 	void *start, *line;
+	uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT;
 
 	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
 		return -EAGAIN;
@@ -237,52 +151,10 @@ static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch)
 	start = vid_priv->fb + linenum * vid_priv->line_length + x * pbytes;
 	line = start;
 
-	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
-		unsigned int idx = (u8)ch * VIDEO_FONT_HEIGHT + row;
-		uchar bits = video_fontdata[idx];
-
-		switch (vid_priv->bpix) {
-		case VIDEO_BPP8:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-				uint8_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_WIDTH; i++) {
-					*dst-- = (bits & 0x80) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-					bits <<= 1;
-				}
-				break;
-			}
-		case VIDEO_BPP16:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-				uint16_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_WIDTH; i++) {
-					*dst-- = (bits & 0x80) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-					bits <<= 1;
-				}
-				break;
-			}
-		case VIDEO_BPP32:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-				uint32_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_WIDTH; i++) {
-					*dst-- = (bits & 0x80) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-					bits <<= 1;
-				}
-				break;
-			}
-		default:
-			return -ENOSYS;
-		}
-		line -= vid_priv->line_length;
-	}
+	ret = fill_char_vertically(pfont, &line, vid_priv, FLIPPED_DIRECTION);
+	if (ret)
+		return ret;
+
 	/* Add 4 bytes to allow for the first pixel writen */
 	ret = vidconsole_sync_copy(dev, start + 4, line);
 	if (ret)
@@ -295,40 +167,15 @@ static int console_set_row_3(struct udevice *dev, uint row, int clr)
 {
 	struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
 	int pbytes = VNBYTES(vid_priv->bpix);
-	void *start, *line;
+	void *start, *dst, *line;
 	int i, j, ret;
 
 	start = vid_priv->fb + row * VIDEO_FONT_HEIGHT * pbytes;
 	line = start;
 	for (j = 0; j < vid_priv->ysize; j++) {
-		switch (vid_priv->bpix) {
-		case VIDEO_BPP8:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-				uint8_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
-					*dst++ = clr;
-				break;
-			}
-		case VIDEO_BPP16:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-				uint16_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
-					*dst++ = clr;
-				break;
-			}
-		case VIDEO_BPP32:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-				uint32_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
-					*dst++ = clr;
-				break;
-			}
-		default:
-			return -ENOSYS;
-		}
+		dst = line;
+		for (i = 0; i < VIDEO_FONT_HEIGHT; i++)
+			fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes);
 		line += vid_priv->line_length;
 	}
 	ret = vidconsole_sync_copy(dev, start, line);
@@ -367,58 +214,21 @@ static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch)
 	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
 	struct udevice *vid = dev->parent;
 	struct video_priv *vid_priv = dev_get_uclass_priv(vid);
-	uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT;
 	int pbytes = VNBYTES(vid_priv->bpix);
-	int i, col, x, ret;
-	int mask = 0x80;
+	int linenum, x, ret;
 	void *start, *line;
+	uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT;
 
 	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
 		return -EAGAIN;
-	x = vid_priv->ysize - VID_TO_PIXEL(x_frac) - 1;
-	start = vid_priv->fb + x * vid_priv->line_length + y * pbytes;
+	x = y;
+	linenum = vid_priv->ysize - VID_TO_PIXEL(x_frac) - 1;
+	start = vid_priv->fb + linenum * vid_priv->line_length + y * pbytes;
 	line = start;
-	for (col = 0; col < VIDEO_FONT_HEIGHT; col++) {
-		switch (vid_priv->bpix) {
-		case VIDEO_BPP8:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
-				uint8_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++) {
-					*dst++ = (pfont[i] & mask) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-				}
-				break;
-			}
-		case VIDEO_BPP16:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP16)) {
-				uint16_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++) {
-					*dst++ = (pfont[i] & mask) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-				}
-				break;
-			}
-		case VIDEO_BPP32:
-			if (IS_ENABLED(CONFIG_VIDEO_BPP32)) {
-				uint32_t *dst = line;
-
-				for (i = 0; i < VIDEO_FONT_HEIGHT; i++) {
-					*dst++ = (pfont[i] & mask) ?
-						vid_priv->colour_fg :
-						vid_priv->colour_bg;
-				}
-				break;
-			}
-		default:
-			return -ENOSYS;
-		}
-		line -= vid_priv->line_length;
-		mask >>= 1;
-	}
+
+	ret = fill_char_horizontally(pfont, &line, vid_priv, NORMAL_DIRECTION);
+	if (ret)
+		return ret;
 	/* Add a line to allow for the first pixels writen */
 	ret = vidconsole_sync_copy(dev, start + vid_priv->line_length, line);
 	if (ret)
@@ -427,36 +237,6 @@ static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch)
 	return VID_TO_POS(VIDEO_FONT_WIDTH);
 }
 
-
-static int console_probe_2(struct udevice *dev)
-{
-	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
-	struct udevice *vid_dev = dev->parent;
-	struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
-
-	vc_priv->x_charsize = VIDEO_FONT_WIDTH;
-	vc_priv->y_charsize = VIDEO_FONT_HEIGHT;
-	vc_priv->cols = vid_priv->xsize / VIDEO_FONT_WIDTH;
-	vc_priv->rows = vid_priv->ysize / VIDEO_FONT_HEIGHT;
-
-	return 0;
-}
-
-static int console_probe_1_3(struct udevice *dev)
-{
-	struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
-	struct udevice *vid_dev = dev->parent;
-	struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
-
-	vc_priv->x_charsize = VIDEO_FONT_WIDTH;
-	vc_priv->y_charsize = VIDEO_FONT_HEIGHT;
-	vc_priv->cols = vid_priv->ysize / VIDEO_FONT_WIDTH;
-	vc_priv->rows = vid_priv->xsize / VIDEO_FONT_HEIGHT;
-	vc_priv->xsize_frac = VID_TO_POS(vid_priv->ysize);
-
-	return 0;
-}
-
 struct vidconsole_ops console_ops_1 = {
 	.putc_xy	= console_putc_xy_1,
 	.move_rows	= console_move_rows_1,
@@ -479,19 +259,19 @@ U_BOOT_DRIVER(vidconsole_1) = {
 	.name	= "vidconsole1",
 	.id	= UCLASS_VIDEO_CONSOLE,
 	.ops	= &console_ops_1,
-	.probe	= console_probe_1_3,
+	.probe	= console_probe,
 };
 
 U_BOOT_DRIVER(vidconsole_2) = {
 	.name	= "vidconsole2",
 	.id	= UCLASS_VIDEO_CONSOLE,
 	.ops	= &console_ops_2,
-	.probe	= console_probe_2,
+	.probe	= console_probe,
 };
 
 U_BOOT_DRIVER(vidconsole_3) = {
 	.name	= "vidconsole3",
 	.id	= UCLASS_VIDEO_CONSOLE,
 	.ops	= &console_ops_3,
-	.probe	= console_probe_1_3,
+	.probe	= console_probe,
 };
diff --git a/drivers/video/vidconsole_internal.h b/drivers/video/vidconsole_internal.h
new file mode 100644
index 0000000000..0dfcd402c5
--- /dev/null
+++ b/drivers/video/vidconsole_internal.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2015 Google, Inc
+ * (C) Copyright 2015
+ * Bernecker & Rainer Industrieelektronik GmbH - http://www.br-automation.com
+ * (C) Copyright 2023 Dzmitry Sankouski <dsankouski@gmail.com>
+ */
+
+#include <video_font.h>		/* Get font data, width and height */
+
+#define VIDEO_FONT_BYTE_WIDTH	((VIDEO_FONT_WIDTH / 8) + (VIDEO_FONT_WIDTH % 8 > 0))
+
+#define FLIPPED_DIRECTION 1
+#define NORMAL_DIRECTION 0
+
+/**
+ * Checks if bits per pixel supported.
+ *
+ * @param bpix	framebuffer bits per pixel.
+ *
+ * @returns 0, if supported, or else -ENOSYS.
+ */
+int check_bpix_support(int bpix);
+
+/**
+ * Fill 1 pixel in framebuffer, and go to next one.
+ *
+ * @param dstp		a pointer to pointer to framebuffer.
+ * @param value		value to write to framebuffer.
+ * @param pbytes	framebuffer bytes per pixel.
+ * @param step		framebuffer pointer increment. Usually is equal to pbytes,
+ *			and may be negative to control filling direction.
+ */
+void fill_pixel_and_goto_next(void **dstp, u32 value, int pbytes, int step);
+
+/**
+ * Fills 1 character in framebuffer vertically. Vertically means we're filling char font data rows
+ * across the lines.
+ *
+ * @param pfont		a pointer to character font data.
+ * @param line		a pointer to pointer to framebuffer. It's a point for upper left char corner
+ * @param vid_priv	driver private data.
+ * @param direction	controls character orientation. Can be normal or flipped.
+ * When normal:               When flipped:
+ *|-----------------------------------------------|
+ *| line stepping        |                        |
+ *|            |         |       stepping ->      |
+ *|     *      |         |       * * *            |
+ *|   * *      v         |         *              |
+ *|     *                |         *              |
+ *|     *                |         * *      ^     |
+ *|   * * *              |         *        |     |
+ *|                      |                  |     |
+ *| stepping ->          |         line stepping  |
+ *|---!!we're starting from upper left char corner|
+ *|-----------------------------------------------|
+ *
+ * @returns 0, if success, or else error code.
+ */
+int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vid_priv,
+			 bool direction);
+
+/**
+ * Fills 1 character in framebuffer horizontally.
+ * Horizontally means we're filling char font data columns across the lines.
+ *
+ * @param pfont		a pointer to character font data.
+ * @param line		a pointer to pointer to framebuffer. It's a point for upper left char corner
+ * @param vid_priv	driver private data.
+ * @param direction	controls character orientation. Can be normal or flipped.
+ * When normal:               When flipped:
+ *|-----------------------------------------------|
+ *|               *        |   line stepping      |
+ *|    ^  * * * * *        |   |                  |
+ *|    |    *     *        |   v   *     *        |
+ *|    |                   |       * * * * *      |
+ *|  line stepping         |       *              |
+ *|                        |                      |
+ *|  stepping ->           |        <- stepping   |
+ *|---!!we're starting from upper left char corner|
+ *|-----------------------------------------------|
+ *
+ * @returns 0, if success, or else error code.
+ */
+int fill_char_horizontally(uchar *pfont, void **line, struct video_priv *vid_priv,
+			   bool direction);
+
+/**
+ * console probe function.
+ *
+ * @param dev	a pointer to device.
+ *
+ * @returns 0, if success, or else error code.
+ */
+int console_probe(struct udevice *dev);
-- 
2.30.2

Re: [PATCH v5 1/1] video console: refactoring and optimization

[Simon Glass]

On Mon, 20 Feb 2023 at 03:49, Dzmitry Sankouski <dsankouski@gmail.com> wrote:
>
> - move common code to vidconsole_internal.h and console_core.c
> - unite probe functions
> - get rid of code duplications in switch across bpp values
> - extract common pixel fill logic in two functions one per
> horizontal and vertical filling
> - rearrange statements in put_xy* methods in unified way
> - replace types - uint*_t to u*
>
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
> Changes for v2: none
> Changes for v3: none
> Changes for v4:
> - move common code to vidconsole_internal.h
> - unite probe functions
> Changes for v5:
> - move common functions to console-core.c file
> - remove static keyword from shared functions
>
>  drivers/video/Makefile              |   6 +
>  drivers/video/console_core.c        | 141 +++++++++++++
>  drivers/video/console_normal.c      | 150 ++++----------
>  drivers/video/console_rotate.c      | 308 ++++------------------------
>  drivers/video/vidconsole_internal.h |  95 +++++++++
>  5 files changed, 324 insertions(+), 376 deletions(-)
>  create mode 100644 drivers/video/console_core.c
>  create mode 100644 drivers/video/vidconsole_internal.h

Reviewed-by: Simon Glass <sjg@chromium.org>