Attention is currently required from: Sean Rhodes.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 )
Change subject: src/mainboard: Add Star Labs labtop series
......................................................................
Patch Set 13:
(1 comment)
File src/mainboard/starlabs/labtop/variants/cml/romstage.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-120645):
https://review.coreboot.org/c/coreboot/+/55128/comment/7e186374_b93f66d9
PS13, Line 115: const uint8_t ht = get_uint_option("hyper_threading", memupd->FspmConfig.HyperThreading);
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/55128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31
Gerrit-Change-Number: 55128
Gerrit-PatchSet: 13
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Thu, 03 Jun 2021 16:06:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Furquan Shaikh, Marshall Dawson, Tim Wawrzynczak, Karthik Ramasubramanian, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54966 )
Change subject: drivers/pcie/rtd3/device: Add PCIe RTD3 driver
......................................................................
Patch Set 4:
(1 comment)
File src/soc/amd/common/block/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/54966/comment/bc1a9a6e_2c34d5af
PS1, Line 210: pci_dev_read_resources
> You mean the root port itself? On Intel platforms at least, the root ports are described in ACPI (\_ […]
No, the root port is handled by the AMD PCIe GPP driver. I mean the actual NVMe device node.
i.e., \_SB.PCI0.GP00.XXXX
The current form of this driver handles writing the device node, but using the `target` method it's not clear who writes the ACPI node.
--
To view, visit https://review.coreboot.org/c/coreboot/+/54966
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2adfc925183ff7a19ab97e89212bc87c40d552d0
Gerrit-Change-Number: 54966
Gerrit-PatchSet: 4
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Mario Limonciello <mario.limonciello(a)amd.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 03 Jun 2021 16:04:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 )
Change subject: src/mainboard: Add Star Labs labtop series
......................................................................
Patch Set 13:
(2 comments)
File src/mainboard/starlabs/labtop/variants/cml/romstage.c:
https://review.coreboot.org/c/coreboot/+/55128/comment/b3149d0e_7c56c09a
PS4, Line 114: const uint8_t ht = get_uint_option("hyper_threading", memupd->FspmConfig.HyperThreading);
> > line over 96 characters […]
1 character is OK with me
File src/mainboard/starlabs/labtop/variants/kbl/romstage.c:
https://review.coreboot.org/c/coreboot/+/55128/comment/743778ea_c18a0825
PS6, Line 26: /* struct region_device spd_rdev; */
> remove
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/55128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31
Gerrit-Change-Number: 55128
Gerrit-PatchSet: 13
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Thu, 03 Jun 2021 16:04:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <admin(a)starlabs.systems>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 )
Change subject: src/mainboard: Add Star Labs labtop series
......................................................................
Patch Set 13:
(1 comment)
Patchset:
PS13:
Thanks for the port Sean, it's always great to see new mainboards here! 🎉
Feel free to update https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/Do… in a followup if you'd like
--
To view, visit https://review.coreboot.org/c/coreboot/+/55128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31
Gerrit-Change-Number: 55128
Gerrit-PatchSet: 13
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Thu, 03 Jun 2021 16:03:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 )
Change subject: src/mainboard: Add Star Labs labtop series
......................................................................
Patch Set 13: Code-Review+2
(1 comment)
File Documentation/mainboard/starlabs/labtop.md:
https://review.coreboot.org/c/coreboot/+/55128/comment/0123d34a_f39be2dc
PS9, Line 1: # Star Labs LabTop
:
: ## Specs
:
: - CPU (full processor specs available at https://ark.intel.com)
: - Intel i7-10710U (Comet Lake)
: - Intel i3-10110U (Comet Lake)
: - Intel i7-8550u (Kaby Lake Refresh)
: - EC
: - ITE IT8987E
: - Backlit Keyboard, with standard PS/2 keycodes and SCI hotkeys
: - Battery
: - Charger, using AC adapter or USB-C PD
: - Suspend / resume
: - GPU
: - Intel UHD Graphics 620
: - GOP driver is recommended, VBT is provided
: - eDP 13-inch 1920x1080 LCD
: - HDMI video
: - USB-C DisplayPort video
: - Memory
: - 16GB on-board for Comet Lake platforms[^1]
: - 8GB on-board for Kaby Lake Refresh platform.
: - Networking
: - AX201 CNVi WiFi / Bluetooth soldered to PCBA (Comet Lake)
: - 8265 PCIe WiFi / Bluetooth soldered to PCBA (Kaby Lake Refresh)
: - Sound
: - Realtek ALC256
: - Internal speakers
: - Internal microphone
: - Combined headphone / microphone 3.5-mm jack
: - HDMI audio
: - USB-C DisplayPort audio
: - Storage
: - M.2 PCIe SSD
: - RTS5129 MicroSD card reader
: - USB
: - 1280x720 CCD camera
: - USB 3.1 Gen 2 Type-C (left)
: - USB 3.1 Gen 2 Type-A (left)
: - USB 3.1 Gen 1 Type-A (right)
:
: [^1] The Comet Lake PCB supports multiple memory variations that are based on hardware configuration resistors see `src/mainboard/starlabs/labtop/variants/cml/romstage.c`
:
: ## Building coreboot
:
: ### Preliminaries
:
: Prior to building coreboot the following files are required:
:
: Comet Lake and Kaby Lake configurations:
: - Intel Flash Descriptor file (descriptor.bin)
: - Intel Management Engine firmware (me.bin)
:
: Comet Lake configuration only:
: - ITE IT8987E firmware (it8987-x.xx.bin)
:
: All Star Labs platforms:
: - Splash screen image in Windows 3.1 BMP format (Logo.bmp)
:
: These files exist in the correct location in the StarLabsLtd/blobs repo on GitHub which is used in place of the standard 3rdparty/blobs repo.
:
: ### Build
:
: The following commands will build a working image:
:
: ##### LabTop Mk IV (Comet Lake)
:
: ```bash
: make distclean
: make defconfig KBUILD_DEFCONFIG=configs/config.starlabs_labtop_cml
: make
: ```
:
: ##### LabTop Mk III (Kaby Lake)
:
: ```bash
: make distclean
: make defconfig KBUILD_DEFCONFIG=configs/config.starlabs_labtop_kbl
: make
: ```
:
: ## Flashing coreboot
:
: ```eval_rst
: +---------------------+------------+
: | Type | Value |
: +=====================+============+
: | Socketed flash | no |
: +---------------------+------------+
: | Vendor | Winbond |
: +---------------------+------------+
: | Model | 25Q128JVSQ |
: +---------------------+------------+
: | Size | 16 MiB |
: +---------------------+------------+
: | Package | SOIC-8 |
: +---------------------+------------+
: | Internal flashing | yes |
: +---------------------+------------+
: | External flashing | no |
: +---------------------+------------+
> That's the last time I used the GUI!
Haha, I never used it before 😊
--
To view, visit https://review.coreboot.org/c/coreboot/+/55128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31
Gerrit-Change-Number: 55128
Gerrit-PatchSet: 13
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Thu, 03 Jun 2021 16:01:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Sean Rhodes <admin(a)starlabs.systems>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55000 )
Change subject: util/spd_tools: Modify MT53E1G32D2NP-046 WT:B LPDDR4 config
......................................................................
util/spd_tools: Modify MT53E1G32D2NP-046 WT:B LPDDR4 config
CB:52586 ("util/spd_tools: Add MT53E1G32D2NP-046 WT:B LPDDR4 config")
incorrectly set ranks per channel to 1. However, MT53E1G32D2NP-046 WT:B
part has 2 channels per die and 2 physical dies. Each channel in each die shares DQ-DQS lines with the channel in other die and uses separate CS lines. Thus, number of ranks per channel is 2.
This change fixes the attribute ranksPerChannel for MT53E1G32D2NP-046 WT:B in LP4x global config by setting it to 2.
BUG=b:186616388
Change-Id: Iba87754ca04c2e026a9cbc8ef07412b467140cba
Signed-off-by: Amanda Huang <amanda_hwang(a)compal.corp-partner.google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55000
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Furquan Shaikh <furquan(a)google.com>
---
M util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Furquan Shaikh: Looks good to me, approved
diff --git a/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt b/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
index d032064..8df4472 100644
--- a/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
+++ b/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
@@ -68,7 +68,7 @@
"channelsPerDie": 2,
"diesPerPackage": 2,
"bitWidthPerChannel": 16,
- "ranksPerChannel": 1,
+ "ranksPerChannel": 2,
"speedMbps": 4267
}
},
--
To view, visit https://review.coreboot.org/c/coreboot/+/55000
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iba87754ca04c2e026a9cbc8ef07412b467140cba
Gerrit-Change-Number: 55000
Gerrit-PatchSet: 6
Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Karthik Ramasubramanian, Felix Held.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54966 )
Change subject: drivers/pcie/rtd3/device: Add PCIe RTD3 driver
......................................................................
Patch Set 4:
(1 comment)
File src/soc/amd/common/block/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/54966/comment/d50fd558_c69a9256
PS1, Line 210: pci_dev_read_resources
> What about the NVMe case where we don't have a PCI driver that writes the ACPI node? […]
You mean the root port itself? On Intel platforms at least, the root ports are described in ACPI (\_SB.PCI0.RPxx), so it's perfectly valid to open up the scope (the RP device is in the DSDT).
That's how the current intel/common/pcie3/rtd3 driver works, it uses the RP device itself (plus the weird "PXSX" device)
--
To view, visit https://review.coreboot.org/c/coreboot/+/54966
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2adfc925183ff7a19ab97e89212bc87c40d552d0
Gerrit-Change-Number: 54966
Gerrit-PatchSet: 4
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Mario Limonciello <mario.limonciello(a)amd.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 03 Jun 2021 15:50:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment