Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 2:
(3 comments)
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG@9 PS1, Line 9: CML
What does CML stand for?
typo mistake
https://review.coreboot.org/#/c/30917/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/2//COMMIT_MSG@9 PS2, Line 9: CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for : CNL only
Does CNL use the midfixed option or not?
yes CNL is using midfixed so keeping for it and removing for others.
https://review.coreboot.org/#/c/30917/2/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/2/src/soc/intel/cannonlake/chip.h@109 PS2, Line 109: SaGv_FixedMid,
this means that SaGv is passed different numbers depending on compile time options. […]
yes so CNL soc can train at 3 different frequency so CNL FSP UPD SaGv can have all this 5 valuse, 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled
but WHL and CFL can train at 2 different frequency so SaGv UPD don't have the option of fixedMid. in that case we need to remove FixedMid option. It is depends on FSP and ultimately on Soc support.