Attention is currently required from: Alicja Michalska, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
Angel Pons has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email )
Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................
Patch Set 13: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Gerrit-Change-Number: 80853
Gerrit-PatchSet: 13
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:35:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Fred Reitberger, Jason Glenesk, Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84376?usp=email )
Change subject: soc/amd/glinda/chipset.cb: Update for glinda
......................................................................
Patch Set 4:
(1 comment)
File src/mainboard/amd/birman/devicetree_glinda.cb:
https://review.coreboot.org/c/coreboot/+/84376/comment/72ae2c99_a77ac0d7?us… :
PS4, Line 211: device ref usb4_xhci_1_root_hub on
shouldn't there be an enabled usb3 port below this?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84376?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: I1138f27bfd47f6fa70a0c2afcc65a5553a609d57
Gerrit-Change-Number: 84376
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:34:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Alicja Michalska, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
Angel Pons has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email )
Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................
Patch Set 13:
(2 comments)
File src/mainboard/erying/tgl/bootblock.c:
https://review.coreboot.org/c/coreboot/+/80853/comment/d34c690a_87a414c8?us… :
PS8, Line 13: ite_reg_write(GPIO_DEV, 0x29, 0xc1); /* 3VSB - RAM loses power in S3 anyway */
> Just noticed this myself yesterday, will definitely give it a try.
Any updates?
File src/mainboard/erying/tgl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80853/comment/737d0857_4bb2acf6?us… :
PS7, Line 125: device ref pch_espi on
> Agreed. Will need to test this, haven't yet had the time to do so.
Any updates?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Gerrit-Change-Number: 80853
Gerrit-PatchSet: 13
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:34:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>
Comment-In-Reply-To: Maxim <max.senia.poliak(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Bao Zheng, Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84373?usp=email )
Change subject: util/amdfwtool: Add binaries
......................................................................
Patch Set 4:
(5 comments)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/84373/comment/e5666656_8e307fbc?us… :
PS4, Line 374: BDT_BOTH
not sure what's correct here, but this is inconsistent with the rest.
also iirc the newer socs don't have a level 2 bios directory table; this might apply to other entries in amd_bios_table too that have the level set or changed to BDT_BOTH in this patch
File util/amdfwtool/data_parse.c:
https://review.coreboot.org/c/coreboot/+/84373/comment/4d3f070f_dd3601ca?us… :
PS4, Line 556: subprog = 0;
why is this removed?
https://review.coreboot.org/c/coreboot/+/84373/comment/7097ccd4_d6bdff3b?us… :
PS4, Line 629: } else if (strcmp(fw_name, "PSP_BDT_UCODE_INS0") == 0) {
: fw_type = AMD_BIOS_UCODE;
: subprog = 0;
: instance = 0;
: } else if (strcmp(fw_name, "PSP_BDT_UCODE_INS1") == 0) {
: fw_type = AMD_BIOS_UCODE;
: subprog = 0;
: instance = 1;
is this needed? for other socs, we keep the ucode update in cbfs and apply it from coreboot. also looks like this is already handled in util/amdfwtool/opts.c?
https://review.coreboot.org/c/coreboot/+/84373/comment/c8dcdf06_e0ea0ed5?us… :
PS4, Line 637: } else if (strcmp(fw_name, "PSP_BDT_APCB_BK_INS0") == 0) {
: fw_type = AMD_BIOS_APCB_BK;
: subprog = 0;
: instance = 0;
: } else if (strcmp(fw_name, "PSP_BDT_APCB_BK_INS8") == 0) {
: fw_type = AMD_BIOS_APCB_BK;
: subprog = 0;
: instance = 8;
this should be already handled in util/amdfwtool/opts.c
https://review.coreboot.org/c/coreboot/+/84373/comment/9e686fdb_516f4924?us… :
PS4, Line 649: fw_type = AMD_BIOS_NV_ST;
probably not the right location in the code to comment about this, but i wonder if this is a file that has the nv range info of if this entry is just supposed to provide the range like it's done for a few other things in util/amdfwtool/opts.c and not have some file added for that
--
To view, visit https://review.coreboot.org/c/coreboot/+/84373?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: I78d7a9dba71de557e0a9a885d8561eea1f4191ef
Gerrit-Change-Number: 84373
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:31:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Martin L Roth, Nicholas Chin.
Raul Rangel has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/84385?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: Makefile: Fix no-op incremental build
......................................................................
Patch Set 2:
(2 comments)
File Makefile:
https://review.coreboot.org/c/coreboot/+/84385/comment/26c387b8_5a43aecc?us… :
PS2, Line 232: # Fix for no-op build
Add a more descriptive comment on why this is needed.
https://review.coreboot.org/c/coreboot/+/84385/comment/e35a736a_5f99ffc2?us… :
PS2, Line 233: $(objutil)/kconfig/conf
Can we just add this as a top-level target:
```
real-all: $(objutil)/kconfig/conf
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84385?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: I98c49d47f811e5cceebce7b7d54b282c773734e3
Gerrit-Change-Number: 84385
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:25:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Angel Pons, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
Alicja Michalska has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email )
Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................
Patch Set 12:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80853/comment/cc385d4c_b6de5ca6?us… :
PS8, Line 31: - Automatic fan control (IT8613E can't read CPU_TIN at the moment)
> Given my issues with the IT8613E on the ODROID-H4, figuring this one out won't be as easy
Yup, we'd need access to IT8613E datasheet for it.
File src/mainboard/erying/tgl/Kconfig:
https://review.coreboot.org/c/coreboot/+/80853/comment/2f27958a_2e70b277?us… :
PS12, Line 14: select HAVE_ACPI_RESUME
> If S3 resume is broken, I would comment out this line
Acknowledged
File src/mainboard/erying/tgl/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/80853/comment/3c32a1bd_e569dbb4?us… :
PS12, Line 27: Scope (\_SB.PCI0.LPCB)
: {
: }
> Still here from PS8
Done
File src/mainboard/erying/tgl/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80853/comment/e5311f62_c6ce743b?us… :
PS12, Line 26: #if (!CONFIG_SOC_INTEL_DISABLE_IGD)
: /* Tigerlake HDMI */
: 0x80862812, /* Vendor ID */
: 0x80860101, /* Subsystem ID */
: 2, /* Number of entries */
: AZALIA_SUBVENDOR(2, 0x80860101),
: AZALIA_PIN_CFG(2, 0x04, 0x18560010),
: #endif
> We don't indent preprocessor in coreboot: […]
Wasn't sure, didn't feel right. Thanks!
File src/mainboard/erying/tgl/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/80853/comment/d02022f8_5896b44a?us… :
PS8, Line 25: mupd->FspmConfig.OcSupport = 1;
: mupd->FspmConfig.OcLock = 0;
> OcSupport may be needed for XMP to be used, OcLock might be to allow changing settings at runtime so […]
I've tested it. OcSupport/OcLock isn't necessary for XMP to work after all
https://review.coreboot.org/c/coreboot/+/80853/comment/5f74ec6a_9d1d38f8?us… :
PS8, Line 43: mupd->FspmConfig.DmiMaxLinkSpeed = 3;
> Interesting... […]
Indeed it is, lol
--
To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Gerrit-Change-Number: 80853
Gerrit-PatchSet: 12
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:17:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Jérémy Compostella, Pranava Y N.
Cliff Huang has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84407?usp=email )
Change subject: mb/google/fatcat: Add override tree
......................................................................
Patch Set 1:
(2 comments)
File src/mainboard/google/fatcat/variants/fatcat/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/84407/comment/7ed829f1_fc09ef30?us… :
PS1, Line 79: [PchSerialIoIndexI2C4] = PchSerialIoPci,
> Disable these as I2C0 and I2C4 are not used?
I2C4 is used for Touchscreen.
https://review.coreboot.org/c/coreboot/+/84407/comment/95668499_87a23245?us… :
PS1, Line 157: device ref thc0 on end
> Please confirm if we are using this
This will be used for touchscreen with THC-i2c or THC-spi mode. In addition, this is function 0. we will need to enable thc0 when thc1 (function 1) is enabled.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84407?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: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d53
Gerrit-Change-Number: 84407
Gerrit-PatchSet: 1
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:16:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pranava Y N <pranavayn(a)google.com>
Attention is currently required from: Alicja Michalska, Angel Pons, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
Hello Angel Pons, Felix Singer, Maxim, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80853?usp=email
to look at the new patch set (#13).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons, Verified-1 by build bot (Jenkins)
Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................
mb/erying: Add Erying Polestar G613 Pro (TGL-H)
Erying is a Chinese manufacturer selling desktop motherboards with
laptop SoCs and custom shim to mount desktop coolers.
Working:
- Serial port (IT8613E 0x3f8)
- All rear USB ports (3.0, 2.0)
- Both HDMI ports
- Realtek GbE NIC
- Internal audio (ALC897/ TGL-H HDMI)
- Environment Controller (SuperIO fan control)
- All SATA ports
- All PCI-E/M.2 ports
- M.2 NGFF WiFi
- PCI-E Resizable BAR (ReBAR)
- VT-x
WIP/Broken:
- PCI-E ASPM (also broken on vendor's FW, clocks are messed up)
- S3/s0ix (also broken on stock, setting 3VSB register didn't help -
system goes to sleep, but RAM loses power)
- DisplayPort on I/O panel (seemingly a simple fix)
- One of USB2 FP connectors, as well as NGFF USB isn't mapped (yet)
- Automatic fan control (IT8613E can't read CPU_TIN at the moment)
Can be flashed using `flashrom -p internal -w build/coreboot.rom`, as
vendor hasn't enabled any protections on SPI chip.
TEST=Flash coreboot build onto the motherboard, install following PCI-E
cards: Radeon RX 7800XT, Kingston KC3000, Optane 900P, Audigy X-Fi.
Power the system up and boot into Windows 10 to check ACPI sanity, then
reboot into Fedora Linux (kernel 6.10.9) and launch 3D application, disk
benchmark, compilation at the same time to check system's stability.
Change-Id: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Signed-off-by: Alicja Michalska <ahplka19(a)gmail.com>
---
A src/mainboard/erying/Kconfig
A src/mainboard/erying/Kconfig.name
A src/mainboard/erying/tgl/Kconfig
A src/mainboard/erying/tgl/Kconfig.name
A src/mainboard/erying/tgl/Makefile.mk
A src/mainboard/erying/tgl/board_info.txt
A src/mainboard/erying/tgl/bootblock.c
A src/mainboard/erying/tgl/cmos.layout
A src/mainboard/erying/tgl/data.vbt
A src/mainboard/erying/tgl/devicetree.cb
A src/mainboard/erying/tgl/dsdt.asl
A src/mainboard/erying/tgl/gpio.h
A src/mainboard/erying/tgl/hda_verb.c
A src/mainboard/erying/tgl/ramstage.c
A src/mainboard/erying/tgl/romstage_fsp_params.c
15 files changed, 837 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/80853/13
--
To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Gerrit-Change-Number: 80853
Gerrit-PatchSet: 13
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Alicja Michalska, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
Angel Pons has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email )
Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................
Patch Set 12: Code-Review+1
(12 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80853/comment/defebb90_d693c428?us… :
PS8, Line 23: broken
> Broken, as it either causes VM to hang, or the entire host 😊
Good grief
https://review.coreboot.org/c/coreboot/+/80853/comment/fe4b1dca_c3a78b34?us… :
PS8, Line 31: - Automatic fan control (IT8613E can't read CPU_TIN at the moment)
> That was my suspicion as well, but it doesn't seem to be the case.
Given my issues with the IT8613E on the ODROID-H4, figuring this one out won't be as easy
File src/mainboard/erying/tgl/Kconfig:
https://review.coreboot.org/c/coreboot/+/80853/comment/cd9c22da_361fb74b?us… :
PS12, Line 14: select HAVE_ACPI_RESUME
If S3 resume is broken, I would comment out this line
File src/mainboard/erying/tgl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80853/comment/7d53f2a0_de653b3d?us… :
PS12, Line 43: device ref peg1 on # SoC x16 (Gen4)
: register "PcieClkSrcUsage[0]" = "0x41"
: register "PcieClkSrcClkReq[0]" = "PCIE_CLK_NOTUSED"
: end
nit: move below peg0
File src/mainboard/erying/tgl/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/80853/comment/eada445c_8c5c5c2e?us… :
PS12, Line 27: Scope (\_SB.PCI0.LPCB)
: {
: }
Still here from PS8
File src/mainboard/erying/tgl/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80853/comment/ce6792d7_43430e28?us… :
PS12, Line 26: #if (!CONFIG_SOC_INTEL_DISABLE_IGD)
: /* Tigerlake HDMI */
: 0x80862812, /* Vendor ID */
: 0x80860101, /* Subsystem ID */
: 2, /* Number of entries */
: AZALIA_SUBVENDOR(2, 0x80860101),
: AZALIA_PIN_CFG(2, 0x04, 0x18560010),
: #endif
We don't indent preprocessor in coreboot:
```suggestion
#if (!CONFIG_SOC_INTEL_DISABLE_IGD)
/* Tigerlake HDMI */
0x80862812, /* Vendor ID */
0x80860101, /* Subsystem ID */
2, /* Number of entries */
AZALIA_SUBVENDOR(2, 0x80860101),
AZALIA_PIN_CFG(2, 0x04, 0x18560010),
#endif
```
File src/mainboard/erying/tgl/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/80853/comment/8f615a0c_287c5646?us… :
PS8, Line 24: mupd->FspmConfig.EnableAbove4GBMmio = 1;
> Technically yes, but I don't see a Kconfig option for it in coreboot (maybe I'm just missing somethi […]
Eh, it's not a big deal (hooking this up in a follow-up is easy)
https://review.coreboot.org/c/coreboot/+/80853/comment/22e4fa91_d42dedad?us… :
PS8, Line 25: mupd->FspmConfig.OcSupport = 1;
: mupd->FspmConfig.OcLock = 0;
> Likewise, though I will check if it's necessary in the first place.
OcSupport may be needed for XMP to be used, OcLock might be to allow changing settings at runtime somehow but I'm not sure
https://review.coreboot.org/c/coreboot/+/80853/comment/9eb30cca_65fdd334?us… :
PS8, Line 28: // iGPU
: mupd->FspmConfig.GttSize = 3; // 8MB
: mupd->FspmConfig.ApertureSize = 3; // 512MB
: mupd->FspmConfig.GtPsmiSupport = 0;
: mupd->FspmConfig.IgdDvmt50PreAlloc = 2; // 64MB
> Yes, without configuring those options, iGPU would crash spectacularly with framebuffer corruption o […]
Whew, that's nasty. Let's keep it as-is for now
https://review.coreboot.org/c/coreboot/+/80853/comment/28243e81_3b35a8ec?us… :
PS8, Line 43: mupd->FspmConfig.DmiMaxLinkSpeed = 3;
> Without setting this parameter, DMI behaves... […]
Interesting... This board is cursed
https://review.coreboot.org/c/coreboot/+/80853/comment/01be5f2d_ca242908?us… :
PS8, Line 54: mupd->FspmConfig.ECT = 1;
> AFAIK necessary for XMP to work?
Weird, I feel manually specifying which training steps to run shouldn't be necessary. But if it works, let's leave it like this.
File src/mainboard/erying/tgl/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/80853/comment/5baef651_63190233?us… :
PS12, Line 40: mupd->FspmConfig.VddVoltage = 1350; // 1.35V
Note: this won't actually change the voltage going to the DIMMs
--
To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Gerrit-Change-Number: 80853
Gerrit-PatchSet: 12
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:07:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Martin L Roth, Nicholas Chin, Raul Rangel.
Nico Huber has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/84385?usp=email )
Change subject: Makefile: Fix no-op incremental build
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Patchset:
PS2:
It's not the first workaround we have for the .SECONDARY, reverting it
would also work for me.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84385?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: I98c49d47f811e5cceebce7b7d54b282c773734e3
Gerrit-Change-Number: 84385
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 18 Sep 2024 22:03:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes