Change in coreboot[master]: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs

[John Zhao (Code Review)]

John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs

The Connect Topology Command(CNTP) is sent with default timeout value
(0x1388) along with FW CM. The CNTP is supposed to be skipped while
using SW CM. While transition from FW CM to SW CM, the default timeout
value could cause boot time delay up to ~10 seconds. Set this FSPS UPD
ITbtConnectTopologyTimeoutInMs to be 0 in order to avoid the 10 seconds
delay. Future FSP release will evaluate this ITbtConnectTopologyTimeoutInMs
value. While FSP finds this UPD value being 0, FSP will skip sendig CNTP.

BUG=b:155893566
TEST=Built image with SW CM Thunderbolt firmware and verified no
outstanding delay time while using FSP v3197 during boot to kernel.

Signed-off-by: John Zhao 
Change-Id: I47e3519fd818cb56e6abd16464d8370ffddabc5b
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/42056/1

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index ed09aaa..e693699 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -238,6 +238,9 @@
     */
    uint16_t TcssAuxOri;

+	/* Connect Topology Command timeout value */
+	uint16_t ITbtConnectTopologyTimeoutInMs;
+
    /*
     * Override GPIO PM configuration:
     * 0: Use FSP default GPIO PM program,
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c
index bdcd357..b615485 100644
--- a/src/soc/intel/tigerlake/fsp_params.c
+++ b/src/soc/intel/tigerlake/fsp_params.c
@@ -114,6 +114,9 @@
    for (i = 0; i < 8; i++)
    	params->IomTypeCPortPadCfg[i] = config->IomTypeCPortPadCfg[i];

+	/* Set FSPS UPD ITbtConnectTopologyTimeoutInMs */
+	params->ITbtConnectTopologyTimeoutInMs = 0;
+
    /* Chipset Lockdown */
    if (get_lockdown_config() == CHIPSET_LOCKDOWN_COREBOOT) {
    	params->PchLockDownGlobalSmi = 0;

[John Zhao (Code Review)]

John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 1: Code-Review-1

DO not merge. It needs to be along with SW CM Thunderbolt firmware update.

[Divya Sasidharan (Code Review)]

Divya Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 1: Code-Review+1

[Divya S Sasidharan (Code Review)]

Divya S Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 1:

@Caveh: Could you please help review the patch to unblock SW CM enablement.

[Caveh Jalali (Code Review)]

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
File src/soc/intel/tigerlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
PS1, Line 117: Set
set, as in disable CNTP entirely?

[John Zhao (Code Review)]

Hello build bot (Jenkins), Wonkyu Kim, Caveh Jalali, Shamile Khan, Prashant Malani, Divya Sasidharan, Nick Vaccaro, Brandon Breitenstein, Patrick Rudolph, Divya S Sasidharan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/42056

to look at the new patch set (#2).

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs

The Connect Topology Command(CNTP) is sent with default timeout value
(0x1388) along with FW CM. The CNTP is supposed to be skipped while
using SW CM. While transition from FW CM to SW CM, the default timeout
value could cause boot time delay up to ~10 seconds. Set this FSPS UPD
ITbtConnectTopologyTimeoutInMs to be 0 in order to avoid the 10 seconds
delay. Future FSP release will evaluate this ITbtConnectTopologyTimeoutInMs
value. While FSP finds this UPD value being 0, FSP will skip sending CNTP.

BUG=b:155893566
TEST=Built image with SW CM Thunderbolt firmware and verified no
outstanding delay time while using FSP v3197 during boot to kernel.

Signed-off-by: John Zhao 
Change-Id: I47e3519fd818cb56e6abd16464d8370ffddabc5b
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/42056/2

[John Zhao (Code Review)]

John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
File src/soc/intel/tigerlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
PS1, Line 117: Set
> set, as in disable CNTP entirely?

CNTP is supposed to be skipped for SW CM. Set this UPD value 0 to avoid ~10 seconds delay now though current FSP v3197 still sends CNTP. Future release FSP will evaluate this UPD and will not send CNTP while finding the UPD value being 0. There will also be no ~10 seconds delay at boot.

[Caveh Jalali (Code Review)]

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
File src/soc/intel/tigerlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
PS1, Line 117: Set
> CNTP is supposed to be skipped for SW CM. […]

sorry, for not being clear.

could you update the comment to indicate that a value of 0 here means
CNTP disabled or no-waiting... the comment doesn't even need to be that
technical, just some words to indicate to the reader that a timeout of 0
disables waiting rather than waiting forever.

[John Zhao (Code Review)]

Hello build bot (Jenkins), Wonkyu Kim, Caveh Jalali, Shamile Khan, Prashant Malani, Divya Sasidharan, Nick Vaccaro, Brandon Breitenstein, Patrick Rudolph, Divya S Sasidharan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/42056

to look at the new patch set (#3).

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs

The Connect Topology Command(CNTP) is sent with default timeout value
(0x1388) along with FW CM. The CNTP is supposed to be skipped while
using SW CM. While transition from FW CM to SW CM, the default timeout
value could cause boot time delay up to ~10 seconds. Set this FSPS UPD
ITbtConnectTopologyTimeoutInMs to be 0 in order to avoid the 10 seconds
delay. Future FSP release will evaluate this ITbtConnectTopologyTimeoutInMs
value. While FSP finds this UPD value being 0, FSP will skip sending CNTP.

BUG=b:155893566
TEST=Built image with SW CM Thunderbolt firmware and verified no
outstanding delay time while using FSP v3197 during boot to kernel.

Signed-off-by: John Zhao 
Change-Id: I47e3519fd818cb56e6abd16464d8370ffddabc5b
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/42056/3

[John Zhao (Code Review)]

John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
File src/soc/intel/tigerlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/42056/1/src/soc/intel/tigerlake/fsp... 
PS1, Line 117: Set
> sorry, for not being clear. […]

no problem. Just updated with comments.

[Caveh Jalali (Code Review)]

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

Patch Set 3: Code-Review+2

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42056 )

Change subject: soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs
......................................................................

soc/intel/tigerlake: Set FSPS UPD ITbtConnectTopologyTimeoutInMs

The Connect Topology Command(CNTP) is sent with default timeout value
(0x1388) along with FW CM. The CNTP is supposed to be skipped while
using SW CM. While transition from FW CM to SW CM, the default timeout
value could cause boot time delay up to ~10 seconds. Set this FSPS UPD
ITbtConnectTopologyTimeoutInMs to be 0 in order to avoid the 10 seconds
delay. Future FSP release will evaluate this ITbtConnectTopologyTimeoutInMs
value. While FSP finds this UPD value being 0, FSP will skip sending CNTP.

BUG=b:155893566
TEST=Built image with SW CM Thunderbolt firmware and verified no
outstanding delay time while using FSP v3197 during boot to kernel.

Signed-off-by: John Zhao 
Change-Id: I47e3519fd818cb56e6abd16464d8370ffddabc5b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42056
Reviewed-by: Caveh Jalali 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 10 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Caveh Jalali: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index ed09aaa..e693699 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -238,6 +238,9 @@
     */
    uint16_t TcssAuxOri;

+	/* Connect Topology Command timeout value */
+	uint16_t ITbtConnectTopologyTimeoutInMs;
+
    /*
     * Override GPIO PM configuration:
     * 0: Use FSP default GPIO PM program,
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c
index bdcd357..926d8eb 100644
--- a/src/soc/intel/tigerlake/fsp_params.c
+++ b/src/soc/intel/tigerlake/fsp_params.c
@@ -114,6 +114,13 @@
    for (i = 0; i < 8; i++)
    	params->IomTypeCPortPadCfg[i] = config->IomTypeCPortPadCfg[i];

+	/*
+	 * Set FSPS UPD ITbtConnectTopologyTimeoutInMs with value 0. FSP will
+	 * evaluate this UPD value and skip sending command. There will be no
+	 * delay for command completion.
+	 */
+	params->ITbtConnectTopologyTimeoutInMs = 0;
+
    /* Chipset Lockdown */
    if (get_lockdown_config() == CHIPSET_LOCKDOWN_COREBOOT) {
    	params->PchLockDownGlobalSmi = 0;