Attention is currently required from: Angel Pons, Varshit Pandya.
Máté Kukri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81529?usp=email )
Change subject: mb/dell/optiplex_9020: Implement late HWM initialization
......................................................................
Patch Set 5:
(1 comment)
File src/mainboard/dell/optiplex_9020/mainboard.c:
https://review.coreboot.org/c/coreboot/+/81529/comment/2d8b2dac_1a8ed024 :
PS5, Line 371: if (CONFIG_MAX_CPUS > 2) {
> Ack. There are CPUs with 2 cores and 2 threads, but they're low-end.
Mine is 4 core 4 thread, and that MSR returns 7 on mine, so I wonder what value would it be on the 2 core 2 thread ones. I have one in a box somewhere I guess I might dig it up later
--
To view, visit https://review.coreboot.org/c/coreboot/+/81529?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: Ibdccd3fc7364e03e84ca606592928410624eed43
Gerrit-Change-Number: 81529
Gerrit-PatchSet: 5
Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 11 Apr 2024 18:29:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Máté Kukri <kukri.mate(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Shelley Chen, Sumeet R Pawnikar.
Hello Karthik Ramasubramanian, Nick Vaccaro, Sumeet R Pawnikar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81789?usp=email
to look at the new patch set (#7).
Change subject: mb/google/brox: Initialize NOTE_BOOK_MODE GPIO
......................................................................
mb/google/brox: Initialize NOTE_BOOK_MODE GPIO
The GPIO for NOTE_BOOK_MODE has changed from GPP_B17 to GPP_E9. Also
initializing it (if ISH is enabled) to be NF2 (ISH_GP4). Also took
the liberty of alphabetizing all the ISH GPIOs to they're easier to
search through.
BUG=b:316421831
BRANCH=None
TEST=emerge-brox coreboot chromeos-bootimage
Make sure that brox device still boots up with this change.
Change-Id: I4a091b58deb855c7a7f1489a9506db2f821503b7
Signed-off-by: Shelley Chen <shchen(a)google.com>
---
M src/mainboard/google/brox/variants/baseboard/brox/gpio.c
M src/mainboard/google/brox/variants/brox/fw_config.c
2 files changed, 12 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/81789/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/81789?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: I4a091b58deb855c7a7f1489a9506db2f821503b7
Gerrit-Change-Number: 81789
Gerrit-PatchSet: 7
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Alexander Couzens, Maciej Pijanowski, Michał Kopeć, Paul Menzel.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80609?usp=email )
Change subject: mb/lenovo: Add ThinkCentre M920q (Cannon Lake)
......................................................................
Patch Set 7: Code-Review+1
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80609/comment/26175a6d_f891a535 :
PS7, Line 7: Cannon Lake
That's Coffee Lake, even if the coreboot code is named `cannonlake`
https://review.coreboot.org/c/coreboot/+/80609/comment/7f5bf373_df06c2c6 :
PS7, Line 28: Display Port
nit: If you mean the VESA standard, it's "DisplayPort" without a space
File src/mainboard/lenovo/m920q/cmos.layout:
https://review.coreboot.org/c/coreboot/+/80609/comment/6a5c2591_61a1eb1f :
PS7, Line 24: 408 1 e 1 nmi
Is this option actually implemented?
File src/mainboard/lenovo/m920q/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80609/comment/68b2dc1c_5276c9c3 :
PS7, Line 7: register "RMT" = "1"
Shouldn't this be a Kconfig option?
File src/mainboard/lenovo/m920q/romstage.c:
https://review.coreboot.org/c/coreboot/+/80609/comment/feada1ee_c85b65f5 :
PS6, Line 33: /* Rcomp target values for CFL-S, DDR4 and 2 DIMMs per channel */
> Updated the comment. […]
I barely remember, but I think the only difference (if any) is the value of the first target (50 or 60)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80609?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: Iea1dc5745c0ecf687fa18b793f0aab4b0855d6d4
Gerrit-Change-Number: 80609
Gerrit-PatchSet: 7
Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Thu, 11 Apr 2024 18:18:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Comment-In-Reply-To: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-MessageType: comment
Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69168?usp=email )
Change subject: soc/intel/**/fast_spi.c: Reorganize some statements
......................................................................
soc/intel/**/fast_spi.c: Reorganize some statements
Avoid calling `acpi_device_scope()` and `fast_spi_acpi_hid()` if the
result won't be used. Also, reorder a condition so that compile-time
constants appear first, so as to help the compiler optimize it out.
Change-Id: I42ce55c2978ad9c593c359c5decd5842fb3a97a1
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/69168
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
---
M src/soc/intel/common/block/fast_spi/fast_spi.c
1 file changed, 5 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Felix Singer: Looks good to me, approved
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi.c b/src/soc/intel/common/block/fast_spi/fast_spi.c
index 74449df..5382644 100644
--- a/src/soc/intel/common/block/fast_spi/fast_spi.c
+++ b/src/soc/intel/common/block/fast_spi/fast_spi.c
@@ -502,17 +502,17 @@
*/
static void fast_spi_fill_ssdt(const struct device *dev)
{
+ /* Do not add SSDT if the fast SPI device is hidden. */
+ if (!CONFIG(FAST_SPI_GENERATE_SSDT) || dev->hidden)
+ return;
+
const char *scope = acpi_device_scope(dev);
const char *hid = fast_spi_acpi_hid(dev);
- struct resource *res;
- /* Do not add SSDT if the fast SPI device is hidden. */
- if (dev->hidden || !CONFIG(FAST_SPI_GENERATE_SSDT))
- return;
if (!scope || !hid)
return;
- res = probe_resource(dev, PCI_BASE_ADDRESS_0);
+ struct resource *res = probe_resource(dev, PCI_BASE_ADDRESS_0);
if (!res)
return;
--
To view, visit https://review.coreboot.org/c/coreboot/+/69168?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: I42ce55c2978ad9c593c359c5decd5842fb3a97a1
Gerrit-Change-Number: 69168
Gerrit-PatchSet: 3
Gerrit-Owner: Angel Pons <th3fanbus(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 Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Angel Pons, Paul Menzel, Werner Zeh.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69168?usp=email )
Change subject: soc/intel/**/fast_spi.c: Reorganize some statements
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/69168?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: I42ce55c2978ad9c593c359c5decd5842fb3a97a1
Gerrit-Change-Number: 69168
Gerrit-PatchSet: 2
Gerrit-Owner: Angel Pons <th3fanbus(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 Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Thu, 11 Apr 2024 18:17:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Shaunak Saha, srinivas.kulkarni(a)intel.com.
Kyoung Il Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81674?usp=email )
Change subject: vc/intel/fsp/mtl: Add Intel Touch Controller UPD in Partial Header
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/81674?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: I9a18b83fbb2d7760bd58027a4203919489b01580
Gerrit-Change-Number: 81674
Gerrit-PatchSet: 4
Gerrit-Owner: srinivas.kulkarni(a)intel.com
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: srinivas.kulkarni(a)intel.com
Gerrit-Comment-Date: Thu, 11 Apr 2024 18:12:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/81854?usp=email )
Change subject: gfxtest: Fix out-of-order components, permanently
......................................................................
Patch Set 1:
(1 comment)
File gfxtest/hw-gfx-gma-gfx_test.adb:
https://review.coreboot.org/c/libgfxinit/+/81854/comment/1ee662d4_31a9a772 :
PS1, Line 54: Blue : Byte;
: Green : Byte;
: Red : Byte;
> I would keep using the RGB order […]
Problem is, this record defines the byte ordering for the hardware. This patch should allow the RGB-to-BGR swizzling to be done at compile time, so that there's no extra runtime cost.
If I were to use RGB ordering here, then `Pixel_To_Word` would have to unscramble the pixel, which is harder to work with.
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/81854?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: I77dbdcd6c235e411585585779c31777adcef57d0
Gerrit-Change-Number: 81854
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 11 Apr 2024 18:06:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-MessageType: comment
Attention is currently required from: Máté Kukri, Varshit Pandya.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81529?usp=email )
Change subject: mb/dell/optiplex_9020: Implement late HWM initialization
......................................................................
Patch Set 5: Code-Review+1
(3 comments)
File src/mainboard/dell/optiplex_9020/mainboard.c:
https://review.coreboot.org/c/coreboot/+/81529/comment/83b5e521_094140e0 :
PS5, Line 355: case 3:
> These are the same chassis type values the vendor firmware uses. […]
Ack
https://review.coreboot.org/c/coreboot/+/81529/comment/cddfcb1e_39717453 :
PS5, Line 368: die("Unknown chassis type\n");
> No table results in the fan running at a low constant speed.
Can the fan still be set to run at full speed? Should work to prevent overheating.
https://review.coreboot.org/c/coreboot/+/81529/comment/8466ad7e_cbe9216a :
PS5, Line 371: if (CONFIG_MAX_CPUS > 2) {
> The vendor firmware bases this check on "max core address MSR" > 2 (i dont remember the id off the t […]
Ack. There are CPUs with 2 cores and 2 threads, but they're low-end.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81529?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: Ibdccd3fc7364e03e84ca606592928410624eed43
Gerrit-Change-Number: 81529
Gerrit-PatchSet: 5
Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Comment-Date: Thu, 11 Apr 2024 18:04:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Máté Kukri <kukri.mate(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons.
Hello Arthur Heymans, Nico Huber, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52651?usp=email
to look at the new patch set (#4).
Change subject: sandybridge,haswell,broadwell: Use DIV_ROUND_CLOSEST macro
......................................................................
sandybridge,haswell,broadwell: Use DIV_ROUND_CLOSEST macro
Integer division in C truncates toward zero. When the dividend and the
divisor are positive, one can add half of the divisor to the dividend to
round the division result towards the closest integer. We already have a
macro in commonlib to do just that, so put it to good use.
Tested with BUILD_TIMELESS=1, coreboot images for the Asus P8Z77-V LX2
and the Asrock B85M Pro4 do not change.
Change-Id: I251af82da15049a3a2aa6ea712ae8c9fe859caf6
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/northbridge/intel/haswell/haswell_mrc/raminit.c
M src/northbridge/intel/sandybridge/raminit_shared.c
M src/soc/intel/broadwell/raminit.c
3 files changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/52651/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/52651?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: I251af82da15049a3a2aa6ea712ae8c9fe859caf6
Gerrit-Change-Number: 52651
Gerrit-PatchSet: 4
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: Patrick Rudolph <rudolphpatrick05(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset