Change in coreboot[master]: soc/intel/tigerlake: Update TCSS PM flow

[John Zhao (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

soc/intel/tigerlake: Update TCSS PM flow

s0xi fails while USB4 device is connected and PCIe tunneling is up.
There is need to change PM flow along with TBT firmware update. This
change invokes D3CE and D3CX in DMA _PS0 when _OFF was not called
earlier.

BUG=b:158777291
TEST=Validated s0ix with USB4 device connected along with TBT firmware
QS variants TBT_TGL_B0_CHROME_Rev33 image.

Signed-off-by: John Zhao 
Change-Id: Iebc8065fe4c8600960d089577608890ab12a95fc
---
M src/soc/intel/tigerlake/acpi/tcss.asl
M src/soc/intel/tigerlake/acpi/tcss_dma.asl
M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
3 files changed, 33 insertions(+), 3 deletions(-)

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

diff --git a/src/soc/intel/tigerlake/acpi/tcss.asl b/src/soc/intel/tigerlake/acpi/tcss.asl
index 9893833..ed43e05 100644
--- a/src/soc/intel/tigerlake/acpi/tcss.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss.asl
@@ -524,6 +524,7 @@
    	If (_SB.PCI0.TDM0.VDID == 0xFFFFFFFF) {
    		Printf("TDM0 does not exist.")
    	} Else {
+			_SB.PCI0.TDM0.SCMP = 3
    		If (_SB.PCI0.TDM0.STAT == 0) {
    			/* DMA0 is in D3Cold early. */
    			_SB.PCI0.TDM0.D3CX()  /* RTD3 Exit */
@@ -554,6 +555,7 @@
    	If (_SB.PCI0.TDM0.VDID == 0xFFFFFFFF) {
    		Printf("TDM0 does not exist.")
    	} Else {
+			_SB.PCI0.TDM0.SCMP = 2
    		If (_SB.PCI0.TDM0.STAT == 1) {
    			/* DMA0 is not in D3Cold now. */
    			_SB.PCI0.TDM0.D3CE()  /* Enable DMA RTD3 */
@@ -579,6 +581,7 @@
    	If (_SB.PCI0.TDM1.VDID == 0xFFFFFFFF) {
    		Printf("TDM1 does not exist.")
    	} Else {
+			_SB.PCI0.TDM1.SCMP = 3
    		If (_SB.PCI0.TDM1.STAT == 0) {
    			/* DMA1 is in D3Cold early. */
    			_SB.PCI0.TDM1.D3CX()  /* RTD3 Exit */
@@ -609,6 +612,7 @@
    	If (_SB.PCI0.TDM1.VDID == 0xFFFFFFFF) {
    		 Printf("TDM1 does not exist.")
    	} Else {
+			_SB.PCI0.TDM1.SCMP = 2
    		If (_SB.PCI0.TDM1.STAT == 1) {
    			/* DMA1 is not in D3Cold now */
    			_SB.PCI0.TDM1.D3CE()  /* Enable DMA RTD3. */
diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl
index da2e8fe..df96576 100644
--- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl
@@ -72,6 +72,28 @@
  */
 Name (SD3C, 0)

+/* SW CM Power State Flow Control */
+Name (SCMP, 0)
+
+Method (_PS0, 0, Serialized)
+{
+	If (SCMP != 3) {
+		If (DUID == 0) {
+			_SB.PCI0.TG0F()
+			_SB.PCI0.TG0N()
+		} Else {
+			_SB.PCI0.TG1F()
+			_SB.PCI0.TG1N()
+		}
+	}
+	SCMP = 0
+}
+
+Method (_PS3, 0, Serialized)
+{
+	SCMP = 1
+}
+
 Method (_DSW, 3)
 {
    /* If entering Sx (Arg1 > 1), need to skip TCSS D3Cold & TBT RTD3/D3Cold. */
diff --git a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
index abca5d1..9571e19 100644
--- a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
@@ -76,10 +76,14 @@

Method (_DSW, 3)
 {
-	C2PM (Arg0, Arg1, Arg2, DCPM)
    /* If entering Sx (Arg1 > 1), need to skip TCSS D3Cold & TBT RTD3/D3Cold. */
-	_SB.PCI0.TDM0.SD3C = Arg1
-	_SB.PCI0.TDM1.SD3C = Arg1
+	If ((TUID == 0) || (TUID == 1)) {
+		_SB.PCI0.TDM0.SD3C = Arg1
+	} Else {
+		_SB.PCI0.TDM1.SD3C = Arg1
+	}
+
+	C2PM (Arg0, Arg1, Arg2, DCPM)
 }

Method (_PRW, 0)

[John Zhao (Code Review)]

Hello build bot (Jenkins), Wonkyu Kim, Tim Wawrzynczak, Duncan Laurie, Shamile Khan, Divya Sasidharan, Utkarsh H Patel, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

soc/intel/tigerlake: Update TCSS PM flow

s0ix fails while USB4 device is connected and PCIe tunneling is up.
There is need to change PM flow along with TBT firmware update. This
change invokes D3CE and D3CX in DMA _PS0 when _OFF was not called
earlier.

BUG=b:158777291
TEST=Validated s0ix with USB4 device connected along with TBT firmware
QS variants TBT_TGL_B0_CHROME_Release_Image_Rev33 image.

Signed-off-by: John Zhao 
Change-Id: Iebc8065fe4c8600960d089577608890ab12a95fc
---
M src/soc/intel/tigerlake/acpi/tcss.asl
M src/soc/intel/tigerlake/acpi/tcss_dma.asl
M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
3 files changed, 33 insertions(+), 3 deletions(-)

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

[John Zhao (Code Review)]

Hello build bot (Jenkins), Wonkyu Kim, Tim Wawrzynczak, Duncan Laurie, Shamile Khan, Divya Sasidharan, Utkarsh H Patel, Patrick Rudolph,

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

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

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

soc/intel/tigerlake: Update TCSS PM flow

s0ix fails while USB4 device is connected and PCIe tunneling is up.
There is need to change PM flow along with TBT firmware update. This
change invokes D3CE and D3CX in DMA _PS0 when _OFF was not called
earlier.

BUG=b:158777291
TEST=Validated s0ix with USB4 device connected along with TBT firmware
QS variants TBT_TGL_B0_CHROME_Release_Image_Rev33 image.

Signed-off-by: John Zhao 
Change-Id: Iebc8065fe4c8600960d089577608890ab12a95fc
---
M src/soc/intel/tigerlake/acpi/tcss.asl
M src/soc/intel/tigerlake/acpi/tcss_dma.asl
M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
3 files changed, 37 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/45014/3

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
File src/soc/intel/tigerlake/acpi/tcss.asl:

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
PS3, Line 622: 2
Is '2' (or '3' above) just for debugging? Because the _PS0 method only cares if SCMP is 1, so why not just set it back to 0 here?

[John Zhao (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
File src/soc/intel/tigerlake/acpi/tcss.asl:

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
PS3, Line 622: 2
> Is '2' (or '3' above) just for debugging? Because the _PS0 method only cares if SCMP is 1, so why no […]

It is about keep tracking PM flow: D0(SCMP=0) -> D3 hot(SCMP=0) -> _PS3 (SCMP=1) -> _OFF(SCMP=2) -> _ON (SCMP=3) ->_PS0(SCMP=0). If simply setting SCMP to 0, we have no clue about the PM flow stage. You are right, it is also good for debugging.

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
File src/soc/intel/tigerlake/acpi/tcss.asl:

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
PS3, Line 622: 2
> It is about keep tracking PM flow: D0(SCMP=0) -> D3 hot(SCMP=0) -> _PS3 (SCMP=1) -> _OFF(SCMP=2) ->  […]

Ok I see in _DSW, gotcha.

[John Zhao (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 3:

(1 comment)

This change is ready for review.

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
File src/soc/intel/tigerlake/acpi/tcss.asl:

https://review.coreboot.org/c/coreboot/+/45014/3/src/soc/intel/tigerlake/acp... 
PS3, Line 622: 2
> Ok I see in _DSW, gotcha.

Sorry, marked WIP as there is proposal change to along with further TBT firmware update.

[John Zhao (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 4:

This change is ready for review.

[Duncan Laurie (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 4: Code-Review+2

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 4: Code-Review+2

[John Zhao (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 4:

Can we consider to merge this patch to align with the merged TBT firmware rev35? This patch is needed and had been validated for TCSS PM flow. Thanks!

[Shamile Khan (Code Review)]

Shamile Khan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45014 )

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

Patch Set 4:
> Patch Set 4:
>
> Can we consider to merge this patch to align with the merged TBT firmware rev35? This patch is needed and had been validated for TCSS PM flow. Thanks!

+1 for above. Can we resolve the conflict by letting this patch in first?

[Duncan Laurie (Code Review)]

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

Change subject: soc/intel/tigerlake: Update TCSS PM flow
......................................................................

soc/intel/tigerlake: Update TCSS PM flow

There is requirement to change PM flow for S0ix along with TBT firmware
update under device attached and no device attached scenarios. This
change invokes D3CE and D3CX in DMA _PS3 and _PS0 respectively.

BUG=b:158777291
TEST=Validated s0ix cycles for USB4 device attached and no device
attached test cases along with updated TBT firmware rev35.

Signed-off-by: John Zhao 
Change-Id: Iebc8065fe4c8600960d089577608890ab12a95fc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45014
Tested-by: build bot (Jenkins) 
Reviewed-by: Duncan Laurie 
Reviewed-by: Tim Wawrzynczak 
---
M src/soc/intel/tigerlake/acpi/tcss_dma.asl
M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
2 files changed, 29 insertions(+), 23 deletions(-)

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

diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl
index da2e8fe..f7c4117 100644
--- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl
@@ -32,20 +32,12 @@

Method (_PR0)
 {
-	If (DUID == 0) {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT0 })
-	} Else {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT1 })
-	}
+	Return (Package() { _SB.PCI0.D3C })
 }

Method (_PR3)
 {
-	If (DUID == 0) {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT0 })
-	} Else {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT1 })
-	}
+	Return (Package() { _SB.PCI0.D3C })
 }

/*
@@ -72,6 +64,24 @@
  */
 Name (SD3C, 0)

+Method (_PS0, 0, Serialized)
+{
+	If (DUID == 0) {
+		_SB.PCI0.TBT0._ON()
+	} Else {
+		_SB.PCI0.TBT1._ON()
+	}
+}
+
+Method (_PS3, 0, Serialized)
+{
+	If (DUID == 0) {
+		_SB.PCI0.TBT0._OFF()
+	} Else {
+		_SB.PCI0.TBT1._OFF()
+	}
+}
+
 Method (_DSW, 3)
 {
    /* If entering Sx (Arg1 > 1), need to skip TCSS D3Cold & TBT RTD3/D3Cold. */
diff --git a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
index abca5d1..b4c0cef 100644
--- a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl
@@ -76,10 +76,14 @@

Method (_DSW, 3)
 {
-	C2PM (Arg0, Arg1, Arg2, DCPM)
    /* If entering Sx (Arg1 > 1), need to skip TCSS D3Cold & TBT RTD3/D3Cold. */
-	_SB.PCI0.TDM0.SD3C = Arg1
-	_SB.PCI0.TDM1.SD3C = Arg1
+	If ((TUID == 0) || (TUID == 1)) {
+		_SB.PCI0.TDM0.SD3C = Arg1
+	} Else {
+		_SB.PCI0.TDM1.SD3C = Arg1
+	}
+
+	C2PM (Arg0, Arg1, Arg2, DCPM)
 }

Method (_PRW, 0)
@@ -302,20 +306,12 @@

Method (_PR0)
 {
-	If ((TUID == 0) || (TUID == 1)) {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT0 })
-	} Else {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT1 })
-	}
+	Return (Package() { _SB.PCI0.D3C })
 }

Method (_PR3)
 {
-	If ((TUID == 0) || (TUID == 1)) {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT0 })
-	} Else {
-		Return (Package() { _SB.PCI0.D3C, _SB.PCI0.TBT1 })
-	}
+	Return (Package() { _SB.PCI0.D3C })
 }

/*