coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: commonlib/bsd/future: Introduce future API
by Raul Rangel (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Aaron Durbin, Furquan Shaikh, Marshall Dawson, Angel Pons, Julius Werner, Eric Peers. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56047 ) Change subject: commonlib/bsd/future: Introduce future API ...................................................................... Patch Set 2: (7 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/56047/comment/72569df3_f2e5ad4d PS1, Line 32: protected. Auditing this is complicated anid error prone. > nit. […] Done File src/commonlib/bsd/include/commonlib/bsd/future.h: https://review.coreboot.org/c/coreboot/+/56047/comment/d3d14d01_2133512e PS1, Line 1: /* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later */ > Why not? Since BSD is more permissive, it's able to be used in more code bases. I'm fine making it GPL only too. No real preference. https://review.coreboot.org/c/coreboot/+/56047/comment/3df074fc_e46e7576 PS1, Line 37: * periodically poll until the second transaction completes. Once the > chaining here to start a decompress would also be useful. "AndThenFut". […] We don't need an `AndThen` future. If a future has dependencies, it is responsible storing their futures and polling them. See the CBFS API for an example. https://review.coreboot.org/c/coreboot/+/56047/comment/90633f29_5c5632f1 PS1, Line 40: * By only polling periodically, we allow the CPU is to continue > nit "is to" Done https://review.coreboot.org/c/coreboot/+/56047/comment/4106a503_970a01c1 PS1, Line 59: !! > nit: redundant, return value already does this conversion Does the fact that this returns a bool cause it to only return 0 or 1? I would assume it would return the truncated pointer. https://review.coreboot.org/c/coreboot/+/56047/comment/55648bc9_0e0044d7 PS1, Line 68: * those dependents should be polled by the containing future and not the > not quite following this. […] Right, the future is no longer polled by the executor once it completes. So a parent future can only complete once all its child futures have completed. https://review.coreboot.org/c/coreboot/+/56047/comment/ce2bdcf2_2caa8c6b PS1, Line 83: > nit: one space too many Done -- To view, visit https://review.coreboot.org/c/coreboot/+/56047 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I125a946233f122c2848704296efbcb3ca3c96079 Gerrit-Change-Number: 56047 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Aaron Durbin <adurbin(a)gmail.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Comment-Date: Wed, 07 Jul 2021 17:05:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Eric Peers <epeers(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: commonlib/bsd/future: Introduce future API
by Raul Rangel (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Aaron Durbin, Furquan Shaikh, Marshall Dawson, Angel Pons, Julius Werner, Eric Peers. Hello build bot (Jenkins), Aaron Durbin, Martin Roth, Furquan Shaikh, Marshall Dawson, Julius Werner, Angel Pons, Eric Peers, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56047 to look at the new patch set (#2). Change subject: commonlib/bsd/future: Introduce future API ...................................................................... commonlib/bsd/future: Introduce future API This CL introduces a futures API. It allows modeling asynchronous operations. This will make it possible to model DMA transfers, I2C transactions, EC operations, etc. It was loosely modeled off the Rust Futures API: https://rust-lang.github.io/async-book/02_execution/02_future.html Since we don't use interrupts while booting, the only option we have is to poll. For this reason I omitted the `wake` callback since there is nothing to call it. I evaluated the existing threads.h API and it has the following down sides: * Each thread requires its own stack. This means that there is a pretty low limit to the number of concurrent async operations, and a lot of overhead. * There are no thread synchronization primitives (e.g., thread handles, join, etc), so it's difficult to wait for an asynchronous operation to complete. * It currently only works in ramstage. * It requires code to be reentrant since we can now have multiple threads of executing going through the same code. This will require adding support for mutexes to make sure critical sections are protected. Auditing this is complicated and error prone. * It makes it hard to tell from looking at the code if something is running in a different thread. * Timestamp tracking becomes difficult because we can context switch to a task that hogs the CPU for a while. The futures API has the following advantages: * We can easily scale the number of async operations since each operation only needs to store the context to run its state machine. * It provides a synchronization primitive so it's possible to block until the operation is complete. * It works in all stages. * There is no need for mutexes since there is only 1 stack and thread of execution. * Async operations are explicitly modeled so it's clear if the code is asynchronous. * The busy_loop parameter allows putting off CPU intensive operations until they are required. This will allow for proper timestamp accounting. * Creating futures that depend on other futures is pretty simple. BUG=b:179699789 TEST=Implemented the API for rdev and cbfs, it seems pretty ergonomic Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I125a946233f122c2848704296efbcb3ca3c96079 --- A src/commonlib/bsd/include/commonlib/bsd/future.h 1 file changed, 86 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/56047/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56047 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I125a946233f122c2848704296efbcb3ca3c96079 Gerrit-Change-Number: 56047 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Aaron Durbin <adurbin(a)gmail.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: grunt/treeya: add Realtek ALC5682 codec support
by Martin Roth (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Daniel Kurtz, Kevin Chang, Paul Menzel. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56100 ) Change subject: grunt/treeya: add Realtek ALC5682 codec support ...................................................................... Patch Set 2: (2 comments) File src/mainboard/google/kahlee/variants/treeya/audio.c: https://review.coreboot.org/c/coreboot/+/56100/comment/e814ef55_c6d022fd PS2, Line 22: 0xfedc2000 > Can you define a macro for this? src/soc/amd/stoneyridge/include/soc/iomap.h:#define I2C_BASE_ADDRESS 0xfedc2000 https://review.coreboot.org/c/coreboot/+/56100/comment/e9c9244a_5a56e193 PS2, Line 32: 0x1a As long as we're updating things, we could turn this into a #define. Same with the "10EC5682" string below. -- To view, visit https://review.coreboot.org/c/coreboot/+/56100 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I49c673fd944b2c2a79c4283eee941a16596ba7fa Gerrit-Change-Number: 56100 Gerrit-PatchSet: 2 Gerrit-Owner: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alec Wang <alec.wang(a)lcfc.corp-partner.google.com> Gerrit-CC: Allen Cheng <allen.cheng(a)lcfc.corp-partner.google.com> Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-Attention: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 07 Jul 2021 16:35:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/redrix: Generate SPD ID for supported parts
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Chen Wisley, Wisley Chen. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55885 ) Change subject: mb/google/brya/var/redrix: Generate SPD ID for supported parts ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55885 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62ee401e43bef22b4b09f41ea59bbdbc479f293c Gerrit-Change-Number: 55885 Gerrit-PatchSet: 6 Gerrit-Owner: Chen Wisley <wisley.chen(a)quantatw.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Chen Wisley <wisley.chen(a)quantatw.com> Gerrit-Attention: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 07 Jul 2021 16:28:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Create redrix variant
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Chen Wisley. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55883 ) Change subject: mb/google/brya: Create redrix variant ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4cfa0bd84e1ba9f8140f95d18a6da960da8124ad Gerrit-Change-Number: 55883 Gerrit-PatchSet: 2 Gerrit-Owner: Chen Wisley <wisley.chen(a)quantatw.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Chen Wisley <wisley.chen(a)quantatw.com> Gerrit-Comment-Date: Wed, 07 Jul 2021 16:27:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/acpi: Add LPM requirements support to PEPD _DSM
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Cliff Huang, Furquan Shaikh, Angel Pons, Michael Niewöhner, Patrick Rudolph, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56005 ) Change subject: soc/intel/common/block/acpi: Add LPM requirements support to PEPD _DSM ...................................................................... Patch Set 2: (6 comments) File src/soc/intel/common/block/acpi/pep.c: https://review.coreboot.org/c/coreboot/+/56005/comment/029dde02_d5cba26c PS1, Line 120: static void pep_s0ix_enum_functions(void *unused) > In general, I don't see Kconfig SOC_INTEL_COMMON_BLOCK_ACPI_PEP_LPM_REQ been used to make pep. […] Yes, in the patches later in this patch train, it is selected for TGL (CB:56007) and ADL (CB:56006). SOC_INTEL_COMMON_BLOCK_ACPI_PEP_LPM_REQ depends on having CONFIG_SOC_INTEL_COMMON_BLOCK_ACPI_PEP selected first (which compiles in pep.c). There shouldn't be any problems here. https://review.coreboot.org/c/coreboot/+/56005/comment/0d82c6fb_ac514b82 PS1, Line 143: PMC_IPC_SUBCMD_GEN_COMM_READ, > Please see comments in pmc_ipc. […] Done https://review.coreboot.org/c/coreboot/+/56005/comment/534516aa_beeddd05 PS1, Line 166: pep_s0ix_collect_lpm_requirements, /* Return LPM requirements */ > use #if for lpm req. If `SOC_INTEL_COMMON_BLOCK_ACPI_PEP_LPM_REQ` if not compiled in, then the _DSM with Arg0 will return `0`, meaning there are no supported functions under this UUID (see lines 122 and 130). https://review.coreboot.org/c/coreboot/+/56005/comment/9ccc7ff6_db023225 PS1, Line 173: DSM_UUID(PEP_S0IX_UUID, pep_s0ix, ARRAY_SIZE(pep_s0ix), NULL), > use #if for lpm req. See above comment. File src/soc/intel/common/block/include/intelblocks/pmc_ipc.h: https://review.coreboot.org/c/coreboot/+/56005/comment/b447b330_1a7b00b4 PS1, Line 25: #define PMC_IPC_CMD_GENERAL_COMM 0xA0 > Please rename to PMC_IPC_CMD_RD_PMC_REG. Done https://review.coreboot.org/c/coreboot/+/56005/comment/2a34c7a1_68cd7d44 PS1, Line 26: #define PMC_IPC_SUBCMD_GEN_COMM_READ 0x02 > When Read PMC reg (0xa0) command is used, there is no subcmd/cmd ID needed. […] Verified, thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/56005 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I542290bd5490aa6580a5ae2b266da3d78bc17e6b Gerrit-Change-Number: 56005 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 07 Jul 2021 16:13:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/acpi: Move pep.asl to acpigen
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Cliff Huang, Furquan Shaikh, Angel Pons, Michael Niewöhner, Patrick Rudolph, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56004 ) Change subject: soc/intel/common/block/acpi: Move pep.asl to acpigen ...................................................................... Patch Set 2: (6 comments) File src/soc/intel/common/block/acpi/Kconfig: https://review.coreboot.org/c/coreboot/+/56004/comment/937d6428_19cf0510 PS1, Line 18: config SOC_INTEL_COMMON_BLOCK_ACPI_PEP > I'm not sure if I follow. […] The files are related, this patch train replaces pep.asl with pep.c (converting all of the ASL to runtime generated AML in the SSDT). pep.asl is currently used by: ADL, CNL, EKL, JSL, SKL and TGL. This patch train intends to make all of the required changes to make pep.asl obsolete. Please see the follower patches to note that each SoC adds the Kconfig and then called generate_acpi_power_engine in their PMC ACPI generation code. File src/soc/intel/common/block/acpi/pep.c: https://review.coreboot.org/c/coreboot/+/56004/comment/10104db1_a2af773c PS1, Line 25: 0x63 > How about: […] Done https://review.coreboot.org/c/coreboot/+/56004/comment/387abdea_e6580c06 PS1, Line 33: static void lpi_get_constraints(void *unused) > > please, no spaces at the start of a line […] Done https://review.coreboot.org/c/coreboot/+/56004/comment/d4bcc91d_b2075cce PS1, Line 40: "\\NULL" > I vaguely recall seeing some ASL that used the host bridge device here. […] Right now, this is (aside from renaming \DUMY to \NULL), a copying of ASL to AML generation. The ASL files uses a non-existent device called \DUMY right now (https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr…). If we want to update this to do something different, how about a different patch? However, do you mean referencing \_SB.PCI0.MCHC instead of a non-existent device? https://review.coreboot.org/c/coreboot/+/56004/comment/50a468da_7092844d PS1, Line 37: acpigen_emit_byte(RETURN_OP); : acpigen_write_package(1); : acpigen_write_package(3); : acpigen_emit_namestring("\\NULL"); /* non-existent */ : acpigen_write_integer(0); /* disabled, no constraints */ : acpigen_write_package(2); : acpigen_write_integer(0); /* revision */ : acpigen_write_package(2); : acpigen_write_integer(0xff); /* apply to all LPIT states */ : acpigen_write_integer(0); /* minimum D-state */ : acpigen_write_package_end(); /* inner 3 */ : acpigen_write_package_end(); /* inner 2 */ : acpigen_write_package_end(); /* inner 1 */ : acpigen_write_package_end(); /* outer */ > How about using scopes to show what closes what? […] I like it, done! File src/soc/intel/common/block/include/intelblocks/acpi.h: https://review.coreboot.org/c/coreboot/+/56004/comment/be18158b_a992f0e6 PS1, Line 94: void generate_acpi_power_engine(const struct device *dev); > No. Please never guard function declarations with preprocessor. […] Cliff, this is how we typically handle things in coreboot, trying to actively minimize `#if` and friends. The compiler and linker are very good at removing dead code, which we can do with a regular `if (CONFIG(...))` statement. -- To view, visit https://review.coreboot.org/c/coreboot/+/56004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie83722e0ed5792e338fc5c39a57eef43b7464e3b Gerrit-Change-Number: 56004 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 07 Jul 2021 16:13:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: acpi: Add function to simplify If (CondRefOf (..)) sequences
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Lance Zhao, Cliff Huang, Furquan Shaikh, Angel Pons, Michael Niewöhner, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56003 ) Change subject: acpi: Add function to simplify If (CondRefOf (..)) sequences ...................................................................... Patch Set 2: (3 comments) File src/acpi/acpigen.c: https://review.coreboot.org/c/coreboot/+/56003/comment/b26866bd_94ae9e37 PS1, Line 2255: acpigen_emit_byte(ZERO_OP); /* ignore COND_REFOF_OP destination */ > What's the output of asl and is that needed? This is required, COND_REFOF always takes 2 arguments, a namestring and then a destination for the result, but when the result is used in an `if` statement, the destination is allowed to be 0. `If (CondRefOf (NAME)) {` See next patch. File src/include/acpi/acpigen.h: https://review.coreboot.org/c/coreboot/+/56003/comment/8145faa2_6e23a916 PS1, Line 590: cond_refof > very pedantic: CondRefOf ---> cond_ref_of Done. https://review.coreboot.org/c/coreboot/+/56003/comment/d369ae69_89105e33 PS1, Line 590: void acpigen_write_if_cond_refof(const char *namestring); > I'd place this function closer to the other if-writing functions. […] Sounds legit, done. -- To view, visit https://review.coreboot.org/c/coreboot/+/56003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6e192a569f550ecb77ad264275d52f219eacaca1 Gerrit-Change-Number: 56003 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 07 Jul 2021 16:12:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Lance Zhao Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/acpi: Add LPM requirements support to PEPD _DSM
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

1 0

Change in coreboot[master]: soc/intel/common/block/acpi: Move pep.asl to acpigen
by Tim Wawrzynczak (Code Review) 07 Jul '21

07 Jul '21

Attention is currently required from: Cliff Huang, Furquan Shaikh, Michael Niewöhner, Angel Pons, Patrick Rudolph, Karthik Ramasubramanian. Hello build bot (Jenkins), Cliff Huang, Furquan Shaikh, Angel Pons, Michael Niewöhner, Angel Pons, Patrick Rudolph, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56004 to look at the new patch set (#2). Change subject: soc/intel/common/block/acpi: Move pep.asl to acpigen ...................................................................... soc/intel/common/block/acpi: Move pep.asl to acpigen There is a use-case for generating the AML bytecode at runtime for the Intel Power Engine device, which comes in a followup patch. BUG=b:185437326 TEST=select this on brya, dump SSDT (verified this matches what was previously in the DSDT): Scope (\_SB.PCI0) { Device (PEPD) { Name (_HID, "INT33A1") Name (_CID, EisaId ("INT33A1") Method (_DSM, 4, Serialized) { ToBuffer (Arg0, Local0) If ((Local0 == ToUUID ("c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"))) { ToInteger (Arg2, Local1) If ((Local1 == Zero)) { Return (Buffer (One) { 0x63 }) } If ((Local1 == One)) { Return (Package (0x01) { Package (0x03) { \NULL, Zero, Package (0x02) { Zero, Package (0x02) { 0xFF, Zero } } } }) } If ((Local1 == 0x02)){} If ((Local1 == 0x03)){} If ((Local1 == 0x04)){} If ((Local1 == 0x05)) { If (CondRefOf (\_SB.PCI0.LPCB.EC0.S0IX)) { \_SB.PCI0.LPCB.EC0.S0IX (One) } If (CondRefOf (\_SB.MS0X)) { \_SB.MS0X (One) } If (CondRefOf (\_SB.PCI0.EGPM)) { \_SB.PCI0.EGPM () } } If ((Local1 == 0x06)) { If (CondRefOf (\_SB.PCI0.LPCB.EC0.S0IX)) { \_SB.PCI0.LPCB.EC0.S0IX (Zero) } If (CondRefOf (\_SB.MS0X)) { \_SB.MS0X (Zero) } If (CondRefOf (\_SB.PCI0.RGPM)) { \_SB.PCI0.RGPM () } } Return (Buffer (One) { 0x00 }) } Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Ie83722e0ed5792e338fc5c39a57eef43b7464e3b --- M src/soc/intel/common/block/acpi/Kconfig M src/soc/intel/common/block/acpi/Makefile.inc A src/soc/intel/common/block/acpi/pep.c M src/soc/intel/common/block/include/intelblocks/acpi.h 4 files changed, 154 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/56004/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie83722e0ed5792e338fc5c39a57eef43b7464e3b Gerrit-Change-Number: 56004 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021