Attention is currently required from: Henry Sun, Chen Wisley, Wisley Chen, Karthik Ramasubramanian.
Hello build bot (Jenkins), Henry Sun, Wisley Chen, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56905
to look at the new patch set (#3).
Change subject: mb/google/dedede/variant/drawcia: Include SPD for MT53E512M32D1NP-046 WT:B
......................................................................
mb/google/dedede/variant/drawcia: Include SPD for MT53E512M32D1NP-046 WT:B
Add SPD support to drawcia for MT53E512M32D1NP-046 WT:B.
This part is already in global_lp4x_mem_parts.json.txt, and use
/util/spd_tool/lp4x/gen_part_id to assigns DRAM IDs.
BUG=b:196951879
BRANCH=firmware-dedede-13606.B
TEST=FW_NAME=drawcia emerge-dedede coreboot chromeos-bootimage
Change-Id: Ic42e6357943ba651ffd92fb2974e9ea52fa19020
Signed-off-by: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
---
M src/mainboard/google/dedede/variants/drawcia/memory/Makefile.inc
M src/mainboard/google/dedede/variants/drawcia/memory/dram_id.generated.txt
M src/mainboard/google/dedede/variants/drawcia/memory/mem_list_variant.txt
3 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/56905/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/56905
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic42e6357943ba651ffd92fb2974e9ea52fa19020
Gerrit-Change-Number: 56905
Gerrit-PatchSet: 3
Gerrit-Owner: Chen Wisley <wisley.chen(a)quantatw.com>
Gerrit-Reviewer: Henry Sun <henrysun(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Henry Sun <henrysun(a)google.com>
Gerrit-Attention: Chen Wisley <wisley.chen(a)quantatw.com>
Gerrit-Attention: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Martin Roth, Idwer Vollering, Paul Menzel, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56803 )
Change subject: util/genbuild_h: micro-adjust the regexp used to set COREBOOT_MAJOR_VERSION
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56803/comment/e5df638f_a16a8537
PS3, Line 7: COREBOOT_MAJOR_VERSION
> COREBOOT_MINOR_VERSION, actually.
Alternatively, borrow CB:55468 's commit message.
File util/genbuild_h/genbuild_h.sh:
https://review.coreboot.org/c/coreboot/+/56803/comment/92a40241_4f7ded0b
PS2, Line 73: printf "#define COREBOOT_MAJOR_VERSION %d\n#define COREBOOT_MINOR_VERSION %d\n" `git describe --match [0-9].[0-9]* | sed 's/\([0-9]\)\.\([0-9][0-9]*\).*/\1 \2/'`
> Please explain in the commit message, what difference \([0-9]\+\) versus \([0-9][0-9]*\) makes.
CB:55468 says that \+ is not part of POSIX Basic Regular Expressions.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56803
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1c0c260fd8d42e23a612a353a288e472cc068c8e
Gerrit-Change-Number: 56803
Gerrit-PatchSet: 3
Gerrit-Owner: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 19 Aug 2021 13:14:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Martin Roth, Idwer Vollering, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56803 )
Change subject: util/genbuild_h: micro-adjust the regexp used to set COREBOOT_MAJOR_VERSION
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56803/comment/7ee54856_639d2f03
PS3, Line 7: COREBOOT_MAJOR_VERSION
COREBOOT_MINOR_VERSION, actually.
https://review.coreboot.org/c/coreboot/+/56803/comment/b109439d_082de6f4
PS3, Line 11: will avoid
nit: avoids
--
To view, visit https://review.coreboot.org/c/coreboot/+/56803
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1c0c260fd8d42e23a612a353a288e472cc068c8e
Gerrit-Change-Number: 56803
Gerrit-PatchSet: 3
Gerrit-Owner: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 19 Aug 2021 13:11:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(1 comment)
File src/drivers/gfx/nvidia/chip.h:
https://review.coreboot.org/c/coreboot/+/57034/comment/9b57f2aa_8c280f23
PS2, Line 7: /* TODO: Set GPIOs in devicetree? */
> yup. […]
It would also be very useful to generate the Optimus ACPI code using the SSDT generator.
The power control sequences for SA PEG and PCH PCIe are different, but it should be possible to generate a SSDT to handle this using information from the devicetree.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Thu, 19 Aug 2021 13:03:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57034/comment/f1e19067_397e5934
PS2, Line 10: GC6
> What does GC6 mean?
It's just graphics power states. https://patents.google.com/patent/US20190163255A1/en
File src/drivers/gfx/nvidia/acpi/gpu.asl:
https://review.coreboot.org/c/coreboot/+/57034/comment/3fb7e2e4_a011d6e3
PS2, Line 108: Debug = " Q0L0 = 1"
: Q0L0 = 1
:
: Debug = " Sleep 16"
: Sleep (16)
:
: Debug = " While Q0L0"
: Local0 = 0
: While (Q0L0) {
: If ((Local0 > 4)) {
: Debug = " While Q0L0 timeout"
: Break
: }
:
: Sleep (16)
: Local0++
: }
:
: Debug = " P0RM = 0"
: P0RM = 0
:
: Debug = " P0AP = 0"
: P0AP = 0
I don't know if you based this on reference code, but this part looks specific to PEG. Optimus on ULT platforms uses a PCH PCIe root port, and this would be different.
Delving deeper into code, looks like this code enables/disables the dGPU without disabling the PCIe link on the PEG side. Looks like one can also power off the PEG port's bundles to save more power, but it rather complicated: it requires saving and restoring register values of its PCI config space and knowing which bundles to power on/off (depends on e.g. PEG bifurcation).
https://review.coreboot.org/c/coreboot/+/57034/comment/69048e87_5a52e556
PS2, Line 153: Debug = " Q0L2 = 1"
: Q0L2 = 1
:
: Debug = " Sleep 16"
: Sleep (16)
:
: Debug = " While Q0L2"
: Local0 = Zero
: While (Q0L2) {
: If ((Local0 > 4)) {
: Debug = " While Q0L2 timeout"
: Break
: }
:
: Sleep (16)
: Local0++
: }
:
: Debug = " P0RM = 1"
: P0RM = 1
:
: Debug = " P0AP = 3"
: P0AP = 3
Same, this part looks specific to PEG.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Thu, 19 Aug 2021 12:53:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Malik Hsu, Paul Menzel.
Casper Chang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57038 )
Change subject: mb/google/brya/primus: modify HID to MX98357 to enable audio function
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57038/comment/617c5771_33dc8f6b
PS3, Line 10: to MX98360A until the kernel SoF driver has support.
> Please summarize the discussion as the bug report is not publically accessible. […]
as the Linux kernel patch is under discussion, we might not be able to provide.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57038
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I893d9a31dc2c7726599c150be01b9585fb6c8a47
Gerrit-Change-Number: 57038
Gerrit-PatchSet: 3
Gerrit-Owner: Casper Chang <casper_chang(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: Malik Hsu <malik_hsu(a)wistron.corp-partner.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-Attention: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Thu, 19 Aug 2021 11:57:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(3 comments)
File src/drivers/gfx/nvidia/acpi/gpu.asl:
https://review.coreboot.org/c/coreboot/+/57034/comment/c86bbf62_dc9cd124
PS2, Line 95: Debug = " DGPU_PWR_EN high"
I would use Printf for debug output; but that's just personal preference
https://review.coreboot.org/c/coreboot/+/57034/comment/44ce1c40_87ba4c42
PS2, Line 160: Zero
0
https://review.coreboot.org/c/coreboot/+/57034/comment/0a687461_b13fac02
PS2, Line 184: Zero
0
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 19 Aug 2021 11:13:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(1 comment)
File src/drivers/gfx/nvidia/chip.h:
https://review.coreboot.org/c/coreboot/+/57034/comment/dab7b20b_f624d096
PS2, Line 7: /* TODO: Set GPIOs in devicetree? */
> Yes
yup. see topic:gpio_ops_and_ipmi for an example
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 19 Aug 2021 11:06:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57009 )
Change subject: tests: Fix function mocking for clang
......................................................................
Patch Set 1:
(1 comment)
File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/57009/comment/f165386b_4caae9c9
PS1, Line 60: TEST_CFLAGS += $(if $(filter clang,$(shell $(HOSTCC) --version)),,-Werror)
In the llvm-project at clang/lib/Sema/Sema.cpp:1167 [1] there is a check that generates those warnings. We could add a new diagnostic flag e.g. "-Wno-weak-identifier-undeclared" and add check:
> if (!Diags.isIgnored(diag::warn_weak_identifier_undeclared, SourceLocation()))
> Diag(WeakID.second.getLocation(), diag::warn_weak_identifier_undeclared)
> << WeakID.first;
Next we would have to add commandline flag for it to:
lang/include/clang/Basic/DiagnosticGroups.td:
> def WarnWeakIdentifierUndeclared : DiagGroup<"warn_weak_identifier_undeclared">
clang/include/clang/Basic/DiagnosticSemaKinds.td:
> def warn_weak_identifier_undeclared : Warning<
> "weak identifier %0 never declared">,
> InGroup<WarnWeakIdentifierUndeclared>;
Probably there will also be a need for new tests or updates of old ones. I am sure, we have to remove warn_weak_identifier_undeclared from clang/test/Misc/warning-flags.c
[1] https://github.com/llvm/llvm-project/blob/acc58504952f0e2ca48a44d868f684e3b…
--
To view, visit https://review.coreboot.org/c/coreboot/+/57009
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1f9011f444248544de7a71bbefc54edc006ae0cd
Gerrit-Change-Number: 57009
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Thu, 19 Aug 2021 10:59:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment