coreboot-gerrit October 2023

coreboot-gerrit@coreboot.org

1 participants
3354 discussions

[M] Change in coreboot[main]: cpu/intel/model_206ax: Use haswell cstate_map
by Felix Held (Code Review) 06 Oct '23

06 Oct '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78193?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: cpu/intel/model_206ax: Use haswell cstate_map ...................................................................... cpu/intel/model_206ax: Use haswell cstate_map Make the code look like on newer platforms. This doesn't change functionality. Test: Lenovo X220 still boots and advertises all C-states as before. Change-Id: Ie7076d11720d55a4ac11318cbbdab9f75d08e15e Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78193 Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/cpu/intel/model_206ax/acpi.c M src/cpu/intel/model_206ax/model_206ax.h 2 files changed, 37 insertions(+), 50 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Felix Held: Looks good to me, approved diff --git a/src/cpu/intel/model_206ax/acpi.c b/src/cpu/intel/model_206ax/acpi.c index 2fceda1..c4f5839 100644 --- a/src/cpu/intel/model_206ax/acpi.c +++ b/src/cpu/intel/model_206ax/acpi.c @@ -13,79 +13,52 @@ #include "model_206ax.h" #include "chip.h" +#define MWAIT_RES(state, sub_state) \ + { \ + .addrl = (((state) << 4) | (sub_state)), \ + .space_id = ACPI_ADDRESS_SPACE_FIXED, \ + .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, \ + .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, \ + .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, \ + } + /* * List of supported C-states in this processor * * Latencies are typical worst-case package exit time in uS * taken from the SandyBridge BIOS specification. */ -static const acpi_cstate_t cstate_map[] = { - { /* 0: C0 */ - }, { /* 1: C1 */ +static acpi_cstate_t cstate_map[NUM_C_STATES] = { + [C_STATE_C0] = { }, + [C_STATE_C1] = { .latency = 1, .power = 1000, - .resource = { - .addrl = 0x00, /* MWAIT State 0 */ - .space_id = ACPI_ADDRESS_SPACE_FIXED, - .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, - .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, - .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, - } + .resource = MWAIT_RES(0, 0), }, - { /* 2: C1E */ + [C_STATE_C1E] = { .latency = 1, .power = 1000, - .resource = { - .addrl = 0x01, /* MWAIT State 0 Sub-state 1 */ - .space_id = ACPI_ADDRESS_SPACE_FIXED, - .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, - .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, - .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, - } + .resource = MWAIT_RES(0, 1), }, - { /* 3: C3 */ + [C_STATE_C3] = { .latency = 63, .power = 500, - .resource = { - .addrl = 0x10, /* MWAIT State 1 */ - .space_id = ACPI_ADDRESS_SPACE_FIXED, - .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, - .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, - .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, - } + .resource = MWAIT_RES(1, 0), }, - { /* 4: C6 */ + [C_STATE_C6] = { .latency = 87, .power = 350, - .resource = { - .addrl = 0x20, /* MWAIT State 2 */ - .space_id = ACPI_ADDRESS_SPACE_FIXED, - .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, - .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, - .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, - } + .resource = MWAIT_RES(2, 0), }, - { /* 5: C7 */ + [C_STATE_C7] = { .latency = 90, .power = 200, - .resource = { - .addrl = 0x30, /* MWAIT State 3 */ - .space_id = ACPI_ADDRESS_SPACE_FIXED, - .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, - .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, - .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, - } + .resource = MWAIT_RES(3, 0), }, - { /* 6: C7S */ + [C_STATE_C7S] = { .latency = 90, .power = 200, - .resource = { - .addrl = 0x31, /* MWAIT State 3 Sub-state 1 */ - .space_id = ACPI_ADDRESS_SPACE_FIXED, - .bit_width = ACPI_FFIXEDHW_VENDOR_INTEL, - .bit_offset = ACPI_FFIXEDHW_CLASS_MWAIT, - .access_size = ACPI_FFIXEDHW_FLAG_HW_COORD, - } + .resource = MWAIT_RES(3, 1), }, }; diff --git a/src/cpu/intel/model_206ax/model_206ax.h b/src/cpu/intel/model_206ax/model_206ax.h index b145523..ecbf323 100644 --- a/src/cpu/intel/model_206ax/model_206ax.h +++ b/src/cpu/intel/model_206ax/model_206ax.h @@ -109,6 +109,20 @@ # error "CONFIG_IED_REGION_SIZE is not a power of 2" #endif +/* + * List of supported C-states for Sandy Bridge/Ivy Bridge. + */ +enum { + C_STATE_C0 = 0, + C_STATE_C1 = 1, + C_STATE_C1E = 2, + C_STATE_C3 = 3, + C_STATE_C6 = 4, + C_STATE_C7 = 5, + C_STATE_C7S = 6, + NUM_C_STATES, +}; + /* Lock MSRs */ void intel_model_206ax_finalize_smm(void); -- To view, visit https://review.coreboot.org/c/coreboot/+/78193?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie7076d11720d55a4ac11318cbbdab9f75d08e15e Gerrit-Change-Number: 78193 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Print supported C-states
by Felix Held (Code Review) 06 Oct '23

06 Oct '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78133?usp=email ) Change subject: cpu/intel/model_206ax: Print supported C-states ...................................................................... cpu/intel/model_206ax: Print supported C-states According to the BWG C-states are processor specific and BIOS must check if a C-state is supported at all. Print the supported C-states in before ACPI _CNT generation. Test: Tested on Lenovo X220 using Intel i5-2540M. All C-states are reported as supported. Change-Id: I713712a1a104714cbf3091782e564e7e784cf21d Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78133 Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/cpu/intel/model_206ax/acpi.c 1 file changed, 21 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved diff --git a/src/cpu/intel/model_206ax/acpi.c b/src/cpu/intel/model_206ax/acpi.c index 994f3bb..2fceda1 100644 --- a/src/cpu/intel/model_206ax/acpi.c +++ b/src/cpu/intel/model_206ax/acpi.c @@ -89,12 +89,31 @@ }, }; +static const char *const c_state_names[] = {"C0", "C1", "C1E", "C3", "C6", "C7", "C7S"}; + static int get_logical_cores_per_package(void) { msr_t msr = rdmsr(MSR_CORE_THREAD_COUNT); return msr.lo & 0xffff; } +static void print_supported_cstates(void) +{ + uint8_t state, substate; + + printk(BIOS_DEBUG, "Supported C-states: "); + + for (size_t i = 0; i < ARRAY_SIZE(cstate_map); i++) { + state = (cstate_map[i].resource.addrl >> 4) + 1; + substate = cstate_map[i].resource.addrl & 0xf; + + /* CPU C0 is always supported */ + if (i == 0 || cpu_get_c_substate_support(state) > substate) + printk(BIOS_DEBUG, " %s", c_state_names[i]); + } + printk(BIOS_DEBUG, "\n"); +} + static void generate_C_state_entries(const struct device *dev) { struct cpu_intel_model_206ax_config *conf = dev->chip_info; @@ -102,7 +121,6 @@ const int acpi_cstates[3] = { conf->acpi_c1, conf->acpi_c2, conf->acpi_c3 }; acpi_cstate_t acpi_cstate_map[ARRAY_SIZE(acpi_cstates)] = { 0 }; - /* Count number of active C-states */ int count = 0; @@ -320,6 +338,8 @@ printk(BIOS_DEBUG, "Found %d CPU(s) with %d core(s) each.\n", numcpus, cores_per_package); + print_supported_cstates(); + for (int cpu_id = 0; cpu_id < numcpus; cpu_id++) for (int core_id = 0; core_id < cores_per_package; core_id++) generate_cpu_entry(device, cpu_id, core_id, cores_per_package); -- To view, visit https://review.coreboot.org/c/coreboot/+/78133?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I713712a1a104714cbf3091782e564e7e784cf21d Gerrit-Change-Number: 78133 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: arch/x86/cpu_common: Add cpu_get_c_substate_support
by Felix Held (Code Review) 06 Oct '23

06 Oct '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78224?usp=email ) Change subject: arch/x86/cpu_common: Add cpu_get_c_substate_support ...................................................................... arch/x86/cpu_common: Add cpu_get_c_substate_support Add a function to get the number of substates supported by an Intel CPU C-state. Test: Can read out the supported C-state substates. Change-Id: Ie57e87609ea5d6ec6f37154e8b84f1e9574aa4a9 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78224 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/arch/x86/cpu_common.c M src/arch/x86/include/arch/cpu.h 2 files changed, 29 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved diff --git a/src/arch/x86/cpu_common.c b/src/arch/x86/cpu_common.c index 9387f3b..0242dce 100644 --- a/src/arch/x86/cpu_common.c +++ b/src/arch/x86/cpu_common.c @@ -187,6 +187,22 @@ return info->num_ways * info->physical_partitions * info->line_size * info->num_sets; } +/* + * Returns the sub-states supported by the specified CPU + * C-state level. + * + * Level 0 corresponds to the lowest C-state (C0). + * Higher levels are processor specific. + */ +uint8_t cpu_get_c_substate_support(const int state) +{ + if ((cpuid_get_max_func() < 5) || + !(cpuid_ecx(5) & CPUID_FEATURE_MONITOR_MWAIT) || (state > 4)) + return 0; + + return (cpuid_edx(5) >> (state * 4)) & 0xf; +} + bool fill_cpu_cache_info(uint8_t level, struct cpu_cache_info *info) { if (!info) diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 2c98d1e..0c4decf 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -99,6 +99,10 @@ #define CPUID_CACHE_NO_OF_SETS_MASK 0xffffffff #define CPUID_CACHE_NO_OF_SETS(res) CPUID_CACHE(NO_OF_SETS, (res).ecx) +// Intel leaf 0x5 +#define CPUID_FEATURE_MONITOR_MWAIT (1 << 0) +#define CPUID_FEATURE_INTERUPT_BREAK_EVENT (1 << 1) + unsigned int cpu_cpuid_extended_level(void); int cpu_have_cpuid(void); @@ -311,6 +315,15 @@ size_t get_cache_size(const struct cpu_cache_info *info); /* + * Returns the sub-states supported by the specified CPU + * C-state level. + * + * Level 0 corresponds to the lowest C-state (C0). + * Higher levels are processor specific. + */ +uint8_t cpu_get_c_substate_support(const int state); + +/* * fill_cpu_cache_info to get all required cache info data and fill into cpu_cache_info * structure by calling CPUID.EAX=leaf and ECX=Cache Level. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/78224?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie57e87609ea5d6ec6f37154e8b84f1e9574aa4a9 Gerrit-Change-Number: 78224 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: mb/google/{rex,ovis}: Disable package C-state auto demotion
by Eric Lai (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Eran Mitrani, Subrata Banik, Sukumar Ghorai, Tarun. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78250?usp=email ) Change subject: mb/google/{rex,ovis}: Disable package C-state auto demotion ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78250?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia4cf4a7cb6bd5eaae26197b55f9385c078960d7b Gerrit-Change-Number: 78250 Gerrit-PatchSet: 4 Gerrit-Owner: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 06 Oct 2023 11:22:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/{rex,ovis}: Disable package C-state auto demotion
by Kapil Porwal (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Eran Mitrani, Eric Lai, Subrata Banik, Sukumar Ghorai, Tarun. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78250?usp=email ) Change subject: mb/google/{rex,ovis}: Disable package C-state auto demotion ...................................................................... Patch Set 4: (1 comment) Patchset: PS3: > LTGM, @Kapil will test and get back Verified. Looks good. -- To view, visit https://review.coreboot.org/c/coreboot/+/78250?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia4cf4a7cb6bd5eaae26197b55f9385c078960d7b Gerrit-Change-Number: 78250 Gerrit-PatchSet: 4 Gerrit-Owner: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 06 Oct 2023 11:13:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/{rex,ovis}: Disable package C-state auto demotion
by Kapil Porwal (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Eran Mitrani, Eric Lai, Subrata Banik, Sukumar Ghorai, Tarun. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78250?usp=email ) Change subject: mb/google/{rex,ovis}: Disable package C-state auto demotion ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78250?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia4cf4a7cb6bd5eaae26197b55f9385c078960d7b Gerrit-Change-Number: 78250 Gerrit-PatchSet: 4 Gerrit-Owner: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 06 Oct 2023 11:12:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal
by Joey Peng (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Derek Huang, Paul Menzel, Reka Norman. Joey Peng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78146?usp=email ) Change subject: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal ...................................................................... Patch Set 8: (1 comment) File src/mainboard/google/dedede/variants/boxy/ramstage.c: https://review.coreboot.org/c/coreboot/+/78146/comment/a46399d8_25ae3b2d : PS5, Line 69: if (display_init_required() : && !gpio_get(GPIO_USB_C0_HPD)) { : /* This has to be done before FSP-S runs. */ : if (google_chromeec_wait_for_displayport(display_timeout_ms)) { : wait_for_hpd(GPIO_USB_C0_HPD, display_timeout_ms); : } : } else if (display_init_required() : && !gpio_get(GPIO_USB_C1_HPD)) { : /* This has to be done before FSP-S runs. */ : if (google_chromeec_wait_for_displayport(display_timeout_ms)) { : wait_for_hpd(GPIO_USB_C1_HPD, display_timeout_ms); : } : } > We have 1 HDMI and 2 Type-C ports for boxy project. […] Also we will not have cases connected to two type-c monitors since boxy need 1 type-c port to be connected to adapter. -- To view, visit https://review.coreboot.org/c/coreboot/+/78146?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I208fdf8234886b2ef43a4b972e6682892238e8e9 Gerrit-Change-Number: 78146 Gerrit-PatchSet: 8 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 06 Oct 2023 10:30:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal
by Joey Peng (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Derek Huang, Paul Menzel, Reka Norman. Hello Derek Huang, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78146?usp=email to look at the new patch set (#8). Change subject: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal ...................................................................... mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal Boxy project needs to wait for HPD ready to continue boot in ramstage to avoid no display in DEV screen and recovery screen via Type-C dongle. BUG=b:303346324 TEST=Both type-c ports can display in DEV screen and recovery screen Signed-off-by: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Change-Id: I208fdf8234886b2ef43a4b972e6682892238e8e9 --- M src/mainboard/google/dedede/mainboard.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/dedede/variants/boxy/ramstage.c 3 files changed, 52 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/78146/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/78146?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I208fdf8234886b2ef43a4b972e6682892238e8e9 Gerrit-Change-Number: 78146 Gerrit-PatchSet: 8 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal
by Joey Peng (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Derek Huang, Paul Menzel, Reka Norman. Joey Peng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78146?usp=email ) Change subject: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal ...................................................................... Patch Set 7: (1 comment) File src/mainboard/google/dedede/variants/boxy/ramstage.c: https://review.coreboot.org/c/coreboot/+/78146/comment/1cfdeeae_fab74e2e : PS1, Line 68: google_chromeec_wait_for_displayport > I just tested on dibbi and this always returns an error (-1) because EC_CMD_USB_PD_GET_AMODE is no l […] Hi Reka, We will remove the google_chromeec_wait_for_displayport() judgement for this patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/78146?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I208fdf8234886b2ef43a4b972e6682892238e8e9 Gerrit-Change-Number: 78146 Gerrit-PatchSet: 7 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 06 Oct 2023 10:27:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal
by Joey Peng (Code Review) 06 Oct '23

06 Oct '23

Attention is currently required from: Derek Huang, Paul Menzel. Joey Peng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78146?usp=email ) Change subject: mb/google/dedede/var/boxy: Wait up for 3s for HPD ready signal ...................................................................... Patch Set 7: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/78146/comment/c7d49ebf_037ad7a8 : PS5, Line 7: Add wait for HPD function > Maybe: […] Done https://review.coreboot.org/c/coreboot/+/78146/comment/230bbd4e_360d59ef : PS5, Line 9: need > needs Done https://review.coreboot.org/c/coreboot/+/78146/comment/0f6937ad_f06b86e1 : PS5, Line 9: Boxy project need to wait for HPD ready to continue boot in ramstage to : avoid no display in DEV screen and recovery screen > Please add a dot/period at the end of sentences. Done File src/mainboard/google/dedede/variants/boxy/ramstage.c: https://review.coreboot.org/c/coreboot/+/78146/comment/a8315664_1a98cf2e : PS5, Line 67: static const long display_timeout_ms = 3000; > Why 3 seconds? The three seconds is referenced form project puff. Also we have tested the delay time needs to be more than 3 seconds to work on boxy. https://review.coreboot.org/c/coreboot/+/78146/comment/7ab99979_523d4ca3 : PS5, Line 69: if (display_init_required() : && !gpio_get(GPIO_USB_C0_HPD)) { : /* This has to be done before FSP-S runs. */ : if (google_chromeec_wait_for_displayport(display_timeout_ms)) { : wait_for_hpd(GPIO_USB_C0_HPD, display_timeout_ms); : } : } else if (display_init_required() : && !gpio_get(GPIO_USB_C1_HPD)) { : /* This has to be done before FSP-S runs. */ : if (google_chromeec_wait_for_displayport(display_timeout_ms)) { : wait_for_hpd(GPIO_USB_C1_HPD, display_timeout_ms); : } : } > Can there be two displays attached at the same time? We have 1 HDMI and 2 Type-C ports for boxy project. This issue only happens when connected to Type-C dongle on DEV mode and recovery mode. Normal mode boot or DEV mode/recovery mode via HDMI works fine. -- To view, visit https://review.coreboot.org/c/coreboot/+/78146?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I208fdf8234886b2ef43a4b972e6682892238e8e9 Gerrit-Change-Number: 78146 Gerrit-PatchSet: 7 Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Reka Norman <rekanorman(a)chromium.org> Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 06 Oct 2023 10:26:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2023