Brandon Breitenstein has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:141608957 BRANCH=NONE TEST: built and booted TGL U RVP
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.c A src/soc/intel/tigerlake/early_tcss.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 5 files changed, 266 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/1
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index fbf56b4..55c06b1 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -208,4 +208,10 @@ config PRERAM_CBMEM_CONSOLE_SIZE hex default 0xe00 + +config EARLY_TCSS + bool "Enable early TCSS" + help + Enable devices to be detected over Type-C ports during boot. + endif diff --git a/src/soc/intel/tigerlake/Makefile.inc b/src/soc/intel/tigerlake/Makefile.inc index c4f71c7..5e60114 100644 --- a/src/soc/intel/tigerlake/Makefile.inc +++ b/src/soc/intel/tigerlake/Makefile.inc @@ -31,6 +31,7 @@ ramstage-y += acpi.c ramstage-y += chip.c ramstage-y += cpu.c +ramstage-$(CONFIG_EARLY_TCSS) += early_tcss.c ramstage-y += elog.c ramstage-y += espi.c ramstage-y += finalize.c diff --git a/src/soc/intel/tigerlake/chip.c b/src/soc/intel/tigerlake/chip.c index 00db2a4..3da5d9a 100644 --- a/src/soc/intel/tigerlake/chip.c +++ b/src/soc/intel/tigerlake/chip.c @@ -10,6 +10,7 @@ #include <intelblocks/itss.h> #include <intelblocks/xdci.h> #include <romstage_handoff.h> +#include <soc/early_tcss.h> #include <soc/intel/common/vbt.h> #include <soc/itss.h> #include <soc/pci_devs.h> @@ -129,6 +130,10 @@ * default policy that doesn't honor boards' requirements. */ itss_snapshot_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
+ /* Check for early TCSS and connect devices */ + if (CONFIG(EARLY_TCSS)) + early_tcss_enable(); + /* Perform silicon specific init. */ fsp_silicon_init(romstage_handoff_is_resume());
diff --git a/src/soc/intel/tigerlake/early_tcss.c b/src/soc/intel/tigerlake/early_tcss.c new file mode 100644 index 0000000..942d490 --- /dev/null +++ b/src/soc/intel/tigerlake/early_tcss.c @@ -0,0 +1,206 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <bootstate.h> +#include <console/console.h> +#include <device/pci.h> +#include <ec/google/chromeec/ec.h> +#include <intelblocks/pmclib.h> +#include <soc/early_tcss.h> +#include <soc/pci_devs.h> +#include <stdlib.h> + +/* send the ipc_command */ +static int send_ipc_command(uint32_t cmd, uint8_t *tcss_req, int req_size, + struct pmc_ipc_buffer *rbuf) +{ + struct pmc_ipc_buffer *wbuf = malloc(sizeof(*tcss_req)); + + /* copy the tcss_req into a buffer for ipc command */ + memcpy(wbuf, tcss_req, req_size); + + return pmc_send_ipc_cmd(cmd, wbuf, rbuf); +} + +static int send_pmc_connect_request(struct tcss_mux mux_data, + struct pmc_ipc_buffer *rbuf) +{ + union pmc_ipc_cmd tcss_cmd = { 0 }; + uint8_t tcss_req[PMC_IPC_CONN_REQ_SIZE] = { 0 }; + + tcss_cmd.cmd = PMC_IPC_USBC_CMD_ID; + tcss_cmd.subcmd = PMC_IPC_USBC_SUBCMD_ID; + + tcss_req[0] = PMC_IPC_TCSS_CONN_REQ_RES | /* Usage */ + mux_data.usb3_port << 4; + tcss_req[1] = mux_data.usb2_port | + mux_data.ufp << 4 | /* 1=UFP/0=DFP */ + mux_data.polarity << 5 | /* ORI-HSL */ + mux_data.polarity << 6 | /* ORI-SBU */ + mux_data.acc << 7; + printk(BIOS_DEBUG, "tcss_req[0]-> 0x%x\n" + "tcss_req[1]-> 0x%x\n\t", tcss_req[0], tcss_req[1]); + + tcss_cmd.len = PMC_IPC_CONN_REQ_SIZE; + + return send_ipc_command(tcss_cmd.cmd_reg, tcss_req, PMC_IPC_CONN_REQ_SIZE, rbuf); +} + +static int send_pmc_safe_mode_request(struct tcss_mux mux_data, + struct pmc_ipc_buffer *rbuf) +{ + union pmc_ipc_cmd tcss_cmd = { 0 }; + uint8_t tcss_req[PMC_IPC_SAFE_REQ_SIZE] = { 0 }; + + tcss_cmd.cmd = PMC_IPC_USBC_CMD_ID; + tcss_cmd.subcmd = PMC_IPC_USBC_SUBCMD_ID; + + tcss_req[0] = PMC_IPC_TCSS_SAFE_MODE_REQ_RES | + mux_data.usb3_port << 4; + printk(BIOS_DEBUG, "tcss_req[0]-> 0x%x\n", tcss_req[0]); + + tcss_cmd.cmd.len = PMC_IPC_SAFE_REQ_SIZE; + + return send_ipc_command(tcss_cmd.cmd_reg, tcss_req, PMC_IPC_SAFE_REQ_SIZE, rbuf); +} + +static int send_pmc_alt_mode_request(struct tcss_mux mux_data, + struct pmc_ipc_buffer *rbuf) +{ + union pmc_ipc_cmd tcss_cmd = { 0 }; + uint8_t tcss_req[PMC_IPC_ALT_REQ_SIZE] = { 0 }; + + tcss_cmd.cmd = PMC_IPC_USBC_CMD_ID; + tcss_cmd.subcmd = PMC_IPC_USBC_SUBCMD_ID; + + tcss_req[0] = PMC_IPC_TCSS_ALTMODE_REQ_RES | mux_data.usb3_port << 4; + tcss_req[1] |= PMC_IPC_DP_MODE; + + tcss_req[4] = mux_data.polarity << 1 | + mux_data.cable << 2 | + mux_data.ufp << 3 | + mux_data.polarity << 4 | + mux_data.polarity << 5; + + if (mux_data.dp_mode <= MODE_DP_PIN_F) { + switch (mux_data.dp_mode) { + case MODE_DP_PIN_A: + tcss_req[5] = 0; + break; + case MODE_DP_PIN_B: + tcss_req[5] = 1; + break; + case MODE_DP_PIN_C: + tcss_req[5] = 2; + break; + case MODE_DP_PIN_D: + tcss_req[5] = 3; + break; + case MODE_DP_PIN_E: + tcss_req[5] = 4; + break; + case MODE_DP_PIN_F: + tcss_req[5] = 5; + break; + } + } + + tcss_req[5] |= mux_data.hpd_lvl << 6; + printk(BIOS_DEBUG, "tcss_req[0]-> 0x%x\n" + "tcss_req[1]-> 0x%x\n" + "tcss_req[4]-> 0x%x\n" + "tcss_req[5]-> 0x%x\n\t", + tcss_req[0], tcss_req[1], tcss_req[4], tcss_req[5]); + + tcss_cmd.len = PMC_IPC_ALT_REQ_SIZE; + + return send_ipc_command(tcss_cmd.cmd_reg, tcss_req, PMC_IPC_ALT_REQ_SIZE, rbuf); +} + +static void update_tcss_mux(int port, struct tcss_mux mux_data) +{ + struct pmc_ipc_buffer *rbuf = malloc(sizeof(*rbuf)); + int ret = 0; + + /* Check if the mux has a USB device */ + if (mux_data.usb) { + ret = send_pmc_connect_request(mux_data, rbuf); + if (ret) { + printk(BIOS_ERR, "Port %d connect request failed\n", port); + return; + } + } + + /* check if mux has a DP device */ + if (mux_data.dp) { + + if (!mux_data.usb) { + ret = send_pmc_connect_request(mux_data, rbuf); + if (ret) { + printk(BIOS_ERR, "Port %d connect request failed\n", port); + return; + } + } + ret = send_pmc_safe_mode_request(mux_data, rbuf); + if (ret) { + printk(BIOS_ERR, "Port %d safe mode request failed\n", port); + return; + } + + ret = send_pmc_alt_mode_request(mux_data, rbuf); + } + + if (ret) + printk(BIOS_ERR, "Port %d mux set failed with error %d\n", port, ret); +} + +void early_tcss_enable(void) +{ + uint8_t num_ports; + int ret, i; + + ret = google_chromeec_get_num_pd_ports(&num_ports); + if (ret < 0) + return; + + for (i = 0; i < num_ports; i++) { + uint8_t port_map, mux_flags; + struct tcss_mux mux_data; + + ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); + if (ret < 0) + continue; + + ret = google_chromeec_pd_get_port_info(i, &port_map); + if (ret < 0) + continue; + + ret = google_chromeec_usb_pd_control(i, &mux_data.ufp, &mux_data.acc, + &mux_data.dp_mode); + if (ret < 0) + continue; + + mux_data.usb = !!(mux_flags & USB_PD_CTRL_USB_ENABLED); + mux_data.dp = !!(mux_flags & USB_PD_CTRL_DP_ENABLED); + mux_data.cable = !!(mux_flags & USB_PD_MUX_TBT_ACTIVE_CABLE); + mux_data.polarity = !!(mux_flags & USB_PD_CTRL_POLARITY_INVERTED); + mux_data.hpd_irq = !!(mux_flags & USB_PD_CTRL_HPD_IRQ); + mux_data.hpd_lvl = !!(mux_flags & USB_PD_CTRL_HPD_LVL); + mux_data.usb2_port = port_map & 0x0F; + mux_data.usb3_port = (port_map & 0xF0) >> 4; + + printk(BIOS_DEBUG, "Port %d mux=0x%x\n" + "USB2 port = %x\n" + "USB3 port = %x\n" + "ufp = %d dbg_acc=%d\n", + i, (unsigned int)mux_flags, mux_data.usb2_port, + mux_data.usb3_port, mux_data.ufp, mux_data.acc); + + update_tcss_mux(i, mux_data); + } +} diff --git a/src/soc/intel/tigerlake/include/soc/early_tcss.h b/src/soc/intel/tigerlake/include/soc/early_tcss.h new file mode 100644 index 0000000..29b7cdd --- /dev/null +++ b/src/soc/intel/tigerlake/include/soc/early_tcss.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* PMC IPC related offsets and commands */ +#define PMC_IPC_USBC_CMD_ID 0xA7 +#define PMC_IPC_USBC_SUBCMD_ID 0x0 +#define PMC_IPC_CMD 0x0 +#define PMC_IPC_TCSS_CONN_REQ_RES 0x0 +#define PMC_IPC_TCSS_SAFE_MODE_REQ_RES 0x2 +#define PMC_IPC_TCSS_ALTMODE_REQ_RES 0x3 +#define PMC_IPC_CONN_REQ_SIZE 2 +#define PMC_IPC_ALT_REQ_SIZE 8 +#define PMC_IPC_SAFE_REQ_SIZE 1 +#define PMC_IPC_DP_MODE BIT(4) + +/* connection modes for pmc */ +enum pmc_ipc_conn_mode { + PMC_IPC_TCSS_DISCONNECT_MODE, + PMC_IPC_TCSS_USB_MODE, + PMC_IPC_TCSS_ALTERNATE_MODE, + PMC_IPC_TCSS_SAFE_MODE, + PMC_IPC_TCSS_HPD_MODE, + PMC_IPC_TCSS_TOTAL_MODES, +}; + +/* DP Mode pin definitions */ +#define MODE_DP_PIN_A BIT(0) +#define MODE_DP_PIN_B BIT(1) +#define MODE_DP_PIN_C BIT(2) +#define MODE_DP_PIN_D BIT(3) +#define MODE_DP_PIN_E BIT(4) +#define MODE_DP_PIN_F BIT(5) + +/* struct to hold all tcss_mux related variables */ +struct tcss_mux { + bool dp; /* DP connected */ + bool usb; /* USB connected */ + bool cable; /* Activ/Passive Cable */ + bool polarity; /* polarity of connected device */ + bool hpd_lvl; /* HPD Level assert */ + bool hpd_irq; /* HPD IRQ assert */ + bool ufp; + bool acc; + uint8_t dp_mode; /* DP Operation Mode */ + uint8_t usb3_port; /* USB2 Port Number */ + uint8_t usb2_port; /* USB3 Port Number */ +}; + +void early_tcss_enable(void);
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Set Ready For Review
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#6).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:141608957 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 526 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/6
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#8).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:141608957 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 527 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/8
Hello build bot (Jenkins), Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#10).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 545 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/10
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 10:
(12 comments)
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright 2020 The coreboot project Authors. : * : * SPDX-License-Identifier: GPL-2.0-or-later : */ /* SPDX-License-Identifier: GPL-2.0-or-later */
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 31: tcss_make_alt_mode_cmd_1 could we think of more descriptive names than alt_mode_cmd_1 and _2 ?
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 55: /* : * commenting this section out for now as probably not needed : static void *iom_reg(unsigned int iom_reg_offset) : { : const uintptr_t iombase = IOM_BASE_ADDRESS; : : return (void *)(iombase + iom_reg_offset); : } : : static const void *port_status_reg(int port) : { : uintptr_t sts_offset; : : sts_offset = IOM_PORT_STATUS_OFFSET + IOM_REG_LEN * port; : return (const void *)iom_reg(sts_offset); : } : : static bool is_port_connected(int port) : { : uint32_t sts; : : sts = read32(port_status_reg(port)); : return !!(sts & IOM_PORT_STATUS_CONNECTED); : } : */ then go ahead and delete it, if you need it later, you can find it here in gerrit 😊
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 81: static int send_conn_disc_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_CONN_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_safe_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_dp_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } All 3 of these are virtually the same, looks like the only difference is the 3rd argument to pmc_make_ipc_cmd(), a refactor would be much easier to read
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 188: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 219: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 221: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 259: if (mux_data.dp_mode <= MODE_DP_PIN_F) { suggestion: remove this if, and use `default` instead
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 319: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 339: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 350: return; probably should have another big error message here.
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 371: //Add check for connected maybe? I think we should check, otherwise we're wasting boot time sending another host command and a PMC IPC as well.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 371: //Add check for connected maybe?
I think we should check, otherwise we're wasting boot time sending another host command and a PMC IP […]
Nothing before this point can set this...which is the only reason I'm not sure this check is needed. It complicates the flow and adds additional delay to check IOM status when there is no possibility for it to be connected
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 31: tcss_make_alt_mode_cmd_1
could we think of more descriptive names than alt_mode_cmd_1 and _2 ?
These literally correspond to Buffer 0 and buffer 1 I can name it as such there really isn't a more descriptive name thatn that though unfortunately
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 81: static int send_conn_disc_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_CONN_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_safe_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_dp_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : }
All 3 of these are virtually the same, looks like the only difference is the 3rd argument to pmc_mak […]
Good point I'll consolidate this
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#11).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 443 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/11
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 11:
(12 comments)
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright 2020 The coreboot project Authors. : * : * SPDX-License-Identifier: GPL-2.0-or-later : */
/* SPDX-License-Identifier: GPL-2. […]
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 31: tcss_make_alt_mode_cmd_1
These literally correspond to Buffer 0 and buffer 1 I can name it as such there really isn't a more […]
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 55: /* : * commenting this section out for now as probably not needed : static void *iom_reg(unsigned int iom_reg_offset) : { : const uintptr_t iombase = IOM_BASE_ADDRESS; : : return (void *)(iombase + iom_reg_offset); : } : : static const void *port_status_reg(int port) : { : uintptr_t sts_offset; : : sts_offset = IOM_PORT_STATUS_OFFSET + IOM_REG_LEN * port; : return (const void *)iom_reg(sts_offset); : } : : static bool is_port_connected(int port) : { : uint32_t sts; : : sts = read32(port_status_reg(port)); : return !!(sts & IOM_PORT_STATUS_CONNECTED); : } : */
then go ahead and delete it, if you need it later, you can find it here in gerrit 😊
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 81: static int send_conn_disc_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_CONN_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_safe_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_dp_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : }
Good point I'll consolidate this
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 188:
nit: extra blank line
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 219:
nit: extra blank line
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 221:
nit: extra blank line
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 259: if (mux_data.dp_mode <= MODE_DP_PIN_F) {
suggestion: remove this if, and use `default` instead
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 319:
nit: extra blank line
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 339:
nit: extra blank line
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 350: return;
probably should have another big error message here.
Done
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 371: //Add check for connected maybe?
Nothing before this point can set this...which is the only reason I'm not sure this check is needed. […]
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#12).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 425 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/12
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 12:
(6 comments)
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 30: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 52: nit: extra blank line
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 201: static void update_tcss_mux(int port, struct tcss_mux mux_data) May be better to pass `mux_data` by reference, i.e., `const struct tcss_mux *mux_data`, to avoid copying
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 203: struct pmc_ipc_buffer *rbuf = malloc(sizeof(*rbuf)); Does you really need malloc here? It doesn't appear that `rbuf` has a lifetime that is longer than this function, so I think you can just do `struct pmc_ipc_buffer rbuf` here
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 269: //Add check for connected maybe? ?
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/fs... PS12, Line 334: DSK what is DSK?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 12:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 30:
nit: extra blank line
Ack
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 52:
nit: extra blank line
Ack
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 201: static void update_tcss_mux(int port, struct tcss_mux mux_data)
May be better to pass `mux_data` by reference, i.e. […]
good point I will change this to a reference
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 203: struct pmc_ipc_buffer *rbuf = malloc(sizeof(*rbuf));
Does you really need malloc here? It doesn't appear that `rbuf` has a lifetime that is longer than t […]
I think there was a reason for this with the old method of creating the buffers but with the new one you are right will just change this to a normal declaration
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 269: //Add check for connected maybe?
?
Will remove this left over comment
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#13).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 477 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/13
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 237: return 0; please, no spaces at the start of a line
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 275: : void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : } This piece is Chrome OS (& EC) specific, so we can't just leave it here in soc/intel/tigerlake. `update_tcss_mux` would need to be added to early_tcss.h instead of this, and then this function should probably go in volteer's mainboard.c or similar.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 275: : void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : }
This piece is Chrome OS (& EC) specific, so we can't just leave it here in soc/intel/tigerlake. […]
Is this not SOC specific? this is only for TGL at this point
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 275: : void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : }
Is this not SOC specific? this is only for TGL at this point
Yes but this code is specific to the Chrome EC (all the google_chromeec_* calls); I understand that right now Volteer & TGLRVP are the only TGL boards, but we shouldn't tie SoC-specific code to Chrome OS.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#14).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 501 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/14
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 275: : void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : }
Yes but this code is specific to the Chrome EC (all the google_chromeec_* calls); I understand that […]
Ok I guess I understand your view I just find it weird to move to mainboard since this will be applicable to any Tigerlake boards in the future as well. So now instead of relying on all SOC function calls fsp_params will be calling a mainboard function to configure the muxes...that just seems a little weird to me (sorry for the delay in response here was working on some other things and thought I had responded)
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#15).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 505 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/15
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#16).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 500 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/16
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#17).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 499 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/17
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#18).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 499 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/18
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 275: : void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : }
Ok I guess I understand your view I just find it weird to move to mainboard since this will be appli […]
We do already have `mainboard_silicon_init_params()` in fsp_params.c for example, which calls into the mainboard to update UPDs. We also can't just assume that necessarily every Tiger Lake board will have the Chrome EC.
I have also been thinking about some re-organization with code that we want to probably share among Chrome OS boards, but not have it live in vendorcode/google/chromeos either (maybe someone wants to run an alternative OS on a chromebook, but they would still need this functionality)... so I hear your point of view as well, and I'm thinking about it, but for now, the mainboard is probably the simplest way.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#19).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 505 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/19
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/13/src/soc/intel/tigerlake/ea... PS13, Line 275: : void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : }
We do already have `mainboard_silicon_init_params()` in fsp_params. […]
updated patchset. Moved code that relies on chromeec to mainboard and made this code only execute in recovery flow.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 19: Code-Review+1
(1 comment)
LGTM except for the commented-out code section
https://review.coreboot.org/c/coreboot/+/42079/19/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/19/src/soc/intel/tigerlake/ea... PS19, Line 287: /* void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : } */ : remove this, next patch has the implementation for Volteer, which serves as an example of how to use the API
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 19:
Patch Set 19: Code-Review+1
(1 comment)
LGTM except for the commented-out code section
Whoops I knew I forgot to remove something
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#20).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 433 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/20
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 20: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/19/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/19/src/soc/intel/tigerlake/ea... PS19, Line 287: /* void early_tcss_enable(void) : { : uint8_t dp_mode; : unsigned int num_ports; : int ret, i; : bool ufp, acc; : : ret = google_chromeec_get_num_pd_ports(&num_ports); : if (ret < 0) { : printk(BIOS_ERR, "get_num_pd_ports failed unable to continue\n"); : return; : } : : for (i = 0; i < num_ports; i++) { : uint8_t port_map, mux_flags; : struct tcss_mux mux_data; : : ret = google_chromeec_usb_get_pd_mux_info(i, &mux_flags); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_pd_mux_info failed\n", i); : continue; : } : : ret = google_chromeec_pd_get_port_info(i, &port_map); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: get_port_info failed\n", i); : continue; : } : : mux_data.usb2_port = port_map & USB_2_PORT_MASK; : mux_data.usb3_port = (port_map & USB_3_PORT_MASK) >> 4; : : //Add check for connected maybe? : : ret = google_chromeec_usb_pd_control(i, &ufp, &acc, &dp_mode); : if (ret < 0) { : printk(BIOS_ERR, "port C%d: pd_control failed\n", i); : continue; : } : : mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED); : mux_data.dp = !!(mux_flags & USB_PD_MUX_DP_ENABLED); : mux_data.cable = !!(mux_flags & USB_PD_CTRL_ACTIVE_CABLE); : mux_data.polarity = !!(mux_flags & USB_PD_MUX_POLARITY_INVERTED); : mux_data.hpd_irq = !!(mux_flags & USB_PD_MUX_HPD_IRQ); : mux_data.hpd_lvl = !!(mux_flags & USB_PD_MUX_HPD_LVL); : mux_data.ufp = !!ufp; : mux_data.acc = !!acc; : mux_data.dp_mode = dp_mode; : : printk(BIOS_DEBUG, "Port %d mux=0x%x\n" : "USB2 port = %x\n" : "USB3 port = %x\n" : "DP Mode = %x\n" : "dp = %d\n" : "usb = %d\n" : "cable = %d\n" : "polarity = %d\n" : "hpd_lvl = %d\n" : "hpd_irq = %d\n" : "ufp = %d\n" : "dbg_acc = %d\n", : i, (unsigned int)mux_flags, mux_data.usb2_port, : mux_data.usb3_port, mux_data.dp_mode, mux_data.dp, : mux_data.usb, mux_data.cable, mux_data.polarity, : mux_data.hpd_lvl, mux_data.hpd_irq, mux_data.ufp, : mux_data.acc); : : update_tcss_mux(i, mux_data); : } : } */ :
remove this, next patch has the implementation for Volteer, which serves as an example of how to use […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 20:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 201: static void update_tcss_mux(int port, struct tcss_mux mux_data)
good point I will change this to a reference
Ack
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 203: struct pmc_ipc_buffer *rbuf = malloc(sizeof(*rbuf));
I think there was a reason for this with the old method of creating the buffers but with the new one […]
Ack
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/ea... PS12, Line 269: //Add check for connected maybe?
Will remove this left over comment
Ack
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/12/src/soc/intel/tigerlake/fs... PS12, Line 334: DSK
what is DSK?
Ack
Duncan Laurie has uploaded a new patch set (#22) to the change originally created by Brandon Breitenstein. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 433 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/22
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 22:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 59: nit: extra tab here
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 79: res_reg = res->buf[0]; nit: blank line after if block
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 144: cmd = tcss_make_safe_mode_cmd( : PMC_IPC_TCSS_SAFE_MODE_REQ_RES, : mux_data.usb3_port); I think this will all fit on 1 line
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 179: static uint8_t hpd_lvl; Shouldn't this be a per-port HPD, since the state is stored across calls?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 22:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 144: cmd = tcss_make_safe_mode_cmd( : PMC_IPC_TCSS_SAFE_MODE_REQ_RES, : mux_data.usb3_port);
I think this will all fit on 1 line
I think it was just long but I will at least move PMC_IPC variable up a line
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 179: static uint8_t hpd_lvl;
Shouldn't this be a per-port HPD, since the state is stored across calls?
I'm going to remove this completely with the new MUX spec the hpd call is required for every dp mode not only when hpd changes
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#23).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST: built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 427 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/23
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 23:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 59:
nit: extra tab here
Done
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 79: res_reg = res->buf[0];
nit: blank line after if block
Done
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 144: cmd = tcss_make_safe_mode_cmd( : PMC_IPC_TCSS_SAFE_MODE_REQ_RES, : mux_data.usb3_port);
I think it was just long but I will at least move PMC_IPC variable up a line
Done
https://review.coreboot.org/c/coreboot/+/42079/22/src/soc/intel/tigerlake/ea... PS22, Line 179: static uint8_t hpd_lvl;
I'm going to remove this completely with the new MUX spec the hpd call is required for every dp mode […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 23: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/23//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42079/23//COMMIT_MSG@16 PS23, Line 16: TEST: nit: `TEST=`
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#24).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST=built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/Kconfig M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 6 files changed, 427 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/24
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/23//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42079/23//COMMIT_MSG@16 PS23, Line 16: TEST:
nit: `TEST=`
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 24:
(8 comments)
https://review.coreboot.org/c/coreboot/+/42079/24/src/mainboard/google/volte... File src/mainboard/google/volteer/Kconfig:
PS24: This is a mainboard change. It should go in the next CL.
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 3: #include <bootstate.h> Why is this required?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 6: #include <ec/google/chromeec/ec.h> Why is this required?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 102: while (--tries >= 0) Just curious: Why are we trying 3 times? Is this operation known to fail for some reason?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 376: DSK What does DSK mean?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 376: WEAK Why WEAK?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 378: vboot_recovery_mode_enabled What about developer mode? Wouldn't you need this in developer mode as well?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/in... PS24, Line 130: mainboard_early_tcss_enable What is the expectation from mainboard here? There should be a comment explaining the expectations.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#25).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST=built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 5 files changed, 433 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/25
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 25:
(8 comments)
https://review.coreboot.org/c/coreboot/+/42079/24/src/mainboard/google/volte... File src/mainboard/google/volteer/Kconfig:
PS24:
This is a mainboard change. It should go in the next CL.
will move to mainboard cl...this one used to encompass both
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 3: #include <bootstate.h>
Why is this required?
removed
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 6: #include <ec/google/chromeec/ec.h>
Why is this required?
leftover from when this was calling ec commands removed
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 102: while (--tries >= 0)
Just curious: Why are we trying 3 times? Is this operation known to fail for some reason?
This is for consistency with the depthcharge code, it can be removed. This loop should almost never be executed cause the only case it executes is if the response buffer is empty.
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 376: DSK
What does DSK mean?
Will clean this up...was using this for debug
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 376: WEAK
Why WEAK?
Ack
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 378: vboot_recovery_mode_enabled
What about developer mode? Wouldn't you need this in developer mode as well?
my understanding was this was mainly for user facing modes that had no OS I will add a developer mode check as well
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/in... PS24, Line 130: mainboard_early_tcss_enable
What is the expectation from mainboard here? There should be a comment explaining the expectations.
adding comment in next patch
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 25:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 157: malloc a local variable is sufficient here
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 245: malloc a local variable is sufficient here.
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 249: mux_data.usb you can add || mux_data.dp here and remove the special case call from the dp case below.
also, since we don't need usb until we get to depthcharge, would it make sense to ignore "mux_data.usb" entirely?
it would help to add a note here that in order to get into ALT mode, the mux has to transition through the USB and SAFE states.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 25:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 249: mux_data.usb
you can add […]
good point here...I think this is a remains of more code being in here in the usb case and since it's only dp now this first section is irrelevant
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 25:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/fs... PS25, Line 376: Calling THe whole "calling ... called" is kinda weird, how about just drop this first 'Calling'
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/in... PS25, Line 133: pmc PMC
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#26).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST=built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 5 files changed, 422 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/26
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 26:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 157: malloc
a local variable is sufficient here
Done
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 245: malloc
a local variable is sufficient here.
Done
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/ea... PS25, Line 249: mux_data.usb
good point here... […]
Done
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/fs... PS25, Line 376: Calling
THe whole "calling ... […]
will remove and update
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/25/src/soc/intel/tigerlake/in... PS25, Line 133: pmc
PMC
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 26:
build failure is not your fault, the tree was broken at the time you uploaded, but it was fixed, see CB:47426. A rebase should fix it
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 26: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/fs... PS26, Line 391: vboot_developer_mode_enabled())) indented, like: ``` if (CONFIG(EARLY_TCSS) && (vboot_recovery_mode_enabled() || vboot_developer_mode_enabled()))
```
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... PS26, Line 131: /* Weak mainboard method to setup any mux configuration needed for early tcss operations nit: ``` /* * Weak mainboard method to setup... ``` with the blank `/*` is coreboot's comment style
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... PS26, Line 131: /* Weak mainboard method to setup any mux configuration needed for early tcss operations : * this function ...TCSS operations. This function will...
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... PS26, Line 135: tcss_mux : * struct `struct tcss_mux`
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#27).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both USB and Display Port.
BUG=b:151731851 BRANCH=NONE TEST=built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 5 files changed, 423 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/27
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 27:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/fs... PS26, Line 391: vboot_developer_mode_enabled()))
indented, like: […]
Done
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... PS26, Line 131: /* Weak mainboard method to setup any mux configuration needed for early tcss operations
nit: […]
Done
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... PS26, Line 131: /* Weak mainboard method to setup any mux configuration needed for early tcss operations : * this function
...TCSS operations. This function will...
Done
https://review.coreboot.org/c/coreboot/+/42079/26/src/soc/intel/tigerlake/in... PS26, Line 135: tcss_mux : * struct
`struct tcss_mux`
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 27:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/Kc... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/Kc... PS27, Line 238: Enable devices to be detected over Type-C ports during boot. This is not completely right. EARLY_TCSS is required only if platform uses external display over Type-C and requires PMC MUX for the port configured correctly before graphics initialization in FSP.
For all other devices over Type-C, it is the responsibility of the payload to configure the MUX as required.
This Kconfig should also add "depends on RUN_FSP_GOP" because that is the use case we are enabling right now.
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... PS27, Line 390: if (CONFIG(EARLY_TCSS) && (vboot_recovery_mode_enabled() || : vboot_developer_mode_enabled())) : mainboard_early_tcss_enable(); Couple of things that I find weird here: 1. `mainboard_early_tcss_enable()` makes a call into mainboard and is expected to call back into SoC. I think it should be possible to simply get required data from mainboard and use that to do the mux configuration.
2. `vboot_recovery_mode_enabled()` and `vboot_developer_mode_enabled()` return 0 if VBOOT is not being used. Even though this is currently used only on Chrome OS platforms, there is nothing stopping non-Chrome OS platforms from using the early tcss feature.
Given that, I think we should organize it as follows:
if (CONFIG(EARLY_TCSS)) tcss_early_configure();
`tcss_early_configure()` will be provided by early_tcss.c and it will take the following actions:
1. if (!display_init_required()) return (i.e. no TCSS mux configuration). `display_init_required()` returns true for non-VBOOT platforms always and returns true for VBOOT platforms only when display needs to be initialized.
2. If `tcss_requires_configuration()` returns true, then call `mainboard_get_pd_mux_info()`. Mainboard can return a table of PD ports that need PMC MUX configuration.
3. Call `update_tcss_mux()` for each entry in the table provided by mainboard.
With this, the mainboard is only responsible for providing a table and all the logic for making calls to configure mux stays within early_tcss driver.
Also, it will be good to add a comment for `tcss_early_configure()` explaining why early TCSS configuration is required and for what platforms it matters. (GOP doing graphics initialization, requires PMC mux to be configured for external display before the initialization, etc.)
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 27:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... PS27, Line 390: if (CONFIG(EARLY_TCSS) && (vboot_recovery_mode_enabled() || : vboot_developer_mode_enabled())) : mainboard_early_tcss_enable();
Couple of things that I find weird here: […]
I will look into making a patch on top of this one that addresses these concerns. In the future it would be nice to get these requests sooner than when the patch is +2 this patch has reached +2 many times and then gets some new request for changes (as you can see it's been up since June)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 27:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... PS27, Line 390: if (CONFIG(EARLY_TCSS) && (vboot_recovery_mode_enabled() || : vboot_developer_mode_enabled())) : mainboard_early_tcss_enable();
I will look into making a patch on top of this one that addresses these concerns. […]
Understood. Apologize for the long review cycle here. I think it's okay if you want to do this in a follow-up change. I would highly recommend updating the commit message and Kconfig to make it clear that this is only for performing graphics initialization for external monitors over Type-C. Currently, the way it is written, it seems to indicate that early TCSS configuration is done for external monitors as well as other devices over type-C.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 27:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... PS27, Line 390: if (CONFIG(EARLY_TCSS) && (vboot_recovery_mode_enabled() || : vboot_developer_mode_enabled())) : mainboard_early_tcss_enable();
Understood. Apologize for the long review cycle here. […]
Sure I will update the commit message and Kconfig message
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42079
to look at the new patch set (#28).
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C idisplays to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both Displays.
BUG=b:151731851 BRANCH=NONE TEST=built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 5 files changed, 424 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/42079/28
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 28: Code-Review+1
Thanks Brandon. +1 to let Tim +2 and move this forward.
Let's please follow-up with the pending comments in https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs.... Thanks!
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/fs... PS27, Line 390: if (CONFIG(EARLY_TCSS) && (vboot_recovery_mode_enabled() || : vboot_developer_mode_enabled())) : mainboard_early_tcss_enable();
Sure I will update the commit message and Kconfig message
I'll have a follow up patch coming soon want to do some initial testing with it and get the correct recipe for determining if the system supports vboot
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 28: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/Kc... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/42079/27/src/soc/intel/tigerlake/Kc... PS27, Line 238: Enable devices to be detected over Type-C ports during boot.
This is not completely right. […]
Ack
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
soc/intel/tigerlake: Add code for early tcss
In order for USB Type-C idisplays to be detected prior to loading Kernel PMC IPC driver is needed to communicate with PMC in order to correctly set the USB Mux settings. This patch is adding in support for early detection of both Displays.
BUG=b:151731851 BRANCH=NONE TEST=built and verified that TCSS MUX is being set on Volteer
Change-Id: I58e66f21210d565fb8145d140d2fc7febecdd21a Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42079 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/include/soc/early_tcss.h 5 files changed, 424 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index f1ac774..507f871 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -232,4 +232,11 @@ config PRERAM_CBMEM_CONSOLE_SIZE hex default 0x2000 + +config EARLY_TCSS_DISPLAY + bool "Enable early TCSS display" + depends on RUN_FSP_GOP + help + Enable displays to be detected over Type-C ports during boot. + endif diff --git a/src/soc/intel/tigerlake/Makefile.inc b/src/soc/intel/tigerlake/Makefile.inc index 3103d60..4a41812 100644 --- a/src/soc/intel/tigerlake/Makefile.inc +++ b/src/soc/intel/tigerlake/Makefile.inc @@ -31,6 +31,7 @@ ramstage-y += acpi.c ramstage-y += chip.c ramstage-y += cpu.c +ramstage-$(CONFIG_EARLY_TCSS_DISPLAY) += early_tcss.c ramstage-y += elog.c ramstage-y += espi.c ramstage-y += finalize.c diff --git a/src/soc/intel/tigerlake/early_tcss.c b/src/soc/intel/tigerlake/early_tcss.c new file mode 100644 index 0000000..3944f61 --- /dev/null +++ b/src/soc/intel/tigerlake/early_tcss.c @@ -0,0 +1,271 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <console/console.h> +#include <device/pci.h> +#include <intelblocks/pmc_ipc.h> +#include <soc/early_tcss.h> +#include <soc/pci_devs.h> +#include <stdlib.h> +#include <inttypes.h> + + +static uint32_t tcss_make_conn_cmd(int u, int u3, int u2, int ufp, int hsl, + int sbu, int acc) +{ + return TCSS_CD_FIELD(USAGE, u) | + TCSS_CD_FIELD(USB3, u3) | + TCSS_CD_FIELD(USB2, u2) | + TCSS_CD_FIELD(UFP, ufp) | + TCSS_CD_FIELD(HSL, hsl) | + TCSS_CD_FIELD(SBU, sbu) | + TCSS_CD_FIELD(ACC, acc); +} + +static uint32_t tcss_make_alt_mode_cmd_buf_0(int u, int u3, int m) +{ + return TCSS_ALT_FIELD(USAGE, u) | + TCSS_ALT_FIELD(USB3, u3) | + TCSS_ALT_FIELD(MODE, m); + +} + +static uint32_t tcss_make_alt_mode_cmd_buf_1(int p, int c, int ufp, int dp) +{ + return TCSS_ALT_FIELD(POLARITY, p) | + TCSS_ALT_FIELD(CABLE, c) | + TCSS_ALT_FIELD(UFP, ufp) | + TCSS_ALT_FIELD(DP_MODE, dp); +} + +static uint32_t tcss_make_safe_mode_cmd(int u, int u3) +{ + return TCSS_CD_FIELD(USAGE, u) | + TCSS_CD_FIELD(USB3, u3); +} + + +static uint32_t tcss_make_hpd_mode_cmd(int u, int u3, int hpd_lvl, int hpd_irq) +{ + return TCSS_HPD_FIELD(USAGE, u) | + TCSS_HPD_FIELD(USB3, u3) | + TCSS_HPD_FIELD(LVL, hpd_lvl) | + TCSS_HPD_FIELD(IRQ, hpd_irq); + +} + +static int send_pmc_req(int cmd_type, const struct pmc_ipc_buffer *req, + struct pmc_ipc_buffer *res, uint32_t size) +{ + + uint32_t cmd_reg; + uint32_t res_reg; + int tries = 2; + int r; + + cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, + size); + + printk(BIOS_DEBUG, "Raw Buffer output 0 %08" PRIx32 "\n", req->buf[0]); + printk(BIOS_DEBUG, "Raw Buffer output 1 %08" PRIx32 "\n", req->buf[1]); + + do { + r = pmc_send_ipc_cmd(cmd_reg, req, res); + if (r < 0) { + printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); + return -1; + } + + res_reg = res->buf[0]; + if (cmd_type == CONNECT_REQ) { + if (!TCSS_CONN_STATUS_HAS_FAILED(res_reg)) { + printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); + return 0; + } + + if (TCSS_CONN_STATUS_IS_FATAL(res_reg)) { + printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); + return -1; + } + } else { + if (!TCSS_STATUS_HAS_FAILED(res_reg)) { + printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); + return 0; + } + + if (TCSS_STATUS_IS_FATAL(res_reg)) { + printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); + return -1; + } + } + } while (--tries >= 0); + + printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); + return -1; +} + +static int send_pmc_connect_request(int port, struct tcss_mux mux_data, + struct pmc_ipc_buffer *res) +{ + uint32_t cmd; + struct pmc_ipc_buffer req = { 0 }; + + cmd = tcss_make_conn_cmd( + PMC_IPC_TCSS_CONN_REQ_RES, + mux_data.usb3_port, + mux_data.usb2_port, + mux_data.ufp, + mux_data.polarity, + mux_data.polarity, + mux_data.acc); + + req.buf[0] = cmd; + + printk(BIOS_DEBUG, "port C%d CONN req: usage %d usb3 %d usb2 %d " + "ufp %d ori_hsl %d ori_sbu %d dbg_acc %d\n", + port, + GET_TCSS_CD_FIELD(USAGE, cmd), + GET_TCSS_CD_FIELD(USB3, cmd), + GET_TCSS_CD_FIELD(USB2, cmd), + GET_TCSS_CD_FIELD(UFP, cmd), + GET_TCSS_CD_FIELD(HSL, cmd), + GET_TCSS_CD_FIELD(SBU, cmd), + GET_TCSS_CD_FIELD(ACC, cmd)); + + return send_pmc_req(CONNECT_REQ, &req, res, PMC_IPC_CONN_REQ_SIZE); +} + +static int send_pmc_safe_mode_request(int port, struct tcss_mux mux_data, + struct pmc_ipc_buffer *res) +{ + uint32_t cmd; + struct pmc_ipc_buffer req = { 0 }; + + cmd = tcss_make_safe_mode_cmd(PMC_IPC_TCSS_SAFE_MODE_REQ_RES, mux_data.usb3_port); + + req.buf[0] = cmd; + + printk(BIOS_DEBUG, "port C%d SAFE req: usage %d usb3 %d\n", + port, + GET_TCSS_CD_FIELD(USAGE, cmd), + GET_TCSS_CD_FIELD(USB3, cmd)); + + return send_pmc_req(SAFE_REQ, &req, res, PMC_IPC_SAFE_REQ_SIZE); +} + +static int send_pmc_dp_hpd_request(int port, struct tcss_mux mux_data) +{ + struct pmc_ipc_buffer *res = NULL; + struct pmc_ipc_buffer req = { 0 }; + uint32_t cmd; + + cmd = tcss_make_hpd_mode_cmd( + PMC_IPC_TCSS_HPD_REQ_RES, + mux_data.usb3_port, + mux_data.hpd_lvl, + mux_data.hpd_irq); + + req.buf[0] = cmd; + + return send_pmc_req(HPD_REQ, &req, res, PMC_IPC_HPD_REQ_SIZE); + +} + +static int send_pmc_dp_mode_request(int port, struct tcss_mux mux_data, + struct pmc_ipc_buffer *res) +{ + uint32_t cmd; + uint8_t dp_mode; + int ret; + + struct pmc_ipc_buffer req = { 0 }; + + cmd = tcss_make_alt_mode_cmd_buf_0( + PMC_IPC_TCSS_ALTMODE_REQ_RES, + mux_data.usb3_port, + PMC_IPC_DP_MODE); + + req.buf[0] = cmd; + + printk(BIOS_DEBUG, "port C%d ALT_1 req: usage %d usb3 %d dp_mode %d\n", + port, + GET_TCSS_ALT_FIELD(USAGE, cmd), + GET_TCSS_ALT_FIELD(USB3, cmd), + GET_TCSS_ALT_FIELD(MODE, cmd)); + + switch (mux_data.dp_mode) { + case MODE_DP_PIN_A: + dp_mode = 1; + break; + case MODE_DP_PIN_B: + dp_mode = 2; + break; + case MODE_DP_PIN_C: + dp_mode = 3; + break; + case MODE_DP_PIN_D: + dp_mode = 4; + break; + case MODE_DP_PIN_E: + dp_mode = 5; + break; + case MODE_DP_PIN_F: + dp_mode = 6; + break; + default: + dp_mode = 0; + break; + } + + cmd = tcss_make_alt_mode_cmd_buf_1( + mux_data.polarity, + mux_data.cable, + 0, /* ufp is not supported in DP ALT Mode request */ + dp_mode); + + printk(BIOS_DEBUG, "port C%d ALT_2 req: polarity %d cable %d ufp %d " + "dp_mode %d\n", + port, + GET_TCSS_ALT_FIELD(POLARITY, cmd), + GET_TCSS_ALT_FIELD(CABLE, cmd), + GET_TCSS_ALT_FIELD(UFP, cmd), + GET_TCSS_ALT_FIELD(DP_MODE, cmd)); + + req.buf[1] = cmd; + + ret = send_pmc_req(DP_REQ, &req, res, PMC_IPC_ALT_REQ_SIZE); + if (ret) + return ret; + + send_pmc_dp_hpd_request(port, mux_data); + return 0; +} + +void update_tcss_mux(int port, struct tcss_mux mux_data) +{ + struct pmc_ipc_buffer *rbuf = NULL; + int ret = 0; + + /* check if mux has a DP device */ + if (mux_data.dp) { + ret = send_pmc_connect_request(port, mux_data, rbuf); + if (ret) { + printk(BIOS_ERR, "Port %d connect request failed\n", port); + return; + } + ret = send_pmc_safe_mode_request(port, mux_data, rbuf); + if (ret) { + printk(BIOS_ERR, "Port %d safe mode request failed\n", port); + return; + } + + ret = send_pmc_dp_mode_request(port, mux_data, rbuf); + } + + if (ret) + printk(BIOS_ERR, "Port C%d mux set failed with error %d\n", port, ret); +} + +__weak void mainboard_early_tcss_enable(void) +{ + /* to be overwritten by each mainboard that needs early tcss */ +} diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index 3427a61..6de098a 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -14,6 +14,7 @@ #include <intelblocks/xdci.h> #include <intelpch/lockdown.h> #include <security/vboot/vboot_common.h> +#include <soc/early_tcss.h> #include <soc/gpio_soc_defs.h> #include <soc/intel/common/vbt.h> #include <soc/pci_devs.h> @@ -384,6 +385,11 @@ switch (phase_index) { case 1: /* TCSS specific initialization here */ + printk(BIOS_DEBUG, "FSP MultiPhaseSiInit %s/%s called\n", + __FILE__, __func__); + if (CONFIG(EARLY_TCSS_DISPLAY) && (vboot_recovery_mode_enabled() || + vboot_developer_mode_enabled())) + mainboard_early_tcss_enable(); break; default: break; diff --git a/src/soc/intel/tigerlake/include/soc/early_tcss.h b/src/soc/intel/tigerlake/include/soc/early_tcss.h new file mode 100644 index 0000000..c009e84 --- /dev/null +++ b/src/soc/intel/tigerlake/include/soc/early_tcss.h @@ -0,0 +1,139 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* PMC IPC related offsets and commands */ +#define PMC_IPC_USBC_CMD_ID 0xA7 +#define PMC_IPC_USBC_SUBCMD_ID 0x0 +#define PMC_IPC_CMD 0x0 +#define PMC_IPC_TCSS_CONN_REQ_RES 0x0 +#define PMC_IPC_TCSS_SAFE_MODE_REQ_RES 0x2 +#define PMC_IPC_TCSS_ALTMODE_REQ_RES 0x3 +#define PMC_IPC_TCSS_HPD_REQ_RES 0x4 +#define PMC_IPC_CONN_REQ_SIZE 2 +#define PMC_IPC_ALT_REQ_SIZE 8 +#define PMC_IPC_SAFE_REQ_SIZE 1 +#define PMC_IPC_HPD_REQ_SIZE 2 +#define PMC_IPC_DP_MODE 1 + +#define TCSS_CD_USAGE_SHIFT 0 +#define TCSS_CD_USAGE_MASK 0x0f +#define TCSS_CD_USB3_SHIFT 4 +#define TCSS_CD_USB3_MASK 0x0f +#define TCSS_CD_USB2_SHIFT 8 +#define TCSS_CD_USB2_MASK 0x0f +#define TCSS_CD_UFP_SHIFT 12 +#define TCSS_CD_UFP_MASK 0x01 +#define TCSS_CD_HSL_SHIFT 13 +#define TCSS_CD_HSL_MASK 0x01 +#define TCSS_CD_SBU_SHIFT 14 +#define TCSS_CD_SBU_MASK 0x01 +#define TCSS_CD_ACC_SHIFT 15 +#define TCSS_CD_ACC_MASK 0x01 +#define TCSS_CD_FAILED_SHIFT 16 +#define TCSS_CD_FAILED_MASK 0x01 +#define TCSS_CD_FATAL_SHIFT 17 +#define TCSS_CD_FATAL_MASK 0x01 + +#define TCSS_ALT_USAGE_SHIFT 0 +#define TCSS_ALT_USAGE_MASK 0x0f +#define TCSS_ALT_USB3_SHIFT 4 +#define TCSS_ALT_USB3_MASK 0x0f +#define TCSS_ALT_MODE_SHIFT 12 +#define TCSS_ALT_MODE_MASK 0x0f +#define TCSS_ALT_POLARITY_SHIFT 1 +#define TCSS_ALT_POLARITY_MASK 0x01 +#define TCSS_ALT_CABLE_SHIFT 2 +#define TCSS_ALT_CABLE_MASK 0x01 +#define TCSS_ALT_UFP_SHIFT 3 +#define TCSS_ALT_UFP_MASK 0x01 +#define TCSS_ALT_DP_MODE_SHIFT 8 +#define TCSS_ALT_DP_MODE_MASK 0x0f +#define TCSS_ALT_FAILED_SHIFT 8 +#define TCSS_ALT_FAILED_MASK 0x01 +#define TCSS_ALT_FATAL_SHIFT 9 +#define TCSS_ALT_FATAL_MASK 0x01 + +#define TCSS_HPD_USAGE_SHIFT 0 +#define TCSS_HPD_USAGE_MASK 0x0f +#define TCSS_HPD_USB3_SHIFT 4 +#define TCSS_HPD_USB3_MASK 0x0f +#define TCSS_HPD_LVL_SHIFT 12 +#define TCSS_HPD_LVL_MASK 0x01 +#define TCSS_HPD_IRQ_SHIFT 13 +#define TCSS_HPD_IRQ_MASK 0x01 + +#define TCSS_CD_FIELD(name, val) \ + (((val) & TCSS_CD_##name##_MASK) << TCSS_CD_##name##_SHIFT) + +#define GET_TCSS_CD_FIELD(name, val) \ + (((val) >> TCSS_CD_##name##_SHIFT) & TCSS_CD_##name##_MASK) + + +#define TCSS_ALT_FIELD(name, val) \ + (((val) & TCSS_ALT_##name##_MASK) << TCSS_ALT_##name##_SHIFT) + +#define TCSS_HPD_FIELD(name, val) \ + (((val) & TCSS_HPD_##name##_MASK) << TCSS_HPD_##name##_SHIFT) + +#define GET_TCSS_ALT_FIELD(name, val) \ + (((val) >> TCSS_ALT_##name##_SHIFT) & TCSS_ALT_##name##_MASK) + +#define TCSS_CONN_STATUS_HAS_FAILED(s) GET_TCSS_CD_FIELD(FAILED, s) +#define TCSS_STATUS_HAS_FAILED(s) GET_TCSS_ALT_FIELD(FAILED, s) +/* !fatal means retry */ +#define TCSS_CONN_STATUS_IS_FATAL(s) GET_TCSS_CD_FIELD(FATAL, s) +#define TCSS_STATUS_IS_FATAL(s) GET_TCSS_ALT_FIELD(FATAL, s) + +#define USB_2_PORT_MASK 0x0f +#define USB_3_PORT_MASK 0xf0 + +/* TCSS connection modes for PMC */ +enum pmc_ipc_conn_mode { + PMC_IPC_TCSS_DISCONNECT_MODE, + PMC_IPC_TCSS_USB_MODE, + PMC_IPC_TCSS_ALTERNATE_MODE, + PMC_IPC_TCSS_SAFE_MODE, + PMC_IPC_TCSS_HPD_MODE, + PMC_IPC_TCSS_TOTAL_MODES, +}; + +enum pmc_ipc_command_type { + CONNECT_REQ, + SAFE_REQ, + DP_REQ, + HPD_REQ, +}; + +/* DP Mode pin definitions */ +#define MODE_DP_PIN_A BIT(0) +#define MODE_DP_PIN_B BIT(1) +#define MODE_DP_PIN_C BIT(2) +#define MODE_DP_PIN_D BIT(3) +#define MODE_DP_PIN_E BIT(4) +#define MODE_DP_PIN_F BIT(5) + +/* struct to hold all tcss_mux related variables */ +struct tcss_mux { + bool dp; /* DP connected */ + bool usb; /* USB connected */ + bool cable; /* Activ/Passive Cable */ + bool polarity; /* polarity of connected device */ + bool hpd_lvl; /* HPD Level assert */ + bool hpd_irq; /* HPD IRQ assert */ + bool ufp; + bool acc; + uint8_t dp_mode; /* DP Operation Mode */ + uint8_t usb3_port; /* USB2 Port Number */ + uint8_t usb2_port; /* USB3 Port Number */ +}; + +void update_tcss_mux(int port, struct tcss_mux mux_data); + +/* + * Weak mainboard method to setup any mux configuration needed for early TCSS operations. + * This function will need to obtain any mux data needed to forward to IOM/PMC and call + * the update_tcss_mux method which will call any PMC commands needed to connect the + * ports. Since the mux data may be stored differently by different mainboards this + * must be overridden by the mainboard with its specific mux data stored in a struct tcss_mux + * struct as defined above. + */ +void mainboard_early_tcss_enable(void);
Attention is currently required from: Brandon Breitenstein. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 29:
(1 comment)
File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/comment/067e5655_6877da02 PS29, Line 391: vboot_developer_mode_enabled())) Sorry, just found this randomly reading through how vboot_developer_mode_enabled() is used in coreboot... shouldn't this be using the display_init_required()/VBOOT_MUST_REQUEST_DISPLAY facility instead? This patch sounds like this code is required for using the display in firmware, but (rec_mode || dev_mode) is not the correct check for that. There are other situations (e.g. TCPC update) that use the display, that's why we have the display_init_required() facility to encapsulate that.
Attention is currently required from: Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 29:
(1 comment)
File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/comment/0cd9a8d6_fc15098e PS29, Line 391: vboot_developer_mode_enabled()))
Sorry, just found this randomly reading through how vboot_developer_mode_enabled() is used in corebo […]
Fixed as part of https://review.coreboot.org/c/coreboot/+/47684/7/src/soc/intel/tigerlake/ear...
-
Brandon Breitenstein (Code Review) -
build bot (Jenkins) (Code Review)
-
Caveh Jalali (Code Review)
-
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Julius Werner (Code Review)
-
Tim Wawrzynczak (Code Review)