Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35880 )
Change subject: src/soc/intel: FSP2.0 platforms: add mainboard option to skip graphics init ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35880/2/src/soc/intel/apollolake/ch... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/35880/2/src/soc/intel/apollolake/ch... PS2, Line 754: #if CONFIG(HAVE_FSP_GOP)
We don't use preprocessor #if's unless necessary. Also, we know that […]
Done
https://review.coreboot.org/c/coreboot/+/35880/2/src/soc/intel/apollolake/ch... PS2, Line 757: RUN_FSP_GOP
...and this even covers HAVE_FSP_GOP too...
Done
https://review.coreboot.org/c/coreboot/+/35880/2/src/soc/intel/apollolake/ch... PS2, Line 759: params->GtFreqMax = 2;
Does it exist?
Done
https://review.coreboot.org/c/coreboot/+/35880/2/src/soc/intel/apollolake/ch... PS2, Line 760: params->CdClock = 3;
Seems like a random choice? and what APL device needs it this high?
on APL the current default that is set/copied is 4: $gBroxtonFspPkgTokenSpaceGuid_CdClock 1 bytes $_DEFAULT_ = 0x04
https://review.coreboot.org/c/coreboot/+/35880/2/src/soc/intel/apollolake/ch... PS2, Line 761: } else {
... […]
Done