Barnali Sarkar has posted comments on this change. ( https://review.coreboot.org/19540 )
Change subject: soc/intel/common/block: Add Intel common CPU code ......................................................................
Patch Set 8:
(12 comments)
https://review.coreboot.org/#/c/19540/8/src/soc/intel/common/block/cpu/Kconf... File src/soc/intel/common/block/cpu/Kconfig:
PS8, Line 4: Intel Common CPU Model Support.
can you please add little bit more like below.
Done
https://review.coreboot.org/#/c/19540/8/src/soc/intel/common/block/cpu/cpu.c File src/soc/intel/common/block/cpu/cpu.c:
PS8, Line 18: #include <assert.h> : #include <bootstate.h> : #include <console/console.h> : #include <cpu/cpu.h> : #include <cpu/x86/mtrr.h> : #include <cpu/x86/msr.h> : #include <cpu/x86/lapic.h> : #include <cpu/x86/mp.h> : #include <cpu/intel/microcode.h> : #include <cpu/intel/speedstep.h> : #include <cpu/intel/turbo.h> : #include <cpu/x86/cache.h> : #include <cpu/x86/name.h> : #include <cpu/x86/smm.h> : #include <delay.h> : #include <device/device.h> : #include <device/pci.h> : #include <intelblocks/cpu.h> : #include <intelblocks/msr.h> : #include <intelblocks/smm.h> : #include <pc80/mc146818rtc.h> : #include <soc/cpu.h> : #include <soc/pci_devs.h> : #include <soc/systemagent.h> : #include <string.h>
let see if we can optimize this
Done
PS8, Line 48: : eax = cpuid_eax(CPUID_LEAF_PM); : : msr = rdmsr(MSR_IA32_MISC_ENABLES); : eax &= 0x2; : if ((!eax) && ((msr.hi & BURST_MODE_DISABLE) == 0)) { : /* Burst Mode has been factory configured as disabled : * and is not available in this physical processor : * package. : */ : printk(BIOS_DEBUG, "Burst Mode is factory disabled\n"); : return; : } : : /* Enable burst mode */ : msr.hi &= ~BURST_MODE_DISABLE; : wrmsr(MSR_IA32_MISC_ENABLES, msr);
can this be put into a function and call as enable/disable turbo for romsta
Done
PS8, Line 66: /* Enable speed step. */ : msr = rdmsr(MSR_IA32_MISC_ENABLES); : msr.lo |= 1 << 16; : wrmsr(MSR_IA32_MISC_ENABLES, msr);
make as function so, mp init feature programming can use this.
Done
https://review.coreboot.org/#/c/19540/8/src/soc/intel/common/block/cpu/cpu_e... File src/soc/intel/common/block/cpu/cpu_early.c:
Line 53: }
can we also have a int get_nominal_ratio() {
Done
PS8, Line 150: void set_P_State_to_turbo_ratio(void)
no camel case
Done
PS8, Line 170: set_P_State_to_nominal_TDP_ratio
same
Done
PS8, Line 189: set_P_State_to_max_non_turbo_ratio
-same
Done
https://review.coreboot.org/#/c/19540/8/src/soc/intel/common/block/cpu/smmre... File src/soc/intel/common/block/cpu/smmrelocate.c:
PS8, Line 34: #include "chip.h"
do we need?
removed and optimized.
https://review.coreboot.org/#/c/19540/8/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/cpu.h:
PS8, Line 25:
one tab less?
Done
https://review.coreboot.org/#/c/19540/8/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/msr.h:
PS8, Line 47:
one tab less
Done
PS8, Line 53: 0x40
(1 << 6)
Done
-
Barnali Sarkar (Code Review)