[PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Hans de Goede]

bgrt_image_size is necessary to (optionally) show the boot graphics from
the efifb code. The efifb driver is a platform driver, using a normal
driver probe() driver callback. So even though it is always builtin it
cannot reference __initdata.

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/firmware/efi/efi-bgrt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
index 50793fda7819..b22ccfb0c991 100644
--- a/drivers/firmware/efi/efi-bgrt.c
+++ b/drivers/firmware/efi/efi-bgrt.c
@@ -20,7 +20,7 @@
 #include <linux/efi-bgrt.h>
 
 struct acpi_table_bgrt bgrt_tab;
-size_t __initdata bgrt_image_size;
+size_t bgrt_image_size;
 
 struct bmp_header {
 	u16 id;
-- 
2.17.1

[PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer

[Hans de Goede]

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 relies on the OS to show the boot graphics.

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>
---
Changes in v2:
-Simplify the comment about acpi_bgrt.status
-Clear the parts of the screen which don't contain the logo to black
 (memset to 0), rather then leaving them as is
---
 drivers/video/fbdev/efifb.c | 140 ++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 46a4484e3da7..fa01eecc0a55 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,121 @@ 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_copy_bmp(u8 *src, u32 *dst, int width, struct screen_info *si)
+{
+	u8 r, g, b;
+
+	while (width--) {
+		b = *src++;
+		g = *src++;
+		r = *src++;
+		*dst++ = (r << si->red_pos)   |
+			 (g << si->green_pos) |
+			 (b << si->blue_pos);
+	}
+}
+
+static void efifb_show_boot_graphics(struct fb_info *info)
+{
+	u32 bmp_width, bmp_height, bmp_pitch, screen_pitch, dst_x, y, src_y;
+	struct screen_info *si = &screen_info;
+	struct bmp_file_header *file_header;
+	struct bmp_dib_header *dib_header;
+	void *bgrt_image = NULL;
+	u8 *dst = info->screen_base;
+
+	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;
+
+	/* bgrt_tab.status is unreliable, so we don't check it */
+
+	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");
+
+	for (y = 0; y < si->lfb_height; y++, dst += si->lfb_linelength) {
+		/* Only background? */
+		if (y < bgrt_tab.image_offset_y ||
+		    y >= (bgrt_tab.image_offset_y + bmp_height)) {
+			memset(dst, 0, 4 * si->lfb_width);
+			continue;
+		}
+
+		src_y = y - bgrt_tab.image_offset_y;
+		/* Positive header height means upside down row order */
+		if (dib_header->height > 0)
+			src_y = (bmp_height - 1) - src_y;
+
+		memset(dst, 0, bgrt_tab.image_offset_x * 4);
+		dst_x = bgrt_tab.image_offset_x;
+		efifb_copy_bmp(bgrt_image + file_header->bitmap_offset +
+					    src_y * bmp_pitch,
+			       (u32 *)dst + dst_x, bmp_width, si);
+		dst_x += bmp_width;
+		memset((u32 *)dst + dst_x, 0, (si->lfb_width - dst_x) * 4);
+	}
+
+	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 +421,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

Re: [PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Hans de Goede]

Bartlomiej,

Now that the fbcon deferred console takeover patches have been
merged I believe this series can be merged too ?

Note the first patch has an ack from Ard for merging the
1 line efi change through the fbdev tree.

Regards,

Hans


On 18-06-18 17:13, Hans de Goede wrote:
> bgrt_image_size is necessary to (optionally) show the boot graphics from
> the efifb code. The efifb driver is a platform driver, using a normal
> driver probe() driver callback. So even though it is always builtin it
> cannot reference __initdata.
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/firmware/efi/efi-bgrt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
> index 50793fda7819..b22ccfb0c991 100644
> --- a/drivers/firmware/efi/efi-bgrt.c
> +++ b/drivers/firmware/efi/efi-bgrt.c
> @@ -20,7 +20,7 @@
>   #include <linux/efi-bgrt.h>
>   
>   struct acpi_table_bgrt bgrt_tab;
> -size_t __initdata bgrt_image_size;
> +size_t bgrt_image_size;
>   
>   struct bmp_header {
>   	u16 id;
> 

Re: [PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Ard Biesheuvel]

On 2 July 2018 at 13:26, Hans de Goede <hdegoede@redhat.com> wrote:
> Bartlomiej,
>
> Now that the fbcon deferred console takeover patches have been
> merged I believe this series can be merged too ?
>
> Note the first patch has an ack from Ard for merging the
> 1 line efi change through the fbdev tree.
>

... or I could take everything through the efi tree instead, as
already discussed between Bartlomiej and me in the context of another
patch series that touches both the fbdev and efi trees.

Bartlomiej, that would require your ack on patch

[PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer

https://marc.info/?l=linux-fbdev&m\x152933484616993&w=2

so if you're ok with that, I will queue both of these for v4.19


>
> On 18-06-18 17:13, Hans de Goede wrote:
>>
>> bgrt_image_size is necessary to (optionally) show the boot graphics from
>> the efifb code. The efifb driver is a platform driver, using a normal
>> driver probe() driver callback. So even though it is always builtin it
>> cannot reference __initdata.
>>
>> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/firmware/efi/efi-bgrt.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/efi/efi-bgrt.c
>> b/drivers/firmware/efi/efi-bgrt.c
>> index 50793fda7819..b22ccfb0c991 100644
>> --- a/drivers/firmware/efi/efi-bgrt.c
>> +++ b/drivers/firmware/efi/efi-bgrt.c
>> @@ -20,7 +20,7 @@
>>   #include <linux/efi-bgrt.h>
>>     struct acpi_table_bgrt bgrt_tab;
>> -size_t __initdata bgrt_image_size;
>> +size_t bgrt_image_size;
>>     struct bmp_header {
>>         u16 id;
>>
>

Re: [PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Bartlomiej Zolnierkiewicz]

On Monday, July 02, 2018 01:46:09 PM Ard Biesheuvel wrote:
> On 2 July 2018 at 13:26, Hans de Goede <hdegoede@redhat.com> wrote:
> > Bartlomiej,
> >
> > Now that the fbcon deferred console takeover patches have been
> > merged I believe this series can be merged too ?
> >
> > Note the first patch has an ack from Ard for merging the
> > 1 line efi change through the fbdev tree.
> >
> 
> ... or I could take everything through the efi tree instead, as
> already discussed between Bartlomiej and me in the context of another
> patch series that touches both the fbdev and efi trees.
> 
> Bartlomiej, that would require your ack on patch
> 
> [PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
> 
> https://marc.info/?l=linux-fbdev&m\x152933484616993&w=2
> 
> so if you're ok with that, I will queue both of these for v4.19

I would really prefer to merge this patchset through fbdev tree
as efi tree doesn't have fbcon deferred console takeover patches
(which are required by efifb changes under discussion).

> > On 18-06-18 17:13, Hans de Goede wrote:
> >>
> >> bgrt_image_size is necessary to (optionally) show the boot graphics from
> >> the efifb code. The efifb driver is a platform driver, using a normal
> >> driver probe() driver callback. So even though it is always builtin it
> >> cannot reference __initdata.
> >>
> >> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   drivers/firmware/efi/efi-bgrt.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/firmware/efi/efi-bgrt.c
> >> b/drivers/firmware/efi/efi-bgrt.c
> >> index 50793fda7819..b22ccfb0c991 100644
> >> --- a/drivers/firmware/efi/efi-bgrt.c
> >> +++ b/drivers/firmware/efi/efi-bgrt.c
> >> @@ -20,7 +20,7 @@
> >>   #include <linux/efi-bgrt.h>
> >>     struct acpi_table_bgrt bgrt_tab;
> >> -size_t __initdata bgrt_image_size;
> >> +size_t bgrt_image_size;
> >>     struct bmp_header {
> >>         u16 id;

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Ard Biesheuvel]

On 2 July 2018 at 13:57, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> On Monday, July 02, 2018 01:46:09 PM Ard Biesheuvel wrote:
>> On 2 July 2018 at 13:26, Hans de Goede <hdegoede@redhat.com> wrote:
>> > Bartlomiej,
>> >
>> > Now that the fbcon deferred console takeover patches have been
>> > merged I believe this series can be merged too ?
>> >
>> > Note the first patch has an ack from Ard for merging the
>> > 1 line efi change through the fbdev tree.
>> >
>>
>> ... or I could take everything through the efi tree instead, as
>> already discussed between Bartlomiej and me in the context of another
>> patch series that touches both the fbdev and efi trees.
>>
>> Bartlomiej, that would require your ack on patch
>>
>> [PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
>>
>> https://marc.info/?l=linux-fbdev&m\x152933484616993&w=2
>>
>> so if you're ok with that, I will queue both of these for v4.19
>
> I would really prefer to merge this patchset through fbdev tree
> as efi tree doesn't have fbcon deferred console takeover patches
> (which are required by efifb changes under discussion).
>

Ah ok, I didn't realise that. I don't think there will be any
conflicts, since the efifb changes in the efi tree and these changes
operate on different parts of the file. But let's double check before
taking stuff into -next.
(efi/next is not pulled into -next directly, but via the tip:efi tree,
and I haven't sent a pull request yet for v4.19)


>> > On 18-06-18 17:13, Hans de Goede wrote:
>> >>
>> >> bgrt_image_size is necessary to (optionally) show the boot graphics from
>> >> the efifb code. The efifb driver is a platform driver, using a normal
>> >> driver probe() driver callback. So even though it is always builtin it
>> >> cannot reference __initdata.
>> >>
>> >> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> >> ---
>> >>   drivers/firmware/efi/efi-bgrt.c | 2 +-
>> >>   1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/firmware/efi/efi-bgrt.c
>> >> b/drivers/firmware/efi/efi-bgrt.c
>> >> index 50793fda7819..b22ccfb0c991 100644
>> >> --- a/drivers/firmware/efi/efi-bgrt.c
>> >> +++ b/drivers/firmware/efi/efi-bgrt.c
>> >> @@ -20,7 +20,7 @@
>> >>   #include <linux/efi-bgrt.h>
>> >>     struct acpi_table_bgrt bgrt_tab;
>> >> -size_t __initdata bgrt_image_size;
>> >> +size_t bgrt_image_size;
>> >>     struct bmp_header {
>> >>         u16 id;
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Bartlomiej Zolnierkiewicz]

On Monday, July 02, 2018 02:02:47 PM Ard Biesheuvel wrote:
> On 2 July 2018 at 13:57, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > On Monday, July 02, 2018 01:46:09 PM Ard Biesheuvel wrote:
> >> On 2 July 2018 at 13:26, Hans de Goede <hdegoede@redhat.com> wrote:
> >> > Bartlomiej,
> >> >
> >> > Now that the fbcon deferred console takeover patches have been
> >> > merged I believe this series can be merged too ?
> >> >
> >> > Note the first patch has an ack from Ard for merging the
> >> > 1 line efi change through the fbdev tree.
> >> >
> >>
> >> ... or I could take everything through the efi tree instead, as
> >> already discussed between Bartlomiej and me in the context of another
> >> patch series that touches both the fbdev and efi trees.
> >>
> >> Bartlomiej, that would require your ack on patch
> >>
> >> [PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
> >>
> >> https://marc.info/?l=linux-fbdev&m\x152933484616993&w=2
> >>
> >> so if you're ok with that, I will queue both of these for v4.19
> >
> > I would really prefer to merge this patchset through fbdev tree
> > as efi tree doesn't have fbcon deferred console takeover patches
> > (which are required by efifb changes under discussion).
> >
> 
> Ah ok, I didn't realise that. I don't think there will be any
> conflicts, since the efifb changes in the efi tree and these changes
> operate on different parts of the file. But let's double check before
> taking stuff into -next.
> (efi/next is not pulled into -next directly, but via the tip:efi tree,
> and I haven't sent a pull request yet for v4.19)

I've verified this with -next from today and it auto-merged fine (for
testing purposes I've applied both patches to -next and then pulled in
efi/next from efi.git tree).

I've applied both patches to fbdev-for-next (I hope it is fine with you).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH v2 1/2] efi/bgrt: Drop __initdata from bgrt_image_size

[Ard Biesheuvel]

On 3 July 2018 at 17:24, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> On Monday, July 02, 2018 02:02:47 PM Ard Biesheuvel wrote:
>> On 2 July 2018 at 13:57, Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>> > On Monday, July 02, 2018 01:46:09 PM Ard Biesheuvel wrote:
>> >> On 2 July 2018 at 13:26, Hans de Goede <hdegoede@redhat.com> wrote:
>> >> > Bartlomiej,
>> >> >
>> >> > Now that the fbcon deferred console takeover patches have been
>> >> > merged I believe this series can be merged too ?
>> >> >
>> >> > Note the first patch has an ack from Ard for merging the
>> >> > 1 line efi change through the fbdev tree.
>> >> >
>> >>
>> >> ... or I could take everything through the efi tree instead, as
>> >> already discussed between Bartlomiej and me in the context of another
>> >> patch series that touches both the fbdev and efi trees.
>> >>
>> >> Bartlomiej, that would require your ack on patch
>> >>
>> >> [PATCH v2 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer
>> >>
>> >> https://marc.info/?l=linux-fbdev&m\x152933484616993&w=2
>> >>
>> >> so if you're ok with that, I will queue both of these for v4.19
>> >
>> > I would really prefer to merge this patchset through fbdev tree
>> > as efi tree doesn't have fbcon deferred console takeover patches
>> > (which are required by efifb changes under discussion).
>> >
>>
>> Ah ok, I didn't realise that. I don't think there will be any
>> conflicts, since the efifb changes in the efi tree and these changes
>> operate on different parts of the file. But let's double check before
>> taking stuff into -next.
>> (efi/next is not pulled into -next directly, but via the tip:efi tree,
>> and I haven't sent a pull request yet for v4.19)
>
> I've verified this with -next from today and it auto-merged fine (for
> testing purposes I've applied both patches to -next and then pulled in
> efi/next from efi.git tree).
>
> I've applied both patches to fbdev-for-next (I hope it is fine with you).
>

Absolutely fine, and thanks for double checking.