Change in coreboot[master]: soc/intel/tigerlake: SATA Port Enable Dito Config

[Shaunak Saha (Code Review)]

Shaunak Saha has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: SATA Port Enable Dito Config
......................................................................

soc/intel/tigerlake: SATA Port Enable Dito Config

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
port specific timeout value used by the HBA for determining when to assert the
DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
too quickly which may result in undesirable latencies.
*PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
Default is 15.
*PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
(DITO), Default is 625.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 24 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/1

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index ed09aaa..d920fc9 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -98,6 +98,18 @@
    uint8_t SataPortsEnable[8];
    uint8_t SataPortsDevSlp[8];

+	/*
+	 * Enable(0)/Disable(1) SATA Power Optimizer on PCH side.
+	 * Default 0. Setting this to 1 disables the SATA Power Optimizer.
+	 */
+	uint8_t SataPwrOptimizeDisable;
+
+	/*
+	 * SATA Port Enable Dito Config.
+	 * Enable DEVSLP Idle Timeout settings (DmVal, DitoVal).
+	 */
+	uint8_t SataPortsEnableDitoConfig[8];
+
    /* Audio related */
    uint8_t PchHdaDspEnable;
    uint8_t PchHdaAudioLinkHdaEnable;
@@ -308,11 +320,6 @@
     */
    uint8_t DmiPwrOptimizeDisable;

-	/*
-	 * Enable(0)/Disable(1) SATA Power Optimizer on PCH side.
-	 * Default 0. Setting this to 1 disables the SATA Power Optimizer.
-	 */
-	uint8_t SataPwrOptimizeDisable;
 };

typedef struct soc_intel_tigerlake_config config_t;
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c
index bdcd357..c2c963f 100644
--- a/src/soc/intel/tigerlake/fsp_params.c
+++ b/src/soc/intel/tigerlake/fsp_params.c
@@ -202,6 +202,18 @@
    params->PchPwrOptEnable = !(config->DmiPwrOptimizeDisable);
    params->SataPwrOptEnable = !(config->SataPwrOptimizeDisable);

+	/*
+	 *  Enable DEVSLP Idle Timeout settings DmVal and DitoVal.
+	 *  SataPortsDmVal is the DITO multiplier. Default is 15.
+	 *  SataPortsDitoVal is the DEVSLP Idle Timeout (DITO), Default is 625.
+	 */
+	for (i = 0; i < ARRAY_SIZE(config->SataPortsEnableDitoConfig); i++) {
+		if (config->SataPortsEnableDitoConfig[i]) {
+			params->SataPortsDmVal[i] = 15;
+			params->SataPortsDitoVal[1] = 625;
+		}
+	}
+
    /* Enable TCPU for processor thermal control */
    params->Device4Enable = config->Device4Enable;

[Shaunak Saha (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Ravishankar Sarawadi, Duncan Laurie, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: SATA Port Enable Dito Config
......................................................................

soc/intel/tigerlake: SATA Port Enable Dito Config

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
port specific timeout value used by the HBA for determining when to assert the
DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
too quickly which may result in undesirable latencies.
*PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
Default is 15.
*PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
(DITO), Default is 625.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 24 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: SATA Port Enable Dito Config
......................................................................

Patch Set 2:

(6 comments)

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@7 
PS2, Line 7: soc/intel/tigerlake: SATA Port Enable Dito Config
Maybe:
> Hook up SATA Port Enable Dito UPDs

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@10 
PS2, Line 10: SataPortsDmVal and SataPortsDitoVal helps to determine when to
            : enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
            : assert the DEVSLP signal as soon as there are no commands outstanding
            : to the device and the port specific Device Sleep idle timer has expired.
            : And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
            : port specific timeout value used by the HBA for determining when to assert the
            : DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
            : amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
            : too quickly which may result in undesirable latencies.
Is that a quote from some datasheet?

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@19 
PS2, Line 19: *PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
Please add a space after the bullet point, and also add a blank line above to visually separate the list from the paragraph.

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@22 
PS2, Line 22: (DITO), Default is 625.
625 ms?

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

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 215: 	 *  SataPortsDitoVal is the DEVSLP Idle Timeout (DITO), Default is 625.
That’s milliseconds? Please add the unit.

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 220: 			params->SataPortsDitoVal[1] = 625;
How can the defaults be changed?

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: SATA Port Enable Dito Config
......................................................................

Patch Set 2:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 219: 15
Please define and use constants for these two settings in place of hard-coding.

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: SATA Port Enable Dito Config
......................................................................

Patch Set 2:

(7 comments)

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@7 
PS2, Line 7: soc/intel/tigerlake: SATA Port Enable Dito Config
> Maybe: […]

Will fix.

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@10 
PS2, Line 10: SataPortsDmVal and SataPortsDitoVal helps to determine when to
            : enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
            : assert the DEVSLP signal as soon as there are no commands outstanding
            : to the device and the port specific Device Sleep idle timer has expired.
            : And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
            : port specific timeout value used by the HBA for determining when to assert the
            : DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
            : amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
            : too quickly which may result in undesirable latencies.
> Is that a quote from some datasheet?

Yes. From the EDS. Will update the document number.

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@19 
PS2, Line 19: *PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
> Please add a space after the bullet point, and also add a blank line above to visually separate the  […]

Will fix.

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@22 
PS2, Line 22: (DITO), Default is 625.
> 625 ms?

Yes.Will update.

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

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 215: 	 *  SataPortsDitoVal is the DEVSLP Idle Timeout (DITO), Default is 625.
> That’s milliseconds? Please add the unit.

Will add.

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 219: 15
> Please define and use constants for these two settings in place of hard-coding.

Will add the constants.

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 220: 			params->SataPortsDitoVal[1] = 625;
> How can the defaults be changed?

Actually we should be able to overwrite the defaults from devicetree. Will add that support here.

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

[Shaunak Saha (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Ravishankar Sarawadi, Duncan Laurie, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs
......................................................................

soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
port specific timeout value used by the HBA for determining when to assert the
DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
too quickly which may result in undesirable latencies. This patch is created
based on Intel Tiger Lake Processor PCH Datasheet with Document number:575857
and Chapter number:12.

* PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
 Default is 15.
* PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
 (DITO), Default is 625ms.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 41 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/4

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

[Shaunak Saha (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Ravishankar Sarawadi, Duncan Laurie, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs
......................................................................

soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
iAnd the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
port specific timeout value used by the HBA for determining when to assert
the DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
too quickly which may result in undesirable latencies. This patch is created
based on Intel Tiger Lake Processor PCH Datasheet with Document number:575857
and Chapter number:12.

* PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
 Default is 15.
* PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
 (DITO), Default is 625ms.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 41 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/5

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

[Shaunak Saha (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Ravishankar Sarawadi, Duncan Laurie, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs
......................................................................

soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM)
is a port specific timeout value used by the HBA for determining when to
assert the DEVSLP signal. It provides a mechanism for the HBA to apply
a programmable amount of hysteresis so as to prevent the HBA from
asserting the DEVSLP signal too quickly which may result in undesirable
latencies. This patch is created based on Intel Tiger Lake Processor
PCH Datasheet with Document number:575857 and Chapter number:12.

* PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
 Default is 15.
* PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
 (DITO), Default is 625ms.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 41 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/6

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs
......................................................................

Patch Set 6:

(7 comments)

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@7 
PS2, Line 7: soc/intel/tigerlake: SATA Port Enable Dito Config
> Will fix.

Done

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@10 
PS2, Line 10: SataPortsDmVal and SataPortsDitoVal helps to determine when to
            : enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
            : assert the DEVSLP signal as soon as there are no commands outstanding
            : to the device and the port specific Device Sleep idle timer has expired.
            : And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM) is a
            : port specific timeout value used by the HBA for determining when to assert the
            : DEVSLP signal. It provides a mechanism for the HBA to apply a programmable
            : amount of hysteresis so as to prevent the HBA from asserting the DEVSLP signal
            : too quickly which may result in undesirable latencies.
> Yes. From the EDS. Will update the document number.

Done

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@19 
PS2, Line 19: *PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
> Will fix.

Done

https://review.coreboot.org/c/coreboot/+/42214/2//COMMIT_MSG@22 
PS2, Line 22: (DITO), Default is 625.
> Yes.Will update.

Done

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

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 215: 	 *  SataPortsDitoVal is the DEVSLP Idle Timeout (DITO), Default is 625.
> Will add.

Done

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 219: 15
> Will add the constants.

Done

https://review.coreboot.org/c/coreboot/+/42214/2/src/soc/intel/tigerlake/fsp... 
PS2, Line 220: 			params->SataPortsDitoVal[1] = 625;
> Actually we should be able to overwrite the defaults from devicetree. Will add that support here.

Done

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable Dito UPDs
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42214/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42214/6//COMMIT_MSG@7 
PS6, Line 7: Dito
uppercase: DITO

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Shaunak Saha (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Ravishankar Sarawadi, Duncan Laurie, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
And the Device Sleep Idle Timeout value (PxDEVSLP.DITO and PxDEVSLP.DM)
is a port specific timeout value used by the HBA for determining when to
assert the DEVSLP signal. It provides a mechanism for the HBA to apply
a programmable amount of hysteresis so as to prevent the HBA from
asserting the DEVSLP signal too quickly which may result in undesirable
latencies. This patch is created based on Intel Tiger Lake Processor
PCH Datasheet with Document number:575857 and Chapter number:12.

* PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
 Default is 15.
* PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
 (DITO), Default is 625ms.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 41 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/7

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42214/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42214/6//COMMIT_MSG@7 
PS6, Line 7: Dito
> uppercase: DITO

Ack

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Caveh Jalali (Code Review)]

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

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

Patch Set 7: Code-Review+1

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/42214/7//COMMIT_MSG@14 
PS7, Line 14: And
remove?

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Shaunak Saha (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Ravishankar Sarawadi, Duncan Laurie, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
Device Sleep Idle Timeout values (PxDEVSLP.DITO and PxDEVSLP.DM) are
port specific timeout values used by the HBA for determining when to
assert the DEVSLP signal. They provides a mechanism for the HBA to apply
a programmable amount of hysteresis so as to prevent the HBA from
asserting the DEVSLP signal too quickly which may result in undesirable
latencies. This patch is created based on Intel Tiger Lake Processor
PCH Datasheet with Document number:575857 and Chapter number:12.

* PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
 Default is 15.
* PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
 (DITO), Default is 625ms.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
---
M src/soc/intel/tigerlake/chip.h
M src/soc/intel/tigerlake/fsp_params.c
2 files changed, 41 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42214/8

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

Patch Set 8:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/42214/7//COMMIT_MSG@14 
PS7, Line 14: And
> remove?

Done

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Wonkyu Kim (Code Review)]

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

Patch Set 8: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42214 )

Change subject: soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs
......................................................................

soc/intel/tigerlake: Hook up SATA Port Enable DITO UPDs

SataPortsEnableDitoConfig Enable DEVSLP Idle Timeout settings DmVal and
DitoVal. SataPortsDmVal and SataPortsDitoVal helps to determine when to
enter Device Sleep. Device Sleep enables the host bus adapter (HBA) to
assert the DEVSLP signal as soon as there are no commands outstanding
to the device and the port specific Device Sleep idle timer has expired.
Device Sleep Idle Timeout values (PxDEVSLP.DITO and PxDEVSLP.DM) are
port specific timeout values used by the HBA for determining when to
assert the DEVSLP signal. They provides a mechanism for the HBA to apply
a programmable amount of hysteresis so as to prevent the HBA from
asserting the DEVSLP signal too quickly which may result in undesirable
latencies. This patch is created based on Intel Tiger Lake Processor
PCH Datasheet with Document number:575857 and Chapter number:12.

* PxDEVSLP.DM ->     SataPortsDmVal: Enable SATA Port DmVal DITO multiplier.
 Default is 15.
* PxDEVSLP.DITO -> SataPortsDitoVal: Enable SATA Port DmVal DEVSLP Idle Timeout
 (DITO), Default is 625ms.

BUG=b:151163106
BRANCH=None
TEST=Build and boot volteer and TGL RVP.

Signed-off-by: Shaunak Saha 
Change-Id: I6a824524738a9e0609f54bea9d892b4a42a1d3db
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42214
Reviewed-by: Wonkyu Kim 
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, 41 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Caveh Jalali: Looks good to me, but someone else must approve
  Wonkyu Kim: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h
index fee7105..59dab58 100644
--- a/src/soc/intel/tigerlake/chip.h
+++ b/src/soc/intel/tigerlake/chip.h
@@ -133,6 +133,23 @@
    uint8_t SataPortsEnable[8];
    uint8_t SataPortsDevSlp[8];

+	/*
+	 * Enable(0)/Disable(1) SATA Power Optimizer on PCH side.
+	 * Default 0. Setting this to 1 disables the SATA Power Optimizer.
+	 */
+	uint8_t SataPwrOptimizeDisable;
+
+	/*
+	 * SATA Port Enable Dito Config.
+	 * Enable DEVSLP Idle Timeout settings (DmVal, DitoVal).
+	 */
+	uint8_t SataPortsEnableDitoConfig[8];
+
+	/* SataPortsDmVal is the DITO multiplier. Default is 15. */
+	uint8_t SataPortsDmVal[8];
+	/* SataPortsDitoVal is the DEVSLP Idle Timeout, default is 625ms */
+	uint16_t SataPortsDitoVal[8];
+
    /* Audio related */
    uint8_t PchHdaDspEnable;
    uint8_t PchHdaAudioLinkHdaEnable;
@@ -337,12 +354,6 @@
     */
    uint8_t DmiPwrOptimizeDisable;

-	/*
-	 * Enable(0)/Disable(1) SATA Power Optimizer on PCH side.
-	 * Default 0. Setting this to 1 disables the SATA Power Optimizer.
-	 */
-	uint8_t SataPwrOptimizeDisable;
-
    /* structure containing various settings for PCH FIVRs */
    struct {
    	bool configure_ext_fivr;
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c
index 3187a33..798c16a 100644
--- a/src/soc/intel/tigerlake/fsp_params.c
+++ b/src/soc/intel/tigerlake/fsp_params.c
@@ -24,6 +24,10 @@
 #define THC_0		1
 #define THC_1		2

+/* SATA DEVSLP idle timeout default values */
+#define DEF_DMVAL	15
+#define DEF_DITOVAL	625
+
 /*
  * Chip config parameter PcieRpL1Substates uses (UPD value + 1)
  * because UPD value of 0 for PcieRpL1Substates means disabled for FSP.
@@ -212,6 +216,26 @@
    params->PchPwrOptEnable = !(config->DmiPwrOptimizeDisable);
    params->SataPwrOptEnable = !(config->SataPwrOptimizeDisable);

+	/*
+	 *  Enable DEVSLP Idle Timeout settings DmVal and DitoVal.
+	 *  SataPortsDmVal is the DITO multiplier. Default is 15.
+	 *  SataPortsDitoVal is the DEVSLP Idle Timeout (DITO), Default is 625ms.
+	 *  The default values can be changed from devicetree.
+	 */
+	for (i = 0; i < ARRAY_SIZE(config->SataPortsEnableDitoConfig); i++) {
+		if (config->SataPortsEnableDitoConfig[i]) {
+			if (config->SataPortsDmVal[i])
+				params->SataPortsDmVal[i] = config->SataPortsDmVal[i];
+			else
+				params->SataPortsDmVal[i] = DEF_DMVAL;
+
+			if (config->SataPortsDitoVal[i])
+				params->SataPortsDitoVal[i] = config->SataPortsDitoVal[i];
+			else
+				params->SataPortsDitoVal[i] = DEF_DITOVAL;
+		}
+	}
+
    /* Enable TCPU for processor thermal control */
    params->Device4Enable = config->Device4Enable;