Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ration on hatch in order to get better boot time numbers in vboot_reference
TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 698,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/1
diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 6c0a94a..6db4581 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -22,6 +22,7 @@ select MB_HAS_ACTIVE_HIGH_SD_PWR_ENABLE select SOC_INTEL_COMETLAKE select SYSTEM_TYPE_LAPTOP + select OVERRIDE_CPU_FLEX_RATIO
if BOARD_GOOGLE_BASEBOARD_HATCH
@@ -129,4 +130,8 @@ select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN select VBOOT_LID_SWITCH
+config CPU_FLEX_RATIO + hex + default 5 + endif # BOARD_GOOGLE_BASEBOARD_HATCH
Hello Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#3).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ration on hatch in order to get better boot time numbers in vboot_reference
TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 698,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/3
Hello Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#4).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ration on hatch in order to get better boot time numbers in vboot_reference
TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/4
Hello Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#6).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ration on hatch in order to get better boot time numbers in vboot_reference
TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/6
Hello Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#7).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ration on hatch in order to get better boot time numbers in vboot_reference
TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/7
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG@9 PS7, Line 9: ration ratio
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG@10 PS7, Line 10: better boot time numbers in vboot_reference Please add a dot/period at the end of sentences.
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG@11 PS7, Line 11: Why was the value of *15* chosen?
https://review.coreboot.org/c/coreboot/+/36865/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36865/7/src/mainboard/google/hatch/... PS7, Line 196: Cpu CPU
Hello Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#8).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ratio on hatch in order to get better boot time numbers in vboot_reference.
TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/8
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 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG@9 PS7, Line 9: ration
ratio
Done
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG@10 PS7, Line 10: better boot time numbers in vboot_reference
Please add a dot/period at the end of sentences.
Done
https://review.coreboot.org/c/coreboot/+/36865/7//COMMIT_MSG@11 PS7, Line 11:
Why was the value of *15* chosen?
its based on results we have run to see what is the best freq to get most savings while running vboot_reference code
https://review.coreboot.org/c/coreboot/+/36865/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36865/7/src/mainboard/google/hatch/... PS7, Line 196: Cpu
CPU
Done
Nico Huber 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:
So, is it just the ratio that should be active while that vboot code runs or is there another effect of flex ratio that helps here?
I'm asking because flex ratio seems to have some side effects that might need more attention. And if just changing the frequency is enough, we could just set a different speed-step state with a single MSR write, no?
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:
So, is it just the ratio that should be active while that vboot code runs or is there another effect of flex ratio that helps here?
this needs a soft reset to apply new flex ratio. hence its better we set this early.
Also as i told we are trying to minimize the platform (+SOC) power consumption in FW boot hence trying not to boot using non-turbo max.
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?
And if just changing the frequency is enough, we could just set a different speed-step state with a single MSR write, no?
Are you referring at EIST here ?
Nico Huber 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:
Patch Set 9:
So, is it just the ratio that should be active while that vboot code runs or is there another effect of flex ratio that helps here?
this needs a soft reset to apply new flex ratio. hence its better we set this early.
Sorry, I'm not asking how to use flex ratio. I'm asking if flex ratio is really needed.
Also as i told we are trying to minimize the platform (+SOC) power consumption in FW boot hence trying not to boot using non-turbo max.
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.
And if just changing the frequency is enough, we could just set a different speed-step state with a single MSR write, no?
Are you referring at EIST here ?
Yes. In `soc/intel/cannonlake/cpu.c` we're already calling cpu_set_max_ratio(). This is the first place I would expect someone to change the ratio. I don't know how (if) it is affected by flex ratio, btw.
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:
Patch Set 9:
Patch Set 9:
So, is it just the ratio that should be active while that vboot code runs or is there another effect of flex ratio that helps here?
this needs a soft reset to apply new flex ratio. hence its better we set this early.
Sorry, I'm not asking how to use flex ratio. I'm asking if flex ratio is really needed.
As i understand overriding CPU strap is the only option to tell CPU to running at different speed
Also as i told we are trying to minimize the platform (+SOC) power consumption in FW boot hence trying not to boot using non-turbo max.
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.
And if just changing the frequency is enough, we could just set a different speed-step state with a single MSR write, no?
Are you referring at EIST here ?
Yes. In `soc/intel/cannonlake/cpu.c` we're already calling cpu_set_max_ratio(). This is the first place I would expect someone to change the ratio. I don't know how (if) it is affected by flex ratio, btw.
EIST is not active as we are using hardware managed P-state and we are in BIOS space where EIST has any role to play
Nico Huber 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:
So, is it just the ratio that should be active while that vboot code runs or is there another effect of flex ratio that helps here?
this needs a soft reset to apply new flex ratio. hence its better we set this early.
Sorry, I'm not asking how to use flex ratio. I'm asking if flex ratio is really needed.
As i understand overriding CPU strap is the only option to tell CPU to running at different speed
Well, that depends on the definition of "running at different speed". If it is about the core's clock, EIST can be used. Generally, not saying this is what you should do, as I've not yet figured out how it interacts with Speed Shift.
Also as i told we are trying to minimize the platform (+SOC) power consumption in FW boot hence trying not to boot using non-turbo max.
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 :)
And if just changing the frequency is enough, we could just set a different speed-step state with a single MSR write, no?
Are you referring at EIST here ?
Yes. In `soc/intel/cannonlake/cpu.c` we're already calling cpu_set_max_ratio(). This is the first place I would expect someone to change the ratio. I don't know how (if) it is affected by flex ratio, btw.
EIST is not active as we are using hardware managed P-state and we are in BIOS space where EIST has any role to play
EIST has a role in BIOS space. If you look for "max efficiency boot" for instance in the BIOS Spec. It tells us to use EIST to put the BSP into HFM and the APs into LFM, so the BSP can use most power. I think this is why we are calling cpu_set_max_ratio().
However, with EIST disabled, this might just do nothing.
I don't yet understand the role of HWP (Speed Shift) during boot. All documentation I read so far seems to assume that the OS configures it before it kicks in. So with EIST disabled, is it possible that we run all cores at the same frequency? I guess it doesn't matter because the APs are parked anyway. But it seems to me, that we could use EIST during boot for your frequency change and HWP for the OS. It seems less invasive than repurposing flex ratio.
Nico Huber 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:
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.
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.
Nico Huber 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:
> 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.
I've played around with it a little bit. It isn't really broken (just one flaw when enabling it). And it can be used to control the CPU frequency within coreboot (after BIOS_RESET_CPL only, though). It also seems that it doesn't conflict with HWP. When the OS opts in for the latter the EIST interface gets disabled (at least the BIOS Spec suggests that). So I wonder now, why we have `chip.h` options for them at all. Would it hurt to always enable both?
Tim Wawrzynczak 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:
Could we get the BUG= too?
Hello Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#10).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ratio on hatch in order to get better boot time numbers in vboot_reference.
BUG=b:142264107 TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/10
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 10:
Patch Set 9:
Could we get the BUG= too?
done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 10: Code-Review+2
Thanks for your patience, Subrata! I think we're in a good place to merge this in.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36865/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36865/10//COMMIT_MSG@14 PS10, Line 14: lower Does this change increase or decrease the flex ratio?
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 10: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36865/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36865/10//COMMIT_MSG@14 PS10, Line 14: lower
Does this change increase or decrease the flex ratio?
Good point, I think this raises it from 5 to 15.
Hello Pratikkumar V Prajapati, Tim Wawrzynczak, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36865
to look at the new patch set (#11).
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ratio on hatch in order to get better boot time numbers in vboot_reference.
BUG=b:142264107 TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio (i.e. freq ~1500MHz)
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36865/11
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 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36865/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36865/10//COMMIT_MSG@14 PS10, Line 14: lower
Good point, I think this raises it from 5 to 15.
Ack
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 11:
Patch Set 10: Code-Review+2
Thanks for your patience, Subrata! I think we're in a good place to merge this in.
thanks Tim
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 11: Code-Review+2
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
mb/google/hatch: Override CPU flex ratio
This patch overrides CPU flex ratio on hatch in order to get better boot time numbers in vboot_reference.
BUG=b:142264107 TEST=Able to save ~100ms of platform boot time while running with lower cpu flex ratio (i.e. freq ~1500MHz)
Without this CL
1100:finished vboot kernel verification 802,443 (148,108)
With this CL
1100:finished vboot kernel verification 685,382 (46,496)
Change-Id: Idd1d1c0c04b1f742f17227a1335f27a956ee940d Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36865 Reviewed-by: V Sowmya v.sowmya@intel.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Pratikkumar V Prajapati pratikkumar.v.prajapati@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Pratikkumar V Prajapati: Looks good to me, but someone else must approve V Sowmya: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb index b4d7afd..e0291bb 100644 --- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb @@ -194,6 +194,9 @@ register "gpio_pm[COMM_3]" = "0" register "gpio_pm[COMM_4]" = "0"
+ # CPU Ratio Override + register "cpu_ratio_override" = "15" + # chipset_lockdown configuration # Use below format to override value in overridetree.cb if required # format:
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 12:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/375 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/374 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/373
Please note: This test is under development and might not be accurate at all!
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 12: Code-Review+2
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 12:
Patch Set 12: Code-Review+2
@Shelly: possible to merge into FW branch as well for hatch ?
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 12:
Sure, can you please add in the line
BRANCH=hatch
to specify that this needs to go into the hatch branch?
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 12:
Patch Set 12:
Sure, can you please add in the line
BRANCH=hatch
to specify that this needs to go into the hatch branch?
@Shelly, I can't edit the commit msg anymore as CL already merge. any other option.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
Sure, can you please add in the line
BRANCH=hatch
to specify that this needs to go into the hatch branch?
@Shelly, I can't edit the commit msg anymore as CL already merge. any other option.
That's my bad, I'll keep an eye out for it.
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 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Sure, can you please add in the line
BRANCH=hatch
to specify that this needs to go into the hatch branch?
@Shelly, I can't edit the commit msg anymore as CL already merge. any other option.
That's my bad, I'll keep an eye out for it.
Hi Tim, No worries, i always appreciate your help, how you were closely following this CL. i would have edit the branch name :)
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Sure, can you please add in the line
BRANCH=hatch
to specify that this needs to go into the hatch branch?
@Shelly, I can't edit the commit msg anymore as CL already merge. any other option.
That's my bad, I'll keep an eye out for it.
Hi Tim, No worries, i always appreciate your help, how you were closely following this CL. i would have edit the branch name :)
Ah! I also did not notice that it already merged. Sorry, should've noticed earlier. Will cherry-pick it over to hatch.
Tim Wawrzynczak has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/36865 )
Change subject: mb/google/hatch: Override CPU flex ratio ......................................................................
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Pratikkumar V Prajapati (Code Review)
-
Shelley Chen (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)
-
V Sowmya (Code Review)