Mimoja has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
drivers/emulation/qemu: Show splashscreen on bochs VGA
Show the coreboot bootsplash in qemu when running with bochs vga.
By default the framebuffer will be initialized in 800x600@32. This can be overwriten by configuration.
The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
This was tested in 1280x1024 by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/drivers/emulation/qemu/bochs.c 2 files changed, 18 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/1
diff --git a/src/device/Kconfig b/src/device/Kconfig index e605bc2..75b25a1 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -392,18 +392,6 @@ default 0x11A if FRAMEBUFFER_VESA_MODE_11A default 0x11B if FRAMEBUFFER_VESA_MODE_11B default 0x118 if FRAMEBUFFER_VESA_MODE_USER - -config BOOTSPLASH - prompt "Show graphical bootsplash" - bool - help - This option shows a graphical bootsplash screen. The graphics are - loaded from the CBFS file bootsplash.jpg. - - You can either specify the location and file name of the - image in the 'General' section or add it manually to CBFS, using, - for example, cbfstool. - endif # FRAMEBUFFER_SET_VESA_MODE
choice @@ -447,6 +435,21 @@ def_bool y depends on VBE_LINEAR_FRAMEBUFFER || GENERIC_LINEAR_FRAMEBUFFER
+config BOOTSPLASH + prompt "Show graphical bootsplash" + bool + depends on LINEAR_FRAMEBUFFER + help + This option shows a graphical bootsplash screen. The graphics are + loaded from the CBFS file bootsplash.jpg. + + You can either specify the location and file name of the + image in the 'General' section or add it manually to CBFS, using, + for example, cbfstool. + + It might not be implemented for every framebuffer initialization platform. + + config LINEAR_FRAMEBUFFER_MAX_WIDTH int "Maximum width in pixels" depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT diff --git a/src/drivers/emulation/qemu/bochs.c b/src/drivers/emulation/qemu/bochs.c index 22095ef..c15255e 100644 --- a/src/drivers/emulation/qemu/bochs.c +++ b/src/drivers/emulation/qemu/bochs.c @@ -15,6 +15,7 @@ #include <edid.h> #include <stdlib.h> #include <arch/io.h> +#include <bootsplash.h> #include <boot/coreboot_tables.h> #include <console/console.h> #include <device/device.h> @@ -122,6 +123,8 @@ edid.panel_bits_per_pixel = 24; edid_set_framebuffer_bits_per_pixel(&edid, 32, 0); set_vbe_mode_info_valid(&edid, addr); + if (CONFIG(BOOTSPLASH)) + set_bootsplash((unsigned char *)addr, width, height, 32); }
static void bochs_init_text_mode(struct device *dev)
Hello Kyösti Mälkki, Paul Menzel, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#2).
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
drivers/emulation/qemu: Show splashscreen on bochs VGA
By default the framebuffer will be initialized in 800x600@32. This can be overwriten by configuration.
The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
This was tested in 1280x1024 by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/drivers/emulation/qemu/bochs.c 2 files changed, 18 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/2
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig@441 PS2, Line 441: depends on LINEAR_FRAMEBUFFER Should the configuration be limited to the qemu target?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig@450 PS2, Line 450: It might not be implemented for every framebuffer initialization platform. I don't see why. See comment in src/drivers/emulation/qemu/bochs.c
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... File src/drivers/emulation/qemu/bochs.c:
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... PS2, Line 126: if (CONFIG(BOOTSPLASH)) implement a platform independent bootsplash driver instead of modifying $n places in coreboot. That can easily done with BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, ...
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... File src/drivers/emulation/qemu/bochs.c:
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... PS2, Line 126: if (CONFIG(BOOTSPLASH))
implement a platform independent bootsplash driver instead of modifying $n places in coreboot. […]
This would require to implement an interface to get the framebuffer parameter in $n places, right?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... File src/drivers/emulation/qemu/bochs.c:
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... PS2, Line 126: if (CONFIG(BOOTSPLASH))
This would require to implement an interface to get the framebuffer parameter in $n places, right?
It's already there: fill_lb_framebuffer() You could add that bootsplash code in edid_fill_fb.c and directly access edid_fb.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... File src/drivers/emulation/qemu/bochs.c:
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... PS2, Line 126: if (CONFIG(BOOTSPLASH))
This would require to implement an interface to get the framebuffer parameter in $n places, right?
We already have the infrastructure to query the framebuffer parameters. With CONFIG_LINEAR_FRAMEBUFFER, there is a fill_lb_framebuffer(). However, I hate boot-state hooks (because of many bugs introduced by their excessive usage) so I propose another scheme (that would be better in the long run anyway):
Currently every linear-framebuffer driver in coreboot has its own means to cache the framebuffer info for future calls of fill_lb_framebuffer(). This has one very bad implication: The driver implements fill_lb_ framebuffer(), so there can only be a single driver in the whole core- boot build (making runtime decisions which driver to choose impossible).
It would be better to reverse the control flow: Instead of querying the framebuffer infos later, let the drivers push them when they set up a framebuffer. Then, the bootsplash code can be in a central place (where the framebuffer info is pushed to), and also rules for multiple reported framebuffers could be implemented there.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: drivers/emulation/qemu: Show splashscreen on bochs VGA ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... File src/drivers/emulation/qemu/bochs.c:
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... PS2, Line 126: if (CONFIG(BOOTSPLASH))
We already have the infrastructure to query the framebuffer parameters. […]
A part of the MULTIBOOT specification (for payloads/kernels) is that the header of such a file in CBFS can contain information about the preferred/required framebuffer. Maybe not related to this directly, but try reduce the use of build-time const CONFIG_FRAMEBUFFER_VESA_MODE.
IMHO that _VESA_MODE parameter should only be applied in the case of no available EDID.
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#3).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call.
Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
This was tested tested on the wip razer blade stealth using vgabios @ 1280x1024 It was also tested in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 25 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34599/3/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/3/src/lib/coreboot_table.c@15... PS3, Line 150: if(CONFIG(BOOTSPLASH)){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/34599/3/src/lib/coreboot_table.c@15... PS3, Line 150: if(CONFIG(BOOTSPLASH)){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34599/3/src/lib/coreboot_table.c@15... PS3, Line 151: unsigned char *fb_ptr =(unsigned char *) (u32) framebuffer->physical_address; spaces required around that '=' (ctx:WxV)
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#4).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call.
Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
This was tested tested on the wip razer blade stealth using vgabios @ 1280x1024 It was also tested in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 25 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/4
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... File src/drivers/emulation/qemu/bochs.c:
https://review.coreboot.org/c/coreboot/+/34599/2/src/drivers/emulation/qemu/... PS2, Line 126: if (CONFIG(BOOTSPLASH))
A part of the MULTIBOOT specification (for payloads/kernels) is that the header of such a file in CB […]
Done
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig@441 PS2, Line 441: depends on LINEAR_FRAMEBUFFER
Should the configuration be limited to the qemu target?
Done
https://review.coreboot.org/c/coreboot/+/34599/2/src/device/Kconfig@450 PS2, Line 450: It might not be implemented for every framebuffer initialization platform.
I don't see why. See comment in src/drivers/emulation/qemu/bochs. […]
Done
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#5).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call.
Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
This was tested tested on the wip razer blade stealth using vgabios @ 1280x1024 It was also tested in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 25 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/5/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/5/src/lib/coreboot_table.c@15... PS5, Line 151: unsigned char *fb_ptr = (unsigned char *) (size_t) framebuffer->physical_address; line over 96 characters
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/6/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/6/src/lib/coreboot_table.c@15... PS6, Line 151: unsigned char *fb_ptr = (unsigned char *) (size_t) framebuffer->physical_address; line over 96 characters
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#7).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call.
Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
This was tested tested on the wip razer blade stealth using vgabios @ 1280x1024 It was also tested in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 26 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/7
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@48 PS7, Line 48: #include <bootsplash.h> Just place it after <bootmem.h>.
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) { Previously, there may have been some combination of Kconfigs where bootsplash was drawn on S3 resume path. I am fine with the change, maybe worth mentioning in the commit message S3 resume will never show bootsplash.
The approach here may delay the showing of bootsplash by some hundred milliseconds or so, depending of how slow PCI init functions the platform has. I would rather see the call made outside coreboot-table but apparently Nico was against hooking to boot states?
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@48 PS7, Line 48: #include <bootsplash.h>
Just place it after <bootmem.h>.
Done
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) {
Previously, there may have been some combination of Kconfigs where bootsplash was drawn on S3 resume […]
Will test it and adapt things acordingly. Thanks for bringing it up. I agree, that we should move it from the "pulling it from the driver" to a "pushing the config" in the long term.
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#8).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call.
Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
This was tested tested on the wip razer blade stealth using vgabios @ 1280x1024 It was also tested in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 26 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/8
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) {
Will test it and adapt things acordingly. Thanks for bringing it up. […]
Yes. This will no longer show the bootsplash on S3 wakeup (which was really delaying wakeup) but everything else still works as expected.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34599/8//COMMIT_MSG@25 PS8, Line 25: The same code does not work on the cirrus vga, as it only shows Add a blank line above?
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#9).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call.
Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
This was tested tested on the wip razer blade stealth using vgabios @ 1280x1024 It was also tested in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES .
The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 26 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/9
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34599/8//COMMIT_MSG@25 PS8, Line 25: The same code does not work on the cirrus vga, as it only shows
Add a blank line above?
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) {
I would rather see the call made outside coreboot-table but apparently Nico was against hooking to boot states?
Complying with my preferences is not really a requirement. Though, even a boot-state hook would delay it a little. My idea was roughly to instantly notify common code (added to, e.g. lib/framebuffer.c) when a framebuffer becomes available. What to do with that information (e.g. cache for later reporting in cbtables, show bootsplash) would be up to the common code.
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) {
I would rather see the call made outside coreboot-table but apparently Nico was against hooking to […]
While I fully agree that that would be nice, I also think that it should not be handled now / in the review. Refactoring the notification system might require more than just 15 lines and a bit of planning.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 9: Code-Review+1
(6 comments)
https://review.coreboot.org/c/coreboot/+/34599/9//COMMIT_MSG Commit Message:
PS9: Some of the paragraphs might read better if you'd join them (generally, there is no need to give every sentence its own paragraph).
https://review.coreboot.org/c/coreboot/+/34599/9//COMMIT_MSG@19 PS9, Line 19: tested double `tested`
https://review.coreboot.org/c/coreboot/+/34599/9/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34599/9/src/device/Kconfig@450 PS9, Line 450: It might not be implemented for every framebuffer initialization platform. Can we drop this now?
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) {
While I fully agree that that would be nice, I also think that it should not be handled now / in the […]
Sure, I was just leaving that thought, if anybody wants to take it, it should be a separate patch (series?), ofc. I might start a discussion on the ML about the API, but don't wait for me if I don't ;)
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c@15... PS9, Line 152: size_t `size_t` only works if it has the same size as a pointer by coincidence. Better use `uintptr_t`.
Also, please remove the spaces around the parentheses. Then, it should also fit on a single line. (You could also use `uint8_t` to save some extra space.)
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c@15... PS9, Line 153: int unsigned int?
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#10).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call. Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future. It was tested on the wip razer blade stealth using vgabios @ 1280x1024 and also in Qemu @ 1280x1024
By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 22 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/10
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34599/9//COMMIT_MSG Commit Message:
PS9:
Some of the paragraphs might read better if you'd join them (generally, […]
Done
https://review.coreboot.org/c/coreboot/+/34599/9//COMMIT_MSG@19 PS9, Line 19: tested
double `tested`
Done
https://review.coreboot.org/c/coreboot/+/34599/9/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34599/9/src/device/Kconfig@450 PS9, Line 450: It might not be implemented for every framebuffer initialization platform.
Can we drop this now?
Yes. Dropped
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c@15... PS9, Line 152: size_t
`size_t` only works if it has the same size as a pointer by coincidence. […]
Done
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c@15... PS9, Line 153: int
unsigned int?
Done
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#11).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call. Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
It was tested on the wip razer blade stealth using vgabios @ 1280x1024 and also in Qemu @ 1280x1024. By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES . The same code does not work on the cirrus vga, as it only shows a white screen - more investigation will be required.
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 22 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/11
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 11:
Is this ready for merge?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 11: Code-Review+1
(3 comments)
Just some nits left. Sorry for taking so long.
https://review.coreboot.org/c/coreboot/+/34599/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34599/11//COMMIT_MSG@11 PS11, Line 11: fill_lb_framebuffer call. The period already signifies the end of the sentence, please don't put line breaks after every sentence.
https://review.coreboot.org/c/coreboot/+/34599/11//COMMIT_MSG@24 PS11, Line 24: a white screen - more investigation will be required. What happens with Cirrus VGA in the payload? does the framebuffer work there? if not, the problem is most likely not related to the splashscreen and not worth mentioning here.
https://review.coreboot.org/c/coreboot/+/34599/11/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/11/src/lib/coreboot_table.c@4... PS11, Line 49: No multiple empty lines, please.
Hello Kyösti Mälkki, Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34599
to look at the new patch set (#12).
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call. Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
It was tested on the wip razer blade stealth using vgabios @ 1280x1024 and also in Qemu @ 1280x1024. By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES .
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 21 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34599/12
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 12:
(3 comments)
"If at first you don't succeed, you try it all again" :)
https://review.coreboot.org/c/coreboot/+/34599/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34599/11//COMMIT_MSG@11 PS11, Line 11: fill_lb_framebuffer call.
The period already signifies the end of the sentence, please don't […]
Ack
https://review.coreboot.org/c/coreboot/+/34599/11//COMMIT_MSG@24 PS11, Line 24: a white screen - more investigation will be required.
What happens with Cirrus VGA in the payload? does the framebuffer […]
Yep. The cirrus framebuffer init appears to not be working on its own with some resolutions..
https://review.coreboot.org/c/coreboot/+/34599/11/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/11/src/lib/coreboot_table.c@4... PS11, Line 49:
No multiple empty lines, please.
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 12: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
lib/coreboot_table: Show splashscreen in lb_table_init
Every vga init implementation needs to cache the framebuffer state to be able to fill the lb_framebuffer struct later on in the fill_lb_framebuffer call. Showing the bootsplash afterwards guarantees to have the same interface into all the vga drivers.
This is by far from ideal, as it only allows for a single driver at compile-time and should be adapted in the future.
It was tested on the wip razer blade stealth using vgabios @ 1280x1024 and also in Qemu @ 1280x1024. By default the qemu framebuffer will be initialized in 800x600@32. This can be overwriten by configuration by setting CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_{X,Y}RES .
Change-Id: I4bec06d22423627e8f429c4b47e0dc9920f1464e Signed-off-by: Johanna Schander coreboot@mimoja.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/34599 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/device/Kconfig M src/device/pci_device.c M src/include/bootsplash.h M src/lib/bootsplash.c M src/lib/coreboot_table.c 5 files changed, 21 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/device/Kconfig b/src/device/Kconfig index e605bc2..0b90833 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -392,18 +392,6 @@ default 0x11A if FRAMEBUFFER_VESA_MODE_11A default 0x11B if FRAMEBUFFER_VESA_MODE_11B default 0x118 if FRAMEBUFFER_VESA_MODE_USER - -config BOOTSPLASH - prompt "Show graphical bootsplash" - bool - help - This option shows a graphical bootsplash screen. The graphics are - loaded from the CBFS file bootsplash.jpg. - - You can either specify the location and file name of the - image in the 'General' section or add it manually to CBFS, using, - for example, cbfstool. - endif # FRAMEBUFFER_SET_VESA_MODE
choice @@ -447,6 +435,18 @@ def_bool y depends on VBE_LINEAR_FRAMEBUFFER || GENERIC_LINEAR_FRAMEBUFFER
+config BOOTSPLASH + prompt "Show graphical bootsplash" + bool + depends on LINEAR_FRAMEBUFFER + help + This option shows a graphical bootsplash screen. The graphics are + loaded from the CBFS file bootsplash.jpg. + + You can either specify the location and file name of the + image in the 'General' section or add it manually to CBFS, using, + for example, cbfstool. + config LINEAR_FRAMEBUFFER_MAX_WIDTH int "Maximum width in pixels" depends on LINEAR_FRAMEBUFFER && MAINBOARD_USE_LIBGFXINIT diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 5765529..84fc82c 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -34,7 +34,6 @@ #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> @@ -766,9 +765,6 @@ 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 index 84ba34c..af09922 100644 --- a/src/include/bootsplash.h +++ b/src/include/bootsplash.h @@ -19,12 +19,6 @@ #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 diff --git a/src/lib/bootsplash.c b/src/lib/bootsplash.c index ee14b92..812a3b7 100644 --- a/src/lib/bootsplash.c +++ b/src/lib/bootsplash.c @@ -21,22 +21,6 @@
#include "jpeg.h"
-void set_vesa_bootsplash(void) -{ - const vbe_mode_info_t *mode_info = vbe_mode_info(); - if (mode_info != NULL) { - 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) diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 95c2ae6..66afcf3 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -31,6 +31,7 @@ #include <cbfs.h> #include <cbmem.h> #include <bootmem.h> +#include <bootsplash.h> #include <spi_flash.h> #include <security/vboot/misc.h> #include <security/vboot/vbnv_layout.h> @@ -144,6 +145,14 @@ memcpy(framebuffer, &fb, sizeof(*framebuffer)); framebuffer->tag = LB_TAG_FRAMEBUFFER; framebuffer->size = sizeof(*framebuffer); + + if (CONFIG(BOOTSPLASH)) { + uint8_t *fb_ptr = (uint8_t *)(uintptr_t)framebuffer->physical_address; + unsigned int width = framebuffer->x_resolution; + unsigned int height = framebuffer->y_resolution; + unsigned int depth = framebuffer->bits_per_pixel; + set_bootsplash(fb_ptr, width, height, depth); + } }
void lb_add_gpios(struct lb_gpios *gpios, const struct lb_gpio *gpio_table,
-
build bot (Jenkins) (Code Review) -
Kyösti Mälkki (Code Review)
-
Mimoja (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)