Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57140 )
Change subject: soc/tigerlake: Make IO decode / enable register configurable
......................................................................
Patch Set 19:
(1 comment)
Patchset:
PS19:
> Thanks for pushing Nico, I'll try to take a look
Tbh I wouldn't do that for each platform but keep using common code, since it handles mirroring already, where required. However, we still can clean up some stuff there. Also, that code already handles the `gen_io_dec` dt options with having mirroring where needed. Copying code from common to the SoCs will just duplicate code once again
--
To view, visit https://review.coreboot.org/c/coreboot/+/57140
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib1a7f2707e565a5651ebe438320de9597f5742c3
Gerrit-Change-Number: 57140
Gerrit-PatchSet: 19
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
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: Andy Pont <andy.pont(a)sdcsystems.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 06 Oct 2021 19:18:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Sumeet R Pawnikar, Patrick Rudolph, Karthik Ramasubramanian, Felix Held.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57925 )
Change subject: drivers/intel/dptf: Add support for PCH methods
......................................................................
Patch Set 7: Code-Review+1
(2 comments)
File src/drivers/intel/dptf/dptf.c:
https://review.coreboot.org/c/coreboot/+/57925/comment/dfb34352_a055c2f2
PS7, Line 230: acpigen_write_package(1);
: acpigen_write_zero();
Since this is unrelated to this change, could this be separate commit? Also please update the comment to explain what the returned value is used for
https://review.coreboot.org/c/coreboot/+/57925/comment/6f1b2171_64d6a32d
PS7, Line 292: "\\_SB_.DPTF.TPCH.PKGC"
I think just
`"PKGC"`
should work here because it's in the same scope
--
To view, visit https://review.coreboot.org/c/coreboot/+/57925
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2d40fff98cb4eb9144d55fd5383d9946e4cb0558
Gerrit-Change-Number: 57925
Gerrit-PatchSet: 7
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 06 Oct 2021 19:14:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Yu-Ping Wu.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58128 )
Change subject: libpayload: cbgfx: Clear screen by memcpy
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58128
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5f08fb50faab48d3db6b61ae022af3226914f72b
Gerrit-Change-Number: 58128
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 06 Oct 2021 19:12:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57815 )
Change subject: soc/intel/alderlake: Skip setting D0I3 bit for HECI devices
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
> Thanks Subrata!
@Tim, if you can take a look into this CL again, +2 lost due to manual rebase
--
To view, visit https://review.coreboot.org/c/coreboot/+/57815
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I86d61c49b8f187611efd495712ad901184665f31
Gerrit-Change-Number: 57815
Gerrit-PatchSet: 9
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 06 Oct 2021 19:10:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Subrata Banik, Angel Pons, Patrick Rudolph.
Hello build bot (Jenkins), Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/57815
to look at the new patch set (#10).
Change subject: soc/intel/alderlake: Skip setting D0I3 bit for HECI devices
......................................................................
soc/intel/alderlake: Skip setting D0I3 bit for HECI devices
This patch skips setting D0I3 bit for all HECI devices by FSP.
BUG=b:200644229
TEST=FSP-S UPD dump suggested `DisableD0I3SettingForHeci` UPD is
set to `1`.
Change-Id: I86d61c49b8f187611efd495712ad901184665f31
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/soc/intel/alderlake/fsp_params.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/57815/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/57815
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I86d61c49b8f187611efd495712ad901184665f31
Gerrit-Change-Number: 57815
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58093 )
Change subject: soc/intel/common: Implement __weak smihandler_soc_disable_busmaster
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> hi Tim, […]
Thanks for always looking out for opportunities for improvement Kane, it is appreciated! You're right that it does still access the PMC I/O space, I am just wondering where the requirement for BME (which is supposed to be for DMA) comes from. coreboot's strategy is to use BME as little as possible; there are very few devices that require it to be set while coreboot is running.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58093
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8ed7e6a3f60588bfb90481ea493495080dbbfd58
Gerrit-Change-Number: 58093
Gerrit-PatchSet: 2
Gerrit-Owner: Kane Chen <kane.chen(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 06 Oct 2021 19:06:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kane Chen <kane.chen(a)intel.corp-partner.google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57140 )
Change subject: soc/tigerlake: Make IO decode / enable register configurable
......................................................................
Patch Set 19:
(1 comment)
Patchset:
PS19:
> Yep. The only difference for newer platforms is that there are these DMI […]
Thanks for pushing Nico, I'll try to take a look
--
To view, visit https://review.coreboot.org/c/coreboot/+/57140
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib1a7f2707e565a5651ebe438320de9597f5742c3
Gerrit-Change-Number: 57140
Gerrit-PatchSet: 19
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
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: Andy Pont <andy.pont(a)sdcsystems.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 06 Oct 2021 19:05:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Angel Pons, Patrick Rudolph.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58040
to look at the new patch set (#10).
Change subject: soc/intel/alderlake: Perform `heci_finalize` prior to booting to OS
......................................................................
soc/intel/alderlake: Perform `heci_finalize` prior to booting to OS
`heci_finalize` ensures to put all heci devices to D3 by setting the
D0i3 bit prior to booting to the OS.
BUG=b:200644229
TEST=Verified D0i3 bit is set for all HECI devices prior to booting
to OS.
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
Change-Id: I86d5959646522f9a2169bf13ae04d88b8f685e14
---
M src/soc/intel/alderlake/finalize.c
1 file changed, 19 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/58040/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/58040
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I86d5959646522f9a2169bf13ae04d88b8f685e14
Gerrit-Change-Number: 58040
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58064 )
Change subject: soc/intel/common: Helper function to check CSE device `devfn` status
......................................................................
soc/intel/common: Helper function to check CSE device `devfn` status
This patch creates a helper function in cse common code block to check
the status of any CSE `devfn`. Example: CSE, CSE_2, IDER, KT, CSE_3 and
CSE_4.
Currently cse common code is only able to read the device state of
`PCH_DEVFN_CSE` CSE device alone.
Additionally, print `slot` and 'func' number of CSE devices in case
the device is either disable or hidden.
BUG=b:200644229
TEST=Able to build and boot ADLRVP-P with this patch where the serial
message listed the CSE devices that are disabled in the device tree
as below:
HECI: CSE device 16.01 is disabled
HECI: CSE device 16.04 is disabled
HECI: CSE device 16.05 is disabled
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
Change-Id: I208b07e89e3aa9d682837380809fbff01ea225b0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58064
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
---
M src/soc/intel/common/block/cse/cse.c
M src/soc/intel/common/block/include/intelblocks/cse.h
2 files changed, 18 insertions(+), 6 deletions(-)
Approvals:
build bot (Jenkins): Verified
Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c
index 471bc1b..e41e337 100644
--- a/src/soc/intel/common/block/cse/cse.c
+++ b/src/soc/intel/common/block/cse/cse.c
@@ -581,23 +581,29 @@
return 0;
}
-bool is_cse_enabled(void)
+bool is_cse_devfn_visible(unsigned int devfn)
{
- const struct device *cse_dev = pcidev_path_on_root(PCH_DEVFN_CSE);
+ int slot = PCI_SLOT(devfn);
+ int func = PCI_FUNC(devfn);
- if (!cse_dev || !cse_dev->enabled) {
- printk(BIOS_WARNING, "HECI: No CSE device\n");
+ if (!is_devfn_enabled(devfn)) {
+ printk(BIOS_WARNING, "HECI: CSE device %02x.%01x is disabled\n", slot, func);
return false;
}
- if (pci_read_config16(PCH_DEV_CSE, PCI_VENDOR_ID) == 0xFFFF) {
- printk(BIOS_WARNING, "HECI: CSE device is hidden\n");
+ if (pci_read_config16(PCI_DEV(0, slot, func), PCI_VENDOR_ID) == 0xFFFF) {
+ printk(BIOS_WARNING, "HECI: CSE device %02x.%01x is hidden\n", slot, func);
return false;
}
return true;
}
+bool is_cse_enabled(void)
+{
+ return is_cse_devfn_visible(PCH_DEVFN_CSE);
+}
+
uint32_t me_read_config32(int offset)
{
return pci_read_config32(PCH_DEV_CSE, offset);
diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h
index 076f294..f162d48 100644
--- a/src/soc/intel/common/block/include/intelblocks/cse.h
+++ b/src/soc/intel/common/block/include/intelblocks/cse.h
@@ -153,6 +153,12 @@
uint32_t me_read_config32(int offset);
/*
+ * Check if the CSE device as per function argument `devfn` is enabled in device tree
+ * and also visible on the PCI bus.
+ */
+bool is_cse_devfn_visible(unsigned int devfn);
+
+/*
* Check if the CSE device is enabled in device tree. Also check if the device
* is visible on the PCI bus by reading config space.
* Return true if device present and config space enabled, else return false.
6 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I208b07e89e3aa9d682837380809fbff01ea225b0
Gerrit-Change-Number: 58064
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged