Re: [PATCH] efi/earlycon: fix early printk for wider fonts

From: Ard Biesheuvel <ardb@kernel.org>
To: Dave Young <dyoung@redhat.com>
Cc: linux-efi <linux-efi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] efi/earlycon: fix early printk for wider fonts
Date: Mon, 13 Apr 2020 12:30:47 +0200	[thread overview]
Message-ID: <CAMj1kXGA-VJdzkWMtWjrXt5HCDmRxdG0YqsoiRUQQS8SL1=Txw@mail.gmail.com> (raw)
In-Reply-To: <20200412024927.GA6884@dhcp-128-65.nay.redhat.com>

On Sun, 12 Apr 2020 at 04:49, Dave Young <dyoung@redhat.com> wrote:
>
> When I play with terminus fonts I noticed the efi early printk does
> not work because the earlycon code assumes font width is 8.
>
> Here add the code to adapt with larger fonts.  Tested with all kinds
> of kernel built-in fonts on my laptop. Also tested with a local draft
> patch for 14x28 !bold terminus font.
>
> Signed-off-by: Dave Young <dyoung@redhat.com>

Thanks Dave. I tested this with the 10x18 font, which is utterly
broken for efifb unless I apply this patch.

I'll queue this as a fix.

> ---
>  drivers/firmware/efi/earlycon.c |   14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> --- linux-x86.orig/drivers/firmware/efi/earlycon.c
> +++ linux-x86/drivers/firmware/efi/earlycon.c
> @@ -114,14 +114,16 @@ static void efi_earlycon_write_char(u32
>         const u32 color_black = 0x00000000;
>         const u32 color_white = 0x00ffffff;
>         const u8 *src;
> -       u8 s8;
> -       int m;
> +       int m, n, bytes;
> +       u8 x;
>
> -       src = font->data + c * font->height;
> -       s8 = *(src + h);
> +       bytes = BITS_TO_BYTES(font->width);
> +       src = font->data + c * font->height * bytes + h * bytes;
>
> -       for (m = 0; m < 8; m++) {
> -               if ((s8 >> (7 - m)) & 1)
> +       for (m = 0; m < font->width; m++) {
> +               n = m % 8;
> +               x = *(src + m / 8);
> +               if ((x >> (7 - n)) & 1)
>                         *dst = color_white;
>                 else
>                         *dst = color_black;
>