Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 4:
(9 comments)
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 114: >
=
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 115: 28
so, 28s is the default if out of range? […]
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 131: BIOS_DEBUG
Even log level INFO?
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 148:
extra indent?
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 161: printk(BIOS_DEBUG, "CPU PL2 = %u Watts\n", tdp_pl2 / power_unit);
Ditto.
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 198:
extra space
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 209: =
be consistent about spaces around '=' in these messages […]
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 75: /* Configure turbo power limits 1ms after reset complete bit */ : mdelay(1);
Why is this? Is this requirement captured anywhere?
We will get back on this.
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 77: 28
If this is statically being set to 28, why does it have to be passed in as a parameter? Can't set_po […]
Need to check this kind of implementation.