coreboot-gerrit December 2024

coreboot-gerrit@coreboot.org

1 participants
3018 discussions

[S] Change in coreboot[main]: soc/intel/xeon_sp/spr/acpi: Fix regression
by Angel Pons (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Patrick Rudolph, Shuo Liu, Tim Chu. Angel Pons has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85562?usp=email ) Change subject: soc/intel/xeon_sp/spr/acpi: Fix regression ...................................................................... Patch Set 4: Code-Review+2 (1 comment) File src/soc/intel/xeon_sp/spr/acpi/uncore.asl: https://review.coreboot.org/c/coreboot/+/85562/comment/461fac72_74f667f2?us… : PS3, Line 15: #define SOCKET_NAME 0 > Arithmetic expression are not evaluated by the preprocessor, nor iasl. Not possible because this uses token pasting. If possible I would try using acpigen for this, but I'm not sure if it's possible -- To view, visit https://review.coreboot.org/c/coreboot/+/85562?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ice168bdebc46dc0cfb9c63c78c46a5d9ff2b7658 Gerrit-Change-Number: 85562 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 23 Dec 2024 10:28:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support
by Yidi Lin (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Andy Hsu, Hung-Te Lin, Jarried Lin. Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85654?usp=email ) Change subject: soc/mediatek/mt8196: Add GPUEB support ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/soc/mediatek/mt8196/include/soc/gpueb_priv.h: https://review.coreboot.org/c/coreboot/+/85654/comment/33b62593_1a216748?us… : PS2, Line 10: #define SPM2GPUPM_CON (SPM_BASE + 0x0410) /* 0x1C004410 */ : #define SPM_REQ_STA_9 (SPM_BASE + 0x0884) /* 0x1C004884 */ : #define SPM_MFG0_PWR_CON (SPM_BASE + 0x0EA8) > Done Sorry, I leaved the comment accidentally. I thought you should use the struct overlay defined in `<soc/spm.h>`. It turns out that the `DUMP_REG` can not be used for those defines. Are you sure GPUEB no longer needs those registers ? -- To view, visit https://review.coreboot.org/c/coreboot/+/85654?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0f10dfc753f73df97ea08a4c23e97de416832be2 Gerrit-Change-Number: 85654 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Mon, 23 Dec 2024 10:26:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[M] Change in coreboot[main]: mb/google/rauru: Add support for getting storage id
by Yu-Ping Wu (Code Review) 23 Dec '24

23 Dec '24

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support
by Yu-Ping Wu (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Andy Hsu, Hung-Te Lin, Jarried Lin, Yidi Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85654?usp=email ) Change subject: soc/mediatek/mt8196: Add GPUEB support ...................................................................... Patch Set 3: (3 comments) File src/soc/mediatek/mt8196/gpueb.c: https://review.coreboot.org/c/coreboot/+/85654/comment/941d76ac_fa51e1ad?us… : PS3, Line 20: BIOS_CRIT BIOS_DEBUG File src/soc/mediatek/mt8196/include/soc/addressmap.h: https://review.coreboot.org/c/coreboot/+/85654/comment/d2bbaa8e_265c6870?us… : PS3, Line 197: GPUEB_BASE MFG_GPUEB_BASE File src/soc/mediatek/mt8196/include/soc/gpueb_priv.h: https://review.coreboot.org/c/coreboot/+/85654/comment/24b75c3c_133a597a?us… : PS2, Line 57: #define MFG_RPC_GHPM_CFG13_CON (GPU_EB_RPC_BASE + 0x0834) /* 0x4B800834 */ : #define MFG_GHPM_RO2_CON (MFG_RPC_BASE + 0x09AC) /* 0x4B8009AC */ > The bases are the same. `MFG_RPC_BASE` should be removed here. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85654?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0f10dfc753f73df97ea08a4c23e97de416832be2 Gerrit-Change-Number: 85654 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Mon, 23 Dec 2024 10:20:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support
by Yu-Ping Wu (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85654?usp=email ) Change subject: soc/mediatek/mt8196: Add GPUEB support ...................................................................... Patch Set 2: (34 comments) File src/soc/mediatek/mt8196/gpueb.c: https://review.coreboot.org/c/coreboot/+/85654/comment/12abaa43_79f9e193?us… : PS2, Line 14: #include <halt.h> Replace `BIOS_CRIT logs + halt()` with `die(...)` with an error message. https://review.coreboot.org/c/coreboot/+/85654/comment/8ec3c471_3d293107?us… : PS2, Line 193: gpueb_parsing_fw `parse_fw` https://review.coreboot.org/c/coreboot/+/85654/comment/f9df3080_57e6688a?us… : PS2, Line 193: imgsize img_size https://review.coreboot.org/c/coreboot/+/85654/comment/795739a4_5f7886f9?us… : PS2, Line 193: void *buf_addr `const void *buf` https://review.coreboot.org/c/coreboot/+/85654/comment/34c5cead_13e20235?us… : PS2, Line 197: ptimg_hdr_t Rename to `gpueb_pt_hdr`. No `_t`. https://review.coreboot.org/c/coreboot/+/85654/comment/d0f59b7e_fe7f170c?us… : PS2, Line 201: while (hdr->magic == PT_MAGIC) { We should also pass `size_t buf_size` to this function, and make sure we never parse beyond that (i.e. `(uintptr_t)hdr + sizeof(*hdr) <= (uintptr_t)buf + buf_size`). https://review.coreboot.org/c/coreboot/+/85654/comment/646b935a_3b5e5ca5?us… : PS2, Line 201: PT Can we rename this to `GPUEB_PT_MAGIC`? https://review.coreboot.org/c/coreboot/+/85654/comment/43c16b2f_875f2ebf?us… : PS2, Line 202: ( remove https://review.coreboot.org/c/coreboot/+/85654/comment/943cb821_74314923?us… : PS2, Line 204: if (hdr->id == 0 || hdr->id >= PT_FW_NUM) { Is this case expected? If so, can't we remove these unused partitions from the gpueb blob? If not, we should print a warning log here. https://review.coreboot.org/c/coreboot/+/85654/comment/30216dce_9024ab39?us… : PS2, Line 209: Check `(uintptr_t)img + hdr->img_size <= (uintptr_t)buf + buf_size` here. https://review.coreboot.org/c/coreboot/+/85654/comment/918936aa_a7ba2a11?us… : PS2, Line 213: auto_dma_fw[PT_FW_IDX_GPUEB_FW].fw = img; : auto_dma_fw[PT_FW_IDX_GPUEB_FW].fw_size = hdr->img_size; Since PT_FW_IDX_GPUEB_FW is NOT auto dma fw (if I understand this correctly), we don't need to store these into the `auto_dma_fw` array. In the check below (line #225), skip `PT_FW_IDX_GPUEB_FW`. Then, check `gpueb_img` and `gpueb_img_size` at the end of this function. https://review.coreboot.org/c/coreboot/+/85654/comment/c87ae1c9_46e0ebea?us… : PS2, Line 235: memcpy((void *)(GPUEB_SRAM_BASE + auto_dma_fw[pt_id].fw_addr), We should check `fw_addr` against a max value for safety. Is `assert(fw_addr + fw_size <= GPUEB_SRAM_SIZE)` correct? https://review.coreboot.org/c/coreboot/+/85654/comment/b8d0df23_94dff19a?us… : PS2, Line 264: ( remove https://review.coreboot.org/c/coreboot/+/85654/comment/7586c4ac_e8593163?us… : PS2, Line 265: CRIT ERR https://review.coreboot.org/c/coreboot/+/85654/comment/cf0a5a60_0250a26b?us… : PS2, Line 271: CRIT ERR https://review.coreboot.org/c/coreboot/+/85654/comment/37b65979_13ef66ad?us… : PS2, Line 289: imgsize img_size https://review.coreboot.org/c/coreboot/+/85654/comment/1a9ebb5f_62df937f?us… : PS2, Line 294: Invalid parameters returned Failed to parse gpueb fw https://review.coreboot.org/c/coreboot/+/85654/comment/d76fee09_08d06ade?us… : PS2, Line 300: memset((void *)GPUEB_SRAM_BASE, 0, GPUEB_SRAM_SIZE); Should check `gpueb_imgsize <= GPUEB_SRAM_SIZE`. https://review.coreboot.org/c/coreboot/+/85654/comment/26e6fb63_48281c7e?us… : PS2, Line 352: .reset = gpueb_reset nit: Add a trailing `,` https://review.coreboot.org/c/coreboot/+/85654/comment/023dc7d7_1cf815c7?us… : PS2, Line 357: ( remove https://review.coreboot.org/c/coreboot/+/85654/comment/508986eb_1c21e0aa?us… : PS2, Line 358: die Is the gpueb init flow critical for the device to boot up? If not, I wonder if we can print an error message and then return, instead of calling `die()` everywhere. File src/soc/mediatek/mt8196/include/soc/addressmap.h: https://review.coreboot.org/c/coreboot/+/85654/comment/7c12cb1b_ba003d55?us… : PS2, Line 188: IO_PHYS + 0x38500000 Rewrite as `MFGSYS_BASE + 0x08500000`. Please rebase onto CB:85740. https://review.coreboot.org/c/coreboot/+/85654/comment/945103a3_a43fb32d?us… : PS2, Line 189: GPU_EB_RPC_BASE Rename to `MFG_GPUEB_RPC_BASE`. File src/soc/mediatek/mt8196/include/soc/gpueb_priv.h: https://review.coreboot.org/c/coreboot/+/85654/comment/fa564194_9f066ba8?us… : PS2, Line 13: define GPUEB_BASE (MFGSYS_BASE + 0x0B000000) /* 0x4B000000 */ : #define MFG_RPC_BASE (MFGSYS_BASE + 0x0B800000) > move to soc/addressmap. […] `MFG_RPC_BASE` already exists there. Please move `GPUEB_BASE` there and rename to `MFG_GPUEB_BASE`. https://review.coreboot.org/c/coreboot/+/85654/comment/04b11b5e_38f396de?us… : PS2, Line 57: #define MFG_RPC_GHPM_CFG13_CON (GPU_EB_RPC_BASE + 0x0834) /* 0x4B800834 */ : #define MFG_GHPM_RO2_CON (MFG_RPC_BASE + 0x09AC) /* 0x4B8009AC */ > why are they using different bases ? The bases are the same. `MFG_RPC_BASE` should be removed here. https://review.coreboot.org/c/coreboot/+/85654/comment/a495d7ed_b1177936?us… : PS2, Line 71: 0xA0000 Write `(640 * KiB)`, and the comment can be removed. https://review.coreboot.org/c/coreboot/+/85654/comment/e53cace2_5d4b0b70?us… : PS2, Line 73: _4B Remove `_4B` https://review.coreboot.org/c/coreboot/+/85654/comment/e927a420_e7d94697?us… : PS2, Line 75: GPUEB_GPR_SIZE + \ > move to the above line. Why do we need to do the subtraction? Can't we define this using `0x0009FD00` directly? https://review.coreboot.org/c/coreboot/+/85654/comment/79de1cb2_85e0b8db?us… : PS2, Line 77: #define GPUEB_GPR0 GPR_BASE_ADDR(0) : #define GPUEB_GPR1 GPR_BASE_ADDR(1) : #define GPUEB_GPR2 GPR_BASE_ADDR(2) : #define GPUEB_GPR6 GPR_BASE_ADDR(6) : #define GPUEB_GPR17 GPR_BASE_ADDR(17) : #define GPUEB_GPR18 GPR_BASE_ADDR(18) : #define GPUEB_GPR23 GPR_BASE_ADDR(23) : #define GPUEB_GPR30 GPR_BASE_ADDR(30) Remove these. Use `GPR_BASE_ADDR(x)` directly. https://review.coreboot.org/c/coreboot/+/85654/comment/86a32472_a28de526?us… : PS2, Line 138: GPUEB_SRAM_GPR25 Remove this. https://review.coreboot.org/c/coreboot/+/85654/comment/4c09b8a8_71ee2786?us… : PS2, Line 152: SWITCH_MUX_WAIT_TIME `SWITCH_MUX_WAIT_US` https://review.coreboot.org/c/coreboot/+/85654/comment/48bc5a69_bdd06df8?us… : PS2, Line 157: #define DUMP_REG(reg) printk(BIOS_CRIT, #reg "(%#x) = %#x\n", (reg), read32p((reg))) > move to gpueb. […] Use `BIOS_DEBUG` instead of `BIOS_CRIT`. https://review.coreboot.org/c/coreboot/+/85654/comment/e246b1ba_3a5c16ec?us… : PS2, Line 186: void const https://review.coreboot.org/c/coreboot/+/85654/comment/18e84fcd_18822b02?us… : PS2, Line 185: struct auto_dma_fw_header { : void *fw; : size_t fw_size; : uint64_t fw_addr; : }; Move to gpueb.c and rename to `struct auto_dma_fw`, as this isn't a header. -- To view, visit https://review.coreboot.org/c/coreboot/+/85654?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0f10dfc753f73df97ea08a4c23e97de416832be2 Gerrit-Change-Number: 85654 Gerrit-PatchSet: 2 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Mon, 23 Dec 2024 10:18:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[M] Change in coreboot[main]: mb/google/corsola: Add new board variant Wyrdeer
by Yidi Lin (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Hung-Te Lin, Xinxiong Xu, Yang Wu, Yu-Ping Wu. Yidi Lin has posted comments on this change by Yang Wu. ( https://review.coreboot.org/c/coreboot/+/85742?usp=email ) Change subject: mb/google/corsola: Add new board variant Wyrdeer ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/corsola/panel_wyrdeer.c: https://review.coreboot.org/c/coreboot/+/85742/comment/eff20de5_b2204d49?us… : PS2, Line 12: static void mipi_panel_power_on(void) : { : const struct tps65132s_reg_setting reg_settings[] = { : { PMIC_TPS65132_VPOS, 0x14, 0x1F }, : { PMIC_TPS65132_VNEG, 0x14, 0x1F }, : { PMIC_TPS65132_DLYX, 0x95, 0xFF }, : { PMIC_TPS65132_ASSDD, 0x5b, 0xFF }, : }; : const struct tps65132s_cfg cfg = { : .i2c_bus = PMIC_I2C_BUS, : .en = GPIO_EN_PP3300_DISP_X, : .sync = GPIO_TCHPAD_INT_ODL, : .settings = reg_settings, : .setting_counts = ARRAY_SIZE(reg_settings), : }; : : mainboard_set_regulator_voltage(MTK_REGULATOR_VIO18, 1800000); : mtk_i2c_bus_init(PMIC_I2C_BUS, I2C_SPEED_FAST); : : if (is_pmic_aw37503(PMIC_I2C_BUS)) { : printk(BIOS_DEBUG, "Initialize and power on PMIC AW37503\n"); : aw37503_init(PMIC_I2C_BUS); : gpio_output(GPIO_EN_PP3300_DISP_X, 1); : mdelay(2); : gpio_output(GPIO_TCHPAD_INT_ODL, 1); : mdelay(3); : } else if (tps65132s_setup(&cfg) != CB_SUCCESS) { : printk(BIOS_ERR, "Failed to setup tps65132s\n"); : } : : /* DISP_RST_1V8_L */ : gpio_output(GPIO_EDPBRDG_RST_L, 1); : mdelay(1); : gpio_output(GPIO_EDPBRDG_RST_L, 0); : udelay(20); : gpio_output(GPIO_EDPBRDG_RST_L, 1); : } This function is identical to the function in `panel_starmie.c`. Move it to a separate file. -- To view, visit https://review.coreboot.org/c/coreboot/+/85742?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I07b73c97d8d51b32f557e31d834ffc6cfb8420ed Gerrit-Change-Number: 85742 Gerrit-PatchSet: 2 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 23 Dec 2024 10:17:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support
by Andy Hsu (Code Review) 23 Dec '24

23 Dec '24

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support
by Andy Hsu (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. Andy Hsu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85654?usp=email ) Change subject: soc/mediatek/mt8196: Add GPUEB support ...................................................................... Patch Set 3: (10 comments) File src/soc/mediatek/mt8196/gpueb.c: https://review.coreboot.org/c/coreboot/+/85654/comment/4554f51b_9ad7d19f?us… : PS2, Line 123: : if (read32p(MFG_RPC_MFG0_PWR_CON) == MFG0_PWR_ON_VALUE) : return; : : if (!(read32p(MFG_RPC_MFG0_PWR_CON) & MFG0_PWR_SRAM_PDN)) : return; > Please add comments why the setting can be omitted in these two conditions. Done https://review.coreboot.org/c/coreboot/+/85654/comment/d1239284_6a928120?us… : PS2, Line 225: 1 > PT_FW_IDX_GPUEB_FW Done https://review.coreboot.org/c/coreboot/+/85654/comment/8ffa006c_2cf09b13?us… : PS2, Line 236: aut > align with `(void *)` Done https://review.coreboot.org/c/coreboot/+/85654/comment/9a38684b_286db84e?us… : PS2, Line 305: write32p(GPUEB_INFO, (uintptr_t)mcu->load_buffer); : write32p(GPUEB_ABNORMAL_BOOT, 0); : write32p(GPUEB_WARM_BOOT, 0); : write32p(GPUMPU_RSV_ADDR, (uintptr_t)_resv_mem_gpueb >> PAGE_SHIFT); > Can you explain this part ? I don't quite understand why it writes `mcu->load_buffer` to `GPUEB_INFO […] Upon confirmation, this section is identified as legacy code and is not needed in the current process. It can be removed. File src/soc/mediatek/mt8196/include/soc/gpueb_priv.h: https://review.coreboot.org/c/coreboot/+/85654/comment/72258711_b90ae8e5?us… : PS2, Line 10: #define SPM2GPUPM_CON (SPM_BASE + 0x0410) /* 0x1C004410 */ : #define SPM_REQ_STA_9 (SPM_BASE + 0x0884) /* 0x1C004884 */ : #define SPM_MFG0_PWR_CON (SPM_BASE + 0x0EA8) > remove Done https://review.coreboot.org/c/coreboot/+/85654/comment/a573ca4e_68d6ce3b?us… : PS2, Line 13: define GPUEB_BASE (MFGSYS_BASE + 0x0B000000) /* 0x4B000000 */ : #define MFG_RPC_BASE (MFGSYS_BASE + 0x0B800000) > move to soc/addressmap. […] Done https://review.coreboot.org/c/coreboot/+/85654/comment/3d2a1f33_d17cc3f4?us… : PS2, Line 57: #define MFG_RPC_GHPM_CFG13_CON (GPU_EB_RPC_BASE + 0x0834) /* 0x4B800834 */ : #define MFG_GHPM_RO2_CON (MFG_RPC_BASE + 0x09AC) /* 0x4B8009AC */ > why are they using different bases ? Done https://review.coreboot.org/c/coreboot/+/85654/comment/051c32cc_753372a4?us… : PS2, Line 75: GPUEB_GPR_SIZE + \ > move to the above line. Done https://review.coreboot.org/c/coreboot/+/85654/comment/f17af5ef_c7785546?us… : PS2, Line 157: #define DUMP_REG(reg) printk(BIOS_CRIT, #reg "(%#x) = %#x\n", (reg), read32p((reg))) > move to gpueb. […] Done https://review.coreboot.org/c/coreboot/+/85654/comment/9cf5e934_3e3b7fb6?us… : PS2, Line 197: memory addr; current setting is within the 4GB memory : * region : */ > The FW address is in the 4GB range. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85654?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0f10dfc753f73df97ea08a4c23e97de416832be2 Gerrit-Change-Number: 85654 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 23 Dec 2024 10:15:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/corsola: Add new board variant Ponyta360
by Yang Wu (Code Review) 23 Dec '24

23 Dec '24

Yang Wu has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/85652?usp=email ) Change subject: mb/google/corsola: Add new board variant Ponyta360 ...................................................................... Abandoned we will share the same name with ponyta -- To view, visit https://review.coreboot.org/c/coreboot/+/85652?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic0ddbfc667d1ba583f3cb95f60c73eb344b454c6 Gerrit-Change-Number: 85652 Gerrit-PatchSet: 1 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add GPUEB support
by Jarried Lin (Code Review) 23 Dec '24

23 Dec '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. Hello Andy Hsu, Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85654?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by Yidi Lin, Verified+1 by build bot (Jenkins) Change subject: soc/mediatek/mt8196: Add GPUEB support ...................................................................... soc/mediatek/mt8196: Add GPUEB support GPUEB is a micro-processor used for GPU power management. It is also responsible for controlling GPU DVFS and GPU thermal throttling. This gpueb load flow adds 47ms to the boot time. coreboot log: CBFS: Found 'tinysys-gpueb-RV33_A.mkpt_fw.img' @0x84740 size 0x29736 in mcache @0xfffdd374 Loaded (and reset) tinysys-gpueb-RV33_A.mkpt_fw.img in 47 msecs. TEST=Boot ok BUG=b:317009620 Signed-off-by: Andy.Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Change-Id: I0f10dfc753f73df97ea08a4c23e97de416832be2 --- M src/soc/mediatek/common/include/soc/symbols.h M src/soc/mediatek/mt8196/Kconfig M src/soc/mediatek/mt8196/Makefile.mk A src/soc/mediatek/mt8196/gpueb.c M src/soc/mediatek/mt8196/include/soc/addressmap.h A src/soc/mediatek/mt8196/include/soc/gpueb.h A src/soc/mediatek/mt8196/include/soc/gpueb_priv.h M src/soc/mediatek/mt8196/include/soc/memlayout.ld M src/soc/mediatek/mt8196/soc.c 9 files changed, 611 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/85654/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/85654?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0f10dfc753f73df97ea08a4c23e97de416832be2 Gerrit-Change-Number: 85654 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Andy Hsu <andy.hsu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2024