Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42547 )
Change subject: cpu/intel/model_206ax: Add overclocking support ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42547/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42547/1//COMMIT_MSG@12 PS1, Line 12: Tested on Asus P8Z77-V LX2 with an i5-3330, CPU now runs at 3.4 GHz.
How did you check that? coreboot output? Linux output? […]
I checked the speed using /proc/cpuinfo while loading all CPU cores. Before, the frequency under the same conditions would be 3.0 GHz (the non-turbo frequency) with coreboot (Asus stock firmware allows one to set the multiplier ratios).
Normally, Turbo Boost uses higher ratios when less CPU cores are active. With this, one can force the maximum turbo ratio for any number of active CPU cores. That 3.2 GHz is most likely the turbo frequency with only one active core.
The maximum non-turbo frequency of 3.0 GHz, which they call `processor base frequency` is on ARK. Intel just uses the larger `max turbo frequency` in the title, because it looks better 😄
https://ark.intel.com/content/www/us/en/ark/products/65509/intel-core-i5-333...
https://review.coreboot.org/c/coreboot/+/42547/1//COMMIT_MSG@13 PS1, Line 13:
It’d be awesome, if you added a reference to some datasheet, so people can look up, what “overclocki […]
Having `n` overclocking bins means that the max turbo ratio is equal to the non-turbo ratio plus `n`. On the i5-3330 I have, `n` is 4, so I can get an extra 0.4 GHz for free 😄
Unfortunately, the relevant MSRs aren't in datasheets, only on non-public material 😞
IIRC there's a comment in finalize.c which mentions the BIOS Writer's Guide, so I could mention this document as well. Even if most people don't have access to the BWG, it would be better than nothing.
https://review.coreboot.org/c/coreboot/+/42547/1/src/cpu/intel/model_206ax/m... File src/cpu/intel/model_206ax/model_206ax_init.c:
https://review.coreboot.org/c/coreboot/+/42547/1/src/cpu/intel/model_206ax/m... PS1, Line 443: xe_init
what's XE? Extreme?
IIRC, it means eXtreme Edition. Apparently, this procedure is called "XE initialization", hence the function name. Shall I put a comment on the function?
https://review.coreboot.org/c/coreboot/+/42547/1/src/cpu/intel/model_206ax/m... PS1, Line 449: printk(BIOS_DEBUG, "XE: not overriding Turbo Ratio limits\n");
since the default is disabled, only print this text when it has been enabled in Kconfig?
The idea is that this function always prints one line when executed. This allows one to know which path was taken when this function has run. However, note that each printk has a different loglevel. This is intentional, as each path has a different severity.
Taking this path is expected, so this message is just for debugging.
The error path is more important, because this option was enabled but it could not do what one expected it to do. Still, it does not prevent a successful boot, so it's only a warning.
The last message is split in two because writing to the MSR could raise a GP# fault. In most circumstances, it should not happen, but splitting it in two allows one to know which value was being programmed, and that the fault happened right there. Furthermore, its loglevel is BIOS_NOTICE because it means the CPU is now overclocked, which might effect stability and is thus important to know when debugging.
https://review.coreboot.org/c/coreboot/+/42547/1/src/cpu/intel/model_206ax/m... PS1, Line 459: 0x206a3
huh, that cpuid isn't in the cpu_table list?
0x206a0 is on the cpu_table list, and the oc_bins value for it would not be valid. Looks like 0x206a3 is a valid CPUID, albeit only used in engineering samples.
http://www.cpu-world.com/cgi-bin/CPUID.pl?CPUID=12861