heads icon indicating copy to clipboard operation
heads copied to clipboard

Set hybrid igpu dgpu by default on boards including dgpu blobs?

Open tlaurion opened this issue 3 years ago • 12 comments

t430/t530/w530 opened relevant (add others)

The problem/feature is that currently deployed coreboot configs are setting both STATIC_OPTION_TABLE and USE_OPTION_TABLE.

USE_OPTION_TABLE instructs coreboot to read CMOS settings from file if not existing in memory, while STATIC_OPTION_TABLE instructs coreboot to overwrite settings in memory with the content of cbfs content on each boot, whatever is set in memory, not letting the user change settings but by flashing changes back into rom.

See https://github.com/coreboot/coreboot/blob/master/src/Kconfig

By removing STATIC_OPTION_TABLE, we could add either nvramtool or another GUI tool under heads rom that can manipulate directly CMOS settings, which would be kept in CMOS memory, and survive firmware upgrades.

The ROM would come with defaults from coreboot, populate cmos if not existing, but otherwise customizations would live until CMOS battery is disconnected.

The danger of removing STATIC_OPTION_TABLE and providing nvramtool is that if the user sets a setting that causes problem, CMOS battery would need disconnection to rollback to what is stored into compiled ROM cmos_layout.txt

tlaurion avatar Aug 03 '22 14:08 tlaurion

Another option would be to create hybrid boards, AND embed a different cmos_layout file into coreboot configurations for boards having dgpus, and keep STATIC_OPTION_TABLE.

https://github.com/coreboot/coreboot/blob/433810a5777cbfb1ee5cae5b284481acdbf63048/src/Kconfig#L734

I think it would make sense to activate hybrid graphics automatically for boards which integrate dgpu blobs, and setting hybrid mode automatically on those machines.

Otherwise I'm not sure why we would want to include such blobs if end user is not intending to activate dgpu. Or why users would want igpu deactivated.

Point being that my understanding for such boards is that hybrid mode is desired, so not sure why it would be anything else then in hybrid mode and defined in ROM with proper, intended, defaults.

Let's remember that as of now, end users have to manually run nvramtool on produced ROM, which changes the cmos_layout in rom file with

hybrid_graphics_mode="Dual Graphics"

As some user reported "there are severe limitations to the Optimus GPU's under coreboot, as you cannot use them for offload rendering, so afaik there is no point to using any other hybrid_graphics_mode setting".

And if someone forgets to run nvramtool to set cmos_layout to be in hybrid mode, hybrid mode won't work on firmware upgrade as of now, until manually reapplied.

Thoughts on best approach to take?

tlaurion avatar Aug 03 '22 14:08 tlaurion

Tagging board owners per https://github.com/osresearch/heads/issues/692:

t420 (xx20): @alexmaloteaux @natterangell (iGPU) @akfhasodh t430 (xx30): @Thrilleratplay @alexmaloteaux @lsafd @bwachter @shamen123 @eganonoa(iGPU) @nitrosimon @jans23 @icequbes1 (iGPU) t520 (xx30): @eganonoa t530 (xx30): @eganonoa w530 (xx30): @eganonoa @zifxify (w530-dgpu-K2000m)

tlaurion avatar Aug 03 '22 14:08 tlaurion

i own w530-dgpu-K2000m, t430-dgpu, and i would like to see this. would it be possible to call nvramtool during make so that the setting is baked into the rom by default?

weyounsix avatar Aug 04 '22 00:08 weyounsix

I have iGPU only, but nvramtool included would be great!

natterangell avatar Aug 06 '22 09:08 natterangell

@natterangell @weyounsix 2 answers, two different opinions, no other participation.

If we refer to OP, choices are

  • remove static option table.
    • Have nvramtool under heads being able to change settings in CMOS. And have user remove battery to revert to default. Would survive firmware upgrade.
  • Keep static option table
    • have nvramutil under heads, have nvramutil modify the ROM directly. Won't survive upgrade.
    • have nvramutil modify ROM post build. Invalidates the ROM hashes.

Not aware of other options. I would tend to prefer the removal of static option table under dgpu boards and have nvramtool under heads, where users can change settings in CMOS and simply disconnect the battery if they mess up, which would get them back to compiled defaults.

tlaurion avatar Aug 17 '22 03:08 tlaurion

Seems removing static option table makes sense.

natterangell avatar Aug 19 '22 19:08 natterangell

@natterangell @weyounsix modified my previous post. Please choose path.

tlaurion avatar Aug 19 '22 23:08 tlaurion

I'm not touching anything with a nvidia chip, so all my thinkpads are iGPU only.

bwachter avatar Aug 25 '22 18:08 bwachter

@bwachter then if you use igpu only you should not compile with a dgpu enabled board?

Modified #692 to specify you use t430 with iGPU only.

tlaurion avatar Aug 25 '22 20:08 tlaurion

Removing static option table seems prudent.

natterangell avatar Aug 30 '22 18:08 natterangell

i own w530-dgpu-K2000m, t430-dgpu, and i would like to see this. would it be possible to call nvramtool during make so that the setting is baked into the rom by default?

I have created a PoC which included a hack around building ifdtool from coreboot to be cross-compiled, so that it could be included into packaged rom. This is pertinent here, because we are talking about another coreboot util here (nvramtool) which would need to be packaged to be run INSIDE of Heads, while not breaking nvramutil that can be packaged to be run on host (builder). So the same process could be followed to have nvramtool build and included in dGPU enabled boards:

This would require:

  • coreboot module file to copy coreboot util (here ifdtool instead of nvramutil) to be copied into its own directory, so that it can thenbe cross-compiled: https://github.com/tlaurion/heads/commit/a4b5381e65f0f2da5fbbe742696ad6b15f1b9bc2#diff-18936189b28399cf48703d0c1ec1df33e57c559de2a12f4438be00e6813bdb68R53
  • Add an environment variable into board configurations to be able to ask for that util to be compiled and packaged under initrd, here NVUTIL: https://github.com/tlaurion/heads/commit/a4b5381e65f0f2da5fbbe742696ad6b15f1b9bc2#diff-fb98d6f892bb15e235b3e7ad3689cc1602832d9fdcb50d1fbe447286973043c9R25
  • Then when declare that variable for a board, so that here-added Makefile magic actually compiles and packages additional ifdtool (here to be replaced with nvramutil): https://github.com/tlaurion/heads/commit/a4b5381e65f0f2da5fbbe742696ad6b15f1b9bc2#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R464-R470

A similar logic could be added for nvramutil to be declared in dGPU enabling boards, and to have nvramutil packaged under dGPU boards roms.

Then, users could call nvramutil from Heads recovery to modify cmos values, which would persist in CMOS, if STATIC_OPTION_TABLE is of course removed under coreboot configuration for a dGPU enabled board.

Someone of yous are interesting into proposing a PR filling your need following that guidance? I would love to give fishlines instead of supplying unlimited supplies of fish here.

I see @weyounsix being interested to test. Someone up to the task of filling shown common needs here and understand and participate in Heads development?

tlaurion avatar Sep 15 '22 20:09 tlaurion

Or simply building nvramcui, which is a elf binary (aimed to be a secondary payload) but built from the crosscompiler (we know by experience that memtest86+ was not working to be launched as a binary, maybe needing to be crosscompiled and resolved by booting latest memtest86 and resolved in seperate issue)? https://github.com/coreboot/coreboot/blob/afbc2c9c0cf0767a95871f18c96b026335cdb55c/payloads/nvramcui/Makefile#L8

tlaurion avatar Sep 15 '22 20:09 tlaurion