Change in coreboot[master]: [WIP]device/Kconfig: Introduce separate graphics menu for Aspeed
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/49218 )
Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Aspeed ......................................................................
[WIP]device/Kconfig: Introduce separate graphics menu for Aspeed
Allows to select which graphics initialisation to do for Aspeed graphics.
Change-Id: I63e58dfacd6cd0b8fab188e27d34ade9b1344a48 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/device/Kconfig A src/device/graphics/Kconfig A src/device/graphics/aspeed/Kconfig M src/drivers/aspeed/ast2050/ast2050.c M src/drivers/aspeed/common/Kconfig 5 files changed, 69 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/49218/1
diff --git a/src/device/Kconfig b/src/device/Kconfig index a176f0a..ca1b4c77 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -46,13 +46,6 @@ Selected by mainboards / drivers that provide native graphics init for Intel GPUs within coreboot.
-config MAINBOARD_HAS_NATIVE_ASPEED_VGA_INIT - def_bool n - select MAINBOARD_HAS_NATIVE_VGA_INIT - help - Selected by mainboards / drivers that provide native graphics - init for Aspeed GPUs within coreboot. - config MAINBOARD_HAS_NATIVE_RK3288_VGA_INIT def_bool n select MAINBOARD_HAS_NATIVE_VGA_INIT @@ -73,6 +66,8 @@ Selected by mainboards that implement support for `libgfxinit`. Usually this requires a list of ports to be probed for displays.
+source "src/device/graphics/Kconfig" + choice prompt "Graphics initialization" default NO_GFX_INIT if VGA_BIOS && PAYLOAD_SEABIOS diff --git a/src/device/graphics/Kconfig b/src/device/graphics/Kconfig new file mode 100644 index 0000000..f45bf30 --- /dev/null +++ b/src/device/graphics/Kconfig @@ -0,0 +1,3 @@ +## SPDX-License-Identifier: GPL-2.0-only + +source "src/device/graphics/aspeed/Kconfig" diff --git a/src/device/graphics/aspeed/Kconfig b/src/device/graphics/aspeed/Kconfig new file mode 100644 index 0000000..1f26629 --- /dev/null +++ b/src/device/graphics/aspeed/Kconfig @@ -0,0 +1,46 @@ +## SPDX-License-Identifier: GPL-2.0-only + +config MAINBOARD_HAS_NATIVE_ASPEED_VGA_INIT + def_bool n + select MAINBOARD_HAS_NATIVE_VGA_INIT + help + Selected by mainboards / drivers that provide native graphics + init for Aspeed GPUs within coreboot. + +config MAINBOARD_FORCE_NATIVE_ASPEED_VGA_INIT + def_bool n + depends on MAINBOARD_HAS_NATIVE_ASPEED_VGA_INIT + help + Selected by mainboards / chipsets whose graphics driver can't or + shouldn't be disabled. + +choice + prompt "Aspeed graphics initialization" + default DO_NATIVE_ASPEED_VGA_INIT if MAINBOARD_FORCE_NATIVE_ASPEED_VGA_INIT + default NO_ASPEED_VGA_INIT if VGA_BIOS && PAYLOAD_SEABIOS + default VGA_ROM_RUN_ASPEED_VGA_INIT if VGA_BIOS + default DO_NATIVE_ASPEED_VGA_INIT + +config DO_NATIVE_ASPEED_VGA_INIT + bool "Native" + depends on MAINBOARD_HAS_NATIVE_ASPEED_VGA_INIT + help + Selected by mainboards / drivers that do native graphics + init for Aspeed GPUs within coreboot. + +config VGA_ROM_RUN_ASPEED_VGA_INIT + bool "Legacy VGA Option ROM" + depends on !MAINBOARD_FORCE_NATIVE_ASPEED_VGA_INIT + depends on VGA_ROM_RUN + help + FIXME: This won't work unless VGA_ROM_RUN is selected as well + Selected by mainboards / drivers that do include a Aspeed VBIOS in + the CBFS for Aspeed GPUs within coreboot. + +config NO_ASPEED_VGA_INIT + bool "None" + depends on !MAINBOARD_FORCE_NATIVE_ASPEED_VGA_INIT + help + Select this to not perform any graphics initialization in + coreboot on the Aspeed GPU. +endchoice diff --git a/src/drivers/aspeed/ast2050/ast2050.c b/src/drivers/aspeed/ast2050/ast2050.c index 5cb7198..17d5439 100644 --- a/src/drivers/aspeed/ast2050/ast2050.c +++ b/src/drivers/aspeed/ast2050/ast2050.c @@ -36,18 +36,24 @@ outb(0xa6, 0x3d4); outb(0x2f, 0x3d5); outb(0xa7, 0x3d4); outb(0x3f, 0x3d5);
- if (CONFIG(VGA_TEXT_FRAMEBUFFER)) { - /* Initialize standard VGA text mode */ - vga_io_init(); + if (CONFIG(VGA_ROM_RUN_ASPEED_VGA_INIT)) { + pci_dev_init(dev); + } else if (CONFIG(DO_NATIVE_ASPEED_VGA_INIT)) { + if (CONFIG(VGA_TEXT_FRAMEBUFFER)) { + /* Initialize standard VGA text mode */ + vga_io_init();
- vga_textmode_init(); - printk(BIOS_INFO, "ASpeed VGA text mode initialized\n"); + vga_textmode_init(); + printk(BIOS_INFO, "ASpeed VGA text mode initialized\n");
- /* if we don't have console, at least print something... */ - vga_line_write(0, "ASpeed VGA text mode initialized"); - } else if (CONFIG(GENERIC_LINEAR_FRAMEBUFFER)) { - ast_driver_framebuffer_init(&drm_dev, 0); - printk(BIOS_INFO, "ASpeed high resolution framebuffer initialized\n"); + /* if we don't have console, at least print something... */ + vga_line_write(0, "ASpeed VGA text mode initialized"); + } else if (CONFIG(GENERIC_LINEAR_FRAMEBUFFER)) { + ast_driver_framebuffer_init(&drm_dev, 0); + printk(BIOS_INFO, "ASpeed high resolution framebuffer initialized\n"); + } + } else { + printk(BIOS_INFO, "ASpeed: Skipping graphics init.\n"); } }
diff --git a/src/drivers/aspeed/common/Kconfig b/src/drivers/aspeed/common/Kconfig index 79c4c75..50d4a38 100644 --- a/src/drivers/aspeed/common/Kconfig +++ b/src/drivers/aspeed/common/Kconfig @@ -1,6 +1,6 @@ config DRIVERS_ASPEED_AST_COMMON bool - select HAVE_LINEAR_FRAMEBUFFER if MAINBOARD_DO_NATIVE_VGA_INIT - select HAVE_VGA_TEXT_FRAMEBUFFER if MAINBOARD_DO_NATIVE_VGA_INIT + select HAVE_LINEAR_FRAMEBUFFER if DO_NATIVE_ASPEED_VGA_INIT + select HAVE_VGA_TEXT_FRAMEBUFFER if DO_NATIVE_ASPEED_VGA_INIT select VGA if VGA_TEXT_FRAMEBUFFER select SOFTWARE_I2C if GENERIC_LINEAR_FRAMEBUFFER
Attention is currently required from: Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49218 )
Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Aspeed ......................................................................
Patch Set 1:
(5 comments)
Patchset:
PS1: Looks good overall. I didn't expect the VGA_ROM_RUN as part of the per-device choices, but I like it. :)
File src/device/graphics/aspeed/Kconfig:
https://review.coreboot.org/c/coreboot/+/49218/comment/6dabe521_46e2bece PS1, Line 2: Put an `if DRIVERS_ASPEED_AST_COMMON` around everything or the `source` line?
https://review.coreboot.org/c/coreboot/+/49218/comment/60f5021d_d56a9f73 PS1, Line 10: config MAINBOARD_FORCE_NATIVE_ASPEED_VGA_INIT : def_bool n : depends on MAINBOARD_HAS_NATIVE_ASPEED_VGA_INIT : help : Selected by mainboards / chipsets whose graphics driver can't or : shouldn't be disabled. I don't think this is needed. Do you have a specific use-case in mind?
IIRC, the original MAINBOARD_FORCE_NATIVE_VGA_INIT was only introduced for cases where the code is written such that we don't have a way to disable it. And it was only selected by the ASpeed driver by accident for bad advertisement.
https://review.coreboot.org/c/coreboot/+/49218/comment/9906157c_d52f4c96 PS1, Line 34: depends on VGA_ROM_RUN I assume this would be changed to `select` later when VGA_ROM_RUN is not part of a choice anymore?
https://review.coreboot.org/c/coreboot/+/49218/comment/03d25d76_adf2b572 PS1, Line 36: FIXME: This won't work unless VGA_ROM_RUN is selected as well The `depends on` already makes sure that it won't show up without VGA_ROM_RUN.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/49218?usp=email )
Change subject: [WIP]device/Kconfig: Introduce separate graphics menu for Aspeed ......................................................................
Abandoned
-
Nico Huber (Code Review) -
Patrick Rudolph (Code Review)
-
Stefan Reinauer (Code Review)