--flash-gpu-escriptor: Keep bay power if already on#283
Conversation
JohnAZoidberg
requested review from
Copilot and
kiram9
and removed request for
kiram9
There was a problem hiding this comment.
Pull request overview
Adjusts --flash_gpu_descriptor behavior to avoid toggling expansion bay GPU power when it’s already enabled, preventing NVIDIA driver crashes/BSOD during EEPROM reprogramming on Windows.
Changes:
- Adds a GPIO read (
gpu_3v_5v_en) to decide whether bay power needs to be forced on during GPU EEPROM writes. - Attempts to only disable bay power after flashing if it was enabled by this command (via
force_power).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Use hide whitespace option to review |
84ffa84 made sure to turn on and off the bay power to enable programming the EEPROM even with bad eeprom (which would cause EC not to turn on bay power). But this would cause issue for reprogramming a good eeprom because the power is already on and it would turn it off afterwards. So if the bay power if turned off for a GPU when windows with nvidia driver is running, the driver crashes and bluescreens Windows. Signed-off-by: Daniel Schaefer <dhs@frame.work>
Signed-off-by: Daniel Schaefer <dhs@frame.work>
JohnAZoidberg
force-pushed
the
keep-bay-power
branch
from
1068457 to
82d1654
Compare
JohnAZoidberg
changed the title
JohnAZoidberg
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if let Ok(ExpansionBayBoard::DualInterposer) = info.expansion_bay_board() { | ||
| /* Force power to the GPU if we are writing the EEPROM */ | ||
| let res = self.set_gpio("gpu_3v_5v_en", true); | ||
| if let Err(err) = res { | ||
| error!("Failed to set ALW power to GPU off {:?}", err); | ||
| return Err(err); | ||
| // If bay power is on already, we don't need to enable/disable it | ||
| if !self.get_gpio("gpu_3v_5v_en")? { | ||
| // Force power to the GPU if we are writing the EEPROM | ||
| let res = self.set_gpio("gpu_3v_5v_en", true); | ||
| if let Err(err) = res { | ||
| error!("Failed to set ALW power to GPU on {:?}", err); | ||
| return Err(err); | ||
| } | ||
| println!("Forcing Power to GPU"); | ||
| os_specific::sleep(100_000); | ||
| force_power = true; | ||
| } |
There was a problem hiding this comment.
dry_run currently skips the actual EEPROM writes inside the chunk loop, but the bay-power GPIO manipulation and the initial sleep still run even in dry-run mode. This means --dry-run can still change hardware power state (and potentially impact a running OS/driver). Consider guarding the get_gpio/set_gpio/sleep block (and the later power-off restore) with if !dry_run, consistent with other flash paths in this module.
There was a problem hiding this comment.
hmm that's a good idea
Signed-off-by: Daniel Schaefer <dhs@frame.work>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
84ffa84 made sure to turn on and off the bay power to enable programming the EEPROM even with bad eeprom (which would cause EC not to turn on bay power).
But this would cause issue for reprogramming a good eeprom because the power is already on and it would turn it off afterwards.
So if the bay power if turned off for a GPU when windows with nvidia driver is running, the driver crashes and bluescreens Windows.
Fixes #240