Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36864 )
Change subject: soc/intel/cannonlake: Add chip config to override CPU flex ratio ......................................................................
Patch Set 6:
(4 comments)
Patch Set 6:
(2 comments)
I have read through the comments but still don't understand a lot of things:
Please explain why a higher ratio results in a lower speed.
Please explain in detail what the `CpuRatio` UPD actually does. How does it relate to `BootFrequency`?
Also, it seems to me that the measurements are void, because the currently simplified tsc_freq_mhz() implementation doesn't support "flex ratio". I think we should implement tsc_freq_mhz() correctly first, before jumping to any conclusions.
Please don't assume thing just because there is one issue going on where tsc_freq is in question. I can ensures that issue doesn't have anything related to this work. We have run some experiment to minimize platform power consumption by running CPU at lower speed and found that running at lower freq provides better boot time number specifically running below code
https://chromium.googlesource.com/chromiumos/platform/vboot/+/refs/heads/mas...
Additionally, for the interested reader: flex ratio adapts the maximum non-turbo ratio. And is mainly a feature to influence the TSC rate...
We are using CPU speed independent timer hence running at HFM or LFM doesn't matter to TSC
https://review.coreboot.org/c/coreboot/+/36864/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36864/6//COMMIT_MSG@13 PS6, Line 13: of
if
Done
https://review.coreboot.org/c/coreboot/+/36864/6/src/soc/intel/cannonlake/ch... File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/c/coreboot/+/36864/6/src/soc/intel/cannonlake/ch... PS6, Line 429: * Override CPU ratio value:
No *flex* in the comment?
Done
https://review.coreboot.org/c/coreboot/+/36864/6/src/soc/intel/cannonlake/ch... PS6, Line 430: * CPU ratio value controls the maximum processor non-turbo ratio
Maybe add the data sheet and section, where this is described.
its part of SDM.
https://review.coreboot.org/c/coreboot/+/36864/6/src/soc/intel/cannonlake/ch... PS6, Line 432: * FSP to skip cpu_ratio override if cpu_ratio set to 0
FSP skips cpu_ratio override if cpu_ratio is 0.
Done