Attention is currently required from: Felix Singer.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60344 )
Change subject: mb/google/octopus: Move selects from Kconfig.name to Kconfig
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60344
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5949fc474c4923beb1f41a40876f03e5c70fc5b1
Gerrit-Change-Number: 60344
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Tue, 28 Dec 2021 10:24:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Tim Wawrzynczak.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60343 )
Change subject: mb/google/octopus/Kconfig: Reorder selects alphabetically
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60343
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I545a1111e24f45f732f1c325c808737dfb6bcae6
Gerrit-Change-Number: 60343
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Tue, 28 Dec 2021 10:22:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Subrata Banik, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60404 )
Change subject: soc/intel/common/cse: Helper function to set D0I3 for all HECI devices
......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/common/block/cse/cse_eop.c:
https://review.coreboot.org/c/coreboot/+/60404/comment/7f9cdc4a_f656c4d8
PS2, Line 17: /* SoC callback to set D0I3 for all HECI devices */
: __weak void soc_heci_set_d0i3(void) { /* no-op */ }
> > Is the weak empty function meaningful on most SoC's or should it be implemented by all and this is just a placeholder?
>
> Ideally should be implemented by other SoC and this is just a placeholder. For example: https://review.coreboot.org/c/coreboot/+/60406/2/src/soc/intel/alderlake/fi…
>
> I wished to make this mandatory for SoC that selects SOC_INTEL_COMMON_BLOCK_HECI_NOTIFY (skips FSP Notify APIs hence, coreboot need to put HECI devices into D0i3) but in that case we might need to guard function declaration as well. WDYT about this ?
I think it's best to avoid weak function as placeholders in this case. If you forgot about it (e.g. a future SoC) you'd have to find why it's not working at runtime. If there is a declaration without a definition, the compiler will pinpoint you towards the problem, before you even ran the image.
Why should the declaration be guarded? Only the function call needs to be.
if (CONFIG(SOC_INTEL_COMMON_BLOCK_HECI_NOTIFY))
soc_heci_set_d0i3()
should work fine. Making a definition mandatory would be my preference.
--
To view, visit https://review.coreboot.org/c/coreboot/+/60404
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If9eb573be096928ccd6cd05b672ec022f5840059
Gerrit-Change-Number: 60404
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.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: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 28 Dec 2021 09:41:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Tim Wawrzynczak, YH Lin.
Kevin Chang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60330 )
Change subject: mb/google/brya/var/taeko: swap TPM i2c with TS i2c for the next build
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/60330/comment/ea35476d_fecbddeb
PS2, Line 9: Taeko will
> 1. Do the schematics have revisions? […]
1. Yes, this change is follow Brya.
2. Update in latest patchset.
--
To view, visit https://review.coreboot.org/c/coreboot/+/60330
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib7273ba107c58e4cd90db00e301a399d7a7df76d
Gerrit-Change-Number: 60330
Gerrit-PatchSet: 3
Gerrit-Owner: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rasheed Hsueh <rasheed.hsueh(a)lcfc.corp-partner.google.com>
Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: YH Lin <yueherngl(a)chromium.org>
Gerrit-Comment-Date: Tue, 28 Dec 2021 09:38:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60392 )
Change subject: soc/mediatek: Prevent passing NULL MIPI commands
......................................................................
soc/mediatek: Prevent passing NULL MIPI commands
Previously (before CB:56965 [1]) mtk_dsi_send_init_command() would
ignore NULL initialization commands passed to it. However, in the
current code mipi_panel_parse_init_commands() doesn't check that (see
CB:57150 [2]), so we should check it on the caller side from
mtk_dsi_init().
[1] b2a1480191 device: Move MIPI panel library from
mainboard/google/kukui into common
[2] 4757a7ea33 mipi: Make panel init callback work directly on DSI
transaction types
BUG=b:202871018, b:209930699
TEST=emerge-corsola coreboot
BRANCH=none
Change-Id: I8196e3b135da273325e2e121523abb7fb230a49c
Signed-off-by: Yu-Ping Wu <yupingso(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60392
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Hung-Te Lin <hungte(a)chromium.org>
Reviewed-by: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
---
M src/soc/mediatek/common/dsi.c
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Hung-Te Lin: Looks good to me, approved
Rex-BC Chen: Looks good to me, but someone else must approve
diff --git a/src/soc/mediatek/common/dsi.c b/src/soc/mediatek/common/dsi.c
index a4556f5..f16d8f2 100644
--- a/src/soc/mediatek/common/dsi.c
+++ b/src/soc/mediatek/common/dsi.c
@@ -377,7 +377,8 @@
mtk_dsi_clk_hs_mode_disable();
mtk_dsi_config_vdo_timing(mode_flags, format, lanes, edid, &phy_timing);
mtk_dsi_clk_hs_mode_enable();
- mipi_panel_parse_init_commands(init_commands, mtk_dsi_cmdq);
+ if (init_commands)
+ mipi_panel_parse_init_commands(init_commands, mtk_dsi_cmdq);
mtk_dsi_set_mode(mode_flags);
mtk_dsi_start();
--
To view, visit https://review.coreboot.org/c/coreboot/+/60392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8196e3b135da273325e2e121523abb7fb230a49c
Gerrit-Change-Number: 60392
Gerrit-PatchSet: 3
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60340 )
Change subject: soc/mediatek/mt8186: Enable VRF12 software control for MT6366
......................................................................
soc/mediatek/mt8186: Enable VRF12 software control for MT6366
PS8640 is a low power MIPI-to-eDP video format converter.
VRF12 does not provide power to PS8640 on krabby.
In original patch, VRF12 is not used, and is set to hardware control
for low power. We change the setting to remove hardware control.
Therefore, if we want to control VRF12 by software, we can control
it directly.
BUG=b:210806060
TEST=build pass
Signed-off-by: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Change-Id: I87d6a94b6fb343590d563ac1554ff87b11c01549
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60340
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso(a)google.com>
---
M src/soc/mediatek/mt8186/mt6366.c
1 file changed, 0 insertions(+), 8 deletions(-)
Approvals:
build bot (Jenkins): Verified
Yu-Ping Wu: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8186/mt6366.c b/src/soc/mediatek/mt8186/mt6366.c
index 450b783..5cec47a 100644
--- a/src/soc/mediatek/mt8186/mt6366.c
+++ b/src/soc/mediatek/mt8186/mt6366.c
@@ -211,10 +211,6 @@
{0x1C1E, 0x1, 0x1, 2},
/* [2:2]: RG_LDO_VRF18_HW1_OP_CFG */
{0x1C24, 0x0, 0x1, 2},
- /* [2:2]: RG_LDO_VRF12_HW1_OP_EN */
- {0x1C32, 0x1, 0x1, 2},
- /* [2:2]: RG_LDO_VRF12_HW1_OP_CFG */
- {0x1C38, 0x0, 0x1, 2},
/* [0:0]: RG_LDO_VEFUSE_SW_OP_EN */
{0x1C46, 0x1, 0x1, 0},
/* [0:0]: RG_LDO_VCN33_SW_OP_EN */
@@ -325,10 +321,6 @@
{0x1C1E, 0x1, 0x1, 2},
/* [2:2]: RG_LDO_VRF18_HW1_OP_CFG */
{0x1C24, 0x0, 0x1, 2},
- /* [2:2]: RG_LDO_VRF12_HW1_OP_EN */
- {0x1C32, 0x1, 0x1, 2},
- /* [2:2]: RG_LDO_VRF12_HW1_OP_CFG */
- {0x1C38, 0x0, 0x1, 2},
/* [0:0]: RG_LDO_VEFUSE_SW_OP_EN */
{0x1C46, 0x1, 0x1, 0},
/* [0:0]: RG_LDO_VCN33_SW_OP_EN */
3 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/+/60340
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I87d6a94b6fb343590d563ac1554ff87b11c01549
Gerrit-Change-Number: 60340
Gerrit-PatchSet: 5
Gerrit-Owner: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60316 )
Change subject: soc/mediatek/mt8186: Adjust usage of SRAM L2C
......................................................................
soc/mediatek/mt8186: Adjust usage of SRAM L2C
We use parts of SRAM_L2C as the memory of PRERAM_CBMEM_CONSOLE before
DRAM calibration. When we check cbmem, we found the content of this
memory is unreadable.
The L3 (can be used as SRAM_L2C) is 1MB in total. However the BootROM
has configured only half of L2/L3 cache as SRAM. Therefore, decrease
the size of each SRAM region to fit into the first half of the cache.
BUG=b:207725851
TEST=Bootblock log looked good in `cbmem -c`
Signed-off-by: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Change-Id: I6041767a1ac0a48ecdda29a0c35d90acf6ad0ef2
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60316
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso(a)google.com>
---
M src/soc/mediatek/mt8186/include/soc/memlayout.ld
1 file changed, 11 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Yu-Ping Wu: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8186/include/soc/memlayout.ld b/src/soc/mediatek/mt8186/include/soc/memlayout.ld
index a1700e6..36078d3 100644
--- a/src/soc/mediatek/mt8186/include/soc/memlayout.ld
+++ b/src/soc/mediatek/mt8186/include/soc/memlayout.ld
@@ -34,6 +34,12 @@
/* MT8186 has 64KB SRAM. */
SRAM_END(0x00110000)
+ /*
+ * The L3 (can be used as SRAM_L2C) is 1MB in total. However the BootROM
+ * has configured only half of L2/L3 cache as SRAM and the rest for cache
+ * so we can't use them unless if we disable L2C and reconfigure (be aware
+ * we can't configure whole L3 to SRAM without any cache).
+ */
SRAM_L2C_START(0x00200000)
/* 4K reserved for BOOTROM until BOOTBLOCK is started */
BOOTBLOCK(0x00201000, 60K)
@@ -41,11 +47,11 @@
* The needed size can be obtained by:
* aarch64-cros-linux-gnu-objdump -x dram.elf | grep memsz
*/
- DRAM_INIT_CODE(0x00210000, 240K)
- OVERLAP_DECOMPRESSOR_VERSTAGE_ROMSTAGE(0x0024c000, 272K)
- PRERAM_CBFS_CACHE(0x00290000, 48K)
- PRERAM_CBMEM_CONSOLE(0x0029C000, 400K)
- SRAM_L2C_END(0x00300000)
+ DRAM_INIT_CODE(0x00210000, 196K)
+ OVERLAP_DECOMPRESSOR_VERSTAGE_ROMSTAGE(0x00241000, 140K)
+ PRERAM_CBFS_CACHE(0x00264000, 48K)
+ PRERAM_CBMEM_CONSOLE(0x00270000, 64K)
+ SRAM_L2C_END(0x00280000)
DRAM_START(0x40000000)
DRAM_DMA(0x40000000, 1M)
--
To view, visit https://review.coreboot.org/c/coreboot/+/60316
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6041767a1ac0a48ecdda29a0c35d90acf6ad0ef2
Gerrit-Change-Number: 60316
Gerrit-PatchSet: 6
Gerrit-Owner: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged