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

Add t430-dgpu support #1219

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Add t430-dgpu support #1219

wants to merge 5 commits into from

Conversation

weyounsix
Copy link

@weyounsix weyounsix commented Sep 24, 2022

apologies for previous closed requests.
learning how to git, closed previous requests due to extraneous commits on my master

Changes:
-Initial t430-dgpu configs from @eganonoa
-patched to match the new build directory structure
-patched vbios script for dependencies, sudo calls, error reporting
-updated README_vbios

replaces PR #1218 (which replaced PR #1177 (which replaced PR #1175 ))

eganonoa and others added 5 commits December 29, 2021 00:26
Maximized boards being supported here only. I do not have a board to test. But these have been tested as working by another user (see linuxboot#1057 (comment)).
Add support for t430 dgpu-versions
patches vbios script dependencies, error reporting, sudo calls
updates board and coreboot configs to match new directory structure of master
changes build dependency to linux-x230-maximized
cleaned up formatting and added instructions for clarity
removed instruction to call script with sudo since sudo calls have been added to script
make executable
@weyounsix
Copy link
Author

weyounsix commented Sep 24, 2022

vscode somehow stripped execute permissions from vbios_t430.sh !! frustrating, but I can do another fresh PR if needed to make this a cleaner pull. let me know.

@weyounsix
Copy link
Author

weyounsix commented Sep 27, 2022

tested only dirty builds from a local repo, but they worked as expected. I will test the circleci artifacts tonight and report back

NOTE: the artifacts will likely not work unless nvramtool is used to set "Dual Graphics" hybrid mode, as specified in README_vbios. my previous experience with the t430-dgpu on the initial builds from @eganonoa's dgpu-t430 branch was that it wouldn't boot at all unless this step was done. the keyboard lights would flash with a steady beep from the speakers. i can confirm this as well.

@tlaurion
Copy link
Collaborator

tlaurion commented Sep 27, 2022

@weyounsix

NOTE: the artifacts will likely not work unless nvramtool is used to set "Dual Graphics" hybrid mode, as specified in README_vbios. my previous experience with the t430-dgpu on the initial builds from @eganonoa's dgpu-t430 branch was that it wouldn't boot at all unless this step was done. the keyboard lights would flash with a steady beep from the speakers. i can confirm this as well.

That goes into actual debate for all dgpu boards: should cmos.layout be changed to enable dual graphics here? (requiring to call nvramutil from the board configuration file, so that the rom itself is enabled for dual graphics? And then STATIC_OPTION being removed from coreboot config, so that a user can disable it from within Heads through command line/GUI option?

Your report here seems to state that t430-dgpu board is different then w530. You seem to report that t430-dgpu MUST have dual graphics enabled in CMOS/cmos.layout, otherwise causing a brick.

I would consequently advise against merging this PR until this is figured out and that board configuration produces the right thing by default. No time to deal with bricking from the masses not reading documentation nor a default behavior that is not desirable at all, causing a brick (think upgrading firmware internally forgetting to apply nvramutil to enable dual graphics for most people not having an external programmer and relaying on a friend/contact for initial external flashing.)

@tlaurion
Copy link
Collaborator

tlaurion commented Sep 27, 2022

@weyounsix Please check weyounsix#3

By compiling nvramtool locally, and adding an entree in Makefile for DGPU ROM name, we make sure in board files to copy to original rom into DGPU ROM, apply nvramtool to apply Dual Graphics on DGPU Rom, and then split the firmware into top and bottom roms.

Please test and review and modify as you please. You are the only T430 DGPU board owner. Own this code, change so it is clear to you and modify the present PR with what you find more clear and adapt documentation entree accordingly.

@weyounsix
Copy link
Author

great news: the circleci artifacts all booted and functioned as expected,
even without setting Dual Graphics mode
so that is clearly no longer an issue.

when i initially built heads for this board, the t430-maximized rom also wouldn't boot, and that was why i initially went down the path to find a build that included dgpu blobs. i can test this too next time to see if that is also no longer an issue.

i still would love to make the dual graphics cmos layout automated! awesome work on that. i will see what i can do with your work as a starting point. perhaps i could also automate the vbios script so that the entire build process is no different from a standard maximized board, then in the docs we could advise board owners how to change the default cmos layout (at their own risk).

@tlaurion
Copy link
Collaborator

tlaurion commented Sep 28, 2022

NOTE: the artifacts will likely not work unless nvramtool is used to set "Dual Graphics" hybrid mode, as specified in README_vbios. my previous experience with the t430-dgpu on the initial builds from @eganonoa's dgpu-t430 branch was that it wouldn't boot at all unless this step was done. the keyboard lights would flash with a steady beep from the speakers. i can confirm this as well.

So that assumption was false: t430 non-dgpu ROMs can boot t430 dGPU boards but without dGPU enabled. Basically like the others.

@tlaurion
Copy link
Collaborator

@weyounsix Can you answer the above unanswered questions?

@tlaurion
Copy link
Collaborator

tlaurion commented Mar 25, 2024

moved as unmaintained under #1604 and put this PR as draft.

@tlaurion tlaurion marked this pull request as draft March 25, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants