Re: [PATCH v2 5/8] video console: allow font size configuration at runtime

Hi Dzmitry,

On Mon, 13 Feb 2023 at 09:57, Dzmitry Sankouski dsankouski@gmail.com wrote:

Allow font size configuration at runtime for console_simple.c driver. This needed for unit testing different fonts.

• move 8x16 font data to video_font_8x16.h file

please do that in a separate patch

• place common font macro in video_font_data.h file

Signed-off-by: Dzmitry Sankouski dsankouski@gmail.com

Changes for v2: N/A

cmd/Kconfig | 8 + cmd/Makefile | 2 +- common/splash.c | 7 +- drivers/video/Kconfig | 16 + drivers/video/console_simple.c | 178 +- include/video_font.h | 19 +- include/video_font_4x6.h | 11 +- include/video_font_8x16.h | 4624 +++++++++++++++++++++++++++++++ include/video_font_data.h | 4644 +------------------------------- 9 files changed, 4802 insertions(+), 4707 deletions(-) create mode 100644 include/video_font_8x16.h

diff --git a/cmd/Kconfig b/cmd/Kconfig index 199a55383e..033095769b 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2211,6 +2211,14 @@ config CMD_VIDCONSOLE The name 'lcdputs' is a bit of a misnomer, but so named because the video device is often an LCD.

+config CMD_SELECT_FONT

• bool "select font size"

•   depends on VIDEO
  

•   default n
  

•   help
  

•     Enabling this will provide 'font' command.
  

•     Allows font selection at runtime.
  

endmenu

source "cmd/ti/Kconfig" diff --git a/cmd/Makefile b/cmd/Makefile index 0b6a96c1d9..80b2796c1d 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -79,7 +79,7 @@ obj-$(CONFIG_CMD_EXT2) += ext2.o obj-$(CONFIG_CMD_FAT) += fat.o obj-$(CONFIG_CMD_FDT) += fdt.o obj-$(CONFIG_CMD_SQUASHFS) += sqfs.o -obj-$(CONFIG_CONSOLE_TRUETYPE) += font.o +obj-$(CONFIG_CMD_SELECT_FONT) += font.o obj-$(CONFIG_CMD_FLASH) += flash.o obj-$(CONFIG_CMD_FPGA) += fpga.o obj-$(CONFIG_CMD_FPGAD) += fpgad.o diff --git a/common/splash.c b/common/splash.c index 2e466a8a0f..3b36876a79 100644 --- a/common/splash.c +++ b/common/splash.c @@ -131,6 +131,7 @@ void splash_get_pos(int *x, int *y) void splash_display_banner(void) { struct udevice *dev;

•   struct video_priv *vid_priv;
    char buf[DISPLAY_OPTIONS_BANNER_LENGTH];
    int col, row, ret;
  
  

@@ -138,9 +139,11 @@ void splash_display_banner(void) if (ret) return;

•   vid_priv = dev_get_uclass_priv(dev->parent);
  

#ifdef CONFIG_VIDEO_LOGO

•   col = BMP_LOGO_WIDTH / VIDEO_FONT_WIDTH + 1;
  

•   row = BMP_LOGO_HEIGHT / VIDEO_FONT_HEIGHT + 1;
  

•   col = BMP_LOGO_WIDTH / vid_priv->fontdata->width + 1;
  

•   row = BMP_LOGO_HEIGHT / vid_priv->fontdata->height + 1;
  

#else col = 0; row = 0; diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 1dfe11d182..31dbf1f98d 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -16,6 +16,21 @@ config VIDEO

if VIDEO

+config VIDEO_FONT_4X6

•   bool "4 x 6 font size"
  

•   help
  

•     Font for video console driver, 4 x 6 pixels.
  

•     Provides character bitmap data in header file.
  

•     When selecting multiple fonts, you may want to enable CMD_SELECT_FONT too.
  

+config VIDEO_FONT_8X16

•   bool "8 x 16 font size"
  

•   default y
  

•   help
  

•     Font for video console driver, 8 x 16 pixels
  

•     Provides character bitmap data in header file.
  

•     When selecting multiple fonts, you may want to enable CMD_SELECT_FONT too.
  

config VIDEO_LOGO bool "Show the U-Boot logo on the display" default y if !SPLASH_SCREEN @@ -147,6 +162,7 @@ config CONSOLE_ROTATION

config CONSOLE_TRUETYPE bool "Support a console that uses TrueType fonts"

•   select CMD_SELECT_FONT
    help
      TrueTrype fonts can provide outline-drawing capability rather than
      needing to provide a bitmap for each font and size that is needed.
  

diff --git a/drivers/video/console_simple.c b/drivers/video/console_simple.c index cdc26cac30..f50a01d22e 100644 --- a/drivers/video/console_simple.c +++ b/drivers/video/console_simple.c @@ -12,12 +12,58 @@ #include <video_console.h> #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 VIDEO_FONT_CHAR_PIXEL_BYTES (VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH)

#define FLIPPED_DIRECTION 1 #define NORMAL_DIRECTION 0

+static int console_set_font(struct udevice *dev, struct video_fontdata *fontdata) +{

•   struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
  

•   struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
  

•   debug("console_simple: setting %s font\n", fontdata->name);
  

•   debug("width: %d\n", fontdata->width);
  

•   debug("byte width: %d\n", fontdata->byte_width);
  

•   debug("height: %d\n", fontdata->height);
  

•   vid_priv->fontdata = fontdata;
  

•   vc_priv->x_charsize = vid_priv->fontdata->width;
  

•   vc_priv->y_charsize = vid_priv->fontdata->height;
  

•   if (vid_priv->rot % 2) {
  

•           vc_priv->cols = vid_priv->ysize / vid_priv->fontdata->width;
  

•           vc_priv->rows = vid_priv->xsize / vid_priv->fontdata->height;
  

•           vc_priv->xsize_frac = VID_TO_POS(vid_priv->ysize);
  

•   } else {
  

•           vc_priv->cols = vid_priv->xsize / vid_priv->fontdata->width;
  

•           vc_priv->rows = vid_priv->ysize / vid_priv->fontdata->height;
  

•   }
  

•   return 0;
  

+}

+void console_simple_list_fonts(struct udevice __maybe_unused *dev)

static?

+{

•   struct video_fontdata *font;
  

•   for (font = fonts; font->name; font++) {
  

•           printf("%s\n", font->name);
  

•   };
  

+}

+int console_simple_select_font(struct udevice *dev, const char *name, uint size)

static?

+{

•   console_set_font(dev, &fonts[1]);
  

•   struct video_fontdata *font;
  

•   for (font = fonts; font->name; font++) {
  

•           if (!strcmp(name, font->name)) {
  

•                   console_set_font(dev, font);
  

•                   return 0;
  

•           }
  

•   };
  

•   printf("no such font: %s, make sure it's name has <width>x<height> format\n", name);
  

its

but ideally we don't print out things in drivers. Could this go in the command instead, perhaps suggesting using 'list' to see the available fonts?

•   return -ENOENT;
  

+}

/**

• Checks if bits per pixel supported.

@@ -113,21 +159,20 @@ static int fill_char_horizontally(uchar *pfont, void **line, struct video_priv * line_step = -vid_priv->line_length; }

•   width_remainder = VIDEO_FONT_WIDTH % 8;
  

•   for (int col = 0; col < VIDEO_FONT_BYTE_WIDTH; col++) {
  

•   width_remainder = vid_priv->fontdata->width % 8;
  

Please put fontdata in a local var to reduce the indirection steps each time. Same below

•   for (int col = 0; col < vid_priv->fontdata->byte_width; col++) {
            mask = 0x80;
            if (width_remainder) {
  

•                   bool is_last_iteration = (VIDEO_FONT_BYTE_WIDTH - col == 1);
  

•                   bool is_last_col = (vid_priv->fontdata->byte_width - col == 1);
  
  

•                   if (is_last_iteration)
  

•                   if (is_last_col)
                            bitcount = width_remainder;
            }
            for (int bit = 0; bit < bitcount; bit++) {
                    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;
  

•                   for (int row = 0; row < vid_priv->fontdata->height; row++) {
  

•                           u32 value = (pfont[row * vid_priv->fontdata->byte_width + col]
  

•                                        & mask) ? vid_priv->colour_fg : vid_priv->colour_bg;
  
                            fill_pixel_and_goto_next(&dst,
                                                     value,
  

@@ -186,17 +231,17 @@ static int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vi line_step = vid_priv->line_length; }

•   width_remainder = VIDEO_FONT_WIDTH % 8;
  

•   for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) {
  

•   width_remainder = vid_priv->fontdata->width % 8;
  

•   for (int row = 0; row < vid_priv->fontdata->height; row++) {
            uchar bits;
  
            bitcount = 8;
            dst = *line;
  

•           for (int col = 0; col < VIDEO_FONT_BYTE_WIDTH; col++) {
  

•           for (int col = 0; col < vid_priv->fontdata->byte_width; col++) {
                    if (width_remainder) {
  

•                           bool is_last_iteration = (VIDEO_FONT_BYTE_WIDTH - col == 1);
  

•                           bool is_last_col = (vid_priv->fontdata->byte_width - col == 1);
  
  

•                           if (is_last_iteration)
  

•                           if (is_last_col)
                                    bitcount = width_remainder;
                    }
                    bits = pfont[col];
  

@@ -215,7 +260,7 @@ static int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vi } } *line += line_step;

•           pfont += VIDEO_FONT_BYTE_WIDTH;
  

•           pfont += vid_priv->fontdata->byte_width;
    }
    return ret;
  

} @@ -224,7 +269,7 @@ 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, *dst, *end;

•   int pixels = VIDEO_FONT_HEIGHT * vid_priv->xsize;
  

•   int pixels = vid_priv->fontdata->height * vid_priv->xsize;
    int ret;
    int i;
    int pbytes;
  

@@ -233,7 +278,7 @@ static int console_set_row(struct udevice *dev, uint row, int clr) if (ret) return ret;

•   line = vid_priv->fb + row * VIDEO_FONT_HEIGHT * vid_priv->line_length;
  

•   line = vid_priv->fb + row * vid_priv->fontdata->height * vid_priv->line_length;
    dst = line;
    pbytes = VNBYTES(vid_priv->bpix);
    for (i = 0; i < pixels; i++)
  

@@ -256,9 +301,9 @@ static int console_move_rows(struct udevice *dev, uint rowdst, int size; int ret;

•   dst = vid_priv->fb + rowdst * VIDEO_FONT_HEIGHT * vid_priv->line_length;
  

•   src = vid_priv->fb + rowsrc * VIDEO_FONT_HEIGHT * vid_priv->line_length;
  

•   size = VIDEO_FONT_HEIGHT * vid_priv->line_length * count;
  

•   dst = vid_priv->fb + rowdst * vid_priv->fontdata->height * vid_priv->line_length;
  

•   src = vid_priv->fb + rowsrc * vid_priv->fontdata->height * vid_priv->line_length;
  

•   size = vid_priv->fontdata->height * vid_priv->line_length * count;
    ret = vidconsole_memmove(dev, dst, src, size);
    if (ret)
            return ret;
  

@@ -274,7 +319,8 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch) int pbytes = VNBYTES(vid_priv->bpix); int x, linenum, ret; void *start, *line;

•   uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_BYTES;
  

•   uchar *pfont = vid_priv->fontdata->video_fontdata +
  

•                   (u8)ch * vid_priv->fontdata->char_pixel_bytes;
  
    if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
            return -EAGAIN;
  

@@ -294,27 +340,20 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch) if (ret) return ret;

•   return VID_TO_POS(VIDEO_FONT_WIDTH);
  

•   return VID_TO_POS(vid_priv->fontdata->width);
  

}

static 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;
  

•   vc_priv->cols = vid_priv->xsize / VIDEO_FONT_WIDTH;
  

•   vc_priv->rows = vid_priv->ysize / VIDEO_FONT_HEIGHT;
  

•   return 0;
  

•   return console_set_font(dev, &fonts[0]);
  

}

struct vidconsole_ops console_ops = { .putc_xy = console_putc_xy, .move_rows = console_move_rows, .set_row = console_set_row,

•   .list_fonts     = console_simple_list_fonts,
  

•   .select_font    = console_simple_select_font,
  

Hmm can you split out the addition of these two items to a separate patch? There is too much going on in this patch.

[..]

Regards, Simon