Nico Huber 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 8:
(1 comment)
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.
No, I was assuming things because the SDM implies the opposite behaviour for CPUID+0x16 (that it doesn't reflect the actual speed). Please don't just accept such documentation problems. If the documentation isn't fixed and there is no comment in the code about the broken documentation, you have to expect such disarray.
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
The current ratio doesn't matter, but the flex ratio does. You can see that the timestamps stay correct with different flex-ratio settings while CPUID +0x16 EAX changes. So the TSC rate has to change too.
https://review.coreboot.org/c/coreboot/+/36864/2/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/36864/2/src/soc/intel/cannonlake/ro... PS2, Line 80: if (CONFIG(OVERRIDE_CPU_FLEX_RATIO))
we are suspecting cache misses more while running this for loop at higher freq, comparing core/uncor […]
Heh, interesting. I would have suspected that such things can impact performance a little, but a 3x speed increase is quite impressive.