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.)