coreboot-gerrit January 2023

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

[M] Change in coreboot[master]: mb/google/brya/var/marasov: Update DPTF parameters
by Frank Chu (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Tarun Tuli, Kyle Lin. Frank Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71634 ) Change subject: mb/google/brya/var/marasov: Update DPTF parameters ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/71634 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I55a3066ef61ce461f40b425a6549d083c29256e5 Gerrit-Change-Number: 71634 Gerrit-PatchSet: 4 Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kyle Lin <kylelinck(a)google.com> Gerrit-Comment-Date: Tue, 03 Jan 2023 09:40:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/intel/mtlrvp: Configure USB devices for MTL-RVP
by Sridhar Siricilla (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Jamie Ryu, Harsha B R, Usha P. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71168 ) Change subject: mb/intel/mtlrvp: Configure USB devices for MTL-RVP ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/71168 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If1a0c31b7bf0f3fc06f039ad76b0cdd41f7cdd90 Gerrit-Change-Number: 71168 Gerrit-PatchSet: 4 Gerrit-Owner: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Attention: Usha P <usha.p(a)intel.com> Gerrit-Comment-Date: Tue, 03 Jan 2023 09:31:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: chromeos/cr50_enable_update.c: Clear EC AP_IDLE flag
by Derek Huang (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Caveh Jalali, Christian Walter, Nick Vaccaro, Daisuke Nojiri, Eric Lai, Boris Mittelberg. Derek Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70773 ) Change subject: chromeos/cr50_enable_update.c: Clear EC AP_IDLE flag ...................................................................... Patch Set 16: (8 comments) File src/mainboard/google/brya/Kconfig: https://review.coreboot.org/c/coreboot/+/70773/comment/0a020ed4_d5e7e1a1 PS12, Line 336: config CR50_RESET_CLEAR_EC_AP_IDLE_FLAG > > this should move to common code. […] Done File src/mainboard/google/brya/mainboard.c: https://review.coreboot.org/c/coreboot/+/70773/comment/043ea430_53db556a PS11, Line 16: #include <ec/google/chromeec/ec.h> > nit: consider alphabetic order please Done https://review.coreboot.org/c/coreboot/+/70773/comment/82f6ec9a_738ff940 PS11, Line 217: __weak > i don't think you need the `weak` implementation anymore now. Done File src/mainboard/google/brya/mainboard.c: https://review.coreboot.org/c/coreboot/+/70773/comment/d7d6224c_6db87f20 PS12, Line 15: #include <vendorcode/google/chromeos/chromeos.h> > we don't need this header now. Done File src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h: https://review.coreboot.org/c/coreboot/+/70773/comment/169b7814_596db0cc PS11, Line 28: variant_prepare_cr50_reset > mainboard_prepare_cr50_reset Done File src/vendorcode/google/chromeos/cr50_enable_update.c: https://review.coreboot.org/c/coreboot/+/70773/comment/8f76a028_438feac5 PS13, Line 159: if (CONFIG(CR50_RESET_CLEAR_EC_AP_IDLE_FLAG)) { : /* Send EC command to clear AP_IDLE flag */ : if (!google_chromeec_reboot(0, EC_REBOOT_NO_OP, : EC_REBOOT_FLAG_CLEAR_AP_IDLE | : EC_REBOOT_FLAG_ON_AP_SHUTDOWN)) : printk(BIOS_INFO, : "Successfully clear AP_IDLE flag"); : else : printk(BIOS_ERR, : "Failed to clear EC AP_IDLE flag"); : } > suggestion: please create a helper function Done File src/vendorcode/google/chromeos/cr50_enable_update.c: https://review.coreboot.org/c/coreboot/+/70773/comment/8ba20e2f_f27560ee PS15, Line 79: printk(BIOS_INFO, > nit:no need warp line. […] Done https://review.coreboot.org/c/coreboot/+/70773/comment/06998666_88f9c720 PS15, Line 82: printk(BIOS_ERR, > nit:no need warp line. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/70773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If97ffbe65f4783f17f4747a87b0bf89a2b021a3b Gerrit-Change-Number: 70773 Gerrit-PatchSet: 16 Gerrit-Owner: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Daisuke Nojiri <dnojiri(a)chromium.org> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-CC: 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: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Daisuke Nojiri <dnojiri(a)chromium.org> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Tue, 03 Jan 2023 09:23:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/intel/mtlrvp: Enable CSE Lite SKU for MTL-RVP
by Sridhar Siricilla (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Jamie Ryu, Harsha B R, Krishna P Bhat D, Usha P. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71638 ) Change subject: mb/intel/mtlrvp: Enable CSE Lite SKU for MTL-RVP ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/71638 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I325405cc304d245871396317c11ac7a5b062a5bd Gerrit-Change-Number: 71638 Gerrit-PatchSet: 1 Gerrit-Owner: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Usha P <usha.p(a)intel.com> Gerrit-Comment-Date: Tue, 03 Jan 2023 09:14:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: chromeos/cr50_enable_update.c: Clear EC AP_IDLE flag
by Derek Huang (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Derek Huang, Caveh Jalali, Christian Walter, Nick Vaccaro, Daisuke Nojiri, Boris Mittelberg. Hello Tarun Tuli, Subrata Banik, Caveh Jalali, Christian Walter, Nick Vaccaro, Daisuke Nojiri, Eric Lai, Boris Mittelberg, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/70773 to look at the new patch set (#16). Change subject: chromeos/cr50_enable_update.c: Clear EC AP_IDLE flag ...................................................................... chromeos/cr50_enable_update.c: Clear EC AP_IDLE flag When AP boots up after Cr50 firmware update and reboot, AP finds that Cr50 reset is required for Cr50 to pick the new firmware so it trigger Cr50 reset and power off the system, AP expects system will power on automatically after Cr50 reset. However this is not the case for Chromebox, Chromebox EC set AP_IDLE flag when system is shutting down, when AP_IDLE flag is set in EC, the system stays at S5/G3 and wait for power button presssend. It cause an issue in factory that the operator needs to press power button to power on the DUT after Cr50 firmware update. This patch sends EC command to direct EC to clear AP_IDLE flag after AP shutdown so AP can boot up when Cr50 reset. BUG=b:261119366 BRANCH=firmware-brya-14505.B TEST=DUT boots up after Cr50 firmware update in factory test flow Change-Id: If97ffbe65f4783f17f4747a87b0bf89a2b021a3b Signed-off-by: Derek Huang <derekhuang(a)google.com> --- M src/ec/google/chromeec/ec_commands.h M src/mainboard/google/brya/Kconfig M src/security/tpm/tss/vendor/cr50/Kconfig M src/vendorcode/google/chromeos/cr50_enable_update.c 4 files changed, 54 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/70773/16 -- To view, visit https://review.coreboot.org/c/coreboot/+/70773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If97ffbe65f4783f17f4747a87b0bf89a2b021a3b Gerrit-Change-Number: 70773 Gerrit-PatchSet: 16 Gerrit-Owner: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Daisuke Nojiri <dnojiri(a)chromium.org> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-CC: 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: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Daisuke Nojiri <dnojiri(a)chromium.org> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: soc/intel/: Rename small and big cores references
by build bot (Jenkins) (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal. build bot (Jenkins) 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 1: (1 comment) Commit Message: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-167535): https://review.coreboot.org/c/coreboot/+/71639/comment/100cce5d_0dcdde89 PS1, Line 8: 'heterogenous' may be misspelled - perhaps 'heterogeneous'? -- 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: 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-CC: 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-Comment-Date: Tue, 03 Jan 2023 09:04:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

03 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/+/71639 ) Change subject: soc/intel/: Rename small and big cores references ...................................................................... soc/intel/: Rename small and big cores references The patch addresses Intel heterogenous cores as `Efficient` and `Performance` cores instead of `small` and `big` cores. It is to ensure coreboot code has uniform reference to heterogernous 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 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, 45 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/71639/1 diff --git a/src/soc/intel/alderlake/cpu.c b/src/soc/intel/alderlake/cpu.c index 4bc0af7..768f2d5 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 *big_core_scal_factor, u16 *small_core_scal_factor) +void soc_get_scaling_factor(u16 *perf_core_scal_factor, u16 *eff_core_scal_factor) { - *big_core_scal_factor = 127; - *small_core_scal_factor = 100; + *perf_core_scal_factor = 127; + *eff_core_scal_factor = 100; } 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 4335a46..ba40db6 100644 --- a/src/soc/intel/common/block/acpi/cpu_hybrid.c +++ b/src/soc/intel/common/block/acpi/cpu_hybrid.c @@ -12,14 +12,14 @@ #define CPPC_NOM_PERF_IDX 3 enum cpu_perf_eff_type { - CPU_TYPE_SMALL, - CPU_TYPE_BIG, + CPU_TYPE_EFF, + CPU_TYPE_PERF, }; DECLARE_SPIN_LOCK(cpu_lock); static u8 global_cpu_type[CONFIG_MAX_CPUS]; -static bool is_big_core(void) +static bool is_perf_core(void) { return get_soc_cpu_type() == CPUID_CORE_TYPE_INTEL_CORE; } @@ -39,9 +39,10 @@ } /* - * This function determines the type (big or small) of the CPU that is executing - * it and stores the information (in a thread-safe manner) in an global_cpu_type - * array. + * This function determines the type (performance or efficient) of the CPU that + * is executing it and stores the information (in a thread-safe manner) in an + * global_cpu_type array. + * * It requires the SoC to implement a function `get_soc_cpu_type()` which will be * called in a critical section to determine the type of the executing CPU. */ @@ -50,8 +51,8 @@ spin_lock(&cpu_lock); u8 cpu_index = get_cpu_index(); - if (is_big_core()) - global_cpu_type[cpu_index] = CPU_TYPE_BIG; + if (is_perf_core()) + global_cpu_type[cpu_index] = CPU_TYPE_PERF; spin_unlock(&cpu_lock); } @@ -64,16 +65,16 @@ } } -static void acpi_get_cpu_nomi_perf(u16 *small_core_nom_perf, u16 *big_core_nom_perf) +static void acpi_get_cpu_nomi_perf(u16 *eff_core_nom_perf, u16 *perf_core_nom_perf) { - u16 big_core_scal_factor, small_core_scal_factor; + u16 perf_core_scal_factor, eff_core_scal_factor; u8 max_non_turbo_ratio = cpu_get_max_non_turbo_ratio(); - soc_get_scaling_factor(&big_core_scal_factor, &small_core_scal_factor); + soc_get_scaling_factor(&perf_core_scal_factor, &eff_core_scal_factor); - *big_core_nom_perf = (u16)((max_non_turbo_ratio * big_core_scal_factor) / 100); + *perf_core_nom_perf = (u16)((max_non_turbo_ratio * perf_core_scal_factor) / 100); - *small_core_nom_perf = (u16)((max_non_turbo_ratio * small_core_scal_factor) / 100); + *eff_core_nom_perf = (u16)((max_non_turbo_ratio * eff_core_scal_factor) / 100); } static u16 acpi_get_cpu_nominal_freq(void) @@ -84,18 +85,18 @@ /* Updates Nominal Frequency and Nominal Performance */ static void acpigen_cppc_update_nominal_freq_perf(const char *pkg_path, s32 core_id) { - u16 small_core_nom_perf, big_core_nom_perf; + u16 eff_core_nom_perf, perf_core_nom_perf; if (!soc_is_nominal_freq_supported()) return; - acpi_get_cpu_nomi_perf(&small_core_nom_perf, &big_core_nom_perf); + acpi_get_cpu_nomi_perf(&eff_core_nom_perf, &perf_core_nom_perf); - if (global_cpu_type[core_id] == CPU_TYPE_BIG) - acpigen_set_package_element_int(pkg_path, CPPC_NOM_PERF_IDX, big_core_nom_perf); + if (global_cpu_type[core_id] == CPU_TYPE_PERF) + acpigen_set_package_element_int(pkg_path, CPPC_NOM_PERF_IDX, perf_core_nom_perf); else acpigen_set_package_element_int(pkg_path, CPPC_NOM_PERF_IDX, - small_core_nom_perf); + eff_core_nom_perf); /* Update CPU's nominal frequency */ acpigen_set_package_element_int(pkg_path, CPPC_NOM_FREQ_IDX, diff --git a/src/soc/intel/common/block/include/intelblocks/acpi.h b/src/soc/intel/common/block/include/intelblocks/acpi.h index c149c69..473ab20 100644 --- a/src/soc/intel/common/block/include/intelblocks/acpi.h +++ b/src/soc/intel/common/block/include/intelblocks/acpi.h @@ -19,7 +19,7 @@ }; /* Gets the scaling factor for small and big core */ -void soc_get_scaling_factor(u16 *big_core_scal_factor, u16 *small_core_scal_factor); +void soc_get_scaling_factor(u16 *perf_core_scal_factor, u16 *eff_core_scal_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/cpu.c b/src/soc/intel/meteorlake/cpu.c index e0ac726..b299cc8 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 *big_core_scal_factor, u16 *small_core_scal_factor) +void soc_get_scaling_factor(u16 *perf_core_scal_factor, u16 *eff_core_scal_factor) { - *big_core_scal_factor = 125; - *small_core_scal_factor = 100; + *perf_core_scal_factor = 125; + *eff_core_scal_factor = 100; } bool soc_is_nominal_freq_supported(void) -- 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: 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) 03 Jan '23

03 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 (#6). 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/cpu/x86/mp_init.c M src/soc/intel/common/block/acpi/cpu_hybrid.c 2 files changed, 86 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/70751/6 -- 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: 6 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

[S] Change in coreboot[master]: soc/intel/meteorlake: Use common gpio.h include
by Eric Lai (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Tarun Tuli, Dinesh Gehlot, Kapil Porwal. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71630 ) Change subject: soc/intel/meteorlake: Use common gpio.h include ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/71630 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I58e428cde5e13f4f0dfe528d798c0613b7f8a94a Gerrit-Change-Number: 71630 Gerrit-PatchSet: 3 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Tue, 03 Jan 2023 08:57:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/brya/var/marasov: Update DPTF parameters
by Frank Chu (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Tarun Tuli, Kyle Lin. Frank Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71634 ) Change subject: mb/google/brya/var/marasov: Update DPTF parameters ...................................................................... Patch Set 3: Code-Review+1 This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/71634 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I55a3066ef61ce461f40b425a6549d083c29256e5 Gerrit-Change-Number: 71634 Gerrit-PatchSet: 3 Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kyle Lin <kylelinck(a)google.com> Gerrit-Comment-Date: Tue, 03 Jan 2023 08:46:28 +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