Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39454 )
Change subject: soc/intel/cannonlake: Steal no memory for disabled IGD ......................................................................
Patch Set 1:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39454/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39454/1//COMMIT_MSG@7 PS1, Line 7: soc/intel/cannonlake: Set IgdDvmt50PreAlloc accordingly to InternalGfx
soc/intel/cannonlake: Steal no memory for disabled IGD?
Ack
https://review.coreboot.org/c/coreboot/+/39454/1//COMMIT_MSG@7 PS1, Line 7: accordingly
according
Ack
https://review.coreboot.org/c/coreboot/+/39454/1//COMMIT_MSG@9 PS1, Line 9: Also
Why *Also*? Isn’t that implied by the first sentence.
Yeah - still the functionality was added.
https://review.coreboot.org/c/coreboot/+/39454/1//COMMIT_MSG@11 PS1, Line 11:
- What does that fix? […]
It does what it says. It sets IgdDvmt50PreAlloc to zero if InternalGfx is disabled. It is 'correct' to do it like this, otherwise the FSP would always allocate memory for the IGD even if it is disabled.
https://review.coreboot.org/c/coreboot/+/39454/1/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39454/1/src/soc/intel/cannonlake/ro... PS1, Line 38: m_cfg->InternalGfx = 1;
With this line missing, did this work before at all?
Right now it seems to work ONLY for CPUs with IGD. Still it was not able to disable the InternalGfx via UPDs because they did not get parsed yet.
https://review.coreboot.org/c/coreboot/+/39454/1/src/soc/intel/cannonlake/ro... PS1, Line 43: }
Why does […]
Would work.