Attention is currently required from: Felix Held, Julius Werner, Marshall Dawson, Maximilian Brune, Zheng Bao, ritul guru.
Bao Zheng has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85466?usp=email )
Change subject: amdfwtool: Set address mode of BIOS binary as context defines
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
> I tested on birman-plus: The binary changed as expected, but the size of all CBFS files is the same […]
Do you use the default setting to build birman-plus.
Based on my test, only majolica and onyx_poc(genoa) have change the final image.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85466?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9c2f2983d03c913b28fbd87aa0925a32a4649d62
Gerrit-Change-Number: 85466
Gerrit-PatchSet: 9
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 12 Feb 2025 09:28:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86343?usp=email )
(
4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: soc/mediatek/mt8196: Remove tvdpll3 disable/enable
......................................................................
soc/mediatek/mt8196: Remove tvdpll3 disable/enable
The tvdpll3 cannot be disabled during suspend because of the enable
operation, so we remove the enable operation. Hardware can now
automatically enable and disable tvdpll3 based on the clock demand of
its downstream.
BRANCH=rauru
BUG=b:377628718
TEST=Bootup OK, Suspend/Resume OK and FW screen shown OK, with MMinfra
kernel/vcp patch, mminfra can be turned off to reduce power consumption.
Signed-off-by: Guangjie Song <guangjie.song(a)mediatek.com>
Change-Id: Ib9c72a1602c1f76dc94cca5c4a61a542a853560b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/86343
Reviewed-by: Yidi Lin <yidilin(a)google.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso(a)google.com>
---
M src/soc/mediatek/mt8196/pll.c
1 file changed, 0 insertions(+), 4 deletions(-)
Approvals:
Yu-Ping Wu: Looks good to me, approved
Yidi Lin: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/soc/mediatek/mt8196/pll.c b/src/soc/mediatek/mt8196/pll.c
index 59a9b09..2007197 100644
--- a/src/soc/mediatek/mt8196/pll.c
+++ b/src/soc/mediatek/mt8196/pll.c
@@ -1572,11 +1572,7 @@
{
const struct pll *pll = &plls[CLK_APMIXED2_TVDPLL3];
- clrbits32(pll->reg, MT8196_PLL_EN);
pll_set_rate(pll, freq);
- setbits32(pll->reg, MT8196_PLL_EN);
-
- udelay(PLL_EN_DELAY);
}
void mt_pll_edp_mux_set_sel(u32 sel)
--
To view, visit https://review.coreboot.org/c/coreboot/+/86343?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib9c72a1602c1f76dc94cca5c4a61a542a853560b
Gerrit-Change-Number: 86343
Gerrit-PatchSet: 6
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com>
Attention is currently required from: Felix Held, Julius Werner, Marshall Dawson, Zheng Bao, ritul guru.
Bao Zheng has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85466?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: amdfwtool: Set address mode of BIOS binary as context defines
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
Hold.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85466?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9c2f2983d03c913b28fbd87aa0925a32a4649d62
Gerrit-Change-Number: 85466
Gerrit-PatchSet: 9
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 12 Feb 2025 08:55:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Elyes Haouas, Martin L Roth.
Elyes Haouas has uploaded a new patch set (#11) to the change originally created by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/85169?usp=email )
Change subject: util/crossgcc: Update LLVM from 18.1.8 to 19.1.7
......................................................................
util/crossgcc: Update LLVM from 18.1.8 to 19.1.7
Change-Id: I4a392b472b7450685343e1343c7b84c78b0cad3d
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M util/crossgcc/buildgcc
R util/crossgcc/patches/clang-19.1.7.src_x86_baremetal.patch
D util/crossgcc/sum/clang-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/clang-19.1.7.src.tar.xz.cksum
D util/crossgcc/sum/clang-tools-extra-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/clang-tools-extra-19.1.7.src.tar.xz.cksum
D util/crossgcc/sum/cmake-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/cmake-19.1.7.src.tar.xz.cksum
D util/crossgcc/sum/compiler-rt-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/compiler-rt-19.1.7.src.tar.xz.cksum
D util/crossgcc/sum/libunwind-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/libunwind-19.1.7.src.tar.xz.cksum
D util/crossgcc/sum/lld-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/lld-19.1.7.src.tar.xz.cksum
D util/crossgcc/sum/llvm-18.1.8.src.tar.xz.cksum
A util/crossgcc/sum/llvm-19.1.7.src.tar.xz.cksum
16 files changed, 8 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/85169/11
--
To view, visit https://review.coreboot.org/c/coreboot/+/85169?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4a392b472b7450685343e1343c7b84c78b0cad3d
Gerrit-Change-Number: 85169
Gerrit-PatchSet: 11
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Attention is currently required from: Dinesh Gehlot, Jayvik Desai, John Su, Kapil Porwal, Nick Vaccaro.
Eric Lai has posted comments on this change by John Su. ( https://review.coreboot.org/c/coreboot/+/86364?usp=email )
Change subject: mb/trulo/var/uldrenite: Support x32 memory configuration
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86364?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Idd3534bba0379a7bb06f8fbbeb9469e938e5a629
Gerrit-Change-Number: 86364
Gerrit-PatchSet: 5
Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com>
Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Wed, 12 Feb 2025 08:17:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Alicja Michalska, Arthur Heymans, Nico Huber.
Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/64189?usp=email )
Change subject: haswell NRI: Add DDR3 JEDEC reset and init
......................................................................
Patch Set 8:
(3 comments)
File src/northbridge/intel/haswell/native_raminit/raminit_native.h:
https://review.coreboot.org/c/coreboot/+/64189/comment/eaeeecc9_e6c3281e?us… :
PS8, Line 79: RAMINIT_STATUS_UNSPECIFIED_ERROR, /** TODO: Deprecated in favor of specific values **/
> Assuming error code reporting isn't ready yet?
This comment is because some of the code I haven't upstreamed yet still uses this, and I'd rather get rid of this one line after the thousands of lines I've still got around are at least upstreamed.
I actually don't use error code values for anything yet (0 = success, anything else = failure), but I felt it'd be nicer than hardcoding magic numbers all over the place (which was the case before). I will eventually use them to try raminit again after a failure, using more relaxed parameters or disabling a failing channel.
https://review.coreboot.org/c/coreboot/+/64189/comment/d4e30546_219bf759?us… :
PS8, Line 176: int8_t rxvref[NUM_CHANNELS][NUM_SLOTRANKS][NUM_LANES];
> uint?
No, this is a signed offset. Zero offset corresponds to VDDQ/2 (half of the I/O voltage), and the offset can be positive or negative.
The timing offsets (tx_dq, txdqs, rcven, rxdqs, clk/ctl/cke pi codes...) are delays measured in "PI ticks" (1/64th of a QCLK (quadrature clock) clock cycle, or 1/128th of a DCLK (DRAM clock) clock cycle). These are unsigned since you cannot go back in time.
I could add comments to these but I think it'd be better to do so in a follow-up as some of the patches in this train add more stuff in here (so adding comments now would cause a bunch of merge conflicts).
File src/southbridge/intel/lynxpoint/pch.h:
https://review.coreboot.org/c/coreboot/+/64189/comment/943cb361_4841a832?us… :
PS8, Line 590: #define PM_CFG2_DRAM_RESET_CTL (1 << 26) /* ULT only */
> NIT: Remove the space to make it look a bit nicer?
The space is there to indicate this is a bitfield within the register. Looks like this file is inconsistent with space usage but I prefer having the space to make it easier to follow
--
To view, visit https://review.coreboot.org/c/coreboot/+/64189?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2a0c066537021b587599228086727cb1e041bff5
Gerrit-Change-Number: 64189
Gerrit-PatchSet: 8
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 12 Feb 2025 08:07:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>
Attention is currently required from: Alicja Michalska, Arthur Heymans, Eric Lai, Felix Held, Felix Singer, Nico Huber, Patrick Rudolph.
Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/64186?usp=email )
Change subject: haswell NRI: Configure initial MC settings
......................................................................
Patch Set 7:
(5 comments)
File src/northbridge/intel/haswell/native_raminit/configure_mc.c:
https://review.coreboot.org/c/coreboot/+/64186/comment/b7281944_0c8bb070?us… :
PS7, Line 16: if (!is_hsw_ult())
> What's the purpose of this check? Does NRI currently support ULT CPUs?
On Haswell Trad (not ULT), we don't program this register. Trad doesn't support configurable channel interleave (it's forced on), and it doesn't support LPDDR (3) either.
I know this code works on Haswell Trad and Haswell ULT with DDR3, but I don't have any Haswell device with LPDDR3.
Should I add any clarification? I use this pattern quite a lot.
https://review.coreboot.org/c/coreboot/+/64186/comment/be653885_09458948?us… :
PS7, Line 641: if (!(reg32 & BIT(31))) { /** TODO: What to do if this is locked? **/
> If it's locked, then reading the register will fail, right? I think it would be nice to put a debug […]
This checks if the lock bit is unset before programming. I believe that *writing* the register will fail, but I'm not sure how to handle it (proceed anyway since it shouldn't affect booting? that's the current approach). I also haven't checked if the register remains locked after a reset. I do know that the register can be locked if we are to retry raminit for some reason (e.g. failed warm/fast boot).
I'll try to refactor this to log an error if the register is locked *and* its value differs from the desired value.
https://review.coreboot.org/c/coreboot/+/64186/comment/58b24c65_80ac7afa?us… :
PS7, Line 660: /** TODO: Remember why this is only done on cold boots **/
> Stored in cache?
I'm pretty sure it's that, yes
File src/northbridge/intel/haswell/native_raminit/raminit_main.c:
https://review.coreboot.org/c/coreboot/+/64186/comment/e2b65ce1_9d31a8dc?us… :
PS7, Line 58: ctrl->vdd_mv = is_hsw_ult() ? 1350 : 1500; /** FIXME: Hardcoded, does it matter? **/
> Probably not, DDR3U isn't very common :)
Yeah, for now it'll do. I know this will need to change once NRI supports voltage switching (which I want to implement someday), but I should first finish the rest of the training steps (I've got a lot to clean up and upstream)
File src/northbridge/intel/haswell/native_raminit/reg_structs.h:
https://review.coreboot.org/c/coreboot/+/64186/comment/34eec391_ac4a0244?us… :
PS7, Line 12: int32_t vref : 6; // Bits 31:26
> s/int/uint, or is there a good reason for it? […]
This is very much intentional - this 6-bit bitfield is actually a signed two's complement value. Conveniently enough, using bitfields for register definitions makes reading and writing these signed bitfields painless.
--
To view, visit https://review.coreboot.org/c/coreboot/+/64186?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If33846b51cb1bab5d0458fe626e13afb1bdc900e
Gerrit-Change-Number: 64186
Gerrit-PatchSet: 7
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <tanleansheng(a)outlook.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 12 Feb 2025 07:47:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86371?usp=email )
Change subject: soc/mediatek/mt8196: Set MT6316 deglitch time from 2ns to 4ns
......................................................................
Patch Set 1:
(2 comments)
File src/soc/mediatek/common/include/soc/mt6316.h:
https://review.coreboot.org/c/coreboot/+/86371/comment/ab12b970_86c85f63?us… :
PS1, Line 32: RG
Remove `RG`. Other values here are also registers, but without `RG` in the names.
Also remove `_ADDR`?
File src/soc/mediatek/common/mt6316.c:
https://review.coreboot.org/c/coreboot/+/86371/comment/074ddc26_a505e9e7?us… :
PS1, Line 177: 0xA
0x0A
--
To view, visit https://review.coreboot.org/c/coreboot/+/86371?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I77bd50cc6c25d6dcded57d9d65d92a0dd19c3c86
Gerrit-Change-Number: 86371
Gerrit-PatchSet: 1
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 07:33:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No