Change in coreboot[master]: soc/tigerlake: Update early TCSS code to not call back and forth betw...
Brandon Breitenstein has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code. This also adds a new check for early display support which removes the requirement for the system to be a vboot compatible system before executing.
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 36 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/1
diff --git a/src/soc/intel/tigerlake/early_tcss.c b/src/soc/intel/tigerlake/early_tcss.c index 3944f61..8616bb1 100644 --- a/src/soc/intel/tigerlake/early_tcss.c +++ b/src/soc/intel/tigerlake/early_tcss.c @@ -3,6 +3,7 @@ #include <console/console.h> #include <device/pci.h> #include <intelblocks/pmc_ipc.h> +#include <security/vboot/vboot_common.h> #include <soc/early_tcss.h> #include <soc/pci_devs.h> #include <stdlib.h> @@ -240,7 +241,7 @@ return 0; }
-void update_tcss_mux(int port, struct tcss_mux mux_data) +static void update_tcss_mux(int port, struct tcss_mux mux_data) { struct pmc_ipc_buffer *rbuf = NULL; int ret = 0; @@ -265,7 +266,34 @@ printk(BIOS_ERR, "Port C%d mux set failed with error %d\n", port, ret); }
-__weak void mainboard_early_tcss_enable(void) +static int system_requires_early_display(void) +{ + if (!CONFIG(VBOOT)) + return 1; + else if (vboot_recovery_mode_enabled() || vboot_developer_mode_enabled()) + return 1; + else + return 0; +} + +void tcss_early_configure(void) +{ + + struct tcss_mux *mux_info = NULL; + unsigned int num_ports; + int i; + + if (!system_requires_early_display()) + return; + + mainboard_early_tcss_enable(mux_info, &num_ports); + + for (i = 0; i < num_ports; i++) + update_tcss_mux(i, mux_info[i]); + +} + +__weak void mainboard_early_tcss_enable(struct tcss_mux *mux_info, unsigned int *num_ports) { /* 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 6de098a..2bec43e 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -387,9 +387,8 @@ /* 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(); + if (CONFIG(EARLY_TCSS_DISPLAY)) + tcss_early_configure(); 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 index c009e84..cee4c12 100644 --- a/src/soc/intel/tigerlake/include/soc/early_tcss.h +++ b/src/soc/intel/tigerlake/include/soc/early_tcss.h @@ -126,7 +126,9 @@ uint8_t usb2_port; /* USB3 Port Number */ };
-void update_tcss_mux(int port, struct tcss_mux mux_data); +//void update_tcss_mux(int port, struct tcss_mux mux_data); + +void tcss_early_configure(void);
/* * Weak mainboard method to setup any mux configuration needed for early TCSS operations. @@ -136,4 +138,4 @@ * 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); +void mainboard_early_tcss_enable(struct tcss_mux *mux_info, unsigned int *num_ports);
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/ear... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/ear... PS1, Line 281: extra line
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/inc... PS1, Line 141: mainboard_early_tcss_enable Could this return num_ports instead of passing it by reference?
Also I wonder if maybe a name like `mainboard_fill_tcss_mux_info` would be little more clear? WDYT?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/inc... PS1, Line 141: mainboard_early_tcss_enable
Could this return num_ports instead of passing it by reference? […]
Yeah I think both of those should work fine I'll change this up to do that.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 1:
(2 comments)
Thanks for following up on this Brandon!
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/ear... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/ear... PS1, Line 286: system_requires_early_display You can replace this with if (!display_init_required()). It does exactly what system_requires_early_display() is doing but using vboot flags that requests display init.
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/1/src/soc/intel/tigerlake/inc... PS1, Line 129: //void update_tcss_mux(int port, struct tcss_mux mux_data); Drop this instead of commenting.
Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47684
to look at the new patch set (#2).
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code. This also adds a new check for early display support which removes the requirement for the system to be a vboot compatible system before executing.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still behaves appropriately
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 36 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/2
Attention is currently required from: Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 2:
(5 comments)
Patchset:
PS2: Comments from patchset#1 still need to be addressed.
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/97c941c2_64d6a1cc PS2, Line 286: system_requires_early_display As commented earlier, you can use `display_init_required()` here.
https://review.coreboot.org/c/coreboot/+/47684/comment/cc39b9d5_7b60a259 PS2, Line 296: __weak I don't think we need the weak implementation. If a mainboard selects EARLY_TCSS then it must provide this function definition.
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/351e5ed0_a31ac184 PS2, Line 129: //void update_tcss_mux(int port, struct tcss_mux mux_data); Drop this instead of commenting out.
https://review.coreboot.org/c/coreboot/+/47684/comment/39471734_50a7ee1f PS2, Line 141: mainboard_early_tcss_enable I think this should be named `mainboard_fill_tcss_mux_info()` since the enabling is performed by SoC code.
Attention is currently required from: Furquan Shaikh. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 2:
(4 comments)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/8f056cd5_53c824b4 PS2, Line 286: system_requires_early_display
As commented earlier, you can use `display_init_required()` here.
will test out with this in next patchset
https://review.coreboot.org/c/coreboot/+/47684/comment/3bf793a1_5cebde10 PS2, Line 296: __weak
I don't think we need the weak implementation. […]
I left a weak here in case it doesn't get implemented and it didn't have a check for display supported before will remove the weak in next patchset
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/bd3dd9d6_27fb2d5f PS2, Line 129: //void update_tcss_mux(int port, struct tcss_mux mux_data);
Drop this instead of commenting out.
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/1fc005ad_80a6d850 PS2, Line 141: mainboard_early_tcss_enable
I think this should be named `mainboard_fill_tcss_mux_info()` since the enabling is performed by SoC […]
I accidentally added this in the next patch instead of updating it here will move that
Attention is currently required from: Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/ff75bf79_fcf89826 PS2, Line 141: mainboard_early_tcss_enable
I accidentally added this in the next patch instead of updating it here will move that
Sounds good. BTW, I think the signature should be something like:
const struct tcss_mux *mainboard_get_tcss_mux_info(size_t *num_ports);
That way, it is clear that the mainboard is supposed to manage the space for the mux table. In the follow-up CLs, it looks like mainboard code is assuming SoC passes in a valid pointer and writes to NULL address.
Attention is currently required from: Furquan Shaikh. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/266baec5_b643557b PS2, Line 141: mainboard_early_tcss_enable
Sounds good. BTW, I think the signature should be something like: […]
that signature makes sense I will update to that over passing the struct by reference
Attention is currently required from: Furquan Shaikh. Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47684
to look at the new patch set (#3).
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code. This also adds a new check for early display support which removes the requirement for the system to be a vboot compatible system before executing.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still behaves appropriately
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 37 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/3
Attention is currently required from: Furquan Shaikh. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 3:
(2 comments)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/4b78e14e_8d2cecb8 PS2, Line 286: system_requires_early_display
will test out with this in next patchset
Done
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/ddc85afe_f7506972 PS2, Line 141: mainboard_early_tcss_enable
that signature makes sense I will update to that over passing the struct by reference
Done
Attention is currently required from: Furquan Shaikh. Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47684
to look at the new patch set (#4).
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code. This also adds a new check for early display support which removes the requirement for the system to be a vboot compatible system before executing.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still behaves appropriately
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 37 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/4
Attention is currently required from: Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 4:
(3 comments)
Patchset:
PS4: I see some comments from earlier patchsets are still not addressed. I will wait for you to go through those and update the CL before reviewing it.
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/fd7c18e6_2395afd2 PS2, Line 286: system_requires_early_display
Done
I don't think this is addressed in latest patchset?
https://review.coreboot.org/c/coreboot/+/47684/comment/29367e64_c9237b79 PS2, Line 296: __weak
I left a weak here in case it doesn't get implemented and it didn't have a check for display support […]
If mainboard selects the respective Kconfig, it should implement the required callback. If it does not, then the build should fail so that the mainboard developer fixes it.
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 4:
(6 comments)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/7ea58017_70729eb8 PS1, Line 281:
extra line
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/84122912_098f4e57 PS1, Line 286: system_requires_early_display
You can replace this with if (!display_init_required()). […]
Done
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/6fcaad20_c567716b PS2, Line 286: system_requires_early_display
I don't think this is addressed in latest patchset?
Oops I added this to the next patch instead of this one I'll move it to this one if that matters
https://review.coreboot.org/c/coreboot/+/47684/comment/d64bbb20_aaa09ce4 PS2, Line 296: __weak
If mainboard selects the respective Kconfig, it should implement the required callback. […]
wouldn't the build fail if EARLY_TCSS_DISPLAY is not selected but EARLY_TCSS is without weak functions?
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/0c802f69_22281570 PS1, Line 129: //void update_tcss_mux(int port, struct tcss_mux mux_data);
Drop this instead of commenting.
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/c96e3c75_a13cbe86 PS1, Line 141: mainboard_early_tcss_enable
Yeah I think both of those should work fine I'll change this up to do that.
Done
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 4:
(2 comments)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/5fac38e6_9960a1cb PS2, Line 286: system_requires_early_display
Oops I added this to the next patch instead of this one I'll move it to this one if that matters
Yes, let's please move it to this CL. There is no use adding a function in this CL and immediately dropping it in the next. Adds to confusion later on when someone is reading these changes.
https://review.coreboot.org/c/coreboot/+/47684/comment/cc9ced03_8e5e83d2 PS2, Line 296: __weak
wouldn't the build fail if EARLY_TCSS_DISPLAY is not selected but EARLY_TCSS is without weak functio […]
If the call to the function is properly guarded with if (CONFIG(EARLY_TCSS_DISPLAY)) then there should not be a build failure.
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 4:
(1 comment)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/55ee0f14_65f8675d PS2, Line 296: __weak
If the call to the function is properly guarded with if (CONFIG(EARLY_TCSS_DISPLAY)) then there shou […]
This is what I am thinking:
```
static void tcss_configure_dp_mode(const struct tcss_port_map *port_map, size_t num_ports) { » int ret; » size_t i; » const struct tcss_port_map *port_info; » const struct tcss_mux_info *mux_info;
» if (!display_init_required()) » » return;
» for (i = 0; i < num_ports; i++) { » » mux_info = mb_get_tcss_mux_info(i); » » port_info = &port_map[i];
» » if (!mux_info->dp) » » » continue;
» » ret = send_pmc_connect_request(i, mux_info, port_info); » » if (ret) { » » » printk(BIOS_ERR, "Port %zd connect request failed\n", i); » » » continue; » » } » » ret = send_pmc_safe_mode_request(i, mux_info, port_info); » » if (ret) { » » » printk(BIOS_ERR, "Port %zd safe mode request failed\n", i); » » » continue; » » }
» » ret = send_pmc_dp_mode_request(i, mux_info, port_info); » » if (ret) » » » printk(BIOS_ERR, "Port C%zd mux set failed with error %d\n", i, ret); » } }
void tcss_early_configure(void) { » const struct tcss_port_map *port_map; » size_t num_ports;
» port_map = mb_get_tcss_port_map(&num_ports); » if (!port_map || !num_ports) » » return;
» if (CONFIG(TCSS_EARLY_DISPLAY)) » » tcss_configure_dp_mode(port_map, num_ports); }
```
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 4:
(2 comments)
Patchset:
PS4:
I see some comments from earlier patchsets are still not addressed. […]
I addressed most of them just hadn't gone back to mark them as such marked them appropriately
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/e27897cb_7e0155a4 PS2, Line 296: __weak
This is what I am thinking: […]
looks good to me I'll add the changes related to port map in the next patch but this flow looks good. I will also rename the functions in the next patch because this was doing the only updates prior to the new changes. Is that appropriate? This patch is only meant to remove calling back and forth so I don't want to unnecessarily complicate that
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 4:
(1 comment)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/8831b9bf_f3f376db PS2, Line 296: __weak
looks good to me I'll add the changes related to port map in the next patch but this flow looks good […]
SGTM.
Attention is currently required from: Tim Wawrzynczak. Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47684
to look at the new patch set (#5).
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code. This also adds a new check for early display support which removes the requirement for the system to be a vboot compatible system before executing.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still behaves appropriately
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 23 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/5
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 5:
(1 comment)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/45f6c983_0cc3fd3b PS2, Line 286: system_requires_early_display
Yes, let's please move it to this CL. […]
Done
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Update early TCSS code to not call back and forth between mainboard and soc ......................................................................
Patch Set 5:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/47684/comment/e6106c88_b02769a3 PS5, Line 9: The original implementation of early tcss resulted in calling to mainboard then back to : soc then back to mainboard to properly configure the muxes. This patch addresses that : issue and instead just gets all the mux information from mainboard and does all config : in the soc code. This also adds a new check for early display support which removes : the requirement for the system to be a vboot compatible system before executing. Commit message lines are limited to 75 characters: https://www.coreboot.org/Git#Commit_messages
Can you please fix this?
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/8c226980_807cb488 PS5, Line 2: #include <bootmode.h>
required for `display_init_required()`
https://review.coreboot.org/c/coreboot/+/47684/comment/27389b5b_b3acde2a PS5, Line 268: NULL Not required. This is set on line 275.
https://review.coreboot.org/c/coreboot/+/47684/comment/22270905_5d15d36f PS5, Line 268: struct const
as mainboard_tcss_fill_mux_info returns `const struct tcss_mux *`
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/c523dd93_760db8d1 PS5, Line 129: //void update_tcss_mux(int port, struct tcss_mux mux_data); Drop commented out code.
https://review.coreboot.org/c/coreboot/+/47684/comment/a397e6f0_b8027792 PS5, Line 134: 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. Comment needs update.
https://review.coreboot.org/c/coreboot/+/47684/comment/19865522_ca60061d PS5, Line 142: mainboard_tcssfill_mux_info This doesn't match the name in early_tcss.c: mainboard_tcss_fill_mux_info
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47684
to look at the new patch set (#6).
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still functions
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 26 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/6
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
Patch Set 6:
(6 comments)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/47684/comment/76fa83b7_9b67f4dd PS5, Line 2:
#include <bootmode.h> […]
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/e8bf01fc_e1ed403f PS5, Line 268: struct
const […]
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/031922e0_637289dc PS5, Line 268: NULL
Not required. This is set on line 275.
Done
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/bf33ca3d_55e08460 PS5, Line 129: //void update_tcss_mux(int port, struct tcss_mux mux_data);
Drop commented out code.
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/3da31381_c1edffa4 PS5, Line 134: 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.
Comment needs update.
Done
https://review.coreboot.org/c/coreboot/+/47684/comment/cf2049d7_77ae95c0 PS5, Line 142: mainboard_tcssfill_mux_info
This doesn't match the name in early_tcss. […]
Done
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/47684/comment/a4e30b4a_d349ed78 PS5, Line 9: The original implementation of early tcss resulted in calling to mainboard then back to : soc then back to mainboard to properly configure the muxes. This patch addresses that : issue and instead just gets all the mux information from mainboard and does all config : in the soc code. This also adds a new check for early display support which removes : the requirement for the system to be a vboot compatible system before executing.
Commit message lines are limited to 75 characters: https://www.coreboot.org/Git#Commit_messages […]
Done
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/5fc5fcb3_67c1f0a2 PS6, Line 139: mainboard_tcssfill_mux_info This is still not correct.
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
Patch Set 6:
(1 comment)
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/9a7f127f_f91e4e8b PS6, Line 139: mainboard_tcssfill_mux_info
This is still not correct.
I don't know how I keep missing to grab this change...I have it on the tree will fix
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47684
to look at the new patch set (#7).
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still functions
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 26 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47684/7
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
Patch Set 6:
(1 comment)
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/47684/comment/eed95d67_3882afd1 PS6, Line 139: mainboard_tcssfill_mux_info
I don't know how I keep missing to grab this change... […]
Done
Attention is currently required from: Tim Wawrzynczak, Brandon Breitenstein. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
Patch Set 7: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47684 )
Change subject: soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc ......................................................................
soc/tigerlake: Fix TCSS code to calling back and forth to mainboard and soc
The original implementation of early tcss resulted in calling to mainboard then back to soc then back to mainboard to properly configure the muxes. This patch addresses that issue and instead just gets all the mux information from mainboard and does all config in the soc code.
BUG=none BRANCH=firmware-volteer-13672.B TEST=Verified functionality is not effected and early TCSS still functions
Change-Id: Idd50b0ffe1d56dffc3698e07c6e4bc4540d45e73 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47684 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/tigerlake/early_tcss.c M src/soc/intel/tigerlake/fsp_params.c M src/soc/intel/tigerlake/include/soc/early_tcss.h 3 files changed, 26 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/early_tcss.c b/src/soc/intel/tigerlake/early_tcss.c index a2810b5..cdc90ad 100644 --- a/src/soc/intel/tigerlake/early_tcss.c +++ b/src/soc/intel/tigerlake/early_tcss.c @@ -1,8 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <bootmode.h> #include <console/console.h> #include <device/pci.h> #include <intelblocks/pmc_ipc.h> +#include <security/vboot/vboot_common.h> #include <soc/early_tcss.h> #include <soc/pci_devs.h> #include <stdlib.h> @@ -237,7 +239,7 @@ return 0; }
-void update_tcss_mux(int port, struct tcss_mux mux_data) +static void update_tcss_mux(int port, struct tcss_mux mux_data) { int ret = 0;
@@ -261,7 +263,18 @@ printk(BIOS_ERR, "Port C%d mux set failed with error %d\n", port, ret); }
-__weak void mainboard_early_tcss_enable(void) +void tcss_early_configure(void) { - /* to be overwritten by each mainboard that needs early tcss */ + const struct tcss_mux *mux_info; + size_t num_ports; + int i; + + if (!display_init_required()) + return; + + mux_info = mainboard_tcss_fill_mux_info(&num_ports); + + for (i = 0; i < num_ports; i++) + update_tcss_mux(i, mux_info[i]); + } diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index 3e440aa..c5501b7 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -462,9 +462,8 @@ /* 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(); + if (CONFIG(EARLY_TCSS_DISPLAY)) + tcss_early_configure(); 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 index c009e84..8b5d802 100644 --- a/src/soc/intel/tigerlake/include/soc/early_tcss.h +++ b/src/soc/intel/tigerlake/include/soc/early_tcss.h @@ -126,14 +126,14 @@ uint8_t usb2_port; /* USB3 Port Number */ };
-void update_tcss_mux(int port, struct tcss_mux mux_data); +void tcss_early_configure(void);
/* - * 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. + * 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. + * Since the mux data may be stored differently by different mainboards this + * must be defined by the mainboard with its specific mux data stored in a struct tcss_mux + * as defined above. + * Returns completed tcss_mux structure */ -void mainboard_early_tcss_enable(void); +const struct tcss_mux *mainboard_tcss_fill_mux_info(size_t *num_ports);
-
Brandon Breitenstein (Code Review) -
Furquan Shaikh (Code Review)
-
Tim Wawrzynczak (Code Review)