* [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-17 15:32 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-17 15:32 UTC (permalink / raw)
To: Ard Biesheuvel, Bartlomiej Zolnierkiewicz
Cc: Hans de Goede, linux-fbdev, linux-efi, dri-devel
On systems where fbcon is configured for deferred console takeover, the
intend is for the framebuffer to show the boot graphics (e.g a vendor
logo) until some message (e.g. an error) is printed or a graphical
session takes over.
Some firmware however relies on the OS to show the boot graphics
(indicated by bgrt_tab.status being 0) and the boot graphics may have
been destroyed by e.g. the grub boot menu.
This patch adds support to efifb to show the boot graphics and
automatically enables this when fbcon is configured for deferred
console takeover.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/video/fbdev/efifb.c | 147 ++++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 46a4484e3da7..b041d936a438 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -9,16 +9,39 @@
#include <linux/kernel.h>
#include <linux/efi.h>
+#include <linux/efi-bgrt.h>
#include <linux/errno.h>
#include <linux/fb.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/printk.h>
#include <linux/screen_info.h>
#include <video/vga.h>
#include <asm/efi.h>
#include <drm/drm_utils.h> /* For drm_get_panel_orientation_quirk */
#include <drm/drm_connector.h> /* For DRM_MODE_PANEL_ORIENTATION_* */
+struct bmp_file_header {
+ u16 id;
+ u32 file_size;
+ u32 reserved;
+ u32 bitmap_offset;
+} __packed;
+
+struct bmp_dib_header {
+ u32 dib_header_size;
+ s32 width;
+ s32 height;
+ u16 planes;
+ u16 bpp;
+ u32 compression;
+ u32 bitmap_size;
+ u32 horz_resolution;
+ u32 vert_resolution;
+ u32 colors_used;
+ u32 colors_important;
+} __packed;
+
static bool request_mem_succeeded = false;
static bool nowc = false;
@@ -66,6 +89,128 @@ static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
return 0;
}
+/*
+ * If fbcon deffered console takeover is configured, the intent is for the
+ * framebuffer to show the boot graphics (e.g. vendor logo) until there is some
+ * (error) message to display. But the boot graphics may have been destroyed by
+ * e.g. option ROM output, detect this and restore the boot graphics.
+ */
+#if defined CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER && \
+ defined CONFIG_ACPI_BGRT
+static void efifb_show_boot_graphics(struct fb_info *info)
+{
+ u32 *dst, bmp_width, bmp_height, bmp_pitch, screen_pitch;
+ struct screen_info *si = &screen_info;
+ struct bmp_file_header *file_header;
+ struct bmp_dib_header *dib_header;
+ void *bgrt_image = NULL;
+ u8 *src, r, g, b;
+ s32 x, y;
+
+ if (!bgrt_tab.image_address) {
+ pr_info("efifb: No BGRT, not showing boot graphics\n");
+ return;
+ }
+
+ /* Avoid flashing the logo if we're going to print std probe messages */
+ if (console_loglevel > CONSOLE_LOGLEVEL_QUIET)
+ return;
+
+ /*
+ * We do not check bgrt_tab.status here because this seems to only
+ * reflect if the firmware has shown the boot graphics at all, if it
+ * later got destroyed by something status will still be 1.
+ * Since we draw the exact same graphic at the exact same place this
+ * will not lead to any tearing if the boot graphic is already there.
+ */
+
+ if (si->lfb_depth != 32) {
+ pr_info("efifb: not 32 bits, not showing boot graphics\n");
+ return;
+ }
+
+ bgrt_image = memremap(bgrt_tab.image_address, bgrt_image_size,
+ MEMREMAP_WB);
+ if (!bgrt_image) {
+ pr_warn("efifb: Ignoring BGRT: failed to map image memory\n");
+ return;
+ }
+
+ if (bgrt_image_size < (sizeof(*file_header) + sizeof(*dib_header)))
+ goto error;
+
+ file_header = bgrt_image;
+ if (file_header->id != 0x4d42 || file_header->reserved != 0)
+ goto error;
+
+ dib_header = bgrt_image + sizeof(*file_header);
+ if (dib_header->dib_header_size != 40 || dib_header->width < 0 ||
+ dib_header->planes != 1 || dib_header->bpp != 24 ||
+ dib_header->compression != 0)
+ goto error;
+
+ bmp_width = dib_header->width;
+ bmp_height = abs(dib_header->height);
+ bmp_pitch = round_up(3 * bmp_width, 4);
+ screen_pitch = si->lfb_linelength;
+
+ if ((file_header->bitmap_offset + bmp_pitch * bmp_height) >
+ bgrt_image_size)
+ goto error;
+
+ if ((bgrt_tab.image_offset_x + bmp_width) > si->lfb_width ||
+ (bgrt_tab.image_offset_y + bmp_height) > si->lfb_height)
+ goto error;
+
+ pr_info("efifb: showing boot graphics\n");
+
+ src = bgrt_image + file_header->bitmap_offset;
+ bmp_pitch -= 3 * bmp_width;
+ if (dib_header->height < 0) {
+ for (y = 0; y < bmp_height; y++) {
+ dst = (u32 *)(
+ info->screen_base +
+ (bgrt_tab.image_offset_y + y) * screen_pitch +
+ bgrt_tab.image_offset_x * 4);
+ for (x = 0; x < bmp_width; x++) {
+ b = *src++;
+ g = *src++;
+ r = *src++;
+ *dst++ = (r << si->red_pos) |
+ (g << si->green_pos) |
+ (b << si->blue_pos);
+ }
+ src += bmp_pitch;
+ }
+ } else {
+ for (y = (bmp_height - 1); y >= 0; y--) {
+ dst = (u32 *)(
+ info->screen_base +
+ (bgrt_tab.image_offset_y + y) * screen_pitch +
+ bgrt_tab.image_offset_x * 4);
+ for (x = 0; x < bmp_width; x++) {
+ b = *src++;
+ g = *src++;
+ r = *src++;
+ *dst++ = (r << si->red_pos) |
+ (g << si->green_pos) |
+ (b << si->blue_pos);
+ }
+ src += bmp_pitch;
+ }
+ }
+
+ memunmap(bgrt_image);
+ return;
+
+error:
+ memunmap(bgrt_image);
+ pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n");
+}
+#else
+static inline void efifb_show_boot_graphics(struct fb_info *info) {}
+#endif
+
static void efifb_destroy(struct fb_info *info)
{
if (info->screen_base)
@@ -283,6 +428,8 @@ static int efifb_probe(struct platform_device *dev)
goto err_release_fb;
}
+ efifb_show_boot_graphics(info);
+
pr_info("efifb: framebuffer at 0x%lx, using %dk, total %dk\n",
efifb_fix.smem_start, size_remap/1024, size_total/1024);
pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
--
2.17.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-17 15:32 ` Hans de Goede
@ 2018-06-18 7:36 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2018-06-18 7:36 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
Hallo Hans,
On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
> On systems where fbcon is configured for deferred console takeover, the
> intend is for the framebuffer to show the boot graphics (e.g a vendor
> logo) until some message (e.g. an error) is printed or a graphical
> session takes over.
>
> Some firmware however relies on the OS to show the boot graphics
> (indicated by bgrt_tab.status being 0) and the boot graphics may have
> been destroyed by e.g. the grub boot menu.
>
> This patch adds support to efifb to show the boot graphics and
> automatically enables this when fbcon is configured for deferred
> console takeover.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
I have tested this code on ARM QEMU/mach-virt, and with a little tweak
(which I will post separately), the code works as expected, i.e., it
redraws the boot logo based on the contents of the BGRT table.
However, what it doesn't do is clear the screen, which means the logo
is drawn on top of whatever the boot environment left behind, and I
end up with something like this.
http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
> ---
> drivers/video/fbdev/efifb.c | 147 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 147 insertions(+)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 46a4484e3da7..b041d936a438 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -9,16 +9,39 @@
>
> #include <linux/kernel.h>
> #include <linux/efi.h>
> +#include <linux/efi-bgrt.h>
> #include <linux/errno.h>
> #include <linux/fb.h>
> #include <linux/pci.h>
> #include <linux/platform_device.h>
> +#include <linux/printk.h>
> #include <linux/screen_info.h>
> #include <video/vga.h>
> #include <asm/efi.h>
> #include <drm/drm_utils.h> /* For drm_get_panel_orientation_quirk */
> #include <drm/drm_connector.h> /* For DRM_MODE_PANEL_ORIENTATION_* */
>
> +struct bmp_file_header {
> + u16 id;
> + u32 file_size;
> + u32 reserved;
> + u32 bitmap_offset;
> +} __packed;
> +
> +struct bmp_dib_header {
> + u32 dib_header_size;
> + s32 width;
> + s32 height;
> + u16 planes;
> + u16 bpp;
> + u32 compression;
> + u32 bitmap_size;
> + u32 horz_resolution;
> + u32 vert_resolution;
> + u32 colors_used;
> + u32 colors_important;
> +} __packed;
> +
> static bool request_mem_succeeded = false;
> static bool nowc = false;
>
> @@ -66,6 +89,128 @@ static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
> return 0;
> }
>
> +/*
> + * If fbcon deffered console takeover is configured, the intent is for the
> + * framebuffer to show the boot graphics (e.g. vendor logo) until there is some
> + * (error) message to display. But the boot graphics may have been destroyed by
> + * e.g. option ROM output, detect this and restore the boot graphics.
> + */
> +#if defined CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER && \
> + defined CONFIG_ACPI_BGRT
> +static void efifb_show_boot_graphics(struct fb_info *info)
> +{
> + u32 *dst, bmp_width, bmp_height, bmp_pitch, screen_pitch;
> + struct screen_info *si = &screen_info;
> + struct bmp_file_header *file_header;
> + struct bmp_dib_header *dib_header;
> + void *bgrt_image = NULL;
> + u8 *src, r, g, b;
> + s32 x, y;
> +
> + if (!bgrt_tab.image_address) {
> + pr_info("efifb: No BGRT, not showing boot graphics\n");
> + return;
> + }
> +
> + /* Avoid flashing the logo if we're going to print std probe messages */
> + if (console_loglevel > CONSOLE_LOGLEVEL_QUIET)
> + return;
> +
> + /*
> + * We do not check bgrt_tab.status here because this seems to only
> + * reflect if the firmware has shown the boot graphics at all, if it
> + * later got destroyed by something status will still be 1.
> + * Since we draw the exact same graphic at the exact same place this
> + * will not lead to any tearing if the boot graphic is already there.
> + */
> +
> + if (si->lfb_depth != 32) {
> + pr_info("efifb: not 32 bits, not showing boot graphics\n");
> + return;
> + }
> +
> + bgrt_image = memremap(bgrt_tab.image_address, bgrt_image_size,
> + MEMREMAP_WB);
> + if (!bgrt_image) {
> + pr_warn("efifb: Ignoring BGRT: failed to map image memory\n");
> + return;
> + }
> +
> + if (bgrt_image_size < (sizeof(*file_header) + sizeof(*dib_header)))
> + goto error;
> +
> + file_header = bgrt_image;
> + if (file_header->id != 0x4d42 || file_header->reserved != 0)
> + goto error;
> +
> + dib_header = bgrt_image + sizeof(*file_header);
> + if (dib_header->dib_header_size != 40 || dib_header->width < 0 ||
> + dib_header->planes != 1 || dib_header->bpp != 24 ||
> + dib_header->compression != 0)
> + goto error;
> +
> + bmp_width = dib_header->width;
> + bmp_height = abs(dib_header->height);
> + bmp_pitch = round_up(3 * bmp_width, 4);
> + screen_pitch = si->lfb_linelength;
> +
> + if ((file_header->bitmap_offset + bmp_pitch * bmp_height) >
> + bgrt_image_size)
> + goto error;
> +
> + if ((bgrt_tab.image_offset_x + bmp_width) > si->lfb_width ||
> + (bgrt_tab.image_offset_y + bmp_height) > si->lfb_height)
> + goto error;
> +
> + pr_info("efifb: showing boot graphics\n");
> +
> + src = bgrt_image + file_header->bitmap_offset;
> + bmp_pitch -= 3 * bmp_width;
> + if (dib_header->height < 0) {
> + for (y = 0; y < bmp_height; y++) {
> + dst = (u32 *)(
> + info->screen_base +
> + (bgrt_tab.image_offset_y + y) * screen_pitch +
> + bgrt_tab.image_offset_x * 4);
> + for (x = 0; x < bmp_width; x++) {
> + b = *src++;
> + g = *src++;
> + r = *src++;
> + *dst++ = (r << si->red_pos) |
> + (g << si->green_pos) |
> + (b << si->blue_pos);
> + }
> + src += bmp_pitch;
> + }
> + } else {
> + for (y = (bmp_height - 1); y >= 0; y--) {
> + dst = (u32 *)(
> + info->screen_base +
> + (bgrt_tab.image_offset_y + y) * screen_pitch +
> + bgrt_tab.image_offset_x * 4);
> + for (x = 0; x < bmp_width; x++) {
> + b = *src++;
> + g = *src++;
> + r = *src++;
> + *dst++ = (r << si->red_pos) |
> + (g << si->green_pos) |
> + (b << si->blue_pos);
> + }
> + src += bmp_pitch;
> + }
> + }
> +
> + memunmap(bgrt_image);
> + return;
> +
> +error:
> + memunmap(bgrt_image);
> + pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n");
> +}
> +#else
> +static inline void efifb_show_boot_graphics(struct fb_info *info) {}
> +#endif
> +
> static void efifb_destroy(struct fb_info *info)
> {
> if (info->screen_base)
> @@ -283,6 +428,8 @@ static int efifb_probe(struct platform_device *dev)
> goto err_release_fb;
> }
>
> + efifb_show_boot_graphics(info);
> +
> pr_info("efifb: framebuffer at 0x%lx, using %dk, total %dk\n",
> efifb_fix.smem_start, size_remap/1024, size_total/1024);
> pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
> --
> 2.17.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 7:36 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2018-06-18 7:36 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
Hallo Hans,
On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
> On systems where fbcon is configured for deferred console takeover, the
> intend is for the framebuffer to show the boot graphics (e.g a vendor
> logo) until some message (e.g. an error) is printed or a graphical
> session takes over.
>
> Some firmware however relies on the OS to show the boot graphics
> (indicated by bgrt_tab.status being 0) and the boot graphics may have
> been destroyed by e.g. the grub boot menu.
>
> This patch adds support to efifb to show the boot graphics and
> automatically enables this when fbcon is configured for deferred
> console takeover.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
I have tested this code on ARM QEMU/mach-virt, and with a little tweak
(which I will post separately), the code works as expected, i.e., it
redraws the boot logo based on the contents of the BGRT table.
However, what it doesn't do is clear the screen, which means the logo
is drawn on top of whatever the boot environment left behind, and I
end up with something like this.
http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
> ---
> drivers/video/fbdev/efifb.c | 147 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 147 insertions(+)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 46a4484e3da7..b041d936a438 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -9,16 +9,39 @@
>
> #include <linux/kernel.h>
> #include <linux/efi.h>
> +#include <linux/efi-bgrt.h>
> #include <linux/errno.h>
> #include <linux/fb.h>
> #include <linux/pci.h>
> #include <linux/platform_device.h>
> +#include <linux/printk.h>
> #include <linux/screen_info.h>
> #include <video/vga.h>
> #include <asm/efi.h>
> #include <drm/drm_utils.h> /* For drm_get_panel_orientation_quirk */
> #include <drm/drm_connector.h> /* For DRM_MODE_PANEL_ORIENTATION_* */
>
> +struct bmp_file_header {
> + u16 id;
> + u32 file_size;
> + u32 reserved;
> + u32 bitmap_offset;
> +} __packed;
> +
> +struct bmp_dib_header {
> + u32 dib_header_size;
> + s32 width;
> + s32 height;
> + u16 planes;
> + u16 bpp;
> + u32 compression;
> + u32 bitmap_size;
> + u32 horz_resolution;
> + u32 vert_resolution;
> + u32 colors_used;
> + u32 colors_important;
> +} __packed;
> +
> static bool request_mem_succeeded = false;
> static bool nowc = false;
>
> @@ -66,6 +89,128 @@ static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
> return 0;
> }
>
> +/*
> + * If fbcon deffered console takeover is configured, the intent is for the
> + * framebuffer to show the boot graphics (e.g. vendor logo) until there is some
> + * (error) message to display. But the boot graphics may have been destroyed by
> + * e.g. option ROM output, detect this and restore the boot graphics.
> + */
> +#if defined CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER && \
> + defined CONFIG_ACPI_BGRT
> +static void efifb_show_boot_graphics(struct fb_info *info)
> +{
> + u32 *dst, bmp_width, bmp_height, bmp_pitch, screen_pitch;
> + struct screen_info *si = &screen_info;
> + struct bmp_file_header *file_header;
> + struct bmp_dib_header *dib_header;
> + void *bgrt_image = NULL;
> + u8 *src, r, g, b;
> + s32 x, y;
> +
> + if (!bgrt_tab.image_address) {
> + pr_info("efifb: No BGRT, not showing boot graphics\n");
> + return;
> + }
> +
> + /* Avoid flashing the logo if we're going to print std probe messages */
> + if (console_loglevel > CONSOLE_LOGLEVEL_QUIET)
> + return;
> +
> + /*
> + * We do not check bgrt_tab.status here because this seems to only
> + * reflect if the firmware has shown the boot graphics at all, if it
> + * later got destroyed by something status will still be 1.
> + * Since we draw the exact same graphic at the exact same place this
> + * will not lead to any tearing if the boot graphic is already there.
> + */
> +
> + if (si->lfb_depth != 32) {
> + pr_info("efifb: not 32 bits, not showing boot graphics\n");
> + return;
> + }
> +
> + bgrt_image = memremap(bgrt_tab.image_address, bgrt_image_size,
> + MEMREMAP_WB);
> + if (!bgrt_image) {
> + pr_warn("efifb: Ignoring BGRT: failed to map image memory\n");
> + return;
> + }
> +
> + if (bgrt_image_size < (sizeof(*file_header) + sizeof(*dib_header)))
> + goto error;
> +
> + file_header = bgrt_image;
> + if (file_header->id != 0x4d42 || file_header->reserved != 0)
> + goto error;
> +
> + dib_header = bgrt_image + sizeof(*file_header);
> + if (dib_header->dib_header_size != 40 || dib_header->width < 0 ||
> + dib_header->planes != 1 || dib_header->bpp != 24 ||
> + dib_header->compression != 0)
> + goto error;
> +
> + bmp_width = dib_header->width;
> + bmp_height = abs(dib_header->height);
> + bmp_pitch = round_up(3 * bmp_width, 4);
> + screen_pitch = si->lfb_linelength;
> +
> + if ((file_header->bitmap_offset + bmp_pitch * bmp_height) >
> + bgrt_image_size)
> + goto error;
> +
> + if ((bgrt_tab.image_offset_x + bmp_width) > si->lfb_width ||
> + (bgrt_tab.image_offset_y + bmp_height) > si->lfb_height)
> + goto error;
> +
> + pr_info("efifb: showing boot graphics\n");
> +
> + src = bgrt_image + file_header->bitmap_offset;
> + bmp_pitch -= 3 * bmp_width;
> + if (dib_header->height < 0) {
> + for (y = 0; y < bmp_height; y++) {
> + dst = (u32 *)(
> + info->screen_base +
> + (bgrt_tab.image_offset_y + y) * screen_pitch +
> + bgrt_tab.image_offset_x * 4);
> + for (x = 0; x < bmp_width; x++) {
> + b = *src++;
> + g = *src++;
> + r = *src++;
> + *dst++ = (r << si->red_pos) |
> + (g << si->green_pos) |
> + (b << si->blue_pos);
> + }
> + src += bmp_pitch;
> + }
> + } else {
> + for (y = (bmp_height - 1); y >= 0; y--) {
> + dst = (u32 *)(
> + info->screen_base +
> + (bgrt_tab.image_offset_y + y) * screen_pitch +
> + bgrt_tab.image_offset_x * 4);
> + for (x = 0; x < bmp_width; x++) {
> + b = *src++;
> + g = *src++;
> + r = *src++;
> + *dst++ = (r << si->red_pos) |
> + (g << si->green_pos) |
> + (b << si->blue_pos);
> + }
> + src += bmp_pitch;
> + }
> + }
> +
> + memunmap(bgrt_image);
> + return;
> +
> +error:
> + memunmap(bgrt_image);
> + pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n");
> +}
> +#else
> +static inline void efifb_show_boot_graphics(struct fb_info *info) {}
> +#endif
> +
> static void efifb_destroy(struct fb_info *info)
> {
> if (info->screen_base)
> @@ -283,6 +428,8 @@ static int efifb_probe(struct platform_device *dev)
> goto err_release_fb;
> }
>
> + efifb_show_boot_graphics(info);
> +
> pr_info("efifb: framebuffer at 0x%lx, using %dk, total %dk\n",
> efifb_fix.smem_start, size_remap/1024, size_total/1024);
> pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-18 7:36 ` Ard Biesheuvel
@ 2018-06-18 8:30 ` Hans de Goede
-1 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-18 8:30 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
Hi,
On 18-06-18 09:36, Ard Biesheuvel wrote:
> Hallo Hans,
>
> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>> On systems where fbcon is configured for deferred console takeover, the
>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>> logo) until some message (e.g. an error) is printed or a graphical
>> session takes over.
>>
>> Some firmware however relies on the OS to show the boot graphics
>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>> been destroyed by e.g. the grub boot menu.
>>
>> This patch adds support to efifb to show the boot graphics and
>> automatically enables this when fbcon is configured for deferred
>> console takeover.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
> (which I will post separately), the code works as expected, i.e., it
> redraws the boot logo based on the contents of the BGRT table.
That is great.
> However, what it doesn't do is clear the screen, which means the logo
> is drawn on top of whatever the boot environment left behind, and I
> end up with something like this.
>
> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
Hmm, less great. I'm not sure how to deal with this, on x86 it is more
or less guaranteed that the screen is already cleared when we load and
clearing a 4k screen means writing about 32MB, which I guess with modern
RAM speeds is not that bad actually.
I see that you got this picture by manual booting from the EFI shell,
in what state does the firmware / bootloader normally leave the
framebuffer? I'm asking because if normally it is either cleared
to black, or already showing the logo I wonder if we should take the
(small) penalty of clearing ?
Given that we are talking about only 32 MB I could do a v2 which just
memsets the rest of the screen to 0.
So we get:
for (y= 0; y < height; y++) {
if (line_part_of_bgrt) {
memset(left-of-bgrt);
draw_bgrt_line(y);
memset(right-of-bgrt);
} else {
memset(line);
}
}
Note I've deliberately done the if on a per line
base to keep the actual blit part of the loop
efficient and without any extra conditionals in
there. I also don't simply first memset the entire
fb to 0 to avoid a flash / tearing if the bgrt
image is already in place (which happens often on
x86).
Implementing this is easy and as said the extra execution time should
be quite small, still I wonder what others think about this?
I'm leaning towards doing the clearing / memsets since I've seen
some firmwares leave some artifacts from not completely clearing
things like a "Press F2 to enter setup" message, missing a few
pixels and leaving those on screen.
Regards,
Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 8:30 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-18 8:30 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
Hi,
On 18-06-18 09:36, Ard Biesheuvel wrote:
> Hallo Hans,
>
> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>> On systems where fbcon is configured for deferred console takeover, the
>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>> logo) until some message (e.g. an error) is printed or a graphical
>> session takes over.
>>
>> Some firmware however relies on the OS to show the boot graphics
>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>> been destroyed by e.g. the grub boot menu.
>>
>> This patch adds support to efifb to show the boot graphics and
>> automatically enables this when fbcon is configured for deferred
>> console takeover.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
> (which I will post separately), the code works as expected, i.e., it
> redraws the boot logo based on the contents of the BGRT table.
That is great.
> However, what it doesn't do is clear the screen, which means the logo
> is drawn on top of whatever the boot environment left behind, and I
> end up with something like this.
>
> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
Hmm, less great. I'm not sure how to deal with this, on x86 it is more
or less guaranteed that the screen is already cleared when we load and
clearing a 4k screen means writing about 32MB, which I guess with modern
RAM speeds is not that bad actually.
I see that you got this picture by manual booting from the EFI shell,
in what state does the firmware / bootloader normally leave the
framebuffer? I'm asking because if normally it is either cleared
to black, or already showing the logo I wonder if we should take the
(small) penalty of clearing ?
Given that we are talking about only 32 MB I could do a v2 which just
memsets the rest of the screen to 0.
So we get:
for (y= 0; y < height; y++) {
if (line_part_of_bgrt) {
memset(left-of-bgrt);
draw_bgrt_line(y);
memset(right-of-bgrt);
} else {
memset(line);
}
}
Note I've deliberately done the if on a per line
base to keep the actual blit part of the loop
efficient and without any extra conditionals in
there. I also don't simply first memset the entire
fb to 0 to avoid a flash / tearing if the bgrt
image is already in place (which happens often on
x86).
Implementing this is easy and as said the extra execution time should
be quite small, still I wonder what others think about this?
I'm leaning towards doing the clearing / memsets since I've seen
some firmwares leave some artifacts from not completely clearing
things like a "Press F2 to enter setup" message, missing a few
pixels and leaving those on screen.
Regards,
Hans
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-18 8:30 ` Hans de Goede
@ 2018-06-18 8:43 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2018-06-18 8:43 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
On 18 June 2018 at 10:30, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 18-06-18 09:36, Ard Biesheuvel wrote:
>>
>> Hallo Hans,
>>
>> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> On systems where fbcon is configured for deferred console takeover, the
>>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>>> logo) until some message (e.g. an error) is printed or a graphical
>>> session takes over.
>>>
>>> Some firmware however relies on the OS to show the boot graphics
>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>>> been destroyed by e.g. the grub boot menu.
>>>
>>> This patch adds support to efifb to show the boot graphics and
>>> automatically enables this when fbcon is configured for deferred
>>> console takeover.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>
>>
>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
>> (which I will post separately), the code works as expected, i.e., it
>> redraws the boot logo based on the contents of the BGRT table.
>
>
> That is great.
>
>> However, what it doesn't do is clear the screen, which means the logo
>> is drawn on top of whatever the boot environment left behind, and I
>> end up with something like this.
>>
>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
>
>
> Hmm, less great. I'm not sure how to deal with this, on x86 it is more
> or less guaranteed that the screen is already cleared when we load and
> clearing a 4k screen means writing about 32MB, which I guess with modern
> RAM speeds is not that bad actually.
>
> I see that you got this picture by manual booting from the EFI shell,
> in what state does the firmware / bootloader normally leave the
> framebuffer?
UEFI does not usually clear the screen when it boots the default
entry, so it is entirely dependent on the boot loader that runs next.
I guess GRUB usually clears the screen unconditionally?
In any case, I think it is reasonable to clear the screen if you
redraw the logo, but I do wonder if it is safe to assume that the
background color is black.
Instead, we may decide to clear the screen before ExitBootServices()
[using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()].
> I'm asking because if normally it is either cleared
> to black, or already showing the logo I wonder if we should take the
> (small) penalty of clearing ?
>
> Given that we are talking about only 32 MB I could do a v2 which just
> memsets the rest of the screen to 0.
>
> So we get:
>
> for (y= 0; y < height; y++) {
> if (line_part_of_bgrt) {
> memset(left-of-bgrt);
> draw_bgrt_line(y);
> memset(right-of-bgrt);
> } else {
> memset(line);
> }
> }
>
> Note I've deliberately done the if on a per line
> base to keep the actual blit part of the loop
> efficient and without any extra conditionals in
> there. I also don't simply first memset the entire
> fb to 0 to avoid a flash / tearing if the bgrt
> image is already in place (which happens often on
> x86).
>
> Implementing this is easy and as said the extra execution time should
> be quite small, still I wonder what others think about this?
>
> I'm leaning towards doing the clearing / memsets since I've seen
> some firmwares leave some artifacts from not completely clearing
> things like a "Press F2 to enter setup" message, missing a few
> pixels and leaving those on screen.
>
I think the overhead of doing the clearing is not going to be the
bottleneck. But redrawing a logo on black background that was designed
to be displayed over another color is going to look really ugly ...
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 8:43 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2018-06-18 8:43 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
On 18 June 2018 at 10:30, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 18-06-18 09:36, Ard Biesheuvel wrote:
>>
>> Hallo Hans,
>>
>> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> On systems where fbcon is configured for deferred console takeover, the
>>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>>> logo) until some message (e.g. an error) is printed or a graphical
>>> session takes over.
>>>
>>> Some firmware however relies on the OS to show the boot graphics
>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>>> been destroyed by e.g. the grub boot menu.
>>>
>>> This patch adds support to efifb to show the boot graphics and
>>> automatically enables this when fbcon is configured for deferred
>>> console takeover.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>
>>
>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
>> (which I will post separately), the code works as expected, i.e., it
>> redraws the boot logo based on the contents of the BGRT table.
>
>
> That is great.
>
>> However, what it doesn't do is clear the screen, which means the logo
>> is drawn on top of whatever the boot environment left behind, and I
>> end up with something like this.
>>
>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
>
>
> Hmm, less great. I'm not sure how to deal with this, on x86 it is more
> or less guaranteed that the screen is already cleared when we load and
> clearing a 4k screen means writing about 32MB, which I guess with modern
> RAM speeds is not that bad actually.
>
> I see that you got this picture by manual booting from the EFI shell,
> in what state does the firmware / bootloader normally leave the
> framebuffer?
UEFI does not usually clear the screen when it boots the default
entry, so it is entirely dependent on the boot loader that runs next.
I guess GRUB usually clears the screen unconditionally?
In any case, I think it is reasonable to clear the screen if you
redraw the logo, but I do wonder if it is safe to assume that the
background color is black.
Instead, we may decide to clear the screen before ExitBootServices()
[using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()].
> I'm asking because if normally it is either cleared
> to black, or already showing the logo I wonder if we should take the
> (small) penalty of clearing ?
>
> Given that we are talking about only 32 MB I could do a v2 which just
> memsets the rest of the screen to 0.
>
> So we get:
>
> for (y= 0; y < height; y++) {
> if (line_part_of_bgrt) {
> memset(left-of-bgrt);
> draw_bgrt_line(y);
> memset(right-of-bgrt);
> } else {
> memset(line);
> }
> }
>
> Note I've deliberately done the if on a per line
> base to keep the actual blit part of the loop
> efficient and without any extra conditionals in
> there. I also don't simply first memset the entire
> fb to 0 to avoid a flash / tearing if the bgrt
> image is already in place (which happens often on
> x86).
>
> Implementing this is easy and as said the extra execution time should
> be quite small, still I wonder what others think about this?
>
> I'm leaning towards doing the clearing / memsets since I've seen
> some firmwares leave some artifacts from not completely clearing
> things like a "Press F2 to enter setup" message, missing a few
> pixels and leaving those on screen.
>
I think the overhead of doing the clearing is not going to be the
bottleneck. But redrawing a logo on black background that was designed
to be displayed over another color is going to look really ugly ...
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-18 8:43 ` Ard Biesheuvel
@ 2018-06-18 9:13 ` Hans de Goede
-1 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-18 9:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
Hi,
On 18-06-18 10:43, Ard Biesheuvel wrote:
> On 18 June 2018 at 10:30, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 18-06-18 09:36, Ard Biesheuvel wrote:
>>>
>>> Hallo Hans,
>>>
>>> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> On systems where fbcon is configured for deferred console takeover, the
>>>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>>>> logo) until some message (e.g. an error) is printed or a graphical
>>>> session takes over.
>>>>
>>>> Some firmware however relies on the OS to show the boot graphics
>>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>>>> been destroyed by e.g. the grub boot menu.
>>>>
>>>> This patch adds support to efifb to show the boot graphics and
>>>> automatically enables this when fbcon is configured for deferred
>>>> console takeover.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>>
>>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
>>> (which I will post separately), the code works as expected, i.e., it
>>> redraws the boot logo based on the contents of the BGRT table.
>>
>>
>> That is great.
>>
>>> However, what it doesn't do is clear the screen, which means the logo
>>> is drawn on top of whatever the boot environment left behind, and I
>>> end up with something like this.
>>>
>>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
>>
>>
>> Hmm, less great. I'm not sure how to deal with this, on x86 it is more
>> or less guaranteed that the screen is already cleared when we load and
>> clearing a 4k screen means writing about 32MB, which I guess with modern
>> RAM speeds is not that bad actually.
>>
>> I see that you got this picture by manual booting from the EFI shell,
>> in what state does the firmware / bootloader normally leave the
>> framebuffer?
>
> UEFI does not usually clear the screen when it boots the default
> entry, so it is entirely dependent on the boot loader that runs next.
> I guess GRUB usually clears the screen unconditionally?
It depends, GRUB either leaves it completely alone (leaving the
BGRT graphics which the firmware hopefully has put up already
in place) or if it actually draws anything, then it clears iit
before starting the kernel.
> In any case, I think it is reasonable to clear the screen if you
> redraw the logo, but I do wonder if it is safe to assume that the
> background color is black.
>
> Instead, we may decide to clear the screen before ExitBootServices()
> [using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()].
On most x86 machines (but not all, GRR) the firmware itself will
draw the logo already. I actually have grub patches pending to make
it not do any text-output related calls at all unless it actually
has a message / menu it wants to display.
Calling EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen() will cause
a bootup sequence like this:
firmware draws logo
clearscreen
redraw logo
Which will cause an ugly flash to black. Where as the purpose
is to have a smooth boot with the logo being on screen all
the time without any flickers / flashes.
I've never seen a machine where the background is not black,
the background not being black would also break the spinning
dots which microsofts draws on top of the background while
booting, so a memset to 0 seems sensible to me.
>> I'm asking because if normally it is either cleared
>> to black, or already showing the logo I wonder if we should take the
>> (small) penalty of clearing ?
>>
>> Given that we are talking about only 32 MB I could do a v2 which just
>> memsets the rest of the screen to 0.
>>
>> So we get:
>>
>> for (y= 0; y < height; y++) {
>> if (line_part_of_bgrt) {
>> memset(left-of-bgrt);
>> draw_bgrt_line(y);
>> memset(right-of-bgrt);
>> } else {
>> memset(line);
>> }
>> }
>>
>> Note I've deliberately done the if on a per line
>> base to keep the actual blit part of the loop
>> efficient and without any extra conditionals in
>> there. I also don't simply first memset the entire
>> fb to 0 to avoid a flash / tearing if the bgrt
>> image is already in place (which happens often on
>> x86).
>>
>> Implementing this is easy and as said the extra execution time should
>> be quite small, still I wonder what others think about this?
>>
>> I'm leaning towards doing the clearing / memsets since I've seen
>> some firmwares leave some artifacts from not completely clearing
>> things like a "Press F2 to enter setup" message, missing a few
>> pixels and leaving those on screen.
>>
>
> I think the overhead of doing the clearing is not going to be the
> bottleneck. But redrawing a logo on black background that was designed
> to be displayed over another color is going to look really ugly ...
Do you know of any examples of this ?
There seems to be no known way to get the background color, so black /
all 0 seems the be the best bet.
I would expect any non black background logos to simply be screen
filling.
Regards,
Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 9:13 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-18 9:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
Hi,
On 18-06-18 10:43, Ard Biesheuvel wrote:
> On 18 June 2018 at 10:30, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 18-06-18 09:36, Ard Biesheuvel wrote:
>>>
>>> Hallo Hans,
>>>
>>> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> On systems where fbcon is configured for deferred console takeover, the
>>>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>>>> logo) until some message (e.g. an error) is printed or a graphical
>>>> session takes over.
>>>>
>>>> Some firmware however relies on the OS to show the boot graphics
>>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>>>> been destroyed by e.g. the grub boot menu.
>>>>
>>>> This patch adds support to efifb to show the boot graphics and
>>>> automatically enables this when fbcon is configured for deferred
>>>> console takeover.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>>
>>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
>>> (which I will post separately), the code works as expected, i.e., it
>>> redraws the boot logo based on the contents of the BGRT table.
>>
>>
>> That is great.
>>
>>> However, what it doesn't do is clear the screen, which means the logo
>>> is drawn on top of whatever the boot environment left behind, and I
>>> end up with something like this.
>>>
>>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
>>
>>
>> Hmm, less great. I'm not sure how to deal with this, on x86 it is more
>> or less guaranteed that the screen is already cleared when we load and
>> clearing a 4k screen means writing about 32MB, which I guess with modern
>> RAM speeds is not that bad actually.
>>
>> I see that you got this picture by manual booting from the EFI shell,
>> in what state does the firmware / bootloader normally leave the
>> framebuffer?
>
> UEFI does not usually clear the screen when it boots the default
> entry, so it is entirely dependent on the boot loader that runs next.
> I guess GRUB usually clears the screen unconditionally?
It depends, GRUB either leaves it completely alone (leaving the
BGRT graphics which the firmware hopefully has put up already
in place) or if it actually draws anything, then it clears iit
before starting the kernel.
> In any case, I think it is reasonable to clear the screen if you
> redraw the logo, but I do wonder if it is safe to assume that the
> background color is black.
>
> Instead, we may decide to clear the screen before ExitBootServices()
> [using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()].
On most x86 machines (but not all, GRR) the firmware itself will
draw the logo already. I actually have grub patches pending to make
it not do any text-output related calls at all unless it actually
has a message / menu it wants to display.
Calling EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen() will cause
a bootup sequence like this:
firmware draws logo
clearscreen
redraw logo
Which will cause an ugly flash to black. Where as the purpose
is to have a smooth boot with the logo being on screen all
the time without any flickers / flashes.
I've never seen a machine where the background is not black,
the background not being black would also break the spinning
dots which microsofts draws on top of the background while
booting, so a memset to 0 seems sensible to me.
>> I'm asking because if normally it is either cleared
>> to black, or already showing the logo I wonder if we should take the
>> (small) penalty of clearing ?
>>
>> Given that we are talking about only 32 MB I could do a v2 which just
>> memsets the rest of the screen to 0.
>>
>> So we get:
>>
>> for (y= 0; y < height; y++) {
>> if (line_part_of_bgrt) {
>> memset(left-of-bgrt);
>> draw_bgrt_line(y);
>> memset(right-of-bgrt);
>> } else {
>> memset(line);
>> }
>> }
>>
>> Note I've deliberately done the if on a per line
>> base to keep the actual blit part of the loop
>> efficient and without any extra conditionals in
>> there. I also don't simply first memset the entire
>> fb to 0 to avoid a flash / tearing if the bgrt
>> image is already in place (which happens often on
>> x86).
>>
>> Implementing this is easy and as said the extra execution time should
>> be quite small, still I wonder what others think about this?
>>
>> I'm leaning towards doing the clearing / memsets since I've seen
>> some firmwares leave some artifacts from not completely clearing
>> things like a "Press F2 to enter setup" message, missing a few
>> pixels and leaving those on screen.
>>
>
> I think the overhead of doing the clearing is not going to be the
> bottleneck. But redrawing a logo on black background that was designed
> to be displayed over another color is going to look really ugly ...
Do you know of any examples of this ?
There seems to be no known way to get the background color, so black /
all 0 seems the be the best bet.
I would expect any non black background logos to simply be screen
filling.
Regards,
Hans
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-18 9:13 ` Hans de Goede
@ 2018-06-18 10:43 ` Ard Biesheuvel
-1 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2018-06-18 10:43 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
On 18 June 2018 at 11:13, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 18-06-18 10:43, Ard Biesheuvel wrote:
>>
>> On 18 June 2018 at 10:30, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 18-06-18 09:36, Ard Biesheuvel wrote:
>>>>
>>>>
>>>> Hallo Hans,
>>>>
>>>> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>>
>>>>> On systems where fbcon is configured for deferred console takeover, the
>>>>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>>>>> logo) until some message (e.g. an error) is printed or a graphical
>>>>> session takes over.
>>>>>
>>>>> Some firmware however relies on the OS to show the boot graphics
>>>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>>>>> been destroyed by e.g. the grub boot menu.
>>>>>
>>>>> This patch adds support to efifb to show the boot graphics and
>>>>> automatically enables this when fbcon is configured for deferred
>>>>> console takeover.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>
>>>>
>>>>
>>>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
>>>> (which I will post separately), the code works as expected, i.e., it
>>>> redraws the boot logo based on the contents of the BGRT table.
>>>
>>>
>>>
>>> That is great.
>>>
>>>> However, what it doesn't do is clear the screen, which means the logo
>>>> is drawn on top of whatever the boot environment left behind, and I
>>>> end up with something like this.
>>>>
>>>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
>>>
>>>
>>>
>>> Hmm, less great. I'm not sure how to deal with this, on x86 it is more
>>> or less guaranteed that the screen is already cleared when we load and
>>> clearing a 4k screen means writing about 32MB, which I guess with modern
>>> RAM speeds is not that bad actually.
>>>
>>> I see that you got this picture by manual booting from the EFI shell,
>>> in what state does the firmware / bootloader normally leave the
>>> framebuffer?
>>
>>
>> UEFI does not usually clear the screen when it boots the default
>> entry, so it is entirely dependent on the boot loader that runs next.
>> I guess GRUB usually clears the screen unconditionally?
>
>
> It depends, GRUB either leaves it completely alone (leaving the
> BGRT graphics which the firmware hopefully has put up already
> in place) or if it actually draws anything, then it clears iit
> before starting the kernel.
>
>> In any case, I think it is reasonable to clear the screen if you
>> redraw the logo, but I do wonder if it is safe to assume that the
>> background color is black.
>>
>> Instead, we may decide to clear the screen before ExitBootServices()
>> [using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()].
>
>
> On most x86 machines (but not all, GRR) the firmware itself will
> draw the logo already. I actually have grub patches pending to make
> it not do any text-output related calls at all unless it actually
> has a message / menu it wants to display.
>
> Calling EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen() will cause
> a bootup sequence like this:
>
> firmware draws logo
> clearscreen
> redraw logo
>
> Which will cause an ugly flash to black. Where as the purpose
> is to have a smooth boot with the logo being on screen all
> the time without any flickers / flashes.
>
> I've never seen a machine where the background is not black,
> the background not being black would also break the spinning
> dots which microsofts draws on top of the background while
> booting, so a memset to 0 seems sensible to me.
>
>
>>> I'm asking because if normally it is either cleared
>>> to black, or already showing the logo I wonder if we should take the
>>> (small) penalty of clearing ?
>>>
>>> Given that we are talking about only 32 MB I could do a v2 which just
>>> memsets the rest of the screen to 0.
>>>
>>> So we get:
>>>
>>> for (y= 0; y < height; y++) {
>>> if (line_part_of_bgrt) {
>>> memset(left-of-bgrt);
>>> draw_bgrt_line(y);
>>> memset(right-of-bgrt);
>>> } else {
>>> memset(line);
>>> }
>>> }
>>>
>>> Note I've deliberately done the if on a per line
>>> base to keep the actual blit part of the loop
>>> efficient and without any extra conditionals in
>>> there. I also don't simply first memset the entire
>>> fb to 0 to avoid a flash / tearing if the bgrt
>>> image is already in place (which happens often on
>>> x86).
>>>
>>> Implementing this is easy and as said the extra execution time should
>>> be quite small, still I wonder what others think about this?
>>>
>>> I'm leaning towards doing the clearing / memsets since I've seen
>>> some firmwares leave some artifacts from not completely clearing
>>> things like a "Press F2 to enter setup" message, missing a few
>>> pixels and leaving those on screen.
>>>
>>
>> I think the overhead of doing the clearing is not going to be the
>> bottleneck. But redrawing a logo on black background that was designed
>> to be displayed over another color is going to look really ugly ...
>
>
> Do you know of any examples of this ?
>
> There seems to be no known way to get the background color, so black /
> all 0 seems the be the best bet.
>
> I would expect any non black background logos to simply be screen
> filling.
>
Ubuntu's GRUB actually fills the screen with a dark grey background,
so if the background color in the logo is black, it may look rather
nasty. Also, it seems to do so unconditionally, rather than leave the
framebuffer contents alone when booting the default entry in hidden
mode.
In any case, I guess it will be up to the distros to fine tune this
once these changes have landed. But clearing the screen to black seems
like a worthwhile improvement.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 10:43 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2018-06-18 10:43 UTC (permalink / raw)
To: Hans de Goede
Cc: open list:EFIFB FRAMEBUFFER DRIVER, linux-efi, dri-devel,
Bartlomiej Zolnierkiewicz
On 18 June 2018 at 11:13, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 18-06-18 10:43, Ard Biesheuvel wrote:
>>
>> On 18 June 2018 at 10:30, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 18-06-18 09:36, Ard Biesheuvel wrote:
>>>>
>>>>
>>>> Hallo Hans,
>>>>
>>>> On 17 June 2018 at 17:32, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>>
>>>>> On systems where fbcon is configured for deferred console takeover, the
>>>>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>>>>> logo) until some message (e.g. an error) is printed or a graphical
>>>>> session takes over.
>>>>>
>>>>> Some firmware however relies on the OS to show the boot graphics
>>>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>>>>> been destroyed by e.g. the grub boot menu.
>>>>>
>>>>> This patch adds support to efifb to show the boot graphics and
>>>>> automatically enables this when fbcon is configured for deferred
>>>>> console takeover.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>
>>>>
>>>>
>>>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak
>>>> (which I will post separately), the code works as expected, i.e., it
>>>> redraws the boot logo based on the contents of the BGRT table.
>>>
>>>
>>>
>>> That is great.
>>>
>>>> However, what it doesn't do is clear the screen, which means the logo
>>>> is drawn on top of whatever the boot environment left behind, and I
>>>> end up with something like this.
>>>>
>>>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png
>>>
>>>
>>>
>>> Hmm, less great. I'm not sure how to deal with this, on x86 it is more
>>> or less guaranteed that the screen is already cleared when we load and
>>> clearing a 4k screen means writing about 32MB, which I guess with modern
>>> RAM speeds is not that bad actually.
>>>
>>> I see that you got this picture by manual booting from the EFI shell,
>>> in what state does the firmware / bootloader normally leave the
>>> framebuffer?
>>
>>
>> UEFI does not usually clear the screen when it boots the default
>> entry, so it is entirely dependent on the boot loader that runs next.
>> I guess GRUB usually clears the screen unconditionally?
>
>
> It depends, GRUB either leaves it completely alone (leaving the
> BGRT graphics which the firmware hopefully has put up already
> in place) or if it actually draws anything, then it clears iit
> before starting the kernel.
>
>> In any case, I think it is reasonable to clear the screen if you
>> redraw the logo, but I do wonder if it is safe to assume that the
>> background color is black.
>>
>> Instead, we may decide to clear the screen before ExitBootServices()
>> [using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()].
>
>
> On most x86 machines (but not all, GRR) the firmware itself will
> draw the logo already. I actually have grub patches pending to make
> it not do any text-output related calls at all unless it actually
> has a message / menu it wants to display.
>
> Calling EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen() will cause
> a bootup sequence like this:
>
> firmware draws logo
> clearscreen
> redraw logo
>
> Which will cause an ugly flash to black. Where as the purpose
> is to have a smooth boot with the logo being on screen all
> the time without any flickers / flashes.
>
> I've never seen a machine where the background is not black,
> the background not being black would also break the spinning
> dots which microsofts draws on top of the background while
> booting, so a memset to 0 seems sensible to me.
>
>
>>> I'm asking because if normally it is either cleared
>>> to black, or already showing the logo I wonder if we should take the
>>> (small) penalty of clearing ?
>>>
>>> Given that we are talking about only 32 MB I could do a v2 which just
>>> memsets the rest of the screen to 0.
>>>
>>> So we get:
>>>
>>> for (y= 0; y < height; y++) {
>>> if (line_part_of_bgrt) {
>>> memset(left-of-bgrt);
>>> draw_bgrt_line(y);
>>> memset(right-of-bgrt);
>>> } else {
>>> memset(line);
>>> }
>>> }
>>>
>>> Note I've deliberately done the if on a per line
>>> base to keep the actual blit part of the loop
>>> efficient and without any extra conditionals in
>>> there. I also don't simply first memset the entire
>>> fb to 0 to avoid a flash / tearing if the bgrt
>>> image is already in place (which happens often on
>>> x86).
>>>
>>> Implementing this is easy and as said the extra execution time should
>>> be quite small, still I wonder what others think about this?
>>>
>>> I'm leaning towards doing the clearing / memsets since I've seen
>>> some firmwares leave some artifacts from not completely clearing
>>> things like a "Press F2 to enter setup" message, missing a few
>>> pixels and leaving those on screen.
>>>
>>
>> I think the overhead of doing the clearing is not going to be the
>> bottleneck. But redrawing a logo on black background that was designed
>> to be displayed over another color is going to look really ugly ...
>
>
> Do you know of any examples of this ?
>
> There seems to be no known way to get the background color, so black /
> all 0 seems the be the best bet.
>
> I would expect any non black background logos to simply be screen
> filling.
>
Ubuntu's GRUB actually fills the screen with a dark grey background,
so if the background color in the logo is black, it may look rather
nasty. Also, it seems to do so unconditionally, rather than leave the
framebuffer contents alone when booting the default entry in hidden
mode.
In any case, I guess it will be up to the distros to fine tune this
once these changes have landed. But clearing the screen to black seems
like a worthwhile improvement.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-17 15:32 ` Hans de Goede
@ 2018-06-18 8:53 ` Môshe van der Sterre
-1 siblings, 0 replies; 26+ messages in thread
From: Môshe van der Sterre @ 2018-06-18 8:53 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-efi, linux-fbdev, Bartlomiej Zolnierkiewicz, dri-devel,
Ard Biesheuvel
Hi Hans,
On 06/17/2018 05:32 PM, Hans de Goede wrote:
> On systems where fbcon is configured for deferred console takeover, the
> intend is for the framebuffer to show the boot graphics (e.g a vendor
> logo) until some message (e.g. an error) is printed or a graphical
> session takes over.
>
> Some firmware however relies on the OS to show the boot graphics
> (indicated by bgrt_tab.status being 0) and the boot graphics may have
> been destroyed by e.g. the grub boot menu.
It may be clearer to just say that the boot graphics may have been destroyed. The reference to the status field and firmware expectations only confuses the intention of this patch, imho.
(This ties in to what I say below)
> This patch adds support to efifb to show the boot graphics and
> automatically enables this when fbcon is configured for deferred
> console takeover.
>
> + /*
> + * We do not check bgrt_tab.status here because this seems to only
> + * reflect if the firmware has shown the boot graphics at all, if it
> + * later got destroyed by something status will still be 1.
> + * Since we draw the exact same graphic at the exact same place this
> + * will not lead to any tearing if the boot graphic is already there.
> + */
I agree that ignoring bgrt_tab.status is the absolute best option.
The status (valid-bit) can, in the real world, be any value with any meaning.
I checked this on a few machines as part of commit 66dbe99cfe30.
- My workstation always has 0.
- An old server that I checked always has 1.
- My laptop has 1 if the boot is uninterrupted, 0 if I request the UEFI boot menu.
So, I have the same reservation about this comment as I have about the commit message. Imho, simply mentioning that the status field cannot be relied upon (in any case), would get the point across.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 8:53 ` Môshe van der Sterre
0 siblings, 0 replies; 26+ messages in thread
From: Môshe van der Sterre @ 2018-06-18 8:53 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-efi, linux-fbdev, Bartlomiej Zolnierkiewicz, dri-devel,
Ard Biesheuvel
Hi Hans,
On 06/17/2018 05:32 PM, Hans de Goede wrote:
> On systems where fbcon is configured for deferred console takeover, the
> intend is for the framebuffer to show the boot graphics (e.g a vendor
> logo) until some message (e.g. an error) is printed or a graphical
> session takes over.
>
> Some firmware however relies on the OS to show the boot graphics
> (indicated by bgrt_tab.status being 0) and the boot graphics may have
> been destroyed by e.g. the grub boot menu.
It may be clearer to just say that the boot graphics may have been destroyed. The reference to the status field and firmware expectations only confuses the intention of this patch, imho.
(This ties in to what I say below)
> This patch adds support to efifb to show the boot graphics and
> automatically enables this when fbcon is configured for deferred
> console takeover.
>
> + /*
> + * We do not check bgrt_tab.status here because this seems to only
> + * reflect if the firmware has shown the boot graphics at all, if it
> + * later got destroyed by something status will still be 1.
> + * Since we draw the exact same graphic at the exact same place this
> + * will not lead to any tearing if the boot graphic is already there.
> + */
I agree that ignoring bgrt_tab.status is the absolute best option.
The status (valid-bit) can, in the real world, be any value with any meaning.
I checked this on a few machines as part of commit 66dbe99cfe30.
- My workstation always has 0.
- An old server that I checked always has 1.
- My laptop has 1 if the boot is uninterrupted, 0 if I request the UEFI boot menu.
So, I have the same reservation about this comment as I have about the commit message. Imho, simply mentioning that the status field cannot be relied upon (in any case), would get the point across.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
2018-06-18 8:53 ` Môshe van der Sterre
@ 2018-06-18 9:08 ` Hans de Goede
-1 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-18 9:08 UTC (permalink / raw)
To: Môshe van der Sterre
Cc: linux-efi, linux-fbdev, Bartlomiej Zolnierkiewicz, dri-devel,
Ard Biesheuvel
Hi,
On 18-06-18 10:53, Môshe van der Sterre wrote:
> Hi Hans,
>
> On 06/17/2018 05:32 PM, Hans de Goede wrote:
>> On systems where fbcon is configured for deferred console takeover, the
>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>> logo) until some message (e.g. an error) is printed or a graphical
>> session takes over.
>>
>> Some firmware however relies on the OS to show the boot graphics
>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>> been destroyed by e.g. the grub boot menu.
>
> It may be clearer to just say that the boot graphics may have been destroyed. The reference to the status field and firmware expectations only confuses the intention of this patch, imho.
> (This ties in to what I say below)
>
>> This patch adds support to efifb to show the boot graphics and
>> automatically enables this when fbcon is configured for deferred
>> console takeover.
>>
>> + /*
>> + * We do not check bgrt_tab.status here because this seems to only
>> + * reflect if the firmware has shown the boot graphics at all, if it
>> + * later got destroyed by something status will still be 1.
>> + * Since we draw the exact same graphic at the exact same place this
>> + * will not lead to any tearing if the boot graphic is already there.
>> + */
>
> I agree that ignoring bgrt_tab.status is the absolute best option.
>
> The status (valid-bit) can, in the real world, be any value with any meaning.
> I checked this on a few machines as part of commit 66dbe99cfe30.
> - My workstation always has 0.
> - An old server that I checked always has 1.
> - My laptop has 1 if the boot is uninterrupted, 0 if I request the UEFI boot menu.
>
> So, I have the same reservation about this comment as I have about the commit message. Imho, simply mentioning that the status field cannot be relied upon (in any case), would get the point across.
Ok, I will simplify both the commit message and comment bits to just state
that the status field is unreliable.
Regards,
Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
@ 2018-06-18 9:08 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2018-06-18 9:08 UTC (permalink / raw)
To: Môshe van der Sterre
Cc: linux-efi, linux-fbdev, Bartlomiej Zolnierkiewicz, dri-devel,
Ard Biesheuvel
Hi,
On 18-06-18 10:53, Môshe van der Sterre wrote:
> Hi Hans,
>
> On 06/17/2018 05:32 PM, Hans de Goede wrote:
>> On systems where fbcon is configured for deferred console takeover, the
>> intend is for the framebuffer to show the boot graphics (e.g a vendor
>> logo) until some message (e.g. an error) is printed or a graphical
>> session takes over.
>>
>> Some firmware however relies on the OS to show the boot graphics
>> (indicated by bgrt_tab.status being 0) and the boot graphics may have
>> been destroyed by e.g. the grub boot menu.
>
> It may be clearer to just say that the boot graphics may have been destroyed. The reference to the status field and firmware expectations only confuses the intention of this patch, imho.
> (This ties in to what I say below)
>
>> This patch adds support to efifb to show the boot graphics and
>> automatically enables this when fbcon is configured for deferred
>> console takeover.
>>
>> + /*
>> + * We do not check bgrt_tab.status here because this seems to only
>> + * reflect if the firmware has shown the boot graphics at all, if it
>> + * later got destroyed by something status will still be 1.
>> + * Since we draw the exact same graphic at the exact same place this
>> + * will not lead to any tearing if the boot graphic is already there.
>> + */
>
> I agree that ignoring bgrt_tab.status is the absolute best option.
>
> The status (valid-bit) can, in the real world, be any value with any meaning.
> I checked this on a few machines as part of commit 66dbe99cfe30.
> - My workstation always has 0.
> - An old server that I checked always has 1.
> - My laptop has 1 if the boot is uninterrupted, 0 if I request the UEFI boot menu.
>
> So, I have the same reservation about this comment as I have about the commit message. Imho, simply mentioning that the status field cannot be relied upon (in any case), would get the point across.
Ok, I will simplify both the commit message and comment bits to just state
that the status field is unreliable.
Regards,
Hans
^ permalink raw reply [flat|nested] 26+ messages in thread
