Attention is currently required from: Martin Roth, Felix Held.
Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Marshall Dawson, Chris Wang, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/50240
to look at the new patch set (#9).
Change subject: soc/amd/picasso: add UPD for RV2 USB3 phy setting adjust
......................................................................
soc/amd/picasso: add UPD for RV2 USB3 phy setting adjust
add UPD for RV2 USB3 phy setting adjust
Usb 3.1 PHY Parameters:
1. RX_EQ_DELTA_IQ_OVRD_VAL
-Override value for rx_eq_delta_iq. Range 0-0xF
2. RX_EQ_DELTA_IQ_OVRD_EN
-Enable override value for rx_eq_delta_iq. Range 0-0x1
3. Override value for rx_vref_ctrl. Range 0 - 0x1F
4. Enable override value for rx_vref_ctrl. Range 0 - 0x1
5. Override value for tx_vboost_lvl: 0 - 0x7.
6. Enable override value for tx_vboost_lvl. Range: 0 - 0x1
7. Override value for rx_vref_ctrl. Range 0 - 0x1F
8. Enable override value for rx_vref_ctrl. Range 0 - 0x1
9. Override value for tx_vboost_lvl: 0 - 0x7.
10. Enable override value for tx_vboost_lvl. Range: 0 - 0x1
BUG=b:175192931
TEST=Build/verify the valule will been apply on dirinboz
Change-Id: I1d5f69e840952cc5171af1ce8597628d1bede5cb
Signed-off-by: Chris Wang <chris.wang(a)amd.corp-partner.google.com>
---
M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb
M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
M src/soc/amd/picasso/chip.h
M src/soc/amd/picasso/fsp_params.c
M src/vendorcode/amd/fsp/picasso/FspsUpd.h
5 files changed, 143 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/50240/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/50240
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d5f69e840952cc5171af1ce8597628d1bede5cb
Gerrit-Change-Number: 50240
Gerrit-PatchSet: 9
Gerrit-Owner: chris wang <Chris.Wang(a)amd.com>
Gerrit-Reviewer: Chris Wang <chris.wang(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Derek Huang, Patrick Rudolph.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50105 )
Change subject: mb/google/volteer: Change cTDP level to level 2 (15W)
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
Did you get chance to check any performance impact due to this change ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/50105
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4420a6a2e463b0a6bd7eb4b81f6a4fb975895ea3
Gerrit-Change-Number: 50105
Gerrit-PatchSet: 3
Gerrit-Owner: Derek Huang <derek.huang(a)intel.corp-partner.google.com>
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: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Derek Huang <derek.huang(a)intel.corp-partner.google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 04 Feb 2021 17:14:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Andrey Petrov, Patrick Rudolph.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 )
Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
......................................................................
Patch Set 1:
(1 comment)
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/50241/comment/1388675f_78bfdc52
PS1, Line 242: !=
> What about <= instead of !=? Assuming UPD only grows at the end, that would allow flexibility to up […]
you mean >, since <= would be break the good case as well? if hdr->cfg_region_size is bigger than sizeof(FSPM_UPD) it's a non-recoverable error and we should call die(), but doing a printk(BIOS_ERR, ...) in the hdr->cfg_region_size < sizeof(FSPM_UPD) case, but continuing would be an option
--
To view, visit https://review.coreboot.org/c/coreboot/+/50241
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d
Gerrit-Change-Number: 50241
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 04 Feb 2021 16:50:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Tim Wawrzynczak, Meera Ravindranath, Andrey Petrov, Patrick Rudolph.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49474 )
Change subject: drivers/intel/fsp2_0: Add support for MP services2 PPI
......................................................................
Patch Set 9:
(3 comments)
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/49474/comment/b55c2bda_a6c5447d
PS6, Line 267:
: if FSP_PEIM_TO_PEIM_INTERFACE
: source "src/drivers/intel/fsp2_0/ppi/Kconfig"
: endif
:
> It's redundant, src/Kconfig already has `source "src/drivers/*/*/*/Kconfig`
yeah.
File src/drivers/intel/fsp2_0/include/fsp/ppi/mp_service_ppi.h:
https://review.coreboot.org/c/coreboot/+/49474/comment/4aecfe34_6f24e8c7
PS6, Line 49: /*
: * switches the requested AP to be the BSP
: */
: efi_return_status_t mp_switch_bsp(void);
:
: /*
: * enable or disable an AP. This service may only be called from the BSP.
: */
: efi_return_status_t mp_enable_disable_ap(void);
:
> We could just have a `efi_return_status_t fsp_ppi_unsupported(void) { return FSP_UNSUPPORTED; }` as […]
wanted to match to :
EDKII_PEI_MP_SERVICES2_PPI mMpServices2Ppi = {
EdkiiPeiGetNumberOfProcessors,
EdkiiPeiGetProcessorInfo,
EdkiiPeiStartupAllAPs,
EdkiiPeiStartupThisAP,
EdkiiPeiSwitchBSP,
EdkiiPeiEnableDisableAP,
EdkiiPeiWhoAmI,
EdkiiPeiStartupAllCPUs
};
As I understand are you suggesting below?
EDKII_PEI_MP_SERVICES2_PPI mMpServices2Ppi = {
EdkiiPeiGetNumberOfProcessors,
EdkiiPeiGetProcessorInfo,
EdkiiPeiStartupAllAPs,
EdkiiPeiStartupThisAP,
fsp_ppi_unsupported,
fsp_ppi_unsupported,
EdkiiPeiWhoAmI,
EdkiiPeiStartupAllCPUs
};
File src/drivers/intel/fsp2_0/ppi/mp_service1.c:
https://review.coreboot.org/c/coreboot/+/49474/comment/4d71611c_15b91d8a
PS6, Line 12:
> nit: extra lines
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/49474
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id74baf17fb90147d229c78be90268fdc3ec1badc
Gerrit-Change-Number: 49474
Gerrit-PatchSet: 9
Gerrit-Owner: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(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)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 04 Feb 2021 16:27:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Meera Ravindranath, Aamir Bohra, Tim Wawrzynczak, Andrey Petrov, Patrick Rudolph.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Subrata Banik, Meera Ravindranath, Tim Wawrzynczak, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49474
to look at the new patch set (#9).
Change subject: drivers/intel/fsp2_0: Add support for MP services2 PPI
......................................................................
drivers/intel/fsp2_0: Add support for MP services2 PPI
Add support for MP services2 PPIs, which is slight modification
over MP services 1 PPIs. A new API StartupAllCPUs have been added
to allow running a task on BSP and all APs. Also the EFI_PEI_SERVICES
parameter has been removed from all MP PPI APIs.
This implementation also selects the respective MP services PPI version
supported for SoCs
BUG=b:169196864
Change-Id: Id74baf17fb90147d229c78be90268fdc3ec1badc
Signed-off-by: Aamir Bohra <aamir.bohra(a)intel.com>
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/include/fsp/ppi/mp_service_ppi.h
M src/drivers/intel/fsp2_0/ppi/Kconfig
M src/drivers/intel/fsp2_0/ppi/Makefile.inc
A src/drivers/intel/fsp2_0/ppi/mp_service1.c
A src/drivers/intel/fsp2_0/ppi/mp_service2.c
M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c
M src/include/efi/efi_datatype.h
M src/soc/intel/alderlake/Kconfig
M src/soc/intel/elkhartlake/Kconfig
M src/soc/intel/icelake/Kconfig
M src/soc/intel/jasperlake/Kconfig
M src/soc/intel/tigerlake/Kconfig
A src/vendorcode/intel/edk2/edk2-stable202005/MdePkg/Include/Ppi/MpServices2.h
14 files changed, 531 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/49474/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/49474
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id74baf17fb90147d229c78be90268fdc3ec1badc
Gerrit-Change-Number: 49474
Gerrit-PatchSet: 9
Gerrit-Owner: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(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)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Attention: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50243 )
Change subject: amd/common/block/acpi/pm_state: fix comparison in get_index_bit
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50243/comment/2c5958e0_6dd27f3d
PS3, Line 10: continued, since 32 is a valid value here.
> And the case of limit > 32 should really be a BUG() or assert() since uint32t for the parameter 'val […]
good point. since you wrote that you're going to look into this, i won't push a patch for that
--
To view, visit https://review.coreboot.org/c/coreboot/+/50243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6ca341841bad62abcb4ea26a350c539813a29de7
Gerrit-Change-Number: 50243
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-Comment-Date: Thu, 04 Feb 2021 16:00:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment