coreboot-gerrit December 2021

coreboot-gerrit@coreboot.org

1 participants
5525 discussions

Change in coreboot[master]: soc/intel/jasperlake: Disable C1 C-state auto demotion
by Ben Kao (Code Review) 20 Dec '21

20 Dec '21

Ben Kao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60254 ) Change subject: soc/intel/jasperlake: Disable C1 C-state auto demotion ...................................................................... soc/intel/jasperlake: Disable C1 C-state auto demotion Disable C1 C-state auto demotion to deduce SOC package power. When enabled, CPU will conditionally demote C3/C6/C7 requests to C1 based on uncore auto-demote information. TEST=test_that --board=dedede [IP] power_VideoCall VideoCall Enable C1 auto demotion Disable C1 auto demotion package-0_pwr_avg 3.92 3.15 Signed-off-by: Ben Kao <ben.kao(a)intel.com> Change-Id: I82565c734b28c60a1eee18ac079f5deaa6939d1e --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/60254/1 diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index 085148f7..c8545dd 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -209,6 +209,9 @@ params->PchFivrExtVnnRailEnabledStates = 0; } + /* Disable C1 C-state Demotion */ + params->C1StateAutoDemotion = 0; + /* Override/Fill FSP Silicon Param for mainboard */ mainboard_silicon_init_params(params); } -- To view, visit https://review.coreboot.org/c/coreboot/+/60254 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I82565c734b28c60a1eee18ac079f5deaa6939d1e Gerrit-Change-Number: 60254 Gerrit-PatchSet: 1 Gerrit-Owner: Ben Kao <ben.kao(a)intel.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/intel/pmc_mux/conn: Change usb{23}_port_number fields to devi...
by Angel Pons (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Reka Norman, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Nick Vaccaro, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60143 ) Change subject: drivers/intel/pmc_mux/conn: Change usb{23}_port_number fields to device pointers ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/60143 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5045b8ea57e8ca6f9ebd7d68a19486736b7e2809 Gerrit-Change-Number: 60143 Gerrit-PatchSet: 4 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)google.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Reka Norman <rekanorman(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:56:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/starlabs/labtop: Store serial number in CMOS
by Angel Pons (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Felix Singer, Sean Rhodes, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59979 ) Change subject: mb/starlabs/labtop: Store serial number in CMOS ...................................................................... Patch Set 11: Code-Review+1 (1 comment) Patchset: PS11: Temporarily reducing my vote to a +1 as there's ongoing discussion -- To view, visit https://review.coreboot.org/c/coreboot/+/59979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70c35c8f99f7e3ac8e45e742156dca741a6c6933 Gerrit-Change-Number: 59979 Gerrit-PatchSet: 11 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:55:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/starlabs/labtop: Store serial number in CMOS
by Angel Pons (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Felix Singer, Sean Rhodes, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59979 ) Change subject: mb/starlabs/labtop: Store serial number in CMOS ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: > I don't think this is a good idea either. […] I haven't used it, but it makes much more sense. -- To view, visit https://review.coreboot.org/c/coreboot/+/59979 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70c35c8f99f7e3ac8e45e742156dca741a6c6933 Gerrit-Change-Number: 59979 Gerrit-PatchSet: 11 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:55:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib: Add a mutex
by Kyösti Mälkki (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Nico Huber, Raul Rangel, Tim Wawrzynczak, Angel Pons, Julius Werner, Rob Barnes, Karthik Ramasubramanian. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59320 ) Change subject: lib: Add a mutex ...................................................................... Patch Set 13: (1 comment) Patchset: PS13: Feeling uncomfortable about this, I had a go, see CB:60253. -- To view, visit https://review.coreboot.org/c/coreboot/+/59320 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I41e02a54a17b1f6513b36a0274e43fc715472d78 Gerrit-Change-Number: 59320 Gerrit-PatchSet: 13 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:46:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP] cpu/x86/lapic: Fix choice X2APIC_ONLY
by Angel Pons (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Nico Huber, Wonkyu Kim, Tim Wawrzynczak, Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60213 ) Change subject: [WIP] cpu/x86/lapic: Fix choice X2APIC_ONLY ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: Will test on Asrock B85M Pro4 -- To view, visit https://review.coreboot.org/c/coreboot/+/60213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78070ae91e78c11c3e3aa225e5673d4667d6f7bb Gerrit-Change-Number: 60213 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:44:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] smp/spinlock: Use compiler builtins
by build bot (Jenkins) (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Raul Rangel. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60253 ) Change subject: [RFC] smp/spinlock: Use compiler builtins ...................................................................... Patch Set 1: (2 comments) File src/include/smp/spinlock.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-135894): https://review.coreboot.org/c/coreboot/+/60253/comment/487cafaf_ab548d7f PS1, Line 11: #define DECLARE_SPIN_LOCK(x) static struct spinlock x; macros should not use a trailing semicolon Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-135894): https://review.coreboot.org/c/coreboot/+/60253/comment/c5a877fd_1fb3b6f2 PS1, Line 27: while (!spinlock_try_lock(lock)) { braces {} are not necessary for single statement blocks -- To view, visit https://review.coreboot.org/c/coreboot/+/60253 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6694784aa0810cd877886ef95434f62e3aefce6c Gerrit-Change-Number: 60253 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:44:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] smp/spinlock: Use compiler builtins
by Kyösti Mälkki (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Raul Rangel. Hello Raul Rangel, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/60253 to review the following change. Change subject: [RFC] smp/spinlock: Use compiler builtins ...................................................................... [RFC] smp/spinlock: Use compiler builtins Change-Id: I6694784aa0810cd877886ef95434f62e3aefce6c Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- D src/arch/x86/include/arch/smp/spinlock.h M src/console/printk.c A src/include/mutex.h M src/include/rules.h M src/include/smp/spinlock.h 5 files changed, 84 insertions(+), 81 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/60253/1 diff --git a/src/arch/x86/include/arch/smp/spinlock.h b/src/arch/x86/include/arch/smp/spinlock.h deleted file mode 100644 index c7008c1..0000000 --- a/src/arch/x86/include/arch/smp/spinlock.h +++ /dev/null @@ -1,70 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef ARCH_SMP_SPINLOCK_H -#define ARCH_SMP_SPINLOCK_H - -#include <thread.h> - -/* - * Your basic SMP spinlocks, allowing only a single CPU anywhere - */ - -typedef struct { - volatile unsigned int lock; -} spinlock_t; - -#define SPIN_LOCK_UNLOCKED { 1 } - -#define DECLARE_SPIN_LOCK(x) \ - static spinlock_t x = SPIN_LOCK_UNLOCKED; - -/* - * Simple spin lock operations. There are two variants, one clears IRQ's - * on the local processor, one does not. - * - * We make no fairness assumptions. They have a cost. - */ -#define barrier() __asm__ __volatile__("" : : : "memory") -#define spin_is_locked(x) (*(volatile char *)(&(x)->lock) <= 0) -#define spin_unlock_wait(x) do { barrier(); } while (spin_is_locked(x)) -#undef barrier - -#define spin_lock_string \ - "\n1:\t" \ - "lock ; decb %0\n\t" \ - "js 2f\n" \ - ".section .text.lock,\"ax\"\n" \ - "2:\t" \ - "cmpb $0,%0\n\t" \ - "rep;nop\n\t" \ - "jle 2b\n\t" \ - "jmp 1b\n" \ - ".previous" - -/* - * This works. Despite all the confusion. - */ -#define spin_unlock_string \ - "movb $1,%0" - -static __always_inline void spin_lock(spinlock_t *lock) -{ - __asm__ __volatile__( - spin_lock_string - : "=m" (lock->lock) : : "memory"); - - /* Switching contexts while holding a spinlock will lead to deadlocks */ - thread_coop_disable(); - -} - -static __always_inline void spin_unlock(spinlock_t *lock) -{ - thread_coop_enable(); - - __asm__ __volatile__( - spin_unlock_string - : "=m" (lock->lock) : : "memory"); -} - -#endif /* ARCH_SMP_SPINLOCK_H */ diff --git a/src/console/printk.c b/src/console/printk.c index 1ed39cb..b54cd9a 100644 --- a/src/console/printk.c +++ b/src/console/printk.c @@ -8,6 +8,7 @@ #include <console/console.h> #include <console/streams.h> #include <console/vtxprintf.h> +#include <mutex.h> #include <smp/spinlock.h> #include <smp/node.h> #include <timer.h> @@ -80,7 +81,7 @@ if (log_this < CONSOLE_LOG_FAST) return 0; - spin_lock(&console_lock); + mutex_lock(&console_lock); console_time_run(); @@ -93,7 +94,7 @@ console_time_stop(); - spin_unlock(&console_lock); + mutex_unlock(&console_lock); return i; } diff --git a/src/include/mutex.h b/src/include/mutex.h new file mode 100644 index 0000000..d4642f9 --- /dev/null +++ b/src/include/mutex.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _MUTEX_H +#define _MUTEX_H + +#include <arch/cpu.h> +#include <smp/spinlock.h> +#include <thread.h> + +static inline void mutex_lock(struct spinlock *lock) +{ + while (!spinlock_try_lock(lock)) { + /* + * If we failed to get the lock, wait until it's been unlocked before trying to + * read-modify-write. This helps prevent excessive cache misses. + */ + while (spinlock_is_locked(lock)) { + /* + * Threads are only available on the BSP, so if this code is running + * on an AP, yielding will fail. In that case just sleep for a bit + * and try again. + */ + if (thread_yield() < 0) + cpu_relax(); + } + } +} + +static inline void mutex_unlock(struct spinlock *lock) +{ + spin_unlock(lock); +} + +#endif /* _MUTEX_H */ diff --git a/src/include/rules.h b/src/include/rules.h index 02b55c5..f1ba3ab 100644 --- a/src/include/rules.h +++ b/src/include/rules.h @@ -300,10 +300,9 @@ #define STAGE_HAS_SPINLOCKS 0 #endif -/* When set <arch/smp/spinlock.h> is included for the spinlock implementation. */ #define ENV_STAGE_SUPPORTS_SMP (CONFIG(SMP) && STAGE_HAS_SPINLOCKS) -#if ENV_X86 && CONFIG(COOP_MULTITASKING) && (ENV_RAMSTAGE || ENV_ROMSTAGE) +#if CONFIG(COOP_MULTITASKING) && (ENV_RAMSTAGE || ENV_ROMSTAGE) /* TODO: Enable in all x86 stages */ #define ENV_STAGE_SUPPORTS_COOP 1 #else diff --git a/src/include/smp/spinlock.h b/src/include/smp/spinlock.h index 116830c..28287ca 100644 --- a/src/include/smp/spinlock.h +++ b/src/include/smp/spinlock.h @@ -1,15 +1,54 @@ #ifndef SMP_SPINLOCK_H #define SMP_SPINLOCK_H -#if ENV_STAGE_SUPPORTS_SMP -#include <arch/smp/spinlock.h> +#include <arch/cpu.h> +#include <types.h> + +struct spinlock { + bool locked; +}; + +#define DECLARE_SPIN_LOCK(x) static struct spinlock x; + +#if ENV_STAGE_SUPPORTS_SMP || ENV_STAGE_SUPPORTS_COOP + +static __always_inline bool spinlock_try_lock(struct spinlock *lock) +{ + return !__atomic_test_and_set(&lock->locked, __ATOMIC_ACQUIRE); +} + +static __always_inline bool spinlock_is_locked(struct spinlock *lock) +{ + return !__atomic_load_1(&lock->locked, __ATOMIC_RELAXED); +} + +static __always_inline void spin_lock(struct spinlock *lock) +{ + while (!spinlock_try_lock(lock)) { + cpu_relax(); + } +} + +static __always_inline void spin_unlock(struct spinlock *lock) +{ + __atomic_clear(&lock->locked, __ATOMIC_RELEASE); +} + #else /* !CONFIG_SMP */ -#define DECLARE_SPIN_LOCK(x) -#define spin_is_locked(lock) 0 -#define spin_unlock_wait(lock) do {} while (0) -#define spin_lock(lock) do {} while (0) -#define spin_unlock(lock) do {} while (0) +static __always_inline bool spinlock_try_lock(struct spinlock *lock) +{ + return true; +} + +static __always_inline bool spinlock_is_locked(struct spinlock *lock) +{ + return false; +} + +static __always_inline void spin_lock(struct spinlock *lock) { } +static __always_inline void spin_unlock(struct spinlock *lock) { } + #endif #endif /* SMP_SPINLOCK_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/60253 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6694784aa0810cd877886ef95434f62e3aefce6c Gerrit-Change-Number: 60253 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available
by Angel Pons (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Wonkyu Kim, Tim Wawrzynczak, Kyösti Mälkki, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58386 ) Change subject: cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available ...................................................................... Patch Set 4: Code-Review+1 (2 comments) File src/cpu/x86/smm/smm_stub.S: https://review.coreboot.org/c/coreboot/+/58386/comment/bff3b227_345619dd PS4, Line 108: jc 1f > I always confuse these conditionals, jumps if eax < 0xb ? JC, JB, JNAE = Jump if CF = 1 The code looks sane to me. https://review.coreboot.org/c/coreboot/+/58386/comment/e8e673cc_d6d054ad PS4, Line 124: cmp (%ebx, %ecx, 2), %ax > Does it make sense to optimise the table to 16bits of LAPIC ID? This is an idea for a follow-up, right? -- To view, visit https://review.coreboot.org/c/coreboot/+/58386 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f1f46fe5091ebeab6dfb4c7e151150cf495d0cb Gerrit-Change-Number: 58386 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:43:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush/var/dewatt: update DRAM ID table
by Kenneth Chan (Code Review) 20 Dec '21

20 Dec '21

Attention is currently required from: Paul Menzel. Kenneth Chan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60090 ) Change subject: mb/google/guybrush/var/dewatt: update DRAM ID table ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/60090/comment/fba6771c_b17ed4bc PS4, Line 9: 1.Samsung LPDDR4X 4266 4G K4UBE3D4AB-MGCL : 2.Hynix LPDDR4X 4266 4G H54G56CYRBX247 > Please add a space after the enumeration dots/periods. […] Done File src/mainboard/google/guybrush/variants/guybrush/memory/mem_parts_used.txt: https://review.coreboot.org/c/coreboot/+/60090/comment/ab8900eb_4b5c79fa PS4, Line 13: H54G56CYRBX247 > Please comment, why this is here already. https://review.coreboot.org/c/coreboot/+/60075 nipperkin used this part,too. It had been merged. -- To view, visit https://review.coreboot.org/c/coreboot/+/60090 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5ece849a86c75be5af9bc0393090b5f1e33bfed Gerrit-Change-Number: 60090 Gerrit-PatchSet: 5 Gerrit-Owner: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bhanu Prakash Maiya <bhanumaiya(a)google.com> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 20 Dec 2021 10:27:04 +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 December 2021