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

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Feb 14 21:13:20 CET 2023



Am 14. Februar 2023 20:48:53 MEZ schrieb Simon Glass <sjg at chromium.org>:
>Hi Dzmitry,
>
>On Mon, 13 Feb 2023 at 09:57, Dzmitry Sankouski <dsankouski at 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 at 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

Shouldn't we have a font that is somehow legible on a 4k display, e.g. 16x32?

Best regards

Heinrich 


 +-------------------------------
>>  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


More information about the U-Boot mailing list