Hi Dzmitry, On Mon, 13 Feb 2023 at 09:57, Dzmitry Sankouski <dsankou...@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 <dsankou...@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