[PATCH 0/5] Preparation work for using font_desc in vc_data

[PATCH 0/5] Preparation work for using font_desc in vc_data

[Peilin Ye]

Hi Daniel, Hi Greg, Hi all,

We are planning to use `font_desc` instead of `console_font` in `vc_data`,
and this is just some prep work for it. It doesn't do much, but at least
it removes two "FIXME"s in fbcon.c :)

Peilin Ye (5):
[1/5] fbdev/atafb: Remove unused extern variables

  Searching for `fontdata` gave me this in fbdev/atafb.c:

  extern unsigned char fontdata_8x8[];
  extern unsigned char fontdata_8x16[];

  ...which freaked me out, since in 6735b4632def ("Fonts: Support
  FONT_EXTRA_WORDS macros for built-in fonts") I changed them from char
  arrays to structures, in lib/fonts/. Fortunately it turns out these
  extern variables have nothing to do with lib/fonts/, and are not being
  used anywhere, so remove them for less confusion.

  m68k cross-compiled.

[2/5] Fonts: Make font size unsigned in font_desc

  Our goal is to use `font_desc` "everywhere" in the kernel, and signed
  `width` and `height` is inappropriate.

  Also, change some printk() format identifiers in console/sticore.c from
  `%d` to `%u`. parisc cross-compiled.

[3/5] Fonts: Add charcount field to font_desc

  Add `unsigned int charcount` to `font_desc`, and update each of our 13
  built-in fonts.

[4/5] fbcon: Avoid hard-coding built-in font charcount
[5/5] parisc/sticore: Avoid hard-coding built-in font charcount

  Everyone (tty, fbcon, sticore, etc.) is assuming that all built-in fonts
  have 256 characters, and is using hard-coded 256 or 255 everywhere.
  These two patches removes some of them. [5/5] is parisc cross-compiled.

  Now is a good time to review all find_font() and get_default_font()
  callers:

  drivers/media/pci/solo6x10/solo6x10-enc.c      133 const struct font_desc *vga = find_font("VGA8x16");
  drivers/media/test-drivers/vimc/vimc-core.c    268 const struct font_desc *font = find_font("VGA8x16");
  drivers/media/test-drivers/vivid/vivid-core.c 1928 const struct font_desc *font = find_font("VGA8x16");
  drivers/usb/misc/sisusbvga/sisusb.c           2285 myfont = find_font("VGA8x16");
    * These 4 only care about font VGA8x16, so let them be for now;

  drivers/video/console/sticore.c                499 fbfont = find_font(fbfont_name);
  drivers/video/console/sticore.c                501 fbfont = get_default_font(1024,768, ~(u32)0, ~(u32)0);
    * Uses 255 and 256, (hopefully) cleaned up by [5/5];

  drivers/video/fbdev/core/fbcon.c               999 if (!fontname[0] || !(font = find_font(fontname)))
  drivers/video/fbdev/core/fbcon.c              1000 font = get_default_font(info->var.xres,
  drivers/video/fbdev/core/fbcon.c              1078 if (!fontname[0] || !(font = find_font(fontname)))
  drivers/video/fbdev/core/fbcon.c              1079 font = get_default_font(info->var.xres,
    * Use 256, cleaned up by [4/5];

  drivers/video/fbdev/core/fbcon.c              2548 else if (!(f = find_font(name)))
  drivers/video/fbdev/core/fbcon.c              2546 f = get_default_font(info->var.xres, info->var.yres,
    * Uses 256 but no easy fix. I'll clean this up after making
      fbcon_do_set_font() pass a `font_desc` as parameter;

  drivers/firmware/efi/earlycon.c               234 font = get_default_font(xres, yres, -1, -1);
    * Does not care about charcount.

Thank you!
Peilin Ye

 drivers/video/console/sticore.c  | 10 +++++-----
 drivers/video/fbdev/atafb.c      |  8 --------
 drivers/video/fbdev/core/fbcon.c |  5 ++---
 include/linux/font.h             |  3 ++-
 lib/fonts/font_10x18.c           |  1 +
 lib/fonts/font_6x10.c            |  1 +
 lib/fonts/font_6x11.c            |  1 +
 lib/fonts/font_6x8.c             |  1 +
 lib/fonts/font_7x14.c            |  1 +
 lib/fonts/font_8x16.c            |  1 +
 lib/fonts/font_8x8.c             |  1 +
 lib/fonts/font_acorn_8x8.c       |  1 +
 lib/fonts/font_mini_4x6.c        |  1 +
 lib/fonts/font_pearl_8x8.c       |  1 +
 lib/fonts/font_sun12x22.c        |  1 +
 lib/fonts/font_sun8x16.c         |  1 +
 lib/fonts/font_ter16x32.c        |  1 +
 17 files changed, 22 insertions(+), 17 deletions(-)

-- 
2.25.1

[PATCH 1/5] fbdev/atafb: Remove unused extern variables

[Peilin Ye]

Remove 6 unused extern variables to reduce confusion. It is worth
mentioning that lib/fonts/font_8x8.c and lib/fonts/font_8x16.c also
declare `fontdata_8x8` and `fontdata_8x16` respectively, and this file
has nothing to do with them.

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
$ # Build-tested (Ubuntu 20.04)
$ sudo apt install gcc-m68k-linux-gnu
$ cp arch/m68k/configs/atari_defconfig .config
$ make ARCH=m68k menuconfig
$ make ARCH=m68k CROSS_COMPILE=m68k-linux-gnu- -j`nproc` all

 drivers/video/fbdev/atafb.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index f253daa05d9d..e3812a8ff55a 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -240,14 +240,6 @@ static int *MV300_reg = MV300_reg_8bit;
 
 static int inverse;
 
-extern int fontheight_8x8;
-extern int fontwidth_8x8;
-extern unsigned char fontdata_8x8[];
-
-extern int fontheight_8x16;
-extern int fontwidth_8x16;
-extern unsigned char fontdata_8x16[];
-
 /*
  * struct fb_ops {
  *	* open/release and usage marking
-- 
2.25.1

[PATCH 2/5] Fonts: Make font size unsigned in font_desc

[Peilin Ye]

It is improper to define `width` and `height` as signed in `struct
font_desc`. Make them unsigned. Also, change the corresponding printk()
format identifiers from `%d` to `%u`, in sti_select_fbfont().

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
Build-tested.

 drivers/video/console/sticore.c | 2 +-
 include/linux/font.h            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
index 6a26a364f9bd..d1bb5915082b 100644
--- a/drivers/video/console/sticore.c
+++ b/drivers/video/console/sticore.c
@@ -502,7 +502,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
 	if (!fbfont)
 		return NULL;
 
-	pr_info("STI selected %dx%d framebuffer font %s for sticon\n",
+	pr_info("STI selected %ux%u framebuffer font %s for sticon\n",
 			fbfont->width, fbfont->height, fbfont->name);
 			
 	bpc = ((fbfont->width+7)/8) * fbfont->height; 
diff --git a/include/linux/font.h b/include/linux/font.h
index b5b312c19e46..4f50d736ea72 100644
--- a/include/linux/font.h
+++ b/include/linux/font.h
@@ -16,7 +16,7 @@
 struct font_desc {
     int idx;
     const char *name;
-    int width, height;
+    unsigned int width, height;
     const void *data;
     int pref;
 };
-- 
2.25.1

[PATCH 3/5] Fonts: Add charcount field to font_desc

[Peilin Ye]

Subsystems are assuming the number of characters of our built-in fonts.
Include that information in our kernel font descriptor, `struct
font_desc`.

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
 include/linux/font.h       | 1 +
 lib/fonts/font_10x18.c     | 1 +
 lib/fonts/font_6x10.c      | 1 +
 lib/fonts/font_6x11.c      | 1 +
 lib/fonts/font_6x8.c       | 1 +
 lib/fonts/font_7x14.c      | 1 +
 lib/fonts/font_8x16.c      | 1 +
 lib/fonts/font_8x8.c       | 1 +
 lib/fonts/font_acorn_8x8.c | 1 +
 lib/fonts/font_mini_4x6.c  | 1 +
 lib/fonts/font_pearl_8x8.c | 1 +
 lib/fonts/font_sun12x22.c  | 1 +
 lib/fonts/font_sun8x16.c   | 1 +
 lib/fonts/font_ter16x32.c  | 1 +
 14 files changed, 14 insertions(+)

diff --git a/include/linux/font.h b/include/linux/font.h
index 4f50d736ea72..abf1442ce719 100644
--- a/include/linux/font.h
+++ b/include/linux/font.h
@@ -17,6 +17,7 @@ struct font_desc {
     int idx;
     const char *name;
     unsigned int width, height;
+    unsigned int charcount;
     const void *data;
     int pref;
 };
diff --git a/lib/fonts/font_10x18.c b/lib/fonts/font_10x18.c
index 0e2deac97da0..4096c6562494 100644
--- a/lib/fonts/font_10x18.c
+++ b/lib/fonts/font_10x18.c
@@ -5137,6 +5137,7 @@ const struct font_desc font_10x18 = {
 	.name	= "10x18",
 	.width	= 10,
 	.height	= 18,
+	.charcount = 256,
 	.data	= fontdata_10x18.data,
 #ifdef __sparc__
 	.pref	= 5,
diff --git a/lib/fonts/font_6x10.c b/lib/fonts/font_6x10.c
index 87da8acd07db..32786674cf65 100644
--- a/lib/fonts/font_6x10.c
+++ b/lib/fonts/font_6x10.c
@@ -3083,6 +3083,7 @@ const struct font_desc font_6x10 = {
 	.name	= "6x10",
 	.width	= 6,
 	.height	= 10,
+	.charcount = 256,
 	.data	= fontdata_6x10.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_6x11.c b/lib/fonts/font_6x11.c
index 5e975dfa10a5..81e4a3aed44a 100644
--- a/lib/fonts/font_6x11.c
+++ b/lib/fonts/font_6x11.c
@@ -3346,6 +3346,7 @@ const struct font_desc font_vga_6x11 = {
 	.name	= "ProFont6x11",
 	.width	= 6,
 	.height	= 11,
+	.charcount = 256,
 	.data	= fontdata_6x11.data,
 	/* Try avoiding this font if possible unless on MAC */
 	.pref	= -2000,
diff --git a/lib/fonts/font_6x8.c b/lib/fonts/font_6x8.c
index 700039a9ceae..5618ae7ef9fa 100644
--- a/lib/fonts/font_6x8.c
+++ b/lib/fonts/font_6x8.c
@@ -2571,6 +2571,7 @@ const struct font_desc font_6x8 = {
 	.name	= "6x8",
 	.width	= 6,
 	.height	= 8,
+	.charcount = 256,
 	.data	= fontdata_6x8.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_7x14.c b/lib/fonts/font_7x14.c
index 86d298f38505..7708e73d491f 100644
--- a/lib/fonts/font_7x14.c
+++ b/lib/fonts/font_7x14.c
@@ -4113,6 +4113,7 @@ const struct font_desc font_7x14 = {
 	.name	= "7x14",
 	.width	= 7,
 	.height	= 14,
+	.charcount = 256,
 	.data	= fontdata_7x14.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_8x16.c b/lib/fonts/font_8x16.c
index 37cedd36ca5e..74125d3570cf 100644
--- a/lib/fonts/font_8x16.c
+++ b/lib/fonts/font_8x16.c
@@ -4627,6 +4627,7 @@ const struct font_desc font_vga_8x16 = {
 	.name	= "VGA8x16",
 	.width	= 8,
 	.height	= 16,
+	.charcount = 256,
 	.data	= fontdata_8x16.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_8x8.c b/lib/fonts/font_8x8.c
index 8ab695538395..96da4bb31ae4 100644
--- a/lib/fonts/font_8x8.c
+++ b/lib/fonts/font_8x8.c
@@ -2578,6 +2578,7 @@ const struct font_desc font_vga_8x8 = {
 	.name	= "VGA8x8",
 	.width	= 8,
 	.height	= 8,
+	.charcount = 256,
 	.data	= fontdata_8x8.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
index 069b3e80c434..ba74053fec7b 100644
--- a/lib/fonts/font_acorn_8x8.c
+++ b/lib/fonts/font_acorn_8x8.c
@@ -270,6 +270,7 @@ const struct font_desc font_acorn_8x8 = {
 	.name	= "Acorn8x8",
 	.width	= 8,
 	.height	= 8,
+	.charcount = 256,
 	.data	= acorndata_8x8.data,
 #ifdef CONFIG_ARCH_ACORN
 	.pref	= 20,
diff --git a/lib/fonts/font_mini_4x6.c b/lib/fonts/font_mini_4x6.c
index 1449876c6a27..637708e8c67e 100644
--- a/lib/fonts/font_mini_4x6.c
+++ b/lib/fonts/font_mini_4x6.c
@@ -2152,6 +2152,7 @@ const struct font_desc font_mini_4x6 = {
 	.name	= "MINI4x6",
 	.width	= 4,
 	.height	= 6,
+	.charcount = 256,
 	.data	= fontdata_mini_4x6.data,
 	.pref	= 3,
 };
diff --git a/lib/fonts/font_pearl_8x8.c b/lib/fonts/font_pearl_8x8.c
index 32d65551e7ed..06cde43c7bd2 100644
--- a/lib/fonts/font_pearl_8x8.c
+++ b/lib/fonts/font_pearl_8x8.c
@@ -2582,6 +2582,7 @@ const struct font_desc font_pearl_8x8 = {
 	.name	= "PEARL8x8",
 	.width	= 8,
 	.height	= 8,
+	.charcount = 256,
 	.data	= fontdata_pearl8x8.data,
 	.pref	= 2,
 };
diff --git a/lib/fonts/font_sun12x22.c b/lib/fonts/font_sun12x22.c
index 641a6b4dca42..d0290d79df2c 100644
--- a/lib/fonts/font_sun12x22.c
+++ b/lib/fonts/font_sun12x22.c
@@ -6156,6 +6156,7 @@ const struct font_desc font_sun_12x22 = {
 	.name	= "SUN12x22",
 	.width	= 12,
 	.height	= 22,
+	.charcount = 256,
 	.data	= fontdata_sun12x22.data,
 #ifdef __sparc__
 	.pref	= 5,
diff --git a/lib/fonts/font_sun8x16.c b/lib/fonts/font_sun8x16.c
index 193fe6d988e0..7d83760354a6 100644
--- a/lib/fonts/font_sun8x16.c
+++ b/lib/fonts/font_sun8x16.c
@@ -268,6 +268,7 @@ const struct font_desc font_sun_8x16 = {
 	.name	= "SUN8x16",
 	.width	= 8,
 	.height	= 16,
+	.charcount = 256,
 	.data	= fontdata_sun8x16.data,
 #ifdef __sparc__
 	.pref	= 10,
diff --git a/lib/fonts/font_ter16x32.c b/lib/fonts/font_ter16x32.c
index 91b9c283bd9c..a2cc25b9f8e3 100644
--- a/lib/fonts/font_ter16x32.c
+++ b/lib/fonts/font_ter16x32.c
@@ -2062,6 +2062,7 @@ const struct font_desc font_ter_16x32 = {
 	.name	= "TER16x32",
 	.width	= 16,
 	.height = 32,
+	.charcount = 256,
 	.data	= fontdata_ter16x32.data,
 #ifdef __sparc__
 	.pref	= 5,
-- 
2.25.1

[PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount

[Peilin Ye]

fbcon_startup() and fbcon_init() are hard-coding the number of characters
of our built-in fonts as 256. Recently, we included that information in
our kernel font descriptor `struct font_desc`, so use `font->charcount`
instead of a hard-coded value.

This patch depends on patch "Fonts: Add charcount field to font_desc".

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
 drivers/video/fbdev/core/fbcon.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index cef437817b0d..e563847991b7 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1004,7 +1004,7 @@ static const char *fbcon_startup(void)
 		vc->vc_font.width = font->width;
 		vc->vc_font.height = font->height;
 		vc->vc_font.data = (void *)(p->fontdata = font->data);
-		vc->vc_font.charcount = 256; /* FIXME  Need to support more fonts */
+		vc->vc_font.charcount = font->charcount;
 	} else {
 		p->fontdata = vc->vc_font.data;
 	}
@@ -1083,8 +1083,7 @@ static void fbcon_init(struct vc_data *vc, int init)
 			vc->vc_font.width = font->width;
 			vc->vc_font.height = font->height;
 			vc->vc_font.data = (void *)(p->fontdata = font->data);
-			vc->vc_font.charcount = 256; /* FIXME  Need to
-							support more fonts */
+			vc->vc_font.charcount = font->charcount;
 		}
 	}
 
-- 
2.25.1

[PATCH 5/5] parisc/sticore: Avoid hard-coding built-in font charcount

[Peilin Ye]

sti_select_fbfont() and sti_cook_fonts() are hard-coding the number of
characters of our built-in fonts as 256. Recently, we included that
information in our kernel font descriptor `struct font_desc`, so use
`fbfont->charcount` instead of hard-coded values.

This patch depends on patch "Fonts: Add charcount field to font_desc".

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
$ # Build-tested (Ubuntu 20.04)
$ sudo apt-get install binutils-hppa64-linux-gnu gcc-7-hppa64-linux-gnu
$ cp arch/parisc/configs/generic-64bit_defconfig .config
$ make -j`nproc` ARCH=parisc CROSS_COMPILE=hppa64-linux-gnu- all

 drivers/video/console/sticore.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
index d1bb5915082b..f869b723494f 100644
--- a/drivers/video/console/sticore.c
+++ b/drivers/video/console/sticore.c
@@ -506,7 +506,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
 			fbfont->width, fbfont->height, fbfont->name);
 			
 	bpc = ((fbfont->width+7)/8) * fbfont->height; 
-	size = bpc * 256;
+	size = bpc * fbfont->charcount;
 	size += sizeof(struct sti_rom_font);
 
 	nf = kzalloc(size, STI_LOWMEM);
@@ -514,7 +514,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
 		return NULL;
 
 	nf->first_char = 0;
-	nf->last_char = 255;
+	nf->last_char = fbfont->charcount - 1;
 	nf->width = fbfont->width;
 	nf->height = fbfont->height;
 	nf->font_type = STI_FONT_HPROMAN8;
@@ -525,7 +525,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
 
 	dest = nf;
 	dest += sizeof(struct sti_rom_font);
-	memcpy(dest, fbfont->data, bpc*256);
+	memcpy(dest, fbfont->data, bpc * fbfont->charcount);
 
 	cooked_font = kzalloc(sizeof(*cooked_font), GFP_KERNEL);
 	if (!cooked_font) {
@@ -660,7 +660,7 @@ static int sti_cook_fonts(struct sti_cooked_rom *cooked_rom,
 void sti_font_convert_bytemode(struct sti_struct *sti, struct sti_cooked_font *f)
 {
 	unsigned char *n, *p, *q;
-	int size = f->raw->bytes_per_char * 256 + sizeof(struct sti_rom_font);
+	int size = f->raw->bytes_per_char * (f->raw->last_char + 1) + sizeof(struct sti_rom_font);
 	struct sti_rom_font *old_font;
 
 	if (sti->wordmode)
-- 
2.25.1

Following up

[Peilin Ye]

Hi Daniel,

More about the 3 things we've discussed before:

  1. Cleaning up con_font_op():

(drivers/tty/vt/vt.c)
int con_font_op(struct vc_data *vc, struct console_font_op *op)
{
	switch (op->op) {
	case KD_FONT_OP_SET:
		return con_font_set(vc, op);
	case KD_FONT_OP_GET:
		return con_font_get(vc, op);
	case KD_FONT_OP_SET_DEFAULT:
		return con_font_default(vc, op);
	case KD_FONT_OP_COPY:
		return con_font_copy(vc, op);
	}
	return -ENOSYS;
}

On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote:
> I think if we change the conf_font_get/set/default/copy functions to not
> take the *op struct (which is take pretty arbitrarily from one of the
> ioctl), but the parameters each needs directly, that would clean up the
> code a _lot_.

    This is on my TODO list! One day I came up with some idea about
    fbcon.c, so I postponed this a bit...

  2. Removing dummy functions, like sisusbdummycon_font_set():
    
    Turns out, before c396a5bf457f ("console: Expand dummy functions for
    CFI"), they were just some macros:

-#define SISUSBCONDUMMY (void *)sisusbdummycon_dummy
+static int sisusbdummycon_font_set(struct vc_data *vc,
+                                  struct console_font *font,
+                                  unsigned int flags)
+{
+       return 0;
+}

    ...and they had been there for a very long (10+ years) time. Removing
    code like this makes me a bit nervous, and...

On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote:
> This actually does something. tbh I would not be surprises if the
> fb_set utility is the only thing that uses this - with a bit of code
> search we could perhaps confirm this, and delete all the other
> implementations.

    ...you mentioned code search, where & what should we look at, in order
    to confirm it's safe to remove them?

  3. Using `font_desc` in `vc_data`:

    Our plan for the gradual conversion was to use a helper function to
    set font for a vc, but after reviewing the 300-ish occurrence of
    `vc_font`, it seems like code doesn't usually set it as a whole:

(drivers/usb/misc/sisusbvga/sisusb_con.c)
	[...]
	c->vc_font.height = sisusb->current_font_height;
	[...]

    ...that's it! It only cares about the height. There are only 4 or 5
    places in fbcon.c that actually set all fields of `vc_font`, like:

    		vc->vc_font.width = font->width;
		vc->vc_font.height = font->height;
		vc->vc_font.data = (void *)(p->fontdata = font->data);
		vc->vc_font.charcount = 256; /* FIXME  Need to support more fonts */

    To make it even more complicated, `p` is a `struct fbcon_display *`,
    containing yet another font data pointer (`fontdata`) that I think
    should be replaced by a `font_desc *`...

    In conclusion, I think it's all about a few hard problems in fbcon.c.
    I'll keep trying and see how it goes.
    
Thank you,
Peilin Ye

Re: Following up

[Daniel Vetter]

On Tue, Oct 27, 2020 at 5:50 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
>
> Hi Daniel,
>
> More about the 3 things we've discussed before:
>
>   1. Cleaning up con_font_op():
>
> (drivers/tty/vt/vt.c)
> int con_font_op(struct vc_data *vc, struct console_font_op *op)
> {
>         switch (op->op) {
>         case KD_FONT_OP_SET:
>                 return con_font_set(vc, op);
>         case KD_FONT_OP_GET:
>                 return con_font_get(vc, op);
>         case KD_FONT_OP_SET_DEFAULT:
>                 return con_font_default(vc, op);
>         case KD_FONT_OP_COPY:
>                 return con_font_copy(vc, op);
>         }
>         return -ENOSYS;
> }
>
> On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote:
> > I think if we change the conf_font_get/set/default/copy functions to not
> > take the *op struct (which is take pretty arbitrarily from one of the
> > ioctl), but the parameters each needs directly, that would clean up the
> > code a _lot_.
>
>     This is on my TODO list! One day I came up with some idea about
>     fbcon.c, so I postponed this a bit...
>
>   2. Removing dummy functions, like sisusbdummycon_font_set():
>
>     Turns out, before c396a5bf457f ("console: Expand dummy functions for
>     CFI"), they were just some macros:
>
> -#define SISUSBCONDUMMY (void *)sisusbdummycon_dummy
> +static int sisusbdummycon_font_set(struct vc_data *vc,
> +                                  struct console_font *font,
> +                                  unsigned int flags)
> +{
> +       return 0;
> +}
>
>     ...and they had been there for a very long (10+ years) time. Removing
>     code like this makes me a bit nervous, and...
>
> On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote:
> > This actually does something. tbh I would not be surprises if the
> > fb_set utility is the only thing that uses this - with a bit of code
> > search we could perhaps confirm this, and delete all the other
> > implementations.
>
>     ...you mentioned code search, where & what should we look at, in order
>     to confirm it's safe to remove them?

Way back there was google's code search, which was awesome. Now I just
put the structure name/ioctl #define/number into
google/bing/duckduckgo and see if anything turns up. Plus check how
it's used in fb tools (although I just recently learned that fb-test
pretty much disappeared from the internet, very hard to find the
original).

If you're unsure, we can merge a patch, then wait about 1 year for any
users to show up with problems. If that's not the case, assume they're
all gone, or it was never used and just implemented because it was
copied from somewhere else, or "just in case". There's lots of dead
uapi around.

>   3. Using `font_desc` in `vc_data`:
>
>     Our plan for the gradual conversion was to use a helper function to
>     set font for a vc, but after reviewing the 300-ish occurrence of
>     `vc_font`, it seems like code doesn't usually set it as a whole:
>
> (drivers/usb/misc/sisusbvga/sisusb_con.c)
>         [...]
>         c->vc_font.height = sisusb->current_font_height;
>         [...]
>
>     ...that's it! It only cares about the height. There are only 4 or 5
>     places in fbcon.c that actually set all fields of `vc_font`, like:
>
>                 vc->vc_font.width = font->width;
>                 vc->vc_font.height = font->height;
>                 vc->vc_font.data = (void *)(p->fontdata = font->data);
>                 vc->vc_font.charcount = 256; /* FIXME  Need to support more fonts */
>
>     To make it even more complicated, `p` is a `struct fbcon_display *`,
>     containing yet another font data pointer (`fontdata`) that I think
>     should be replaced by a `font_desc *`...
>
>     In conclusion, I think it's all about a few hard problems in fbcon.c.
>     I'll keep trying and see how it goes.

Yeah fbcon.c is pretty good horrors unfortunately :-/
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/5] fbdev/atafb: Remove unused extern variables

[Daniel Vetter]

On Tue, Oct 27, 2020 at 12:31:08PM -0400, Peilin Ye wrote:
> Remove 6 unused extern variables to reduce confusion. It is worth
> mentioning that lib/fonts/font_8x8.c and lib/fonts/font_8x16.c also
> declare `fontdata_8x8` and `fontdata_8x16` respectively, and this file
> has nothing to do with them.
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

This was unused ever since this driver was merged into 2.1.67 (I looked at
historical linux git trees quickly). Save to delete I'd say, probably just
copypasted from some outdated driver template that was even older.

Applied to drm-misc-next.
-Daniel
> ---
> $ # Build-tested (Ubuntu 20.04)
> $ sudo apt install gcc-m68k-linux-gnu
> $ cp arch/m68k/configs/atari_defconfig .config
> $ make ARCH=m68k menuconfig
> $ make ARCH=m68k CROSS_COMPILE=m68k-linux-gnu- -j`nproc` all
> 
>  drivers/video/fbdev/atafb.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
> index f253daa05d9d..e3812a8ff55a 100644
> --- a/drivers/video/fbdev/atafb.c
> +++ b/drivers/video/fbdev/atafb.c
> @@ -240,14 +240,6 @@ static int *MV300_reg = MV300_reg_8bit;
>  
>  static int inverse;
>  
> -extern int fontheight_8x8;
> -extern int fontwidth_8x8;
> -extern unsigned char fontdata_8x8[];
> -
> -extern int fontheight_8x16;
> -extern int fontwidth_8x16;
> -extern unsigned char fontdata_8x16[];
> -
>  /*
>   * struct fb_ops {
>   *	* open/release and usage marking
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc

[Daniel Vetter]

On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote:
> It is improper to define `width` and `height` as signed in `struct
> font_desc`. Make them unsigned. Also, change the corresponding printk()
> format identifiers from `%d` to `%u`, in sti_select_fbfont().
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

I'm not entirely sure of the motivation here ... height/width should never
ever be even close to the limit here. Or have you seen integer math that
could potentially go wrong if we go with unsigned instead of int?
-Daniel

> ---
> Build-tested.
> 
>  drivers/video/console/sticore.c | 2 +-
>  include/linux/font.h            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
> index 6a26a364f9bd..d1bb5915082b 100644
> --- a/drivers/video/console/sticore.c
> +++ b/drivers/video/console/sticore.c
> @@ -502,7 +502,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
>  	if (!fbfont)
>  		return NULL;
>  
> -	pr_info("STI selected %dx%d framebuffer font %s for sticon\n",
> +	pr_info("STI selected %ux%u framebuffer font %s for sticon\n",
>  			fbfont->width, fbfont->height, fbfont->name);
>  			
>  	bpc = ((fbfont->width+7)/8) * fbfont->height; 
> diff --git a/include/linux/font.h b/include/linux/font.h
> index b5b312c19e46..4f50d736ea72 100644
> --- a/include/linux/font.h
> +++ b/include/linux/font.h
> @@ -16,7 +16,7 @@
>  struct font_desc {
>      int idx;
>      const char *name;
> -    int width, height;
> +    unsigned int width, height;
>      const void *data;
>      int pref;
>  };
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 3/5] Fonts: Add charcount field to font_desc

[Daniel Vetter]

On Tue, Oct 27, 2020 at 12:34:26PM -0400, Peilin Ye wrote:
> Subsystems are assuming the number of characters of our built-in fonts.
> Include that information in our kernel font descriptor, `struct
> font_desc`.
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

atm can't merge this because we need a backmerge of maybe -rc2 into
drm-misc-next first. Please remind me if this doesn't land next week.
-Daniel

> ---
>  include/linux/font.h       | 1 +
>  lib/fonts/font_10x18.c     | 1 +
>  lib/fonts/font_6x10.c      | 1 +
>  lib/fonts/font_6x11.c      | 1 +
>  lib/fonts/font_6x8.c       | 1 +
>  lib/fonts/font_7x14.c      | 1 +
>  lib/fonts/font_8x16.c      | 1 +
>  lib/fonts/font_8x8.c       | 1 +
>  lib/fonts/font_acorn_8x8.c | 1 +
>  lib/fonts/font_mini_4x6.c  | 1 +
>  lib/fonts/font_pearl_8x8.c | 1 +
>  lib/fonts/font_sun12x22.c  | 1 +
>  lib/fonts/font_sun8x16.c   | 1 +
>  lib/fonts/font_ter16x32.c  | 1 +
>  14 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/font.h b/include/linux/font.h
> index 4f50d736ea72..abf1442ce719 100644
> --- a/include/linux/font.h
> +++ b/include/linux/font.h
> @@ -17,6 +17,7 @@ struct font_desc {
>      int idx;
>      const char *name;
>      unsigned int width, height;
> +    unsigned int charcount;
>      const void *data;
>      int pref;
>  };
> diff --git a/lib/fonts/font_10x18.c b/lib/fonts/font_10x18.c
> index 0e2deac97da0..4096c6562494 100644
> --- a/lib/fonts/font_10x18.c
> +++ b/lib/fonts/font_10x18.c
> @@ -5137,6 +5137,7 @@ const struct font_desc font_10x18 = {
>  	.name	= "10x18",
>  	.width	= 10,
>  	.height	= 18,
> +	.charcount = 256,
>  	.data	= fontdata_10x18.data,
>  #ifdef __sparc__
>  	.pref	= 5,
> diff --git a/lib/fonts/font_6x10.c b/lib/fonts/font_6x10.c
> index 87da8acd07db..32786674cf65 100644
> --- a/lib/fonts/font_6x10.c
> +++ b/lib/fonts/font_6x10.c
> @@ -3083,6 +3083,7 @@ const struct font_desc font_6x10 = {
>  	.name	= "6x10",
>  	.width	= 6,
>  	.height	= 10,
> +	.charcount = 256,
>  	.data	= fontdata_6x10.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_6x11.c b/lib/fonts/font_6x11.c
> index 5e975dfa10a5..81e4a3aed44a 100644
> --- a/lib/fonts/font_6x11.c
> +++ b/lib/fonts/font_6x11.c
> @@ -3346,6 +3346,7 @@ const struct font_desc font_vga_6x11 = {
>  	.name	= "ProFont6x11",
>  	.width	= 6,
>  	.height	= 11,
> +	.charcount = 256,
>  	.data	= fontdata_6x11.data,
>  	/* Try avoiding this font if possible unless on MAC */
>  	.pref	= -2000,
> diff --git a/lib/fonts/font_6x8.c b/lib/fonts/font_6x8.c
> index 700039a9ceae..5618ae7ef9fa 100644
> --- a/lib/fonts/font_6x8.c
> +++ b/lib/fonts/font_6x8.c
> @@ -2571,6 +2571,7 @@ const struct font_desc font_6x8 = {
>  	.name	= "6x8",
>  	.width	= 6,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= fontdata_6x8.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_7x14.c b/lib/fonts/font_7x14.c
> index 86d298f38505..7708e73d491f 100644
> --- a/lib/fonts/font_7x14.c
> +++ b/lib/fonts/font_7x14.c
> @@ -4113,6 +4113,7 @@ const struct font_desc font_7x14 = {
>  	.name	= "7x14",
>  	.width	= 7,
>  	.height	= 14,
> +	.charcount = 256,
>  	.data	= fontdata_7x14.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_8x16.c b/lib/fonts/font_8x16.c
> index 37cedd36ca5e..74125d3570cf 100644
> --- a/lib/fonts/font_8x16.c
> +++ b/lib/fonts/font_8x16.c
> @@ -4627,6 +4627,7 @@ const struct font_desc font_vga_8x16 = {
>  	.name	= "VGA8x16",
>  	.width	= 8,
>  	.height	= 16,
> +	.charcount = 256,
>  	.data	= fontdata_8x16.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_8x8.c b/lib/fonts/font_8x8.c
> index 8ab695538395..96da4bb31ae4 100644
> --- a/lib/fonts/font_8x8.c
> +++ b/lib/fonts/font_8x8.c
> @@ -2578,6 +2578,7 @@ const struct font_desc font_vga_8x8 = {
>  	.name	= "VGA8x8",
>  	.width	= 8,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= fontdata_8x8.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
> index 069b3e80c434..ba74053fec7b 100644
> --- a/lib/fonts/font_acorn_8x8.c
> +++ b/lib/fonts/font_acorn_8x8.c
> @@ -270,6 +270,7 @@ const struct font_desc font_acorn_8x8 = {
>  	.name	= "Acorn8x8",
>  	.width	= 8,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= acorndata_8x8.data,
>  #ifdef CONFIG_ARCH_ACORN
>  	.pref	= 20,
> diff --git a/lib/fonts/font_mini_4x6.c b/lib/fonts/font_mini_4x6.c
> index 1449876c6a27..637708e8c67e 100644
> --- a/lib/fonts/font_mini_4x6.c
> +++ b/lib/fonts/font_mini_4x6.c
> @@ -2152,6 +2152,7 @@ const struct font_desc font_mini_4x6 = {
>  	.name	= "MINI4x6",
>  	.width	= 4,
>  	.height	= 6,
> +	.charcount = 256,
>  	.data	= fontdata_mini_4x6.data,
>  	.pref	= 3,
>  };
> diff --git a/lib/fonts/font_pearl_8x8.c b/lib/fonts/font_pearl_8x8.c
> index 32d65551e7ed..06cde43c7bd2 100644
> --- a/lib/fonts/font_pearl_8x8.c
> +++ b/lib/fonts/font_pearl_8x8.c
> @@ -2582,6 +2582,7 @@ const struct font_desc font_pearl_8x8 = {
>  	.name	= "PEARL8x8",
>  	.width	= 8,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= fontdata_pearl8x8.data,
>  	.pref	= 2,
>  };
> diff --git a/lib/fonts/font_sun12x22.c b/lib/fonts/font_sun12x22.c
> index 641a6b4dca42..d0290d79df2c 100644
> --- a/lib/fonts/font_sun12x22.c
> +++ b/lib/fonts/font_sun12x22.c
> @@ -6156,6 +6156,7 @@ const struct font_desc font_sun_12x22 = {
>  	.name	= "SUN12x22",
>  	.width	= 12,
>  	.height	= 22,
> +	.charcount = 256,
>  	.data	= fontdata_sun12x22.data,
>  #ifdef __sparc__
>  	.pref	= 5,
> diff --git a/lib/fonts/font_sun8x16.c b/lib/fonts/font_sun8x16.c
> index 193fe6d988e0..7d83760354a6 100644
> --- a/lib/fonts/font_sun8x16.c
> +++ b/lib/fonts/font_sun8x16.c
> @@ -268,6 +268,7 @@ const struct font_desc font_sun_8x16 = {
>  	.name	= "SUN8x16",
>  	.width	= 8,
>  	.height	= 16,
> +	.charcount = 256,
>  	.data	= fontdata_sun8x16.data,
>  #ifdef __sparc__
>  	.pref	= 10,
> diff --git a/lib/fonts/font_ter16x32.c b/lib/fonts/font_ter16x32.c
> index 91b9c283bd9c..a2cc25b9f8e3 100644
> --- a/lib/fonts/font_ter16x32.c
> +++ b/lib/fonts/font_ter16x32.c
> @@ -2062,6 +2062,7 @@ const struct font_desc font_ter_16x32 = {
>  	.name	= "TER16x32",
>  	.width	= 16,
>  	.height = 32,
> +	.charcount = 256,
>  	.data	= fontdata_ter16x32.data,
>  #ifdef __sparc__
>  	.pref	= 5,
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount

[Daniel Vetter]

On Tue, Oct 27, 2020 at 12:37:29PM -0400, Peilin Ye wrote:
> fbcon_startup() and fbcon_init() are hard-coding the number of characters
> of our built-in fonts as 256. Recently, we included that information in
> our kernel font descriptor `struct font_desc`, so use `font->charcount`
> instead of a hard-coded value.
> 
> This patch depends on patch "Fonts: Add charcount field to font_desc".
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

So I think this is correct, but it also doesn't do a hole lot yet. fbcon.c
still has tons of hard-coded 256 all over, and if (p->userfont).

I think if we instead set vc->vc_font.charcount both in fbcon_init and in
fbcon_do_set_font (probably just replace the userfont parameter with
font_charcount for now), then we could replace these all with
vc->vc_font.charcount. And the code would already improve quite a bit I
think.

With just this change here I think we have even more inconsistency, since
for built-in fonts vc->vc_font.charcount is now set correctly, but for
userfonts we need to instead look at FNTCHARCNT(vc->vc_font.data).

We'd still need to maintain p->userfont because of the refcount chaos, but
that is much more work.

Or do I miss something here?
-Daniel

> ---
>  drivers/video/fbdev/core/fbcon.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index cef437817b0d..e563847991b7 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -1004,7 +1004,7 @@ static const char *fbcon_startup(void)
>  		vc->vc_font.width = font->width;
>  		vc->vc_font.height = font->height;
>  		vc->vc_font.data = (void *)(p->fontdata = font->data);
> -		vc->vc_font.charcount = 256; /* FIXME  Need to support more fonts */
> +		vc->vc_font.charcount = font->charcount;
>  	} else {
>  		p->fontdata = vc->vc_font.data;
>  	}
> @@ -1083,8 +1083,7 @@ static void fbcon_init(struct vc_data *vc, int init)
>  			vc->vc_font.width = font->width;
>  			vc->vc_font.height = font->height;
>  			vc->vc_font.data = (void *)(p->fontdata = font->data);
> -			vc->vc_font.charcount = 256; /* FIXME  Need to
> -							support more fonts */
> +			vc->vc_font.charcount = font->charcount;
>  		}
>  	}
>  
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 5/5] parisc/sticore: Avoid hard-coding built-in font charcount

[Daniel Vetter]

On Tue, Oct 27, 2020 at 12:41:02PM -0400, Peilin Ye wrote:
> sti_select_fbfont() and sti_cook_fonts() are hard-coding the number of
> characters of our built-in fonts as 256. Recently, we included that
> information in our kernel font descriptor `struct font_desc`, so use
> `fbfont->charcount` instead of hard-coded values.
> 
> This patch depends on patch "Fonts: Add charcount field to font_desc".
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
> $ # Build-tested (Ubuntu 20.04)
> $ sudo apt-get install binutils-hppa64-linux-gnu gcc-7-hppa64-linux-gnu
> $ cp arch/parisc/configs/generic-64bit_defconfig .config
> $ make -j`nproc` ARCH=parisc CROSS_COMPILE=hppa64-linux-gnu- all
> 
>  drivers/video/console/sticore.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
> index d1bb5915082b..f869b723494f 100644
> --- a/drivers/video/console/sticore.c
> +++ b/drivers/video/console/sticore.c
> @@ -506,7 +506,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
>  			fbfont->width, fbfont->height, fbfont->name);
>  			
>  	bpc = ((fbfont->width+7)/8) * fbfont->height; 
> -	size = bpc * 256;
> +	size = bpc * fbfont->charcount;
>  	size += sizeof(struct sti_rom_font);
>  
>  	nf = kzalloc(size, STI_LOWMEM);
> @@ -514,7 +514,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
>  		return NULL;
>  
>  	nf->first_char = 0;
> -	nf->last_char = 255;
> +	nf->last_char = fbfont->charcount - 1;
>  	nf->width = fbfont->width;
>  	nf->height = fbfont->height;
>  	nf->font_type = STI_FONT_HPROMAN8;
> @@ -525,7 +525,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
>  
>  	dest = nf;
>  	dest += sizeof(struct sti_rom_font);
> -	memcpy(dest, fbfont->data, bpc*256);
> +	memcpy(dest, fbfont->data, bpc * fbfont->charcount);
>  
>  	cooked_font = kzalloc(sizeof(*cooked_font), GFP_KERNEL);
>  	if (!cooked_font) {
> @@ -660,7 +660,7 @@ static int sti_cook_fonts(struct sti_cooked_rom *cooked_rom,
>  void sti_font_convert_bytemode(struct sti_struct *sti, struct sti_cooked_font *f)
>  {
>  	unsigned char *n, *p, *q;
> -	int size = f->raw->bytes_per_char * 256 + sizeof(struct sti_rom_font);
> +	int size = f->raw->bytes_per_char * (f->raw->last_char + 1) + sizeof(struct sti_rom_font);
>  	struct sti_rom_font *old_font;
>  
>  	if (sti->wordmode)
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount

[Peilin Ye]

On Tue, Oct 27, 2020 at 08:13:53PM +0100, Daniel Vetter wrote:
> On Tue, Oct 27, 2020 at 12:37:29PM -0400, Peilin Ye wrote:
> > fbcon_startup() and fbcon_init() are hard-coding the number of characters
> > of our built-in fonts as 256. Recently, we included that information in
> > our kernel font descriptor `struct font_desc`, so use `font->charcount`
> > instead of a hard-coded value.
> > 
> > This patch depends on patch "Fonts: Add charcount field to font_desc".
> > 
> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> 
> So I think this is correct, but it also doesn't do a hole lot yet. fbcon.c
> still has tons of hard-coded 256 all over, and if (p->userfont).
> 
> I think if we instead set vc->vc_font.charcount both in fbcon_init and in
> fbcon_do_set_font (probably just replace the userfont parameter with
> font_charcount for now), then we could replace these all with
> vc->vc_font.charcount. And the code would already improve quite a bit I
> think.
> 
> With just this change here I think we have even more inconsistency, since
> for built-in fonts vc->vc_font.charcount is now set correctly, but for
> userfonts we need to instead look at FNTCHARCNT(vc->vc_font.data).

You are right, let's remove FNTCHARCNT() altogether. fbcon_do_set_font()
still needs a userfont parameter for refcount handling, I'll just add a
charcount parameter to it.

Peilin

Re: Following up

[Peilin Ye]

On Tue, Oct 27, 2020 at 07:36:54PM +0100, Daniel Vetter wrote:
> On Tue, Oct 27, 2020 at 5:50 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> >     ...you mentioned code search, where & what should we look at, in order
> >     to confirm it's safe to remove them?
> 
> Way back there was google's code search, which was awesome. Now I just
> put the structure name/ioctl #define/number into
> google/bing/duckduckgo and see if anything turns up. Plus check how
> it's used in fb tools (although I just recently learned that fb-test
> pretty much disappeared from the internet, very hard to find the
> original).
> 
> If you're unsure, we can merge a patch, then wait about 1 year for any
> users to show up with problems. If that's not the case, assume they're
> all gone, or it was never used and just implemented because it was
> copied from somewhere else, or "just in case". There's lots of dead
> uapi around.

I see, it will be my next thing to do. Hopefully this will remove a lot
of console_font occurrences.

Peilin

Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc

[Peilin Ye]

On Tue, Oct 27, 2020 at 07:50:58PM +0100, Daniel Vetter wrote:
> On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote:
> > It is improper to define `width` and `height` as signed in `struct
> > font_desc`. Make them unsigned. Also, change the corresponding printk()
> > format identifiers from `%d` to `%u`, in sti_select_fbfont().
> > 
> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> 
> I'm not entirely sure of the motivation here ... height/width should never
> ever be even close to the limit here. Or have you seen integer math that
> could potentially go wrong if we go with unsigned instead of int?

Oh... No, I have not. I just thought we shouldn't represent a length
using a signed value. Also, width and height in console_font are
unsigned int - that shouldn't matter that much though.

[3/5] doesn't hunk properly without this patch, I'll send a v2 for [3/5]
soon.

Peilin

[PATCH 3/5 v2] Fonts: Add charcount field to font_desc

[Peilin Ye]

Subsystems are assuming the number of characters of our built-in fonts.
Include that information in our kernel font descriptor, `struct
font_desc`.

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Change in v2:
  - Rebase onto 5.10-rc1.

 include/linux/font.h       | 1 +
 lib/fonts/font_10x18.c     | 1 +
 lib/fonts/font_6x10.c      | 1 +
 lib/fonts/font_6x11.c      | 1 +
 lib/fonts/font_6x8.c       | 1 +
 lib/fonts/font_7x14.c      | 1 +
 lib/fonts/font_8x16.c      | 1 +
 lib/fonts/font_8x8.c       | 1 +
 lib/fonts/font_acorn_8x8.c | 1 +
 lib/fonts/font_mini_4x6.c  | 1 +
 lib/fonts/font_pearl_8x8.c | 1 +
 lib/fonts/font_sun12x22.c  | 1 +
 lib/fonts/font_sun8x16.c   | 1 +
 lib/fonts/font_ter16x32.c  | 1 +
 14 files changed, 14 insertions(+)

diff --git a/include/linux/font.h b/include/linux/font.h
index b5b312c19e46..54e60ad2252b 100644
--- a/include/linux/font.h
+++ b/include/linux/font.h
@@ -17,6 +17,7 @@ struct font_desc {
     int idx;
     const char *name;
     int width, height;
+    unsigned int charcount;
     const void *data;
     int pref;
 };
diff --git a/lib/fonts/font_10x18.c b/lib/fonts/font_10x18.c
index 0e2deac97da0..4096c6562494 100644
--- a/lib/fonts/font_10x18.c
+++ b/lib/fonts/font_10x18.c
@@ -5137,6 +5137,7 @@ const struct font_desc font_10x18 = {
 	.name	= "10x18",
 	.width	= 10,
 	.height	= 18,
+	.charcount = 256,
 	.data	= fontdata_10x18.data,
 #ifdef __sparc__
 	.pref	= 5,
diff --git a/lib/fonts/font_6x10.c b/lib/fonts/font_6x10.c
index 87da8acd07db..32786674cf65 100644
--- a/lib/fonts/font_6x10.c
+++ b/lib/fonts/font_6x10.c
@@ -3083,6 +3083,7 @@ const struct font_desc font_6x10 = {
 	.name	= "6x10",
 	.width	= 6,
 	.height	= 10,
+	.charcount = 256,
 	.data	= fontdata_6x10.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_6x11.c b/lib/fonts/font_6x11.c
index 5e975dfa10a5..81e4a3aed44a 100644
--- a/lib/fonts/font_6x11.c
+++ b/lib/fonts/font_6x11.c
@@ -3346,6 +3346,7 @@ const struct font_desc font_vga_6x11 = {
 	.name	= "ProFont6x11",
 	.width	= 6,
 	.height	= 11,
+	.charcount = 256,
 	.data	= fontdata_6x11.data,
 	/* Try avoiding this font if possible unless on MAC */
 	.pref	= -2000,
diff --git a/lib/fonts/font_6x8.c b/lib/fonts/font_6x8.c
index 700039a9ceae..5618ae7ef9fa 100644
--- a/lib/fonts/font_6x8.c
+++ b/lib/fonts/font_6x8.c
@@ -2571,6 +2571,7 @@ const struct font_desc font_6x8 = {
 	.name	= "6x8",
 	.width	= 6,
 	.height	= 8,
+	.charcount = 256,
 	.data	= fontdata_6x8.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_7x14.c b/lib/fonts/font_7x14.c
index 86d298f38505..7708e73d491f 100644
--- a/lib/fonts/font_7x14.c
+++ b/lib/fonts/font_7x14.c
@@ -4113,6 +4113,7 @@ const struct font_desc font_7x14 = {
 	.name	= "7x14",
 	.width	= 7,
 	.height	= 14,
+	.charcount = 256,
 	.data	= fontdata_7x14.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_8x16.c b/lib/fonts/font_8x16.c
index 37cedd36ca5e..74125d3570cf 100644
--- a/lib/fonts/font_8x16.c
+++ b/lib/fonts/font_8x16.c
@@ -4627,6 +4627,7 @@ const struct font_desc font_vga_8x16 = {
 	.name	= "VGA8x16",
 	.width	= 8,
 	.height	= 16,
+	.charcount = 256,
 	.data	= fontdata_8x16.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_8x8.c b/lib/fonts/font_8x8.c
index 8ab695538395..96da4bb31ae4 100644
--- a/lib/fonts/font_8x8.c
+++ b/lib/fonts/font_8x8.c
@@ -2578,6 +2578,7 @@ const struct font_desc font_vga_8x8 = {
 	.name	= "VGA8x8",
 	.width	= 8,
 	.height	= 8,
+	.charcount = 256,
 	.data	= fontdata_8x8.data,
 	.pref	= 0,
 };
diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
index 069b3e80c434..ba74053fec7b 100644
--- a/lib/fonts/font_acorn_8x8.c
+++ b/lib/fonts/font_acorn_8x8.c
@@ -270,6 +270,7 @@ const struct font_desc font_acorn_8x8 = {
 	.name	= "Acorn8x8",
 	.width	= 8,
 	.height	= 8,
+	.charcount = 256,
 	.data	= acorndata_8x8.data,
 #ifdef CONFIG_ARCH_ACORN
 	.pref	= 20,
diff --git a/lib/fonts/font_mini_4x6.c b/lib/fonts/font_mini_4x6.c
index 1449876c6a27..637708e8c67e 100644
--- a/lib/fonts/font_mini_4x6.c
+++ b/lib/fonts/font_mini_4x6.c
@@ -2152,6 +2152,7 @@ const struct font_desc font_mini_4x6 = {
 	.name	= "MINI4x6",
 	.width	= 4,
 	.height	= 6,
+	.charcount = 256,
 	.data	= fontdata_mini_4x6.data,
 	.pref	= 3,
 };
diff --git a/lib/fonts/font_pearl_8x8.c b/lib/fonts/font_pearl_8x8.c
index 32d65551e7ed..06cde43c7bd2 100644
--- a/lib/fonts/font_pearl_8x8.c
+++ b/lib/fonts/font_pearl_8x8.c
@@ -2582,6 +2582,7 @@ const struct font_desc font_pearl_8x8 = {
 	.name	= "PEARL8x8",
 	.width	= 8,
 	.height	= 8,
+	.charcount = 256,
 	.data	= fontdata_pearl8x8.data,
 	.pref	= 2,
 };
diff --git a/lib/fonts/font_sun12x22.c b/lib/fonts/font_sun12x22.c
index 641a6b4dca42..d0290d79df2c 100644
--- a/lib/fonts/font_sun12x22.c
+++ b/lib/fonts/font_sun12x22.c
@@ -6156,6 +6156,7 @@ const struct font_desc font_sun_12x22 = {
 	.name	= "SUN12x22",
 	.width	= 12,
 	.height	= 22,
+	.charcount = 256,
 	.data	= fontdata_sun12x22.data,
 #ifdef __sparc__
 	.pref	= 5,
diff --git a/lib/fonts/font_sun8x16.c b/lib/fonts/font_sun8x16.c
index 193fe6d988e0..7d83760354a6 100644
--- a/lib/fonts/font_sun8x16.c
+++ b/lib/fonts/font_sun8x16.c
@@ -268,6 +268,7 @@ const struct font_desc font_sun_8x16 = {
 	.name	= "SUN8x16",
 	.width	= 8,
 	.height	= 16,
+	.charcount = 256,
 	.data	= fontdata_sun8x16.data,
 #ifdef __sparc__
 	.pref	= 10,
diff --git a/lib/fonts/font_ter16x32.c b/lib/fonts/font_ter16x32.c
index 91b9c283bd9c..a2cc25b9f8e3 100644
--- a/lib/fonts/font_ter16x32.c
+++ b/lib/fonts/font_ter16x32.c
@@ -2062,6 +2062,7 @@ const struct font_desc font_ter_16x32 = {
 	.name	= "TER16x32",
 	.width	= 16,
 	.height = 32,
+	.charcount = 256,
 	.data	= fontdata_ter16x32.data,
 #ifdef __sparc__
 	.pref	= 5,
-- 
2.25.1

Re: [PATCH 3/5] Fonts: Add charcount field to font_desc

[Peilin Ye]

On Tue, Oct 27, 2020 at 07:59:35PM +0100, Daniel Vetter wrote:
> On Tue, Oct 27, 2020 at 12:34:26PM -0400, Peilin Ye wrote:
> > Subsystems are assuming the number of characters of our built-in fonts.
> > Include that information in our kernel font descriptor, `struct
> > font_desc`.
> > 
> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> atm can't merge this because we need a backmerge of maybe -rc2 into
> drm-misc-next first. Please remind me if this doesn't land next week.

Sure, thanks for reviewing these!

Peilin

Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc

[Daniel Vetter]

On Wed, Oct 28, 2020 at 01:43:07AM -0400, Peilin Ye wrote:
> On Tue, Oct 27, 2020 at 07:50:58PM +0100, Daniel Vetter wrote:
> > On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote:
> > > It is improper to define `width` and `height` as signed in `struct
> > > font_desc`. Make them unsigned. Also, change the corresponding printk()
> > > format identifiers from `%d` to `%u`, in sti_select_fbfont().
> > > 
> > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> > 
> > I'm not entirely sure of the motivation here ... height/width should never
> > ever be even close to the limit here. Or have you seen integer math that
> > could potentially go wrong if we go with unsigned instead of int?
> 
> Oh... No, I have not. I just thought we shouldn't represent a length
> using a signed value. Also, width and height in console_font are
> unsigned int - that shouldn't matter that much though.

Oh this is actually a good reason, since that's the uapi structure. And so
using the exact same signedness should help a bit with accidental casting
bugs.

If you mention this in the commit message I think this is good to go.
-Daniel

> 
> [3/5] doesn't hunk properly without this patch, I'll send a v2 for [3/5]
> soon.
> 
> Peilin
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/5] fbdev/atafb: Remove unused extern variables

[Geert Uytterhoeven]

On Tue, Oct 27, 2020 at 6:12 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> Remove 6 unused extern variables to reduce confusion. It is worth
> mentioning that lib/fonts/font_8x8.c and lib/fonts/font_8x16.c also
> declare `fontdata_8x8` and `fontdata_8x16` respectively, and this file
> has nothing to do with them.
>
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc

[Peilin Ye]

On Wed, Oct 28, 2020 at 09:18:44AM +0100, Daniel Vetter wrote:
> On Wed, Oct 28, 2020 at 01:43:07AM -0400, Peilin Ye wrote:
> > On Tue, Oct 27, 2020 at 07:50:58PM +0100, Daniel Vetter wrote:
> > > On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote:
> > > > It is improper to define `width` and `height` as signed in `struct
> > > > font_desc`. Make them unsigned. Also, change the corresponding printk()
> > > > format identifiers from `%d` to `%u`, in sti_select_fbfont().
> > > > 
> > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> > > 
> > > I'm not entirely sure of the motivation here ... height/width should never
> > > ever be even close to the limit here. Or have you seen integer math that
> > > could potentially go wrong if we go with unsigned instead of int?
> > 
> > Oh... No, I have not. I just thought we shouldn't represent a length
> > using a signed value. Also, width and height in console_font are
> > unsigned int - that shouldn't matter that much though.
> 
> Oh this is actually a good reason, since that's the uapi structure. And so
> using the exact same signedness should help a bit with accidental casting
> bugs.
> 
> If you mention this in the commit message I think this is good to go.

Ah, I see, v2 on the way. Please ignore [v2 3/5], that doesn't hunk with
this patch in effect...

One newbie question, should I mention in the commit message, if a patch
depends on another patch in the series in order to hunk properly?

Peilin

[PATCH v2 2/5] Fonts: Make font size unsigned in font_desc

[Peilin Ye]

`width` and `height` are defined as unsigned in our UAPI font descriptor
`struct console_font`. Make them unsigned in our kernel font descriptor
`struct font_desc`, too.

Also, change the corresponding printk() format identifiers from `%d` to
`%u`, in sti_select_fbfont().

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
Change in v2:
  - Mention `struct console_font` in the commit message. (Suggested by
    Daniel Vetter <daniel@ffwll.ch>)

 drivers/video/console/sticore.c | 2 +-
 include/linux/font.h            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
index 6a26a364f9bd..d1bb5915082b 100644
--- a/drivers/video/console/sticore.c
+++ b/drivers/video/console/sticore.c
@@ -502,7 +502,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
 	if (!fbfont)
 		return NULL;
 
-	pr_info("STI selected %dx%d framebuffer font %s for sticon\n",
+	pr_info("STI selected %ux%u framebuffer font %s for sticon\n",
 			fbfont->width, fbfont->height, fbfont->name);
 			
 	bpc = ((fbfont->width+7)/8) * fbfont->height; 
diff --git a/include/linux/font.h b/include/linux/font.h
index b5b312c19e46..4f50d736ea72 100644
--- a/include/linux/font.h
+++ b/include/linux/font.h
@@ -16,7 +16,7 @@
 struct font_desc {
     int idx;
     const char *name;
-    int width, height;
+    unsigned int width, height;
     const void *data;
     int pref;
 };
-- 
2.25.1

[PATCH RFC v2 4/5] fbdev: Avoid using FNTCHARCNT() and hard-coded font charcount

[Peilin Ye]

For user-provided fonts, the framebuffer layer is using a
negative-indexing macro, FNTCHARCNT(), to keep track of their number of
characters. For built-in fonts, it is using hard-coded values (256). This
results in something like the following:

		map.length = (ops->p->userfont) ?
			FNTCHARCNT(ops->p->fontdata) : 256;

Recently we added a `charcount` field to `struct font_desc`. Use it, and
set `vc->vc_font.charcount` properly whenever `vc->vc_font.data` is set,
so that we can use `vc->vc_font.charcount` instead of an if-else between
FNTCHARCNT() and 256.

This patch depends on patch "Fonts: Add charcount field to font_desc".

Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
FNTCHARCNT() now disappeared from drivers/video/fbdev/core/. There is one
more in drivers/video/console/newport_con.c, but I think that file needs
to be handled separately.

Changes in v2:
  - Try avoid using FNTCHARCNT() altogether, instead of only changing a
    little bit (Suggested by Daniel Vetter <daniel.vetter@ffwll.ch>)
  - Change the title, since we are now removing FNTCHARCNT()
  - Set `vc->vc_font.charcount` properly, in fbcon_startup(),
    fbcon_init(), fbcon_set_disp() and fbcon_do_set_font()
  - Replace hard-coded 256 whenever possible

 drivers/video/fbdev/core/fbcon.c        | 58 ++++++++++---------------
 drivers/video/fbdev/core/fbcon_rotate.c |  2 +-
 drivers/video/fbdev/core/tileblit.c     |  3 +-
 3 files changed, 25 insertions(+), 38 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index cef437817b0d..253eb9bcff25 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1004,7 +1004,7 @@ static const char *fbcon_startup(void)
 		vc->vc_font.width = font->width;
 		vc->vc_font.height = font->height;
 		vc->vc_font.data = (void *)(p->fontdata = font->data);
-		vc->vc_font.charcount = 256; /* FIXME  Need to support more fonts */
+		vc->vc_font.charcount = font->charcount;
 	} else {
 		p->fontdata = vc->vc_font.data;
 	}
@@ -1032,7 +1032,7 @@ static void fbcon_init(struct vc_data *vc, int init)
 	struct vc_data **default_mode = vc->vc_display_fg;
 	struct vc_data *svc = *default_mode;
 	struct fbcon_display *t, *p = &fb_display[vc->vc_num];
-	int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256;
+	int logo = 1, new_rows, new_cols, rows, cols;
 	int cap, ret;
 
 	if (WARN_ON(info_idx == -1))
@@ -1068,6 +1068,7 @@ static void fbcon_init(struct vc_data *vc, int init)
 						    fvc->vc_font.data);
 			vc->vc_font.width = fvc->vc_font.width;
 			vc->vc_font.height = fvc->vc_font.height;
+			vc->vc_font.charcount = fvc->vc_font.charcount;
 			p->userfont = t->userfont;
 
 			if (p->userfont)
@@ -1083,17 +1084,13 @@ static void fbcon_init(struct vc_data *vc, int init)
 			vc->vc_font.width = font->width;
 			vc->vc_font.height = font->height;
 			vc->vc_font.data = (void *)(p->fontdata = font->data);
-			vc->vc_font.charcount = 256; /* FIXME  Need to
-							support more fonts */
+			vc->vc_font.charcount = font->charcount;
 		}
 	}
 
-	if (p->userfont)
-		charcnt = FNTCHARCNT(p->fontdata);
-
 	vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1);
 	vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
-	if (charcnt == 256) {
+	if (vc->vc_font.charcount == 256) {
 		vc->vc_hi_font_mask = 0;
 	} else {
 		vc->vc_hi_font_mask = 0x100;
@@ -1358,7 +1355,7 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
 	struct vc_data **default_mode, *vc;
 	struct vc_data *svc;
 	struct fbcon_ops *ops = info->fbcon_par;
-	int rows, cols, charcnt = 256;
+	int rows, cols;
 
 	p = &fb_display[unit];
 
@@ -1378,12 +1375,11 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
 		vc->vc_font.data = (void *)(p->fontdata = t->fontdata);
 		vc->vc_font.width = (*default_mode)->vc_font.width;
 		vc->vc_font.height = (*default_mode)->vc_font.height;
+		vc->vc_font.charcount = (*default_mode)->vc_font.charcount;
 		p->userfont = t->userfont;
 		if (p->userfont)
 			REFCOUNT(p->fontdata)++;
 	}
-	if (p->userfont)
-		charcnt = FNTCHARCNT(p->fontdata);
 
 	var->activate = FB_ACTIVATE_NOW;
 	info->var.activate = var->activate;
@@ -1393,7 +1389,7 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
 	ops->var = info->var;
 	vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1);
 	vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
-	if (charcnt == 256) {
+	if (vc->vc_font.charcount == 256) {
 		vc->vc_hi_font_mask = 0;
 	} else {
 		vc->vc_hi_font_mask = 0x100;
@@ -2027,7 +2023,7 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width,
 		 */
 		if (pitch <= 0)
 			return -EINVAL;
-		size = CALC_FONTSZ(vc->vc_font.height, pitch, FNTCHARCNT(vc->vc_font.data));
+		size = CALC_FONTSZ(vc->vc_font.height, pitch, vc->vc_font.charcount);
 		if (size > FNTSIZE(vc->vc_font.data))
 			return -EINVAL;
 	}
@@ -2075,7 +2071,7 @@ static int fbcon_switch(struct vc_data *vc)
 	struct fbcon_ops *ops;
 	struct fbcon_display *p = &fb_display[vc->vc_num];
 	struct fb_var_screeninfo var;
-	int i, ret, prev_console, charcnt = 256;
+	int i, ret, prev_console;
 
 	info = registered_fb[con2fb_map[vc->vc_num]];
 	ops = info->fbcon_par;
@@ -2152,10 +2148,7 @@ static int fbcon_switch(struct vc_data *vc)
 	vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1);
 	vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800;
 
-	if (p->userfont)
-		charcnt = FNTCHARCNT(vc->vc_font.data);
-
-	if (charcnt > 256)
+	if (vc->vc_font.charcount > 256)
 		vc->vc_complement_mask <<= 1;
 
 	updatescrollmode(p, info, vc);
@@ -2289,7 +2282,7 @@ static int fbcon_get_font(struct vc_data *vc, struct console_font *font)
 
 	font->width = vc->vc_font.width;
 	font->height = vc->vc_font.height;
-	font->charcount = vc->vc_hi_font_mask ? 512 : 256;
+	font->charcount = vc->vc_font.charcount;
 	if (!font->data)
 		return 0;
 
@@ -2405,31 +2398,27 @@ static void set_vc_hi_font(struct vc_data *vc, bool set)
 	}
 }
 
-static int fbcon_do_set_font(struct vc_data *vc, int w, int h,
+static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 			     const u8 * data, int userfont)
 {
 	struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
 	struct fbcon_ops *ops = info->fbcon_par;
 	struct fbcon_display *p = &fb_display[vc->vc_num];
 	int resize;
-	int cnt;
 	char *old_data = NULL;
 
 	resize = (w != vc->vc_font.width) || (h != vc->vc_font.height);
 	if (p->userfont)
 		old_data = vc->vc_font.data;
-	if (userfont)
-		cnt = FNTCHARCNT(data);
-	else
-		cnt = 256;
 	vc->vc_font.data = (void *)(p->fontdata = data);
 	if ((p->userfont = userfont))
 		REFCOUNT(data)++;
 	vc->vc_font.width = w;
 	vc->vc_font.height = h;
-	if (vc->vc_hi_font_mask && cnt == 256)
+	vc->vc_font.charcount = charcount;
+	if (vc->vc_hi_font_mask && charcount == 256)
 		set_vc_hi_font(vc, false);
-	else if (!vc->vc_hi_font_mask && cnt == 512)
+	else if (!vc->vc_hi_font_mask && charcount == 512)
 		set_vc_hi_font(vc, true);
 
 	if (resize) {
@@ -2458,7 +2447,7 @@ static int fbcon_copy_font(struct vc_data *vc, int con)
 
 	if (od->fontdata == f->data)
 		return 0;	/* already the same font... */
-	return fbcon_do_set_font(vc, f->width, f->height, od->fontdata, od->userfont);
+	return fbcon_do_set_font(vc, f->width, f->height, f->charcount, od->fontdata, od->userfont);
 }
 
 /*
@@ -2506,9 +2495,10 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
 	if (!new_data)
 		return -ENOMEM;
 
+	memset(new_data, 0, FONT_EXTRA_WORDS * sizeof(int));
+
 	new_data += FONT_EXTRA_WORDS * sizeof(int);
 	FNTSIZE(new_data) = size;
-	FNTCHARCNT(new_data) = charcount;
 	REFCOUNT(new_data) = 0;	/* usage counter */
 	for (i=0; i< charcount; i++) {
 		memcpy(new_data + i*h*pitch, data +  i*32*pitch, h*pitch);
@@ -2534,7 +2524,7 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
 			break;
 		}
 	}
-	return fbcon_do_set_font(vc, font->width, font->height, new_data, 1);
+	return fbcon_do_set_font(vc, font->width, font->height, charcount, new_data, 1);
 }
 
 static int fbcon_set_def_font(struct vc_data *vc, struct console_font *font, char *name)
@@ -2550,7 +2540,7 @@ static int fbcon_set_def_font(struct vc_data *vc, struct console_font *font, cha
 
 	font->width = f->width;
 	font->height = f->height;
-	return fbcon_do_set_font(vc, f->width, f->height, f->data, 0);
+	return fbcon_do_set_font(vc, f->width, f->height, f->charcount, f->data, 0);
 }
 
 static u16 palette_red[16];
@@ -3031,8 +3021,7 @@ void fbcon_get_requirement(struct fb_info *info,
 				p = &fb_display[i];
 				caps->x |= 1 << (vc->vc_font.width - 1);
 				caps->y |= 1 << (vc->vc_font.height - 1);
-				charcnt = (p->userfont) ?
-					FNTCHARCNT(p->fontdata) : 256;
+				charcnt = vc->vc_font.charcount;
 				if (caps->len < charcnt)
 					caps->len = charcnt;
 			}
@@ -3045,8 +3034,7 @@ void fbcon_get_requirement(struct fb_info *info,
 			p = &fb_display[fg_console];
 			caps->x = 1 << (vc->vc_font.width - 1);
 			caps->y = 1 << (vc->vc_font.height - 1);
-			caps->len = (p->userfont) ?
-				FNTCHARCNT(p->fontdata) : 256;
+			caps->len = vc->vc_font.charcount;
 		}
 	}
 }
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index ac72d4f85f7d..9072d5947d16 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -33,7 +33,7 @@ static int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
 
 	src = ops->fontdata = vc->vc_font.data;
 	ops->cur_rotate = ops->p->con_rotate;
-	len = (!ops->p->userfont) ? 256 : FNTCHARCNT(src);
+	len = vc->vc_font.charcount;
 	s_cellsize = ((vc->vc_font.width + 7)/8) *
 		vc->vc_font.height;
 	d_cellsize = s_cellsize;
diff --git a/drivers/video/fbdev/core/tileblit.c b/drivers/video/fbdev/core/tileblit.c
index 628fe5e010c0..e9ab90d1b67c 100644
--- a/drivers/video/fbdev/core/tileblit.c
+++ b/drivers/video/fbdev/core/tileblit.c
@@ -145,8 +145,7 @@ void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info)
 		map.width = vc->vc_font.width;
 		map.height = vc->vc_font.height;
 		map.depth = 1;
-		map.length = (ops->p->userfont) ?
-			FNTCHARCNT(ops->p->fontdata) : 256;
+		map.length = vc->vc_font.charcount;
 		map.data = ops->p->fontdata;
 		info->tileops->fb_settile(info, &map);
 	}
-- 
2.25.1

Re: [PATCH v2 2/5] Fonts: Make font size unsigned in font_desc

[Daniel Vetter]

On Wed, Oct 28, 2020 at 06:56:47AM -0400, Peilin Ye wrote:
> `width` and `height` are defined as unsigned in our UAPI font descriptor
> `struct console_font`. Make them unsigned in our kernel font descriptor
> `struct font_desc`, too.
> 
> Also, change the corresponding printk() format identifiers from `%d` to
> `%u`, in sti_select_fbfont().
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

Pushed to drm-misc-next, thanks for the patch.
-Daniel

> ---
> Change in v2:
>   - Mention `struct console_font` in the commit message. (Suggested by
>     Daniel Vetter <daniel@ffwll.ch>)
> 
>  drivers/video/console/sticore.c | 2 +-
>  include/linux/font.h            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
> index 6a26a364f9bd..d1bb5915082b 100644
> --- a/drivers/video/console/sticore.c
> +++ b/drivers/video/console/sticore.c
> @@ -502,7 +502,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name)
>  	if (!fbfont)
>  		return NULL;
>  
> -	pr_info("STI selected %dx%d framebuffer font %s for sticon\n",
> +	pr_info("STI selected %ux%u framebuffer font %s for sticon\n",
>  			fbfont->width, fbfont->height, fbfont->name);
>  			
>  	bpc = ((fbfont->width+7)/8) * fbfont->height; 
> diff --git a/include/linux/font.h b/include/linux/font.h
> index b5b312c19e46..4f50d736ea72 100644
> --- a/include/linux/font.h
> +++ b/include/linux/font.h
> @@ -16,7 +16,7 @@
>  struct font_desc {
>      int idx;
>      const char *name;
> -    int width, height;
> +    unsigned int width, height;
>      const void *data;
>      int pref;
>  };
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/5] fbdev/atafb: Remove unused extern variables

[Thomas Zimmermann]

Am 27.10.20 um 17:31 schrieb Peilin Ye:
> Remove 6 unused extern variables to reduce confusion. It is worth
> mentioning that lib/fonts/font_8x8.c and lib/fonts/font_8x16.c also
> declare `fontdata_8x8` and `fontdata_8x16` respectively, and this file
> has nothing to do with them.
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> $ # Build-tested (Ubuntu 20.04)
> $ sudo apt install gcc-m68k-linux-gnu
> $ cp arch/m68k/configs/atari_defconfig .config
> $ make ARCH=m68k menuconfig
> $ make ARCH=m68k CROSS_COMPILE=m68k-linux-gnu- -j`nproc` all
> 
>  drivers/video/fbdev/atafb.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
> index f253daa05d9d..e3812a8ff55a 100644
> --- a/drivers/video/fbdev/atafb.c
> +++ b/drivers/video/fbdev/atafb.c
> @@ -240,14 +240,6 @@ static int *MV300_reg = MV300_reg_8bit;
>  
>  static int inverse;
>  
> -extern int fontheight_8x8;
> -extern int fontwidth_8x8;
> -extern unsigned char fontdata_8x8[];
> -
> -extern int fontheight_8x16;
> -extern int fontwidth_8x16;
> -extern unsigned char fontdata_8x16[];
> -
>  /*
>   * struct fb_ops {
>   *	* open/release and usage marking
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

Re: [PATCH 0/5] Preparation work for using font_desc in vc_data

[Daniel Vetter]

On Tue, Oct 27, 2020 at 12:27:35PM -0400, Peilin Ye wrote:
> Hi Daniel, Hi Greg, Hi all,
> 
> We are planning to use `font_desc` instead of `console_font` in `vc_data`,
> and this is just some prep work for it. It doesn't do much, but at least
> it removes two "FIXME"s in fbcon.c :)

Btw nitpeek on how you submit patches: The threading you're using here is
"deep" i.e. every patch is  reply to the previous patch. The usual thing
is flat, i.e. all patches are replies to the cover letter.

I think only very old versions of git had the former as a default, so not
sure what's going on. But the deep threading makes piecing the
conversation together quite a bit harder, at least here in mutt.
-Daniel


> 
> Peilin Ye (5):
> [1/5] fbdev/atafb: Remove unused extern variables
> 
>   Searching for `fontdata` gave me this in fbdev/atafb.c:
> 
>   extern unsigned char fontdata_8x8[];
>   extern unsigned char fontdata_8x16[];
> 
>   ...which freaked me out, since in 6735b4632def ("Fonts: Support
>   FONT_EXTRA_WORDS macros for built-in fonts") I changed them from char
>   arrays to structures, in lib/fonts/. Fortunately it turns out these
>   extern variables have nothing to do with lib/fonts/, and are not being
>   used anywhere, so remove them for less confusion.
> 
>   m68k cross-compiled.
> 
> [2/5] Fonts: Make font size unsigned in font_desc
> 
>   Our goal is to use `font_desc` "everywhere" in the kernel, and signed
>   `width` and `height` is inappropriate.
> 
>   Also, change some printk() format identifiers in console/sticore.c from
>   `%d` to `%u`. parisc cross-compiled.
> 
> [3/5] Fonts: Add charcount field to font_desc
> 
>   Add `unsigned int charcount` to `font_desc`, and update each of our 13
>   built-in fonts.
> 
> [4/5] fbcon: Avoid hard-coding built-in font charcount
> [5/5] parisc/sticore: Avoid hard-coding built-in font charcount
> 
>   Everyone (tty, fbcon, sticore, etc.) is assuming that all built-in fonts
>   have 256 characters, and is using hard-coded 256 or 255 everywhere.
>   These two patches removes some of them. [5/5] is parisc cross-compiled.
> 
>   Now is a good time to review all find_font() and get_default_font()
>   callers:
> 
>   drivers/media/pci/solo6x10/solo6x10-enc.c      133 const struct font_desc *vga = find_font("VGA8x16");
>   drivers/media/test-drivers/vimc/vimc-core.c    268 const struct font_desc *font = find_font("VGA8x16");
>   drivers/media/test-drivers/vivid/vivid-core.c 1928 const struct font_desc *font = find_font("VGA8x16");
>   drivers/usb/misc/sisusbvga/sisusb.c           2285 myfont = find_font("VGA8x16");
>     * These 4 only care about font VGA8x16, so let them be for now;
> 
>   drivers/video/console/sticore.c                499 fbfont = find_font(fbfont_name);
>   drivers/video/console/sticore.c                501 fbfont = get_default_font(1024,768, ~(u32)0, ~(u32)0);
>     * Uses 255 and 256, (hopefully) cleaned up by [5/5];
> 
>   drivers/video/fbdev/core/fbcon.c               999 if (!fontname[0] || !(font = find_font(fontname)))
>   drivers/video/fbdev/core/fbcon.c              1000 font = get_default_font(info->var.xres,
>   drivers/video/fbdev/core/fbcon.c              1078 if (!fontname[0] || !(font = find_font(fontname)))
>   drivers/video/fbdev/core/fbcon.c              1079 font = get_default_font(info->var.xres,
>     * Use 256, cleaned up by [4/5];
> 
>   drivers/video/fbdev/core/fbcon.c              2548 else if (!(f = find_font(name)))
>   drivers/video/fbdev/core/fbcon.c              2546 f = get_default_font(info->var.xres, info->var.yres,
>     * Uses 256 but no easy fix. I'll clean this up after making
>       fbcon_do_set_font() pass a `font_desc` as parameter;
> 
>   drivers/firmware/efi/earlycon.c               234 font = get_default_font(xres, yres, -1, -1);
>     * Does not care about charcount.
> 
> Thank you!
> Peilin Ye
> 
>  drivers/video/console/sticore.c  | 10 +++++-----
>  drivers/video/fbdev/atafb.c      |  8 --------
>  drivers/video/fbdev/core/fbcon.c |  5 ++---
>  include/linux/font.h             |  3 ++-
>  lib/fonts/font_10x18.c           |  1 +
>  lib/fonts/font_6x10.c            |  1 +
>  lib/fonts/font_6x11.c            |  1 +
>  lib/fonts/font_6x8.c             |  1 +
>  lib/fonts/font_7x14.c            |  1 +
>  lib/fonts/font_8x16.c            |  1 +
>  lib/fonts/font_8x8.c             |  1 +
>  lib/fonts/font_acorn_8x8.c       |  1 +
>  lib/fonts/font_mini_4x6.c        |  1 +
>  lib/fonts/font_pearl_8x8.c       |  1 +
>  lib/fonts/font_sun12x22.c        |  1 +
>  lib/fonts/font_sun8x16.c         |  1 +
>  lib/fonts/font_ter16x32.c        |  1 +
>  17 files changed, 22 insertions(+), 17 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 3/5 v2] Fonts: Add charcount field to font_desc

[Daniel Vetter]

On Wed, Oct 28, 2020 at 02:05:33AM -0400, Peilin Ye wrote:
> Subsystems are assuming the number of characters of our built-in fonts.
> Include that information in our kernel font descriptor, `struct
> font_desc`.
> 
> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> Change in v2:
>   - Rebase onto 5.10-rc1.

Hm there's a tiny conflict now with this because of the unsigned vs signed
discussion we've had.

Can you please rebase the remaining 3 patches onto linux-next or
drm-misc-next or so and then resubmit?

Thanks, Daniel

> 
>  include/linux/font.h       | 1 +
>  lib/fonts/font_10x18.c     | 1 +
>  lib/fonts/font_6x10.c      | 1 +
>  lib/fonts/font_6x11.c      | 1 +
>  lib/fonts/font_6x8.c       | 1 +
>  lib/fonts/font_7x14.c      | 1 +
>  lib/fonts/font_8x16.c      | 1 +
>  lib/fonts/font_8x8.c       | 1 +
>  lib/fonts/font_acorn_8x8.c | 1 +
>  lib/fonts/font_mini_4x6.c  | 1 +
>  lib/fonts/font_pearl_8x8.c | 1 +
>  lib/fonts/font_sun12x22.c  | 1 +
>  lib/fonts/font_sun8x16.c   | 1 +
>  lib/fonts/font_ter16x32.c  | 1 +
>  14 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/font.h b/include/linux/font.h
> index b5b312c19e46..54e60ad2252b 100644
> --- a/include/linux/font.h
> +++ b/include/linux/font.h
> @@ -17,6 +17,7 @@ struct font_desc {
>      int idx;
>      const char *name;
>      int width, height;
> +    unsigned int charcount;
>      const void *data;
>      int pref;
>  };
> diff --git a/lib/fonts/font_10x18.c b/lib/fonts/font_10x18.c
> index 0e2deac97da0..4096c6562494 100644
> --- a/lib/fonts/font_10x18.c
> +++ b/lib/fonts/font_10x18.c
> @@ -5137,6 +5137,7 @@ const struct font_desc font_10x18 = {
>  	.name	= "10x18",
>  	.width	= 10,
>  	.height	= 18,
> +	.charcount = 256,
>  	.data	= fontdata_10x18.data,
>  #ifdef __sparc__
>  	.pref	= 5,
> diff --git a/lib/fonts/font_6x10.c b/lib/fonts/font_6x10.c
> index 87da8acd07db..32786674cf65 100644
> --- a/lib/fonts/font_6x10.c
> +++ b/lib/fonts/font_6x10.c
> @@ -3083,6 +3083,7 @@ const struct font_desc font_6x10 = {
>  	.name	= "6x10",
>  	.width	= 6,
>  	.height	= 10,
> +	.charcount = 256,
>  	.data	= fontdata_6x10.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_6x11.c b/lib/fonts/font_6x11.c
> index 5e975dfa10a5..81e4a3aed44a 100644
> --- a/lib/fonts/font_6x11.c
> +++ b/lib/fonts/font_6x11.c
> @@ -3346,6 +3346,7 @@ const struct font_desc font_vga_6x11 = {
>  	.name	= "ProFont6x11",
>  	.width	= 6,
>  	.height	= 11,
> +	.charcount = 256,
>  	.data	= fontdata_6x11.data,
>  	/* Try avoiding this font if possible unless on MAC */
>  	.pref	= -2000,
> diff --git a/lib/fonts/font_6x8.c b/lib/fonts/font_6x8.c
> index 700039a9ceae..5618ae7ef9fa 100644
> --- a/lib/fonts/font_6x8.c
> +++ b/lib/fonts/font_6x8.c
> @@ -2571,6 +2571,7 @@ const struct font_desc font_6x8 = {
>  	.name	= "6x8",
>  	.width	= 6,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= fontdata_6x8.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_7x14.c b/lib/fonts/font_7x14.c
> index 86d298f38505..7708e73d491f 100644
> --- a/lib/fonts/font_7x14.c
> +++ b/lib/fonts/font_7x14.c
> @@ -4113,6 +4113,7 @@ const struct font_desc font_7x14 = {
>  	.name	= "7x14",
>  	.width	= 7,
>  	.height	= 14,
> +	.charcount = 256,
>  	.data	= fontdata_7x14.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_8x16.c b/lib/fonts/font_8x16.c
> index 37cedd36ca5e..74125d3570cf 100644
> --- a/lib/fonts/font_8x16.c
> +++ b/lib/fonts/font_8x16.c
> @@ -4627,6 +4627,7 @@ const struct font_desc font_vga_8x16 = {
>  	.name	= "VGA8x16",
>  	.width	= 8,
>  	.height	= 16,
> +	.charcount = 256,
>  	.data	= fontdata_8x16.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_8x8.c b/lib/fonts/font_8x8.c
> index 8ab695538395..96da4bb31ae4 100644
> --- a/lib/fonts/font_8x8.c
> +++ b/lib/fonts/font_8x8.c
> @@ -2578,6 +2578,7 @@ const struct font_desc font_vga_8x8 = {
>  	.name	= "VGA8x8",
>  	.width	= 8,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= fontdata_8x8.data,
>  	.pref	= 0,
>  };
> diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c
> index 069b3e80c434..ba74053fec7b 100644
> --- a/lib/fonts/font_acorn_8x8.c
> +++ b/lib/fonts/font_acorn_8x8.c
> @@ -270,6 +270,7 @@ const struct font_desc font_acorn_8x8 = {
>  	.name	= "Acorn8x8",
>  	.width	= 8,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= acorndata_8x8.data,
>  #ifdef CONFIG_ARCH_ACORN
>  	.pref	= 20,
> diff --git a/lib/fonts/font_mini_4x6.c b/lib/fonts/font_mini_4x6.c
> index 1449876c6a27..637708e8c67e 100644
> --- a/lib/fonts/font_mini_4x6.c
> +++ b/lib/fonts/font_mini_4x6.c
> @@ -2152,6 +2152,7 @@ const struct font_desc font_mini_4x6 = {
>  	.name	= "MINI4x6",
>  	.width	= 4,
>  	.height	= 6,
> +	.charcount = 256,
>  	.data	= fontdata_mini_4x6.data,
>  	.pref	= 3,
>  };
> diff --git a/lib/fonts/font_pearl_8x8.c b/lib/fonts/font_pearl_8x8.c
> index 32d65551e7ed..06cde43c7bd2 100644
> --- a/lib/fonts/font_pearl_8x8.c
> +++ b/lib/fonts/font_pearl_8x8.c
> @@ -2582,6 +2582,7 @@ const struct font_desc font_pearl_8x8 = {
>  	.name	= "PEARL8x8",
>  	.width	= 8,
>  	.height	= 8,
> +	.charcount = 256,
>  	.data	= fontdata_pearl8x8.data,
>  	.pref	= 2,
>  };
> diff --git a/lib/fonts/font_sun12x22.c b/lib/fonts/font_sun12x22.c
> index 641a6b4dca42..d0290d79df2c 100644
> --- a/lib/fonts/font_sun12x22.c
> +++ b/lib/fonts/font_sun12x22.c
> @@ -6156,6 +6156,7 @@ const struct font_desc font_sun_12x22 = {
>  	.name	= "SUN12x22",
>  	.width	= 12,
>  	.height	= 22,
> +	.charcount = 256,
>  	.data	= fontdata_sun12x22.data,
>  #ifdef __sparc__
>  	.pref	= 5,
> diff --git a/lib/fonts/font_sun8x16.c b/lib/fonts/font_sun8x16.c
> index 193fe6d988e0..7d83760354a6 100644
> --- a/lib/fonts/font_sun8x16.c
> +++ b/lib/fonts/font_sun8x16.c
> @@ -268,6 +268,7 @@ const struct font_desc font_sun_8x16 = {
>  	.name	= "SUN8x16",
>  	.width	= 8,
>  	.height	= 16,
> +	.charcount = 256,
>  	.data	= fontdata_sun8x16.data,
>  #ifdef __sparc__
>  	.pref	= 10,
> diff --git a/lib/fonts/font_ter16x32.c b/lib/fonts/font_ter16x32.c
> index 91b9c283bd9c..a2cc25b9f8e3 100644
> --- a/lib/fonts/font_ter16x32.c
> +++ b/lib/fonts/font_ter16x32.c
> @@ -2062,6 +2062,7 @@ const struct font_desc font_ter_16x32 = {
>  	.name	= "TER16x32",
>  	.width	= 16,
>  	.height = 32,
> +	.charcount = 256,
>  	.data	= fontdata_ter16x32.data,
>  #ifdef __sparc__
>  	.pref	= 5,
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch