coreboot-gerrit January 2023

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

[S] Change in coreboot[master]: src/lib: Include LZMA in romstage for FSP-M
by Julius Werner (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Martin L Roth, Martin Roth. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71674 ) Change subject: src/lib: Include LZMA in romstage for FSP-M ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File src/lib/Makefile.inc: https://review.coreboot.org/c/coreboot/+/71674/comment/14034f3b_93818003 PS1, Line 98: ifneq ($(CONFIG_COMPRESS_RAMSTAGE_LZMA)$(CONFIG_FSP_COMPRESS_FSP_M_LZMA),) Could also just always compile it in and let the linker garbage collection take care of it. -- To view, visit https://review.coreboot.org/c/coreboot/+/71674 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id52d25a13420f05db8b2b563de0448f9d44638e0 Gerrit-Change-Number: 71674 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 16:34:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/: Rename small and big cores references
by Kapil Porwal (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Sridhar Siricilla, Werner Zeh. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71639 ) Change subject: soc/intel/: Rename small and big cores references ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/71639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I98c9c0ed86b211d736a0a1738b47410faa13a39f Gerrit-Change-Number: 71639 Gerrit-PatchSet: 3 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 16:34:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Kconfig: Add option to compress ramstage with LZ4
by Julius Werner (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Martin L Roth, Martin Roth. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71673 ) Change subject: Kconfig: Add option to compress ramstage with LZ4 ...................................................................... Patch Set 1: (4 comments) File src/Kconfig: https://review.coreboot.org/c/coreboot/+/71673/comment/cc924098_27e5c7f0 PS1, Line 173: config MB_COMPRESS_RAMSTAGE_LZ4 I wonder if this should be a variable affecting both payload compression and ramstage compression, because if LZ4 is better for one of them it would likely be better for the other as well? It could be a sort of generic PREFER_LZ4 option that affects all compression choices. https://review.coreboot.org/c/coreboot/+/71673/comment/b18bd944_6463e019 PS1, Line 180: depends on HAVE_RAMSTAGE && !UNCOMPRESSED_RAMSTAGE I forget, what actually happens when the depends on in a choice is not satisfied? Does that mean both of the choice options will be set to false? I thought one of the choice options always has to be true but maybe I misremember that case... https://review.coreboot.org/c/coreboot/+/71673/comment/f01a6a49_f1250ed9 PS1, Line 181: default COMPRESS_RAMSTAGE_LZ4 if MB_COMPRESS_RAMSTAGE_LZ4 Don't you still need a `default COMPRESS_RAMSTAGE_LZMA` after this as well? https://review.coreboot.org/c/coreboot/+/71673/comment/bb004cce_51c79cf2 PS1, Line 192: Compress ramstage with LZ4 for faster decompression It would be good to explain the trade-offs involved in this decision (e.g. flash reading speed vs CPU speed and flash space) in more detail in one of these help strings. -- To view, visit https://review.coreboot.org/c/coreboot/+/71673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27dd1a8def024e0efd466cef9ffd9ca71717486a Gerrit-Change-Number: 71673 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 16:33:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel: Add Kconfigs to define scaling factor for cores
by Kapil Porwal (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Sridhar Siricilla. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71687 ) Change subject: soc/intel: Add Kconfigs to define scaling factor for cores ...................................................................... Patch Set 1: (2 comments) File src/soc/intel/common/block/acpi/cpu_hybrid.c: https://review.coreboot.org/c/coreboot/+/71687/comment/0a877cd6_98b6bd05 PS1, Line 78: soc_get_scaling_factor Do we even need this function now? https://review.coreboot.org/c/coreboot/+/71687/comment/4b645762_ec6f2abf PS1, Line 79: scal_factor.performance_core Can't we use `CONFIG_SOC_INTEL_PERF_CORE_SCAL_FACTOR`? -- To view, visit https://review.coreboot.org/c/coreboot/+/71687 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I55e4d815116ef40c5f33be64ab495e942bf35ee8 Gerrit-Change-Number: 71687 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 16:31:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: soc/intel/mtl: Add missing claimed memory regions
by Kapil Porwal (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Tarun Tuli, Eran Mitrani, Subrata Banik. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71114 ) Change subject: soc/intel/mtl: Add missing claimed memory regions ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/71114/comment/c2e38f49_f7614a65 PS3, Line 13: Please add more details, e.g. 'cbmem -c' snippet: ``` DOMAIN: 0000 read_resources bus 0 link: 0 SA MMIO resource: PCIEXBAR -> base = 0xc0000000, size = 0x10000000 dev: PCI: 00:00.0, index: 0x0, base: 0xc0000000, size: 0x10000000 SA MMIO resource: MCHBAR -> base = 0xfedc0000, size = 0x00020000 dev: PCI: 00:00.0, index: 0x1, base: 0xfedc0000, size: 0x20000 SA MMIO resource: DMIBAR -> base = 0xfeda0000, size = 0x00001000 dev: PCI: 00:00.0, index: 0x2, base: 0xfeda0000, size: 0x1000 SA MMIO resource: EPBAR -> base = 0xfeda1000, size = 0x00001000 dev: PCI: 00:00.0, index: 0x3, base: 0xfeda1000, size: 0x1000 SA MMIO resource: REGBAR -> base = 0xd0000000, size = 0x10000000 dev: PCI: 00:00.0, index: 0x4, base: 0xd0000000, size: 0x10000000 SA MMIO resource: EDRAMBAR -> base = 0xfed80000, size = 0x00004000 dev: PCI: 00:00.0, index: 0x5, base: 0xfed80000, size: 0x4000 SA MMIO resource: CRAB_ABORT -> base = 0xfeb00000, size = 0x00080000 dev: PCI: 00:00.0, index: 0x6, base: 0xfeb00000, size: 0x80000 SA MMIO resource: TPM -> base = 0xfed40000, size = 0x00010000 dev: PCI: 00:00.0, index: 0x7, base: 0xfed40000, size: 0x10000 SA MMIO resource: LT_SECURITY -> base = 0xfed50000, size = 0x00020000 dev: PCI: 00:00.0, index: 0x8, base: 0xfed50000, size: 0x20000 SA MMIO resource: APIC -> base = 0xfec00000, size = 0x00100000 dev: PCI: 00:00.0, index: 0x9, base: 0xfec00000, size: 0x100000 SA MMIO resource: PCH_RESERVED -> base = 0xfd800000, size = 0x01000000 dev: PCI: 00:00.0, index: 0xa, base: 0xfd800000, size: 0x1000000 SA MMIO resource: MMCONF -> base = 0xc0000000, size = 0x10000000 dev: PCI: 00:00.0, index: 0xb, base: 0xc0000000, size: 0x10000000 SA MMIO resource: DSM -> base = 0x7c000000, size = 0x04000000 dev: PCI: 00:00.0, index: 0xc, base: 0x7c000000, size: 0x4000000 SA MMIO resource: TSEG -> base = 0x7b000000, size = 0x00800000 dev: PCI: 00:00.0, index: 0xd, base: 0x7b000000, size: 0x800000 SA MMIO resource: GSM -> base = 0x7b800000, size = 0x00800000 dev: PCI: 00:00.0, index: 0xe, base: 0x7b800000, size: 0x800000 dev: PCI: 00:00.0, index: 0xf, base: 0x0, size: 0xa0000 dev: PCI: 00:00.0, index: 0x10, base: 0xc0000, size: 0x75f40000 dev: PCI: 00:00.0, index: 0x11, base: 0x76000000, size: 0xa000000 Available memory above 4GB: 6144M dev: PCI: 00:00.0, index: 0x12, base: 0x100000000, size: 0x180000000 dev: PCI: 00:00.0, index: 0x13, base: 0xa0000, size: 0x20000 dev: PCI: 00:00.0, index: 0x14, base: 0xc0000, size: 0x40000 ``` 'dmesg' snippet: ``` BIOS-e820: [mem 0x0000000000000000-0x0000000000000fff] reserved BIOS-e820: [mem 0x0000000000001000-0x000000000009ffff] usable BIOS-e820: [mem 0x00000000000a0000-0x00000000000fffff] reserved BIOS-e820: [mem 0x0000000000100000-0x00000000759a9fff] usable BIOS-e820: [mem 0x00000000759aa000-0x000000007fffffff] reserved BIOS-e820: [mem 0x00000000c0000000-0x00000000e0ffffff] reserved BIOS-e820: [mem 0x00000000f8000000-0x00000000f9ffffff] reserved BIOS-e820: [mem 0x00000000fd800000-0x00000000fe7fffff] reserved BIOS-e820: [mem 0x00000000feb00000-0x00000000feb7ffff] reserved BIOS-e820: [mem 0x00000000fec00000-0x00000000fecfffff] reserved BIOS-e820: [mem 0x00000000fed40000-0x00000000fed6ffff] reserved BIOS-e820: [mem 0x00000000fed80000-0x00000000fed83fff] reserved BIOS-e820: [mem 0x00000000feda0000-0x00000000feda1fff] reserved BIOS-e820: [mem 0x00000000fedc0000-0x00000000feddffff] reserved BIOS-e820: [mem 0x00000000ff000000-0x00000000ffffffff] reserved BIOS-e820: [mem 0x0000000100000000-0x000000027fffffff] usable BIOS-e820: [mem 0x000003fff0aa0000-0x000003fff0aa1fff] reserved ``` File src/soc/intel/meteorlake/include/soc/systemagent.h: https://review.coreboot.org/c/coreboot/+/71114/comment/52570d94_68d2d1a4 PS3, Line 49: 0xFED50000 As per MTL FAS, There are 2 ranges for LT: FED20000-FED7FFFF FED50000-FED6FFFF File src/soc/intel/meteorlake/systemagent.c: https://review.coreboot.org/c/coreboot/+/71114/comment/3d4f25e4_dedf87a9 PS3, Line 27: { PCIEXBAR, CONFIG_ECAM_MMCONF_BASE_ADDRESS, CONFIG_ECAM_MMCONF_LENGTH, : "PCIEXBAR" }, : { MCHBAR, MCH_BASE_ADDRESS, MCH_BASE_SIZE, "MCHBAR" }, : { DMIBAR, DMI_BASE_ADDRESS, DMI_BASE_SIZE, "DMIBAR" }, : { EPBAR, EP_BASE_ADDRESS, EP_BASE_SIZE, "EPBAR" }, : { REGBAR, REG_BASE_ADDRESS, REG_BASE_SIZE, "REGBAR" }, : { EDRAMBAR, EDRAM_BASE_ADDRESS, EDRAM_BASE_SIZE, "EDRAMBAR" }, : : /* first field (sa_mmio_descriptor.index) is not used, setting to 0: */ : { 0, CRAB_ABORT_BASE_ADDR, CRAB_ABORT_SIZE, "CRAB_ABORT" }, : { 0, TPM_BASE_ADDRESS, TPM_SIZE, "TPM" }, : { 0, LT_SECURITY_BASE_ADDR, LT_SECURITY_SIZE, "LT_SECURITY" }, : { 0, IO_APIC_ADDR, APIC_SIZE, "APIC" }, : // PCH_PRESERVERD covers: : // TraceHub SW BAR, SBREG, PMC MBAR, SPI BAR0, SerialIo BAR in ACPI mode : // eSPI LGMR BAR, eSPI2 SEGMR BAR, TraceHub MTB BAR, TraceHub FW BAR : // see fsp/ClientOneSiliconPkg/Fru/AdlPch/Include/PchReservedResourcesAdpP.h : { 0, PCH_PRESERVED_BASE_ADDRESS, PCH_PRESERVED_BASE_SIZE, "PCH_RESERVED" }, Have you already checked these ranges against MTL FAS sec5.5? -- To view, visit https://review.coreboot.org/c/coreboot/+/71114 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I749e7b6e969f8d6314fcd2906acd7de69d4d9f9c Gerrit-Change-Number: 71114 Gerrit-PatchSet: 3 Gerrit-Owner: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 16:22:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Fix cpu index calculation
by Sridhar Siricilla (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Subrata Banik, Ravishankar Sarawadi, Kapil Porwal, Angel Pons, Arthur Heymans, Lean Sheng Tan, Werner Zeh, Sukumar Ghorai, Raj Astekar. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70751 ) Change subject: soc/intel/common: Fix cpu index calculation ...................................................................... Patch Set 8: (1 comment) File src/soc/intel/common/block/acpi/cpu_hybrid.c: https://review.coreboot.org/c/coreboot/+/70751/comment/f84ffd78_7292dd6d PS7, Line 34: g_cpu_bus > Hmm, I had tested this, but this is not working. Thanks, I have implemented. -- To view, visit https://review.coreboot.org/c/coreboot/+/70751 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69e5e6231dd18b43d439340aaed50eb9edeca3b7 Gerrit-Change-Number: 70751 Gerrit-PatchSet: 8 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 15:34:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel: Add Kconfigs to define scaling factor for cores
by Sridhar Siricilla (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal. Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/71687 ) Change subject: soc/intel: Add Kconfigs to define scaling factor for cores ...................................................................... soc/intel: Add Kconfigs to define scaling factor for cores The patch adds Kconfigs to define scaling factor for Efficient and Performance cores instead of using hard coded values in the soc code. Also, the patch defines core_scaling_factor structure to group performance and efficient scaling factor variables. TEST=Build the code for Gimble and Rex. Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I55e4d815116ef40c5f33be64ab495e942bf35ee8 --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/cpu.c M src/soc/intel/common/block/acpi/cpu_hybrid.c M src/soc/intel/common/block/include/intelblocks/acpi.h M src/soc/intel/meteorlake/Kconfig M src/soc/intel/meteorlake/cpu.c 6 files changed, 50 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/71687/1 diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index 9d950a7..42719b7 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -290,6 +290,14 @@ int default 100 +config SOC_INTEL_PERF_CORE_SCAL_FACTOR + int + default 127 + +config SOC_INTEL_EFFICIENT_CORE_SCAL_FACTOR + int + default 100 + config SOC_INTEL_COMMON_BLOCK_GSPI_CLOCK_MHZ int default 120 @@ -459,6 +467,7 @@ endchoice + if STITCH_ME_BIN config CSE_BPDT_VERSION diff --git a/src/soc/intel/alderlake/cpu.c b/src/soc/intel/alderlake/cpu.c index 768f2d5..0a0eb34 100644 --- a/src/soc/intel/alderlake/cpu.c +++ b/src/soc/intel/alderlake/cpu.c @@ -99,10 +99,10 @@ return CPUID_CORE_TYPE_INTEL_CORE; } -void soc_get_scaling_factor(u16 *perf_core_scal_factor, u16 *eff_core_scal_factor) +void soc_get_scaling_factor(struct core_scaling_factor *scaling_factor) { - *perf_core_scal_factor = 127; - *eff_core_scal_factor = 100; + scaling_factor->performance_core = CONFIG_SOC_INTEL_PERF_CORE_SCAL_FACTOR; + scaling_factor->efficient_core = CONFIG_SOC_INTEL_EFFICIENT_CORE_SCAL_FACTOR; } bool soc_is_nominal_freq_supported(void) diff --git a/src/soc/intel/common/block/acpi/cpu_hybrid.c b/src/soc/intel/common/block/acpi/cpu_hybrid.c index 8f0da02..836f808 100644 --- a/src/soc/intel/common/block/acpi/cpu_hybrid.c +++ b/src/soc/intel/common/block/acpi/cpu_hybrid.c @@ -72,14 +72,12 @@ static void acpi_get_cpu_nomi_perf(u16 *eff_core_nom_perf, u16 *perf_core_nom_perf) { - u16 perf_core_scal_factor, eff_core_scal_factor; + struct core_scaling_factor scal_factor; u8 max_non_turbo_ratio = cpu_get_max_non_turbo_ratio(); - soc_get_scaling_factor(&perf_core_scal_factor, &eff_core_scal_factor); - - *perf_core_nom_perf = (u16)((max_non_turbo_ratio * perf_core_scal_factor) / 100); - - *eff_core_nom_perf = (u16)((max_non_turbo_ratio * eff_core_scal_factor) / 100); + soc_get_scaling_factor(&scal_factor); + *perf_core_nom_perf = (u16)((max_non_turbo_ratio * scal_factor.performance_core) / 100); + *eff_core_nom_perf = (u16)((max_non_turbo_ratio * scal_factor.efficient_core) / 100); } static u16 acpi_get_cpu_nominal_freq(void) diff --git a/src/soc/intel/common/block/include/intelblocks/acpi.h b/src/soc/intel/common/block/include/intelblocks/acpi.h index 63f38bf..d6e5974 100644 --- a/src/soc/intel/common/block/include/intelblocks/acpi.h +++ b/src/soc/intel/common/block/include/intelblocks/acpi.h @@ -18,8 +18,13 @@ CPUID_UNKNOWN = 0xff, }; +struct core_scaling_factor { + uint16_t performance_core; + uint16_t efficient_core; +}; + /* Gets the scaling factor for Efficient and Performance core */ -void soc_get_scaling_factor(u16 *perf_core_scal_factor, u16 *eff_core_scal_factor); +void soc_get_scaling_factor(struct core_scaling_factor *scaling_factor); /* Generates ACPI code to define _CPC control method */ void acpigen_write_CPPC_hybrid_method(int core_id); diff --git a/src/soc/intel/meteorlake/Kconfig b/src/soc/intel/meteorlake/Kconfig index 4d69566..d5beb75 100644 --- a/src/soc/intel/meteorlake/Kconfig +++ b/src/soc/intel/meteorlake/Kconfig @@ -210,6 +210,14 @@ config ECAM_MMCONF_BASE_ADDRESS default 0xc0000000 +config SOC_INTEL_PERF_CORE_SCAL_FACTOR + int + default 125 + +config SOC_INTEL_EFFICIENT_CORE_SCAL_FACTOR + int + default 100 + config CPU_BCLK_MHZ int default 100 diff --git a/src/soc/intel/meteorlake/cpu.c b/src/soc/intel/meteorlake/cpu.c index b299cc8..435ddb1 100644 --- a/src/soc/intel/meteorlake/cpu.c +++ b/src/soc/intel/meteorlake/cpu.c @@ -87,10 +87,10 @@ return CPUID_CORE_TYPE_INTEL_CORE; } -void soc_get_scaling_factor(u16 *perf_core_scal_factor, u16 *eff_core_scal_factor) +void soc_get_scaling_factor(struct core_scaling_factor *scaling_factor) { - *perf_core_scal_factor = 125; - *eff_core_scal_factor = 100; + scaling_factor->performance_core = CONFIG_SOC_INTEL_PERF_CORE_SCAL_FACTOR; + scaling_factor->efficient_core = CONFIG_SOC_INTEL_EFFICIENT_CORE_SCAL_FACTOR; } bool soc_is_nominal_freq_supported(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/71687 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I55e4d815116ef40c5f33be64ab495e942bf35ee8 Gerrit-Change-Number: 71687 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: soc/intel/common: Fix cpu index calculation
by Sridhar Siricilla (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Subrata Banik, Ravishankar Sarawadi, Kapil Porwal, Angel Pons, Arthur Heymans, Lean Sheng Tan, Werner Zeh, Sukumar Ghorai, Raj Astekar. Hello build bot (Jenkins), Subrata Banik, Ravishankar Sarawadi, Kapil Porwal, Angel Pons, Arthur Heymans, Lean Sheng Tan, Werner Zeh, Raj Astekar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/70751 to look at the new patch set (#8). Change subject: soc/intel/common: Fix cpu index calculation ...................................................................... soc/intel/common: Fix cpu index calculation get_cpu_index() helper function returns cpu's index based on it's APIC id position from the ascending order list of cpus' APIC IDs. In order to calculate the cpu's index, the helper function needs to traverse through each cpu node to find their APIC IDs. So, the function traverse the CPU node list from the cpu whose APIC ID is 0 assuming it is the first cpu node in the list. This logic works fine where BSP's APIC ID is 0. But, starting from MTL, APIC ID for BSP need not be 0 as APIC ID numbering first get assigned for CPU Die Efficient cores, then Performance cores. Please refer section# 6.1 of doc#643504 for more details on APIC IDs. Considering the APIC Id allotment for MTL cores, as existing code traversing begins from the cpu that has APIC Id#0 which may not be the first cpu node in the list so index calculation results in wrong value. The patch addresses above described issue by traversing all the CPU nodes to calculate the cpu index. Also, prevents inconsistent report of /sys/devices/system/cpu/cpu*/cpufreq/* and /sys/devices/system/cpu/cpuXX/acpi_cppc on each reboot. TEST=Verified that the get_cpu_index helper function returns the correct index id for a CPU on Rex. The coreboot log with code instrumentation, before this patch: [DEBUG] my_apic_id:0x10 cpu_index: 0x6 [DEBUG] my_apic_id:0x11 cpu_index: 0x6 [DEBUG] my_apic_id:0x42 cpu_index: 0x6 [DEBUG] my_apic_id:0x21 cpu_index: 0x6 [DEBUG] my_apic_id:0x40 cpu_index: 0x6 [DEBUG] my_apic_id:0x31 cpu_index: 0x6 [DEBUG] my_apic_id:0x39 cpu_index: 0x6 [DEBUG] my_apic_id:0xa cpu_index: 0x3 [DEBUG] my_apic_id:0x0 cpu_index: 0x0 [DEBUG] my_apic_id:0x8 cpu_index: 0x2 [DEBUG] my_apic_id:0x4 cpu_index: 0x2 [DEBUG] my_apic_id:0x28 cpu_index: 0x6 [DEBUG] my_apic_id:0x2 cpu_index: 0x1 [DEBUG] my_apic_id:0x38 cpu_index: 0x6 [DEBUG] my_apic_id:0x29 cpu_index: 0x6 [DEBUG] my_apic_id:0xe cpu_index: 0x5 [DEBUG] my_apic_id:0x6 cpu_index: 0x2 [DEBUG] my_apic_id:0x20 cpu_index: 0x6 [DEBUG] my_apic_id:0x30 cpu_index: 0x6 [DEBUG] my_apic_id:0x19 cpu_index: 0x6 [DEBUG] my_apic_id:0xc cpu_index: 0x4 [DEBUG] my_apic_id:0x18 cpu_index: 0x6 We can see same cpu_index for multiple cores before fix. After this patch.. [DEBUG] my_apic_id:0x10 cpu_index: 0x8 [DEBUG] my_apic_id:019 cpu_index: 0xb [DEBUG] my_apic_id:0x11 cpu_index: 0x9 [DEBUG] my_apic_id:0x18 cpu_index: 0xa [DEBUG] my_apic_id:0x40 cpu_index: 0x14 [DEBUG] my_apic_id:0x30 cpu_index: 0x10 [DEBUG] my_apic_id:0x42 cpu_index: 0x15 [DEBUG] my_apic_id:0xc cpu_index: 0x6 [DEBUG] my_apic_id:0x2 cpu_index: 0x1 [DEBUG] my_apic_id:0x29 cpu_index: 0xf [DEBUG] my_apic_id:0xe cpu_index: 0x7 [DEBUG] my_apic_id:0x20 cpu_index: 0xc [DEBUG] my_apic_id:0x0 cpu_index: 0x0 [DEBUG] my_apic_id:0x31 cpu_index: 0x11 [DEBUG] my_apic_id:0x28 cpu_index: 0xe [DEBUG] my_apic_id:0x21 cpu_index: 0xd [DEBUG] my_apic_id:0xa cpu_index: 0x5 [DEBUG] my_apic_id:0x38 cpu_index: 0x12 [DEBUG] my_apic_id:0x8 cpu_index: 0x4 [DEBUG] my_apic_id:0x4 cpu_index: 0x2 [DEBUG] my_apic_id:0x39 cpu_index: 0x13 Change-Id: I69e5e6231dd18b43d439340aaed50eb9edeca3b7 Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/acpi/cpu_hybrid.c 1 file changed, 91 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/70751/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/70751 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69e5e6231dd18b43d439340aaed50eb9edeca3b7 Gerrit-Change-Number: 70751 Gerrit-PatchSet: 8 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: soc/intel/: Rename small and big cores references
by Sridhar Siricilla (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal, Werner Zeh. Hello build bot (Jenkins), Tarun Tuli, Subrata Banik, Kapil Porwal, Werner Zeh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/71639 to look at the new patch set (#3). Change subject: soc/intel/: Rename small and big cores references ...................................................................... soc/intel/: Rename small and big cores references The patch addresses Intel heterogeneous cores as `Efficient` and `Performance` cores instead of `small` and `big` cores. It is to ensure coreboot code has uniform reference to the heterogeneous cores. So, the patch renames all `small` and `big` core references to `efficient` (eff) and `performance` (perf) cores respectively. TEST=Build the code for Brya and Rex boards Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I98c9c0ed86b211d736a0a1738b47410faa13a39f --- M src/soc/intel/alderlake/cpu.c M src/soc/intel/common/block/acpi/cpu_hybrid.c M src/soc/intel/common/block/include/intelblocks/acpi.h M src/soc/intel/meteorlake/cpu.c 4 files changed, 44 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/71639/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/71639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I98c9c0ed86b211d736a0a1738b47410faa13a39f Gerrit-Change-Number: 71639 Gerrit-PatchSet: 3 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: soc/intel/common: Add common function to disable LT memory lock
by Tarun Tuli (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Jérémy Compostella, Angel Pons. Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71655 ) Change subject: soc/intel/common: Add common function to disable LT memory lock ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/71655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic666919ed51f683c9a1ef0b7ba061cd2949faf1e Gerrit-Change-Number: 71655 Gerrit-PatchSet: 4 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 05 Jan 2023 15:14:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2023