Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

t420 owners (and t430 users) : Why CONFIG_USE_OPTION_TABLE, CONFIG_STATIC_OPTION_TABLE and CONFIG_GENERIC_LINEAR_FRAMEBUFFER are defined under coreboot configs? #1293

Open
tlaurion opened this issue Jan 24, 2023 · 6 comments

Comments

@tlaurion
Copy link
Collaborator

As part of #1251, it was observed that x220/t420 coreboot configurations are different, causing t420 boards to be even more limited in space then I first thought (and t420 being the only ones failing without #1184 or #1121 merged in).

Trace of the difference:

user@heads-tests:~/heads$ diff -u config/coreboot-x220-maximized.config config/coreboot-t420-maximized.config 
--- config/coreboot-x220-maximized.config	2022-12-21 14:56:14.515000000 -0500
+++ config/coreboot-t420-maximized.config	2022-12-21 14:56:14.513000000 -0500
@@ -1,4 +1,6 @@
 # CONFIG_USE_BLOBS is not set
+CONFIG_USE_OPTION_TABLE=y
+CONFIG_STATIC_OPTION_TABLE=y
 CONFIG_VENDOR_LENOVO=y
 CONFIG_NO_POST=y
 CONFIG_CBFS_SIZE=0x750000
@@ -6,12 +8,12 @@
 CONFIG_ME_BIN_PATH="@BLOB_DIR@/xx20/me.bin"
 CONFIG_GBE_BIN_PATH="@BLOB_DIR@/xx20/gbe.bin"
 CONFIG_HAVE_IFD_BIN=y
-CONFIG_BOARD_LENOVO_X220=y
+CONFIG_BOARD_LENOVO_T420=y
 CONFIG_LINUX_COMMAND_LINE="intel_iommu=igfx_off quiet"
 CONFIG_UART_PCI_ADDR=0
 CONFIG_HAVE_ME_BIN=y
 CONFIG_HAVE_GBE_BIN=y
-CONFIG_NO_GFX_INIT=y
+CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
 CONFIG_DRIVERS_PS2_KEYBOARD=y
 CONFIG_TPM_MEASURED_BOOT=y
 CONFIG_CONSOLE_CBMEM_BUFFER_SIZE=0x80000

For non-developers:

  • '---' lines corresponds to lines removed from x220 when compared to t420
  • '+++' lines corresponds to lines added to t420 when compared to x220

This means that (if we take x220 as reference, since really similar to x230)

  • t420 includes cmos configuration put at build time (CONFIG_USE_OPTION_TABLE) and loads it unconditionally at each init (CONFIG_STATIC_OPTION_TABLE). Why? dGPU (nvidia graphics) variants used amongst end users?
  • t420 includes libgfxinit (CONFIG_GENERIC_LINEAR_FRAMEBUFFER) graphic init, replacing x220's CONFIG_NO_GFX_INIT which uses exclusively the kernel's i915 kernel driver+drm driver to init display at kernel boot.

Same applies to t430 users:

user@heads-tests:~/heads$ diff -u config/coreboot-t430-maximized.config config/coreboot-t420-maximized.config 
--- config/coreboot-t430-maximized.config	2022-12-21 14:56:14.513000000 -0500
+++ config/coreboot-t420-maximized.config	2022-12-21 14:56:14.513000000 -0500
@@ -3,12 +3,12 @@
 CONFIG_STATIC_OPTION_TABLE=y
 CONFIG_VENDOR_LENOVO=y
 CONFIG_NO_POST=y
-CONFIG_CBFS_SIZE=0xB80000
-CONFIG_IFD_BIN_PATH="@BLOB_DIR@/xx30/ifd.bin"
-CONFIG_ME_BIN_PATH="@BLOB_DIR@/xx30/me.bin"
-CONFIG_GBE_BIN_PATH="@BLOB_DIR@/xx30/gbe.bin"
+CONFIG_CBFS_SIZE=0x750000
+CONFIG_IFD_BIN_PATH="@BLOB_DIR@/xx20/ifd.bin"
+CONFIG_ME_BIN_PATH="@BLOB_DIR@/xx20/me.bin"
+CONFIG_GBE_BIN_PATH="@BLOB_DIR@/xx20/gbe.bin"
 CONFIG_HAVE_IFD_BIN=y
-CONFIG_BOARD_LENOVO_THINKPAD_T430=y
+CONFIG_BOARD_LENOVO_T420=y
 CONFIG_LINUX_COMMAND_LINE="intel_iommu=igfx_off quiet"
 CONFIG_UART_PCI_ADDR=0
 CONFIG_HAVE_ME_BIN=y

Any historical reasons for that?

Tagging board owners per #692:
t420 (xx20): @alexmaloteaux @natterangell (iGPU) @akfhasodh @doob85
t430 (xx30): @Thrilleratplay @alexmaloteaux @lsafd @bwachter(iGPU) @shamen123 @eganonoa(iGPU) @nitrosimon @jans23 @icequbes1 (iGPU) @weyounsix (t430-dgpu)

@natterangell
Copy link
Contributor

I have no idea. Although the fact that the t420 came with an optional dGPU might be related. I started using Heads rather recently, so no historical knowledge on this point.

@tlaurion
Copy link
Collaborator Author

Any of yous being willing to test versions neither including libgfxinit (relying solely on i915 internal Intel graphics) nor cmos options fixated at compile time?

My current understanding is that intel integrated gpu (iGPU) would boot fine as primary display.

To dGPU users out there: you have a problem with Heads booting only on laptop screens? I'm not sure I understand the use case here. I would need some clarifications, since as of today, cmos included at build time is enforcing intel GPU, not enforcing dual display. So users relaying on CircleCI's builds to provide firmware images would either have to apply nvramutil on the rom or modify the coreboot sources to patch cmos file prior of building.

My point here is the following: what use case is intended per current enforced coreboot configuration?

@natterangell
Copy link
Contributor

I can test.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 24, 2023

@natterangell : thank you for your testing proposition, but looking for historical reasons first.

Changing configs would only create more problems then resolve the one I have now (which I have a fix: removing DUMMY programmer inside of flashrom should resolve my issue under #1251 which is currently unused anyway: EDIT: nope, that failed https://app.circleci.com/pipelines/github/tlaurion/heads/1371/workflows/eb87f072-135a-4493-9a45-08630b007761).

Let's clarify functionality provided through options currently defined:

  • By having CONFIG_USE_OPTION_TABLE + CONFIG_USE_OPTION_TABLE, a user applying nvramutil to change the default behavior of CMOS settings have to do so externally on the rom prior of flashing, either by modifying the coreboot source tree directly prior of building rom, or by applying new settings on the ROM, which would modify the cmos files (CMOS memory is totally bypassed when those options are active) This is dicussed here: Set hybrid igpu dgpu by default on boards including dgpu blobs? #1197 (comment)
  • By removing libgfxinit (CONFIG_GENERIC_LINEAR_FRAMEBUFFER) coreboot would be in the dark until kernel's graphic drivers kick in. On t430/t420(?)/t530/t520/w530 where dGPU is desired, separate boards configs should exist which includes additional dGPU blobs (this is the case now for some boards where community work happened) where CONFIG_GENERIC_LINEAR_FRAMEBUFFER provides basic graphics from coreboot to kernel kicking in with kernel drivers where hardware needs to be initialized with the blobs. But as of now, I am unclear on why t420 and t430, not including those blobs (as per seperate board config's coreboot and proper download scripts as for other boards) would not define CONFIG_NO_GFX_INIT and get rid of libgfxinit.

The point here is that for other boards (eg: t530/w530), I kinda understand why CONFIG_GENERIC_LINEAR_FRAMEBUFFER is baked in so that basic support COULD be provided in case a user decides to change cmos settings to enable hybrid graphics, since coreboot includes the additional blobs required and where the OS would be responsible to handle it, in combination with nvramutil setting up cmos settings to enable hybrid graphics.

But for t420/t430 where no dGPU blobs are provided,

  • i'm not sure I understand why CONFIG_GENERIC_LINEAR_FRAMEBUFFER is active
  • i'm not sure I understand why CONFIG_USE_OPTION_TABLE + CONFIG_USE_OPTION_TABLE are fixated, unless some users are modifying other stuff under cmos settings (what?) at build time.

@tlaurion
Copy link
Collaborator Author

t420 (xx20): @alexmaloteaux @akfhasodh @doob85 : do you have dGPUs ? (Nvidia?)

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 24, 2023

t430 (xx30): @weyounsix (t430-dgpu)

@weyounsix Maybe you will be my blank filler here.
Without #1219, you only have dGPU initialized (nvidia) AND available in booted OS. nvramutil NEEDS to be ran to enable hybrid graphic to make the dGPU enabled in final OS but Heads STILL only outputs on iGPU. Correct?

@tlaurion tlaurion changed the title t420 owners (and t430 users) please feed me in historically t420 owners (and t430 users) : Why CONFIG_USE_OPTION_TABLE, CONFIG_STATIC_OPTION_TABLE and CONFIG_GENERIC_LINEAR_FRAMEBUFFER are defined under coreboot configs? Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants