Attention is currently required from: Mike Banon, Idwer Vollering, Paul Menzel, Angel Pons, Arthur Heymans, Kyösti Mälkki, Felix Held.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/53982 )
Change subject: AGESA f15tn boards: Sync IDS option values
......................................................................
Patch Set 6:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/53982/comment/2ef45fc2_b62923b6
PS6, Line 14: Note that the `IDSOPT_TRACING_ENABLED` option currently fails to build.
> Well, a88xm-e was copy-pasta'd from f2a85-m, but the idea still applies.
Ack
https://review.coreboot.org/c/coreboot/+/53982/comment/1f68950c_811274dd
PS6, Line 14: Note that the `IDSOPT_TRACING_ENABLED` option currently fails to build.
> Well, a88xm-e was copy-pasta'd from f2a85-m, but the idea still applies.
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/53982
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iedd4d1f255650012f3efd9a27718e18c1c904dc1
Gerrit-Change-Number: 53982
Gerrit-PatchSet: 6
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Sun, 22 Aug 2021 22:14:22 +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: Mike Banon, Idwer Vollering, Paul Menzel, Angel Pons, Arthur Heymans, Kyösti Mälkki, Felix Held.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/53982 )
Change subject: AGESA f15tn boards: Sync IDS option values
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/53982
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iedd4d1f255650012f3efd9a27718e18c1c904dc1
Gerrit-Change-Number: 53982
Gerrit-PatchSet: 6
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Sun, 22 Aug 2021 22:13:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Kyösti Mälkki, Patrick Rudolph.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54976 )
Change subject: Revert "soc/intel/broadwell/pch: Drop device NVS remainders"
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS2:
> Should this be submitted?
Thanks for the reminder, Paul. I had assumed that this got submitted back then. Looks like I had missed submitting it. Manually rebased this.
--
To view, visit https://review.coreboot.org/c/coreboot/+/54976
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic31d7ae62c5df72708b724160e96e10b46002eb1
Gerrit-Change-Number: 54976
Gerrit-PatchSet: 4
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Sun, 22 Aug 2021 21:59:33 +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: Furquan Shaikh, Kyösti Mälkki, Patrick Rudolph.
Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Kyösti Mälkki, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/54976
to look at the new patch set (#4).
Change subject: Revert "soc/intel/broadwell/pch: Drop device NVS remainders"
......................................................................
Revert "soc/intel/broadwell/pch: Drop device NVS remainders"
This reverts commit 34bd6ba97917b0bc54bb1f1e106a56b5c03e19ac.
Reason for revert: Device NVS is expected by mainboard samus
in payload depthcharge:
https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/932c6ba…
Not reverted:
* ACPI_HAS_DEVICE_NVS does not exist anymore in ToT and hence it's
selection in broadwell is not required.
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
Change-Id: Ic31d7ae62c5df72708b724160e96e10b46002eb1
---
A src/soc/intel/broadwell/include/soc/device_nvs.h
M src/soc/intel/broadwell/include/soc/pch.h
M src/soc/intel/broadwell/pch/acpi.c
M src/soc/intel/broadwell/pch/adsp.c
M src/soc/intel/broadwell/pch/serialio.c
5 files changed, 89 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/54976/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/54976
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic31d7ae62c5df72708b724160e96e10b46002eb1
Gerrit-Change-Number: 54976
Gerrit-PatchSet: 4
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Martin Roth, Idwer Vollering, Paul Menzel, Arthur Heymans.
Nico Huber 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: Code-Review+2
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56803/comment/26743925_41801d09
PS3, Line 12:
Please break long lines; line-length limit for commit messages is 72 chars.
Patchset:
PS3:
Commit message should get an update, looks fine otherwise.
--
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: 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: Sun, 22 Aug 2021 21:38:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56218 )
Change subject: acpi: Fill fadt->century based on Kconfig
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56218/comment/c9f98137_a3ced72b
PS1, Line 7: acpi: Fill fadt->century based on Kconfig
> AIUI, the hardcoded values that this patch removes can cause problems depending on the state of the […]
Only in combination with the usual option tables. If the FADT entry
is not 0, Linux for instance will write the reported CMOS byte when
the RTC time gets updated.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56218
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I916f19e022633b316fbc0c6bf38bbd58228412be
Gerrit-Change-Number: 56218
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sun, 22 Aug 2021 21:34:00 +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: Furquan Shaikh, Kyösti Mälkki, Patrick Rudolph.
Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Kyösti Mälkki, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/54976
to look at the new patch set (#3).
Change subject: Revert "soc/intel/broadwell/pch: Drop device NVS remainders"
......................................................................
Revert "soc/intel/broadwell/pch: Drop device NVS remainders"
This reverts commit 34bd6ba97917b0bc54bb1f1e106a56b5c03e19ac.
Reason for revert: Device NVS is expected by mainboard samus
in payload depthcharge:
https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/932c6ba…
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
Change-Id: Ic31d7ae62c5df72708b724160e96e10b46002eb1
---
M src/soc/intel/broadwell/Kconfig
A src/soc/intel/broadwell/include/soc/device_nvs.h
M src/soc/intel/broadwell/include/soc/pch.h
M src/soc/intel/broadwell/pch/acpi.c
M src/soc/intel/broadwell/pch/adsp.c
M src/soc/intel/broadwell/pch/serialio.c
6 files changed, 90 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/54976/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/54976
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic31d7ae62c5df72708b724160e96e10b46002eb1
Gerrit-Change-Number: 54976
Gerrit-PatchSet: 3
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Michael Niewöhner.
Furquan Shaikh 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)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57034/comment/bfd438e5_12a25a7f
PS2, Line 12:
> I found "Switchable Graphics Design Guidelines" in the KBL PDG (Intel doc #564042) for example
Thanks for pointer Michael! I think there would be two set of requirements:
1. dGPU part requirements:
- How the PM control looks like for the part? Is it only GPIO based? Is it just PCIe-link based? Or are there any additional steps required to perform configuration to put the device into appropriate state.
2. SoC requirements:
- This would be any root port requirements and/or PEG requirements, etc.
--
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: Furquan Shaikh <furquan(a)google.com>
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: Sun, 22 Aug 2021 21:26:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Sugnan Prabhu S, Tim Wawrzynczak, Paul Menzel.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56750 )
Change subject: vc/google/chromeos: Add support for new SAR tables revisions
......................................................................
Patch Set 17:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56750/comment/0df600d7_b30ec1b8
PS17, Line 7: vc/google/chromeos
This is touching more than just vc/google/chromeos.
File src/include/sar.h:
https://review.coreboot.org/c/coreboot/+/56750/comment/b65d1209_abf1b06f
PS17, Line 21: struct wifi_sar_delta_table {
: uint8_t version;
: union {
: struct {
: uint8_t power_max_2400mhz;
: uint8_t power_chain_a_2400mhz;
: uint8_t power_chain_b_2400mhz;
: uint8_t power_max_5200mhz;
: uint8_t power_chain_a_5200mhz;
: uint8_t power_chain_b_5200mhz;
: } __packed group_rev0[SAR_NUM_WGDS_GROUPS];
: struct {
: uint8_t power_max_2400mhz;
: uint8_t power_chain_a_2400mhz;
: uint8_t power_chain_b_2400mhz;
: uint8_t power_max_5200mhz;
: uint8_t power_chain_a_5200mhz;
: uint8_t power_chain_b_5200mhz;
: uint8_t power_max_6000mhz;
: uint8_t power_chain_a_6000mhz;
: uint8_t power_chain_b_6000mhz;
: } __packed group_rev1[SAR_NUM_WGDS_GROUPS];
: } __packed;
: } __packed;
I think we should hide the differences of different revisions on the SAR file generation side so that there are minimal changes in coreboot (especially the ACPI generation code) to support new revisions. I am thinking something like this:
```
struct sar_profile {
uint8_t revision;
uint8_t dsar_set_count;
uint8_t chains_count;
uint8_t subbands_count;
uint8_t sar_table[0]; /* This will hold (dsar_set_count + 1) * chains_count * subbands_count bytes */
} __packed;
struct geo_profile {
uint8_t revision;
uint8_t chains_count;
uint8_t bands_count;
uint8_t wgds_table[0]; /* This will hold chains_count * bands_count bytes */
} __packed;
```
So, the SAR file generation will emit out:
```
sar_profile {
.revision -> 0
.dsar_set_count -> 2
.chains_count -> 2
.subbands_count -> 5
.sar_table -> { set0 -> { .... }, set1 -> { .... }, set2 -> { .... } }
}
```
`acpi.c` can then use this information to generate the required nodes:
```
static uint8_t *sar_fetch_set(const struct sar_profile *sar, size_t set_num)
{
uint8_t *sar_table = &sar->sar_table[0];
sar_table += sar->chains_count * sar->subbands_count * set;
return sar_table;
}
static void sar_emit_wrds(const struct sar_profile *sar)
{
size_t package_size, i;
uint8_t *set;
acpigen_write_name("WRDS");
acpigen_write_package(2);
acpigen_write_dword(sar->revision);
table_size = sar->chains_count * sar->subbands_count;
/* Domain type + SAR enable + chains_count * subbands_count */
package_size = 1 + 1 + table_size;
acpigen_write_package(package_size);
acpigen_write_dword(WRDS_DOMAIN_TYPE_WIFI);
acpigen_write_dword(1);
set = sar_fetch_set(sar, 0);
for (i = 0; i < table_size; i++) {
acpigen_write_byte(set[i]);
}
}
static void sar_emit_ewrd(const struct sar_profile *sar)
{
size_t package_size, i;
uint8_t *set;
acpigen_write_name("EWRD");
acpigen_write_package(2);
acpigen_write_dword(sar->revision);
table_size = sar->chains_count * sar->subbands_count;
/* Domain type + DSAR enable + DSAR sets + chains_count * subbands_count * set_count */
package_size = 1 + 1 + 1 + table_size * sar->dsar_set_count;
acpigen_write_package(package_size);
acpigen_write_dword(WRDS_DOMAIN_TYPE_WIFI);
acpigen_write_dword(sar->dsar_set_count ? 1 : 0);
acpigen_write_dword(sar->dsar_set_count);
for (set_num = 1; set_num <= sar->dsar_set_count; i++) {
set = sar_fetch_set(sar, set_num);
for (i = 0; i < table_size; i++) {
acpigen_write_byte(set[i]);
}
}
}
```
Similarly other tables can be handled. What do you think?
https://review.coreboot.org/c/coreboot/+/56750/comment/f77551cc_9f845ad1
PS17, Line 51: sar_limit
I think you meant sar_limit[0] to indicate that this is a variable-length array. At least, that is how you are using it in acpi.c
File src/vendorcode/google/chromeos/sar.c:
https://review.coreboot.org/c/coreboot/+/56750/comment/ee7c8bf0_b908e32e
PS17, Line 138: if (CONFIG(USE_SAR)) {
: sar_limits->sar_table->version = 0;
: sar_limits->sar_table->sar_enable = CONFIG(SAR_ENABLE);
: sar_limits->sar_table->dsar_enable = CONFIG(DSAR_ENABLE);
: sar_limits->sar_table->dsar_set_num = CONFIG_DSAR_SET_NUM;
: }
I like the fact that you are trying to fix the problems with the original representation of SAR tables in CBFS. It assumed that the revision will always be 0. And now we need to support the newer revisions as well. So, encoding the new revisions while massaging the older format to convert it into appropriate format here seems like the right thing to do.
One comment I have is that we should not really add another Kconfig `USE_SAR_V2` and instead rely on the CBFS data to make the decision. This is because the way you are organizing CBFS format eliminates the need for Kconfig-based version tracking (in fact it will be really confusing for partners to determine the correct version and keep those in sync in coreboot and the CBFS table). The only reason why you need `USE_SAR_V2` is to identify if that extra processing is required on the CBFS SAR table.
This is not the cleanest of approaches, but my recommendation is:
1. Read SAR table from CBFS.
2. Compare length to expected length in old format. If it matches, then assume old format.
3. Else assume that the format encodes revision information correctly.
If you really want to keep it foolproof and not rely on length, you can add a string "$SAR" at the start of the CBFS file to identify the new format. But, I think that is not really necessary.
With this, you can eliminate the need for `USE_SAR_V2` and all platforms are expected to only select `USE_SAR`. One thing that you will have to co-ordinate is the SAR table generation code to ensure that all *new* platforms move to this new format even if they are not using any of the new revisions i.e. even if a platform plans to use revision 0, it should still encode in the new format. This ensures consistency going forward. However, *older* platforms that have already shipped should continue using the old format. Thus, you don't have to go back and fix the firmware branches for each.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56750
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I08c3f321938eba04e8bcff4d87cb215422715bb2
Gerrit-Change-Number: 56750
Gerrit-PatchSet: 17
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.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: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sun, 22 Aug 2021 21:22:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment