Mimoja has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: Fix bootsplash display code for optionroms ......................................................................
Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer were setup as 1027x768@16. This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel. For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 83 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/1
diff --git a/src/device/oprom/Makefile.inc b/src/device/oprom/Makefile.inc index 61970ae..9d49796 100644 --- a/src/device/oprom/Makefile.inc +++ b/src/device/oprom/Makefile.inc @@ -16,3 +16,5 @@ subdirs-$(CONFIG_PCI_OPTION_ROM_RUN_YABEL) += x86emu subdirs-$(CONFIG_PCI_OPTION_ROM_RUN_YABEL) += yabel subdirs-$(CONFIG_PCI_OPTION_ROM_RUN_REALMODE) += realmode + +ramstage-$(CONFIG_BOOTSPLASH) += bootsplash.c \ No newline at end of file diff --git a/src/device/oprom/bootsplash.c b/src/device/oprom/bootsplash.c new file mode 100644 index 0000000..a1b804c --- /dev/null +++ b/src/device/oprom/bootsplash.c @@ -0,0 +1,33 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Johanna Schander coreboot@mimoja.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <lib/jpeg.h> +#include <cbfs.h> + +#include "bootsplash.h" + +int oprom_set_bootsplash(unsigned char *framebuffer, u16 x_resolution, u16 y_resolution, + u16 fb_resolution) +{ + struct jpeg_decdata *decdata; + unsigned char *jpeg = + cbfs_boot_map_with_leak("bootsplash.jpg", CBFS_TYPE_BOOTSPLASH, NULL); + if (!jpeg) { + return -1; + } + decdata = malloc(sizeof(*decdata)); + return jpeg_decode(jpeg, framebuffer, x_resolution, y_resolution, fb_resolution, + decdata); +} \ No newline at end of file diff --git a/src/device/oprom/bootsplash.h b/src/device/oprom/bootsplash.h new file mode 100644 index 0000000..0bf7f26 --- /dev/null +++ b/src/device/oprom/bootsplash.h @@ -0,0 +1,18 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Johanna Schander coreboot@mimoja.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <stdint.h> + +int oprom_set_bootsplash(unsigned char *framebuffer, u16 x_resolution, u16 y_resolution, u16 fb_resolution); diff --git a/src/device/oprom/realmode/x86.c b/src/device/oprom/realmode/x86.c index 67e550c..79586a5 100644 --- a/src/device/oprom/realmode/x86.c +++ b/src/device/oprom/realmode/x86.c @@ -18,19 +18,21 @@ #include <arch/interrupt.h> #include <arch/registers.h> #include <boot/coreboot_tables.h> -#include <cbfs.h> #include <console/console.h> #include <cpu/amd/lxdef.h> #include <cpu/amd/vr.h> #include <delay.h> #include <device/pci.h> #include <device/pci_ids.h> -#include <lib/jpeg.h> #include <pc80/i8259.h> #include <pc80/i8254.h> #include <string.h> #include <vbe.h>
+#if CONFIG(BOOTSPLASH) +#include "../bootsplash.h" +#endif + /* we use x86emu's register file representation */ #include <x86emu/regs.h>
@@ -347,12 +349,17 @@ { mode_info.video_mode = (1 << 14) | CONFIG_FRAMEBUFFER_VESA_MODE; vbe_get_mode_info(&mode_info); + + u16 x_resolution = le16_to_cpu(mode_info.vesa.x_resolution); + u16 y_resolution = le16_to_cpu(mode_info.vesa.y_resolution); + u16 fb_resolution = mode_info.vesa.bits_per_pixel; + unsigned char *framebuffer = (unsigned char *)mode_info.vesa.phys_base_ptr; printk(BIOS_DEBUG, "VBE: resolution: %dx%d@%d\n", - le16_to_cpu(mode_info.vesa.x_resolution), - le16_to_cpu(mode_info.vesa.y_resolution), - mode_info.vesa.bits_per_pixel); + x_resolution, + y_resolution, + fb_resolution); printk(BIOS_DEBUG, "VBE: framebuffer: %p\n", framebuffer); if (!framebuffer) { printk(BIOS_DEBUG, "VBE: Mode does not support linear " @@ -362,17 +369,15 @@
vbe_set_mode(&mode_info); #if CONFIG(BOOTSPLASH) - struct jpeg_decdata *decdata; - unsigned char *jpeg = cbfs_boot_map_with_leak("bootsplash.jpg", - CBFS_TYPE_BOOTSPLASH, - NULL); - if (!jpeg) { - printk(BIOS_DEBUG, "VBE: No bootsplash found.\n"); - return; + printk(BIOS_INFO, "VBE: Setting up Bootsplash\n"); + int ret = oprom_set_bootsplash(framebuffer, x_resolution, y_resolution, fb_resolution); + if (ret == -1) { + printk(BIOS_WARNING, "VBE: No bootsplash found.\n"); + } else if (ret > 0) { + printk(BIOS_WARNING, "VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", ret); + } else { + printk(BIOS_INFO, "VBE: Bootsplash loaded\n"); } - decdata = malloc(sizeof(*decdata)); - int ret = 0; - ret = jpeg_decode(jpeg, framebuffer, 1024, 768, 16, decdata); #endif }
diff --git a/src/device/oprom/yabel/vbe.c b/src/device/oprom/yabel/vbe.c index 8116c6b..5ab6c68 100644 --- a/src/device/oprom/yabel/vbe.c +++ b/src/device/oprom/yabel/vbe.c @@ -52,12 +52,12 @@ #include "interrupt.h" #include "device.h"
-#include <cbfs.h> - #include <delay.h> -#include "../../src/lib/jpeg.h"
#include <vbe.h> +#if CONFIG(BOOTSPLASH) +#include "../bootsplash.h" +#endif
// these structs only store a subset of the VBE defined fields // only those needed. @@ -747,31 +747,15 @@ vbe_set_mode(&mode_info);
#if CONFIG(BOOTSPLASH) - unsigned char *framebuffer = - (unsigned char *) le32_to_cpu(mode_info.vesa.phys_base_ptr); - DEBUG_PRINTF_VBE("FRAMEBUFFER: 0x%p\n", framebuffer); - - struct jpeg_decdata *decdata; - - /* Switching Intel IGD to 1MB video memory will break this. Who - * cares. */ - // int imagesize = 1024*768*2; - - unsigned char *jpeg = cbfs_boot_map_with_leak("bootsplash.jpg", - CBFS_TYPE_BOOTSPLASH, - NULL); - if (!jpeg) { + printk(BIOS_INFO, "VBE: Setting up Bootsplash\n"); + int ret = oprom_set_bootsplash(framebuffer, x_resolution, y_resolution, fb_resolution) + if (ret == -1) { DEBUG_PRINTF_VBE("Could not find bootsplash.jpg\n"); - return; + } else if (ret > 0){ + DEBUG_PRINTF_VBE("VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", ret); + } else { + DEBUG_PRINTF_VBE("VBE: Bootsplash loaded\n"); } - DEBUG_PRINTF_VBE("Splash at %p ...\n", jpeg); - dump(jpeg, 64); - - decdata = malloc(sizeof(*decdata)); - int ret = 0; - DEBUG_PRINTF_VBE("Decompressing boot splash screen...\n"); - ret = jpeg_decode(jpeg, framebuffer, 1024, 768, 16, decdata); - DEBUG_PRINTF_VBE("returns %x\n", ret); #endif }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: Fix bootsplash display code for optionroms ......................................................................
Patch Set 1:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/bootsplash... PS1, Line 18: int oprom_set_bootsplash(unsigned char *framebuffer, u16 x_resolution, u16 y_resolution, u16 fb_resolution); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/bootsplash... PS1, Line 27: if (!jpeg) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/bootsplash... PS1, Line 33: } adding a line without newline at end of file
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/realmode/x... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/realmode/x... PS1, Line 374: if (ret == -1) { braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/realmode/x... PS1, Line 377: printk(BIOS_WARNING, "VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", ret); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/yabel/vbe.... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/yabel/vbe.... PS1, Line 752: if (ret == -1) { braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/yabel/vbe.... PS1, Line 754: } else if (ret > 0){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/34537/1/src/device/oprom/yabel/vbe.... PS1, Line 755: DEBUG_PRINTF_VBE("VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", ret); line over 96 characters
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#2).
Change subject: Fix bootsplash display code for optionroms ......................................................................
Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer were setup as 1027x768@16. This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel. For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: Fix bootsplash display code for optionroms ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/bootsplash... PS2, Line 18: int oprom_set_bootsplash(unsigned char *framebuffer, trailing whitespace
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/bootsplash... PS2, Line 19: u16 x_resolution, u16 y_resolution, u16 fb_resolution); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/bootsplash... PS2, Line 19: u16 x_resolution, u16 y_resolution, u16 fb_resolution); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/bootsplash... PS2, Line 29: trailing whitespace
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/realmode/x... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/realmode/x... PS2, Line 377: printk(BIOS_WARNING, trailing whitespace
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/realmode/x... PS2, Line 378: "VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", trailing whitespace
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/yabel/vbe.... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/2/src/device/oprom/yabel/vbe.... PS2, Line 756: "VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", trailing whitespace
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#3).
Change subject: Fix bootsplash display code for optionroms ......................................................................
Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer were setup as 1027x768@16. This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel. For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/3
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#4).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer were setup as 1027x768@16. This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel. For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/4
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#5).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer were setup as 1027x768@16. This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel. For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/5
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 5:
(7 comments)
Nice.
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: So far the bootsplash is only correctly rendered if the framebuffer were : setup as 1027x768@16. On what device?
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: were : setup as 1027x768@16 is set up as …
(Really 1027?)
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: So far the bootsplash is only correctly rendered if the framebuffer were : setup as 1027x768@16. : This commit removes this limit by using the actual resolutions and combines : the code for x86 and yabel. : For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init. Please add an empty line between paragraphs.
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... PS5, Line 19: u16 x_resolution, u16 y_resolution, u16 fb_resolution); Please add a documentation for the function (return code), and so on.
Why u16, and not `unsigned int`?
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... PS5, Line 28: -1 Please think about using the enums `CB_SUCCESS`, and friends.
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/realmode/x... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/realmode/x... PS5, Line 372: Bootsplash bootsplash
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/realmode/x... PS5, Line 377: BIOS_WARNING BIOS_ERROR
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#6).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/6
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34537/6/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/6/src/device/oprom/bootsplash... PS6, Line 17: unsigned int x_resolution, unsigned int y_resolution, unsigned int fb_resolution); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34537/6/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/6/src/device/oprom/bootsplash... PS6, Line 21: int oprom_set_bootsplash(unsigned char *framebuffer, trailing whitespace
https://review.coreboot.org/c/coreboot/+/34537/6/src/device/oprom/bootsplash... PS6, Line 22: unsigned int x_resolution, unsigned int y_resolution, unsigned int fb_resolution) line over 96 characters
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 6:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: were : setup as 1027x768@16
is set up as … […]
Nope. Typo by me. Thanks
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: So far the bootsplash is only correctly rendered if the framebuffer were : setup as 1027x768@16.
On what device?
It was not device specific to my understanding of the word..
If the image was != 1024x768 jpeg_decode would error out, if the framebuffer were not 1024x768@16 the result would have looked wrong (And maybe even overflowed the FB).
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: So far the bootsplash is only correctly rendered if the framebuffer were : setup as 1027x768@16. : This commit removes this limit by using the actual resolutions and combines : the code for x86 and yabel. : For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Please add an empty line between paragraphs.
Done
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... PS5, Line 19: u16 x_resolution, u16 y_resolution, u16 fb_resolution);
Please add a documentation for the function (return code), and so on. […]
The rest of the optionrom Code uses u16... from the mode_info.vesa objects are 16 bit values extracted, therefore i went with it. Changed it now.
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/bootsplash... PS5, Line 28: -1
Please think about using the enums `CB_SUCCESS`, and friends.
Returning CB_ERR here
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/realmode/x... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/realmode/x... PS5, Line 372: Bootsplash
bootsplash
done
https://review.coreboot.org/c/coreboot/+/34537/5/src/device/oprom/realmode/x... PS5, Line 377: BIOS_WARNING
BIOS_ERROR
Done
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#7).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34537/7/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/7/src/device/oprom/bootsplash... PS7, Line 17: unsigned int x_resolution, unsigned int y_resolution, unsigned int fb_resolution); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34537/7/src/device/oprom/bootsplash... File src/device/oprom/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/7/src/device/oprom/bootsplash... PS7, Line 22: unsigned int x_resolution, unsigned int y_resolution, unsigned int fb_resolution) line over 96 characters
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#8).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 88 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/8
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#9).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 90 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/9
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#10).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 90 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/yabel/vbe... PS10, Line 763: "VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", ret); line over 96 characters
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 10:
Patch Set 10:
(1 comment)
Jenkins and post commit hooks cannot agree about this. Giving up now. Which way to do it?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 8:
IMHO it is now good time to move bootsplash code entirely outside oprom directory, even if the dependency still remains after this commit.
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 10:
Patch Set 8:
IMHO it is now good time to move bootsplash code entirely outside oprom directory, even if the dependency still remains after this commit.
I thought about it and decided against it: - It is only called from optionroms - It should only be compiled, if oprom execution is selected as well.
Therefore i left it there. (For now)
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/5//COMMIT_MSG@9 PS5, Line 9: So far the bootsplash is only correctly rendered if the framebuffer were : setup as 1027x768@16.
It was not device specific to my understanding of the word.. […]
Done
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#11).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/Makefile.inc A src/device/oprom/bootsplash.c A src/device/oprom/bootsplash.h M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c 5 files changed, 96 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/11
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 763: "VBE: Bootsplash could not be decoded. jpeg_decode returned %d.\n", ret); line over 96 characters
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 11:
(5 comments)
A reminder to reviewers, no board selects BOOTSPLASH so jenkins score for passing the build is not of any value.
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/realmode/... PS10, Line 359: (unsigned char *)mode_info.vesa.phys_base_ptr; Either this should have le32_to_cpu() or the two le16_to_cpu() above should be removed, for consistency.
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 58: #if CONFIG(BOOTSPLASH) No guards required. But I would appreciate a more thorough fix.
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 750: #if CONFIG(BOOTSPLASH) This is what I consider the most "broken" part calling for attention. None of the bootsplash should happen inside "vbe_set_graphics()" or even it's parent function "run_bios()".
If you change this such that bootsplash code evaluates vbe_mode_info_t after run_bios() returns, this might become much cleaner and closer to supporting native graphics init in one go.
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 758: int ret = oprom_set_bootsplash(framebuffer, x_resolution, y_resolution, fb_resolution) Missing ; at end of line?
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 760: DEBUG_PRINTF_VBE("Could not find bootsplash.jpg\n"); Strongly recommend braces here. Probably does not build currently.
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 11:
(5 comments)
Thanks for the review. Move to lib/bootsplash.c for good.
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/10/src/device/oprom/realmode/... PS10, Line 359: (unsigned char *)mode_info.vesa.phys_base_ptr;
Either this should have le32_to_cpu() or the two le16_to_cpu() above should be removed, for consiste […]
I agree. Changed to le32_to_cpu
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 58: #if CONFIG(BOOTSPLASH)
No guards required. But I would appreciate a more thorough fix.
Removed. Could you explain briefly, if you have the time, so i can improve in the future?
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 750: #if CONFIG(BOOTSPLASH)
This is what I consider the most "broken" part calling for attention. […]
Ahh! Now i get what you mean. Hopefully I did exactly that :)
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 758: int ret = oprom_set_bootsplash(framebuffer, x_resolution, y_resolution, fb_resolution)
Missing ; at end of line?
Yep. Thanks
https://review.coreboot.org/c/coreboot/+/34537/11/src/device/oprom/yabel/vbe... PS11, Line 760: DEBUG_PRINTF_VBE("Could not find bootsplash.jpg\n");
Strongly recommend braces here. Probably does not build currently.
Done
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#12).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c 4 files changed, 30 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/12
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#13).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c M src/include/vbe.h 5 files changed, 40 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/13
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 13:
(6 comments)
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/oprom/realmode/... PS13, Line 349: unsigned char *framebuffer = trailing whitespace
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/oprom/realmode/... PS13, Line 355: printk(BIOS_DEBUG, "VBE: framebuffer: %p\n" ,framebuffer); space prohibited before that ',' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/oprom/realmode/... PS13, Line 355: printk(BIOS_DEBUG, "VBE: framebuffer: %p\n" ,framebuffer); space required after that ',' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/pci_device.c@77... PS13, Line 773: if(vbe_mode_info_valid()){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/pci_device.c@77... PS13, Line 773: if(vbe_mode_info_valid()){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34537/13/src/device/pci_device.c@77... PS13, Line 778: unsigned char *framebuffer = (unsigned char *)le32_to_cpu(mode_info.vesa.phys_base_ptr); line over 96 characters
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#14).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c M src/include/vbe.h A src/lib/bootsplash.c A src/lib/bootsplash.h 7 files changed, 102 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/14
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 14:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34537/14/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/14/src/device/pci_device.c@77... PS14, Line 773: if(vbe_mode_info_valid()){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/34537/14/src/device/pci_device.c@77... PS14, Line 773: if(vbe_mode_info_valid()){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34537/14/src/device/pci_device.c@77... PS14, Line 778: unsigned char *framebuffer = (unsigned char *)le32_to_cpu(mode_info.vesa.phys_base_ptr); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34537/14/src/lib/bootsplash.h File src/lib/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/14/src/lib/bootsplash.h@25 PS14, Line 25: unsigned int y_resolution, unsigned int fb_resolution); adding a line without newline at end of file
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#15).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is setup as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c M src/include/vbe.h A src/lib/bootsplash.c A src/lib/bootsplash.h 7 files changed, 102 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/15
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 15:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 773: if(vbe_mode_info_valid()){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 773: if(vbe_mode_info_valid()){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 778: unsigned char *framebuffer = (unsigned char *)le32_to_cpu(mode_info.vesa.phys_base_ptr); line over 96 characters
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 15:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG@10 PS15, Line 10: setup The verb is spelled with a space: set up. (You can think on where the third person s would go: he/she/it sets up the table ….
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG@15 PS15, Line 15: For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init. Just for my memory, what happens if the bootsplash image has different dimensions? Is it cropped or put on the top left side?
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG@16 PS15, Line 16: Please document how you tested this.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... PS15, Line 354: For the future, please do clean-ups in separate commits.
https://review.coreboot.org/c/coreboot/+/34537/15/src/lib/bootsplash.h File src/lib/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/15/src/lib/bootsplash.h@20 PS15, Line 20: * Returns 0 on success The return type is void currently. Is that wanted?
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#16).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c M src/include/vbe.h A src/lib/bootsplash.c A src/lib/bootsplash.h 7 files changed, 103 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/16
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/16/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/16/src/device/pci_device.c@77... PS16, Line 773: if (vbe_mode_info_valid()){ space required before the open brace '{'
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 16:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc@51 PS15, Line 51: ramstage-$(CONFIG_BOOTSPLASH) += ../lib/bootsplash.c You can just modify lib/Makefile.inc instead, jpeg.c is probably there already.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... PS15, Line 225: int mode_info_valid; I think you can declare those static, and do:
const vbe_mode_info_t *vbe_mode_info(void) { if (!mode_info_valid) return NULL; return &mode_info; }
The name mode_info is sort of ugly to have in global namespace.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... PS15, Line 722: } Like before.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 772: #if CONFIG(BOOTSPLASH) I would move the details to lib/bootsplash.c leaving only following here:
if (CONFIG(BOOTSPLASH)) show_bootsplash();
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@78... PS15, Line 788: timestamp_add_now(TS_OPROM_END); Would be more correct to render image after TS_OPROM_END.
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@105 PS15, Line 105: #if CONFIG(FRAMEBUFFER_SET_VESA_MODE) Generally such guards are only used when it does not build without them.
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@107 PS15, Line 107: int vbe_mode_info_valid(void); There was suggestion to return vbe_mode_info_t pointer instead.
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 16:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG@10 PS15, Line 10: setup
The verb is spelled with a space: set up. […]
Done
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG@15 PS15, Line 15: For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
Just for my memory, what happens if the bootsplash image has different dimensions? Is it cropped or […]
Old requirement: framebuffer >= image == 1024x768@16 New requirement: image == framebuffer
Else it would have overflown the FB.
I would like to add the code to center the image in the FB later, so that it can be used to display independent of the resolution
https://review.coreboot.org/c/coreboot/+/34537/15//COMMIT_MSG@16 PS15, Line 16:
Please document how you tested this.
parially Done. vgabios tested on razer blade stealth no clue how to test yabel tbh :/
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... PS15, Line 354:
For the future, please do clean-ups in separate commits.
Okay. Thanks :)
https://review.coreboot.org/c/coreboot/+/34537/15/src/lib/bootsplash.h File src/lib/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/15/src/lib/bootsplash.h@20 PS15, Line 20: * Returns 0 on success
The return type is void currently. […]
Yes. My thought: It is not a critical part boot-up and does its own logging. Also there is no expected behaviour if something goes wrong. There is not really a way to recover.
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#17).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c A src/lib/bootsplash.h 8 files changed, 113 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/17
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 17:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc@51 PS15, Line 51: ramstage-$(CONFIG_BOOTSPLASH) += ../lib/bootsplash.c
You can just modify lib/Makefile.inc instead, jpeg.c is probably there already.
THanks nice! thanks
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... PS15, Line 225: int mode_info_valid;
I think you can declare those static, and do: […]
Using it! Thanks a lot!
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... PS15, Line 722: }
Like before.
Done
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 772: #if CONFIG(BOOTSPLASH)
I would move the details to lib/bootsplash.c leaving only following here: […]
I disagree, as lib/bootsplash should be generic without the dependency of handling vesa related things.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@78... PS15, Line 788: timestamp_add_now(TS_OPROM_END);
Would be more correct to render image after TS_OPROM_END.
I agree. Changed
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@105 PS15, Line 105: #if CONFIG(FRAMEBUFFER_SET_VESA_MODE)
Generally such guards are only used when it does not build without them.
Okay. Thanks
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@107 PS15, Line 107: int vbe_mode_info_valid(void);
There was suggestion to return vbe_mode_info_t pointer instead.
Will use the function approach instead.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 17:
(10 comments)
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/Makefile.inc@51 PS15, Line 51: ramstage-$(CONFIG_BOOTSPLASH) += ../lib/bootsplash.c
You can just modify lib/Makefile.inc instead, jpeg.c is probably there already.
Done
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/realmode/... PS15, Line 225: int mode_info_valid;
I think you can declare those static, and do: […]
Done
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/oprom/yabel/vbe... PS15, Line 722: }
Like before.
Done
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 772: #if CONFIG(BOOTSPLASH)
I disagree, as lib/bootsplash should be generic without the dependency of handling vesa related thin […]
And pci_device.c should be even more unaware of vesa headers than bootsplash.c.
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@78... PS15, Line 788: timestamp_add_now(TS_OPROM_END);
I agree. […]
Done
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@105 PS15, Line 105: #if CONFIG(FRAMEBUFFER_SET_VESA_MODE)
Okay. […]
Done
https://review.coreboot.org/c/coreboot/+/34537/15/src/include/vbe.h@107 PS15, Line 107: int vbe_mode_info_valid(void);
Will use the function approach instead.
Done
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.h File src/lib/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.h@4 PS17, Line 4: * Copyright (C) 2019 Johanna Schander coreboot@mimoja.de We have sort of stopped filling names in license headers. You can have it but it may just get stripped out in near future.
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.h@15 PS17, Line 15: We generally always guard header files
#ifndef __BOOTSPLASH_H__ #define __BOOTSPLASH_H__ .... #endif
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.c@19 PS17, Line 19: #include "bootsplash.h" IMHO <bootsplash.h> and the file added would be src/include/bootsplash.h
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#18).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c A src/lib/bootsplash.h 8 files changed, 130 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/18
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/18/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/18/src/lib/bootsplash.c@23 PS18, Line 23: void set_vesa_bootsplash() Bad function definition - void set_vesa_bootsplash() should probably be void set_vesa_bootsplash(void)
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 18:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/15/src/device/pci_device.c@77... PS15, Line 772: #if CONFIG(BOOTSPLASH)
And pci_device.c should be even more unaware of vesa headers than bootsplash.c.
i created a set_vesa_bootsplash wrapper around set_bootsplash
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.h File src/lib/bootsplash.h:
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.h@15 PS17, Line 15:
We generally always guard header files […]
Done
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.c@19 PS17, Line 19: #include "bootsplash.h"
IMHO <bootsplash.h> and the file added would be src/include/bootsplash. […]
For some reason there are a lot of headers in src/lib. I will move it
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#19).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 8 files changed, 130 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/19
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/19/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/19/src/lib/bootsplash.c@23 PS19, Line 23: void set_vesa_bootsplash() Bad function definition - void set_vesa_bootsplash() should probably be void set_vesa_bootsplash(void)
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#20).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 8 files changed, 130 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/20
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/19/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/19/src/lib/bootsplash.c@23 PS19, Line 23: void set_vesa_bootsplash()
Bad function definition - void set_vesa_bootsplash() should probably be void set_vesa_bootsplash(voi […]
Jenkins sees it all ^^
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 20:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34537/19/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/19/src/device/pci_device.c@54 PS19, Line 54: #include <bootsplash.h> Move it up after <bootmode.h>
https://review.coreboot.org/c/coreboot/+/34537/19/src/device/pci_device.c@77... PS19, Line 776: set_vesa_bootsplash(); No preprocessor use here, just regular if (CONFIG(xxx)).
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/17/src/lib/bootsplash.c@19 PS17, Line 19: #include "bootsplash.h"
For some reason there are a lot of headers in src/lib. […]
Done
https://review.coreboot.org/c/coreboot/+/34537/19/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/19/src/lib/bootsplash.c@23 PS19, Line 23: void set_vesa_bootsplash()
Jenkins sees it all ^^
Done
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#21).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 8 files changed, 130 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/21
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/21/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/21/src/device/pci_device.c@77... PS21, Line 775: if (CONFIG(BOOTSPLASH)) { braces {} are not necessary for single statement blocks
Hello Kyösti Mälkki, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#22).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was testend on the wip razer blade stealth using the intel vgabios. The yabel changes were untested.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Makefile.inc M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 8 files changed, 129 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/22
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 22:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34537/19/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/34537/19/src/device/pci_device.c@54 PS19, Line 54: #include <bootsplash.h>
Move it up after <bootmode. […]
Done
https://review.coreboot.org/c/coreboot/+/34537/19/src/device/pci_device.c@77... PS19, Line 776: set_vesa_bootsplash();
No preprocessor use here, just regular if (CONFIG(xxx)).
Done
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 22:
(8 comments)
Thanks, I'm happy about the overall layout of the files now.
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/Makefile.inc@51 PS22, Line 51: Unrelated whitespace change.
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... PS22, Line 224: vbe_mode_info_t mode_info; static, if possible
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... PS22, Line 235: return NULL; Change to:
if (!mode_info_valid || !mode_info.vesa.phys_base_ptr) return NULL;
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... PS22, Line 366: return; Change for line 234 is to cover this case, bootsplash requires linear framebuffer address.
Not in the scope of this patch, but yabel continues with the call to vbe_set_mode() regardless here.
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... PS22, Line 716: vbe_mode_info_t mode_info; static, if possible
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... PS22, Line 722: } Remove 717-722, duplicates of 166-171. Try build with oprom in "secure mode" to combile yabel code.
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... PS22, Line 726: if (!mode_info_valid) Like the other file.
https://review.coreboot.org/c/coreboot/+/34537/22/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/22/src/include/vbe.h@105 PS22, Line 105: const vbe_mode_info_t *vbe_mode_info(void); Document use, noting it returns NULL in case there is no linear framebuffer.
Hello Kyösti Mälkki, Patrick Rudolph, ron minnich, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#23).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16.
This commit removes this limit by using the actual resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
This was tested on the wip razer blade stealth using the intel vgabios.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 7 files changed, 128 insertions(+), 48 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/23
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 23:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/Makefile.inc@51 PS22, Line 51:
Unrelated whitespace change.
Done
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... PS22, Line 224: vbe_mode_info_t mode_info;
static, if possible
Done
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/realmode/... PS22, Line 235: return NULL;
Change to: […]
Done
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... PS22, Line 716: vbe_mode_info_t mode_info;
static, if possible
Done
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... PS22, Line 722: }
Remove 717-722, duplicates of 166-171. Try build with oprom in "secure mode" to combile yabel code.
Done
https://review.coreboot.org/c/coreboot/+/34537/22/src/device/oprom/yabel/vbe... PS22, Line 726: if (!mode_info_valid)
Like the other file.
Done
https://review.coreboot.org/c/coreboot/+/34537/22/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/22/src/include/vbe.h@105 PS22, Line 105: const vbe_mode_info_t *vbe_mode_info(void);
Document use, noting it returns NULL in case there is no linear framebuffer.
Done
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 23: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34537/23/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/23/src/device/oprom/yabel/vbe... PS23, Line 38: #include <boot/coreboot_tables.h> I think it's not needed anymore
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 23:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG@10 PS22, Line 10: set up as 1024x768@16. Sorry, can you please add what happens if a different resolution/depth is used?
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG@17 PS22, Line 17: testend tested
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG@17 PS22, Line 17: This was testend on the wip razer blade stealth using the intel vgabios. What resolution did you test with?
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 23:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG@10 PS22, Line 10: set up as 1024x768@16.
Sorry, can you please add what happens if a different resolution/depth is used?
Done
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG@17 PS22, Line 17: This was testend on the wip razer blade stealth using the intel vgabios.
What resolution did you test with?
Done
https://review.coreboot.org/c/coreboot/+/34537/22//COMMIT_MSG@17 PS22, Line 17: testend
tested
Done
https://review.coreboot.org/c/coreboot/+/34537/23/src/device/oprom/yabel/vbe... File src/device/oprom/yabel/vbe.c:
https://review.coreboot.org/c/coreboot/+/34537/23/src/device/oprom/yabel/vbe... PS23, Line 38: #include <boot/coreboot_tables.h>
I think it's not needed anymore
Done
Hello Kyösti Mälkki, Patrick Rudolph, ron minnich, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#24).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16. Different resolutions did not show anything, differnent depth resulted in the distorted images.
This commit removes this limit by using the actual framebuffer resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
It was tested in 1280x1024@32 on the wip razer blade stealth using the intel vgabios.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 7 files changed, 128 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/24
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 24:
(2 comments)
nice cleanup, thanks!
https://review.coreboot.org/c/coreboot/+/34537/24/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/24/src/include/vbe.h@106 PS24, Line 106: mode_indo mode_info
https://review.coreboot.org/c/coreboot/+/34537/24/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/24/src/lib/bootsplash.c@18 PS24, Line 18: <lib/jpeg.h> "jpeg.h" (as in: search in current directory)?
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 24:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34537/24/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/34537/24/src/include/vbe.h@106 PS24, Line 106: mode_indo
mode_info
Done
https://review.coreboot.org/c/coreboot/+/34537/24/src/lib/bootsplash.c File src/lib/bootsplash.c:
https://review.coreboot.org/c/coreboot/+/34537/24/src/lib/bootsplash.c@18 PS24, Line 18: <lib/jpeg.h>
"jpeg. […]
Done
Hello Kyösti Mälkki, Patrick Rudolph, ron minnich, Christoph Pomaska, Stefan Reinauer, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#25).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16. Different resolutions did not show anything, differnent depth resulted in the distorted images.
This commit removes this limit by using the actual framebuffer resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
It was tested in 1280x1024@32 on the wip razer blade stealth using the intel vgabios.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 7 files changed, 129 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/25
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 25: Code-Review+1
Commit message text width is 75 characters.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 25: Code-Review+1
One last tweak on the commit message (see Paul's comment) and I think it can go in.
Hello Kyösti Mälkki, Patrick Rudolph, Paul Menzel, Stefan Reinauer, build bot (Jenkins), Patrick Georgi, ron minnich, Christoph Pomaska, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34537
to look at the new patch set (#26).
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16. Different resolutions did not show anything, differnent depth resulted in the distorted images.
This commit removes this limit by using the actual framebuffer resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
It was tested in 1280x1024@32 on the wip razer blade stealth using the intel vgabios.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 7 files changed, 129 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34537/26
Johanna Schander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 26:
one 75 char line. rest below. thanks for all the support
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 26: Code-Review+2
Caught a nunrelated build error with yabel, CB:34578 should go in first.
Thanks. And welcome to project!
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
Patch Set 26: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34537 )
Change subject: src/device/oprom: Fix bootsplash display code for optionroms ......................................................................
src/device/oprom: Fix bootsplash display code for optionroms
So far the bootsplash is only correctly rendered if the framebuffer is set up as 1024x768@16. Different resolutions did not show anything, differnent depth resulted in the distorted images.
This commit removes this limit by using the actual framebuffer resolutions and combines the code for x86 and yabel.
For the moment the bootsplash is still limited to VGA-OptionROM framebuffer init.
It was tested in 1280x1024@32 on the wip razer blade stealth using the intel vgabios.
Change-Id: I5ab7b8a0f28badaa16e25dbe807158870d06e26a Signed-off-by: Johanna Schander coreboot@mimoja.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/34537 Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/device/oprom/realmode/x86.c M src/device/oprom/yabel/vbe.c M src/device/pci_device.c A src/include/bootsplash.h M src/include/vbe.h M src/lib/Makefile.inc A src/lib/bootsplash.c 7 files changed, 129 insertions(+), 51 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Kyösti Mälkki: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/src/device/oprom/realmode/x86.c b/src/device/oprom/realmode/x86.c index 67e550c..1a80a00 100644 --- a/src/device/oprom/realmode/x86.c +++ b/src/device/oprom/realmode/x86.c @@ -18,14 +18,12 @@ #include <arch/interrupt.h> #include <arch/registers.h> #include <boot/coreboot_tables.h> -#include <cbfs.h> #include <console/console.h> #include <cpu/amd/lxdef.h> #include <cpu/amd/vr.h> #include <delay.h> #include <device/pci.h> #include <device/pci_ids.h> -#include <lib/jpeg.h> #include <pc80/i8259.h> #include <pc80/i8254.h> #include <string.h> @@ -223,7 +221,7 @@ }
#if CONFIG(FRAMEBUFFER_SET_VESA_MODE) -vbe_mode_info_t mode_info; +static vbe_mode_info_t mode_info; static int mode_info_valid;
static int vbe_mode_info_valid(void) @@ -231,6 +229,13 @@ return mode_info_valid; }
+const vbe_mode_info_t *vbe_mode_info(void) +{ + if (!mode_info_valid || !mode_info.vesa.phys_base_ptr) + return NULL; + return &mode_info; +} + static int vbe_check_for_failure(int ah);
static void vbe_get_ctrl_info(vbe_info_block *info) @@ -353,6 +358,7 @@ le16_to_cpu(mode_info.vesa.x_resolution), le16_to_cpu(mode_info.vesa.y_resolution), mode_info.vesa.bits_per_pixel); + printk(BIOS_DEBUG, "VBE: framebuffer: %p\n", framebuffer); if (!framebuffer) { printk(BIOS_DEBUG, "VBE: Mode does not support linear " @@ -361,19 +367,6 @@ }
vbe_set_mode(&mode_info); -#if CONFIG(BOOTSPLASH) - struct jpeg_decdata *decdata; - unsigned char *jpeg = cbfs_boot_map_with_leak("bootsplash.jpg", - CBFS_TYPE_BOOTSPLASH, - NULL); - if (!jpeg) { - printk(BIOS_DEBUG, "VBE: No bootsplash found.\n"); - return; - } - decdata = malloc(sizeof(*decdata)); - int ret = 0; - ret = jpeg_decode(jpeg, framebuffer, 1024, 768, 16, decdata); -#endif }
void vbe_textmode_console(void) diff --git a/src/device/oprom/yabel/vbe.c b/src/device/oprom/yabel/vbe.c index 8116c6b..9a7fa04 100644 --- a/src/device/oprom/yabel/vbe.c +++ b/src/device/oprom/yabel/vbe.c @@ -34,9 +34,6 @@
#include <string.h> #include <types.h> -#if CONFIG(FRAMEBUFFER_SET_VESA_MODE) -#include <boot/coreboot_tables.h> -#endif
#include <endian.h>
@@ -52,10 +49,7 @@ #include "interrupt.h" #include "device.h"
-#include <cbfs.h> - #include <delay.h> -#include "../../src/lib/jpeg.h"
#include <vbe.h>
@@ -717,7 +711,14 @@ } #endif
-vbe_mode_info_t mode_info; +static vbe_mode_info_t mode_info; + +const vbe_mode_info_t *vbe_mode_info(void) +{ + if (!mode_info_valid || !mode_info.vesa.phys_base_ptr) + return NULL; + return &mode_info; +}
void vbe_set_graphics(void) { @@ -745,34 +746,6 @@ mode_info.video_mode = (1 << 14) | CONFIG_FRAMEBUFFER_VESA_MODE; vbe_get_mode_info(&mode_info); vbe_set_mode(&mode_info); - -#if CONFIG(BOOTSPLASH) - unsigned char *framebuffer = - (unsigned char *) le32_to_cpu(mode_info.vesa.phys_base_ptr); - DEBUG_PRINTF_VBE("FRAMEBUFFER: 0x%p\n", framebuffer); - - struct jpeg_decdata *decdata; - - /* Switching Intel IGD to 1MB video memory will break this. Who - * cares. */ - // int imagesize = 1024*768*2; - - unsigned char *jpeg = cbfs_boot_map_with_leak("bootsplash.jpg", - CBFS_TYPE_BOOTSPLASH, - NULL); - if (!jpeg) { - DEBUG_PRINTF_VBE("Could not find bootsplash.jpg\n"); - return; - } - DEBUG_PRINTF_VBE("Splash at %p ...\n", jpeg); - dump(jpeg, 64); - - decdata = malloc(sizeof(*decdata)); - int ret = 0; - DEBUG_PRINTF_VBE("Decompressing boot splash screen...\n"); - ret = jpeg_decode(jpeg, framebuffer, 1024, 768, 16, decdata); - DEBUG_PRINTF_VBE("returns %x\n", ret); -#endif }
int fill_lb_framebuffer(struct lb_framebuffer *framebuffer) diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 9c47085..7786043 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -34,6 +34,7 @@ #include <arch/acpi.h> #include <device/pci_ops.h> #include <bootmode.h> +#include <bootsplash.h> #include <console/console.h> #include <stdlib.h> #include <stdint.h> @@ -49,6 +50,8 @@ #include <pc80/i8259.h> #include <security/vboot/vbnv.h> #include <timestamp.h> +#include <types.h> +
u8 pci_moving_config8(struct device *dev, unsigned int reg) { @@ -764,9 +767,13 @@ return;
run_bios(dev, (unsigned long)ram); + gfx_set_init_done(1); printk(BIOS_DEBUG, "VGA Option ROM was run\n"); timestamp_add_now(TS_OPROM_END); + + if (CONFIG(BOOTSPLASH)) + set_vesa_bootsplash(); }
/** Default device operation for PCI devices */ diff --git a/src/include/bootsplash.h b/src/include/bootsplash.h new file mode 100644 index 0000000..84ba34c --- /dev/null +++ b/src/include/bootsplash.h @@ -0,0 +1,36 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Johanna Schander coreboot@mimoja.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __BOOTSPLASH_H__ +#define __BOOTSPLASH_H__ + +#include <types.h> + +/** + * Wraps bootsplash setup for vesa + */ +void set_vesa_bootsplash(void); + + +/** + * Sets up the framebuffer with the bootsplash.jpg from cbfs. + * Returns 0 on success + * CB_ERR on cbfs errors + * and >0 on jpeg errors. + */ +void set_bootsplash(unsigned char *framebuffer, unsigned int x_resolution, + unsigned int y_resolution, unsigned int fb_resolution); + +#endif diff --git a/src/include/vbe.h b/src/include/vbe.h index 67049be..cfae7e4 100644 --- a/src/include/vbe.h +++ b/src/include/vbe.h @@ -102,4 +102,10 @@ void vbe_set_graphics(void); void vbe_textmode_console(void);
+/** + * Returns the mode_info struct from the vbe context, + * if initialized. NULL on invalid mode_infos. + */ +const vbe_mode_info_t *vbe_mode_info(void); + #endif // VBE_H diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 3fad0b8..89ed4b0 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -124,6 +124,7 @@ ramstage-y += hexstrtobin.c ramstage-y += wrdd.c ramstage-$(CONFIG_CONSOLE_CBMEM) += cbmem_console.c +ramstage-$(CONFIG_BOOTSPLASH) += bootsplash.c ramstage-$(CONFIG_BOOTSPLASH) += jpeg.c ramstage-$(CONFIG_TRACE) += trace.c postcar-$(CONFIG_TRACE) += trace.c diff --git a/src/lib/bootsplash.c b/src/lib/bootsplash.c new file mode 100644 index 0000000..5527b23 --- /dev/null +++ b/src/lib/bootsplash.c @@ -0,0 +1,62 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Johanna Schander coreboot@mimoja.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <cbfs.h> +#include <vbe.h> +#include <console/console.h> +#include <endian.h> +#include <bootsplash.h> + +#include "jpeg.h" + +void set_vesa_bootsplash(void) +{ + const vbe_mode_info_t *mode_info = vbe_mode_info(); + if (mode_info != NULL) { + printk(BIOS_INFO, "Setting up bootsplash\n"); + unsigned int x_resolution = le16_to_cpu(mode_info->vesa.x_resolution); + unsigned int y_resolution = le16_to_cpu(mode_info->vesa.y_resolution); + unsigned int fb_resolution = mode_info->vesa.bits_per_pixel; + unsigned char *framebuffer = + (unsigned char *)le32_to_cpu(mode_info->vesa.phys_base_ptr); + + set_bootsplash(framebuffer, x_resolution, y_resolution, fb_resolution); + } else { + printk(BIOS_ERR, "VBE modeinfo invalid\n"); + } +} + + +void set_bootsplash(unsigned char *framebuffer, unsigned int x_resolution, + unsigned int y_resolution, unsigned int fb_resolution) +{ + struct jpeg_decdata *decdata; + unsigned char *jpeg = + cbfs_boot_map_with_leak("bootsplash.jpg", CBFS_TYPE_BOOTSPLASH, NULL); + if (!jpeg) { + printk(BIOS_ERR, "Could not find bootsplash.jpg\n"); + return; + } + + decdata = malloc(sizeof(*decdata)); + int ret = jpeg_decode(jpeg, framebuffer, x_resolution, y_resolution, fb_resolution, + decdata); + if (ret != 0) { + printk(BIOS_ERR, "Bootsplash could not be decoded. jpeg_decode returned %d.\n", + ret); + return; + } + printk(BIOS_INFO, "Bootsplash loaded\n"); +}
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Johanna Schander (Code Review)
-
Kyösti Mälkki (Code Review)
-
Mimoja (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)