Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 9:
Patch Set 9:
I'm asking because flex ratio seems to have some side effects that might need more attention.
I don't get this part, can you please help to explain about side effect?
It changes CPUID and MSR values that otherwise can't be changed (mostly to align multiple processor packages, is what I found in documentation). This affects all the code in coreboot that reads those registers. For instance, I suspect that our ACPI p-state table generation might be wrong of flex ratio is used.
We are running full BAT with CPU bench marking to see if any issue. Although i don't think that matter.
It might not. There is a lot of dead, broken code for these platforms, it seems. EIST is not enabled on any of them and enabling doesn't work. So no p-state tables are generated at all. That should be safe :)
Just tried CFL-H with EIST instead of HWP and flex ratio reduced to 13 (27 default). This makes coreboot generate p-state table entries only up to 1301MHz. It seems to run at full-speed anyway, but something is definitely fishy.
Again, this is not a problem for existing boards, because EIST is never enabled. But at least worth a comment in the previous patch, that the current code is incompatible with flex ratio. Alternatively, we could also just remove all EIST code from cannonlake/ on. If nobody uses it and it becomes a burden, I see no reason to keep it.
make sense to remove EIST from CNL and other. if you like to do it, please pick it.