Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to disable Precision Time Management

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46856 )

Change subject: soc/intel/tigerlake: Expose UPD to disable Precision Time Management
......................................................................

soc/intel/tigerlake: Expose UPD to disable Precision Time Management

Expose a config option that allows disabling the FSP UPD which controls
Precision Time Management for a particular PCIe root port.  Since this
is enabled by default the option is inverted to allow disabling for
a particular port while not affecting others.

BUG=b:160996445
TEST=boot on volteer with PTM disabled for the NVMe root port

Change-Id: Icb51b256eb581d942b2d30fcabfae52fa90e48d4
Signed-off-by: Duncan Laurie 
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 4 insertions(+), 0 deletions(-)

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

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index f752b5f..fcd6b0c 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -242,6 +242,9 @@
    /* Probe CLKREQ# signal before enabling CLKREQ# based power management.*/
    uint8_t PcieRpClkReqDetect[CONFIG_MAX_ROOT_PORTS];

+	/* Disable PCIe Precision Time Measurement for Root Ports (enabled by default) */
+	uint8_t PciePtmDisable[CONFIG_MAX_ROOT_PORTS];
+
    /* PCIe RP L1 substate */
    enum L1_substates_control {
    	L1_SS_FSP_DEFAULT,
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c
index 887241b..1520cfe 100644
--- a/src/soc/intel/tigerlake/fsp_params.c
+++ b/src/soc/intel/tigerlake/fsp_params.c
@@ -186,6 +186,7 @@
    	params->PcieRpAdvancedErrorReporting[i] =
    		config->PcieRpAdvancedErrorReporting[i];
    	params->PcieRpHotPlug[i] = config->PcieRpHotPlug[i];
+		params->PciePtm[i] = !config->PciePtmDisable[i];
    }

/* Enable ClkReqDetect for enabled port */

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to disable Precision Time Measurement

[Duncan Laurie (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Expose UPD to disable Precision Time Measurement
......................................................................

soc/intel/tigerlake: Expose UPD to disable Precision Time Measurement

Expose a config option that allows disabling the FSP UPD which controls
Precision Time Measurement for a particular PCIe root port.  Since this
is enabled by default the option is inverted to allow disabling for
a particular port while not affecting others.

BUG=b:160996445
TEST=boot on volteer with PTM disabled for the NVMe root port

Change-Id: Icb51b256eb581d942b2d30fcabfae52fa90e48d4
Signed-off-by: Duncan Laurie 
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 4 insertions(+), 0 deletions(-)

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

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to disable Precision Time Measurement

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46856 )

Change subject: soc/intel/tigerlake: Expose UPD to disable Precision Time Measurement
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

[Duncan Laurie (Code Review)]

Hello build bot (Jenkins), Tim Wawrzynczak, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement
......................................................................

soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

Expose a config option that allows enabling the FSP UPD which controls
Precision Time Measurement for a particular PCIe root port.

This UPD is enabled by default in FSP but interferes with achieving
deeper S0ix substates so in order to prevent it from needing to be
explicitly disabled for every root port this change makes disabling it
the default and allows it to be enabled if needed.

BUG=b:160996445
TEST=boot on volteer with PTM disabled by default for all root ports
and ensure S0i3.2 substate can be achieved.

Change-Id: Icb51b256eb581d942b2d30fcabfae52fa90e48d4
Signed-off-by: Duncan Laurie 
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/46856/6

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46856 )

Change subject: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement
......................................................................

Patch Set 7: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46856 )

Change subject: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement
......................................................................

Patch Set 7: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/46856/7//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/46856/7//COMMIT_MSG@12 
PS7, Line 12: interferes with achieving
            : deeper S0ix substates
Did we ever get an answer why this is the case?

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46856 )

Change subject: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46856/7//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/46856/7//COMMIT_MSG@12 
PS7, Line 12: interferes with achieving
            : deeper S0ix substates
> Did we ever get an answer why this is the case?

No, I have had a hard time getting info about this feature at all.  It seems to be mostly focused on industrial applications for IOT devices so it doesn't really seem important for a client system anyway.

Change in coreboot[master]: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

[Duncan Laurie (Code Review)]

Duncan Laurie has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46856 )

Change subject: soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement
......................................................................

soc/intel/tigerlake: Expose UPD to enable Precision Time Measurement

Expose a config option that allows enabling the FSP UPD which controls
Precision Time Measurement for a particular PCIe root port.

This UPD is enabled by default in FSP but interferes with achieving
deeper S0ix substates so in order to prevent it from needing to be
explicitly disabled for every root port this change makes disabling it
the default and allows it to be enabled if needed.

BUG=b:160996445
TEST=boot on volteer with PTM disabled by default for all root ports
and ensure S0i3.2 substate can be achieved.

Change-Id: Icb51b256eb581d942b2d30fcabfae52fa90e48d4
Signed-off-by: Duncan Laurie 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46856
Reviewed-by: Tim Wawrzynczak 
Reviewed-by: Furquan Shaikh 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 4 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index 6a48ad0..5c124de 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -242,6 +242,9 @@
    /* Probe CLKREQ# signal before enabling CLKREQ# based power management.*/
    uint8_t PcieRpClkReqDetect[CONFIG_MAX_ROOT_PORTS];

+	/* Enable PCIe Precision Time Measurement for Root Ports (disabled by default) */
+	uint8_t PciePtm[CONFIG_MAX_ROOT_PORTS];
+
    /* PCIe RP L1 substate */
    enum L1_substates_control {
    	L1_SS_FSP_DEFAULT,
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c
index 6de098a..ed34897 100644
--- a/src/soc/intel/tigerlake/fsp_params.c
+++ b/src/soc/intel/tigerlake/fsp_params.c
@@ -187,6 +187,7 @@
    	params->PcieRpAdvancedErrorReporting[i] =
    		config->PcieRpAdvancedErrorReporting[i];
    	params->PcieRpHotPlug[i] = config->PcieRpHotPlug[i];
+		params->PciePtm[i] = config->PciePtm[i];
    }

/* Enable ClkReqDetect for enabled port */