Change in coreboot[master]: mainboard: Set Tiger Lake platforms to have retimer config Aux orient...
Brandon Breitenstein has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
mainboard: Set Tiger Lake platforms to have retimer config Aux orientation
Since the Type-C ports on all the Tiger Lake platforms have retimers set the TcssAuxOri UPD to 0 in order for the SoC to not misconfigure the ports. Volteer will need some additional changes after this is implemented to account for ports that do not have a retimer.
BUG=b:145943811 BRANCH=none TEST=Boot to OS and check TypeC port1 Display, Connecting type-c display should work regardless of type-c cable orientation.
Change-Id: I29eb0513299126ad8d1ee11ded2c771f28ad13f3 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 3 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39460/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index a65e5f1..0352b62 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -112,6 +112,7 @@
# TCSS USB3 register "TcssXhciEn" = "1" + register "TcssAuxOri" = "0"
# DP port register "DdiPortAConfig" = "1" # eDP diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index 4492acb..11a2883 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -95,6 +95,9 @@ [PchSerialIoIndexUART2] = PchSerialIoPci, }"
+ # TCSS USB3 + register "TcssAuxOri" = "0" + #HD Audio register "PchHdaDspEnable" = "1" register "PchHdaAudioLinkHdaEnable" = "0" diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb index 643db36..1fc1b62 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb @@ -91,6 +91,9 @@ [PchSerialIoIndexUART2] = PchSerialIoPci, }"
+ # TCSS USB3 + register "TcssAuxOri" = "0" + #HD Audio register "PchHdaDspEnable" = "1" register "PchHdaAudioLinkHdaEnable" = "0"
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 1: Code-Review+2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG@9 PS1, Line 9: Type-C ports on all the Tiger Lake platforms have retimers I don't think this is true. We can have type c ports w/o a retimer.
https://review.coreboot.org/c/coreboot/+/39460/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/1/src/mainboard/google/voltee... PS1, Line 115: register "TcssAuxOri" = "0" What are the value options for this field? If there's not a retimer that fixes the orientation what's the appropriate value?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG@9 PS1, Line 9: Type-C ports on all the Tiger Lake platforms have retimers
I don't think this is true. We can have type c ports w/o a retimer.
I will reword this...I clarified the difference for Volteer below
https://review.coreboot.org/c/coreboot/+/39460/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/1/src/mainboard/google/voltee... PS1, Line 115: register "TcssAuxOri" = "0"
What are the value options for this field? If there's not a retimer that fixes the orientation what' […]
this is a bitwise field relating to each port if the flip is needed the corresponding bit x2 needs to be set please see the FspsUpd.h file for more info on this
/** Offset 0x04CE - TCSS Aux Orientation Override Enable Bits 0, 2, ... 10 control override enables, bits 1, 3, ... 11 control overrides **/
Divya S Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG@14 PS1, Line 14: BUG=b:145943811 Doesn't seem like the right partner bug?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39460/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/1/src/mainboard/google/voltee... PS1, Line 115: register "TcssAuxOri" = "0"
this is a bitwise field relating to each port if the flip is needed the corresponding bit x2 needs t […]
ok... I think we should make sure and document things clearly in chip.h as well as in commentary here.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39460
to look at the new patch set (#2).
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
mainboard: Set Tiger Lake platforms to have retimer config Aux orientation
In order to create a working baseline all ports are being set to have retimers. Setting the TcssAuxOri UPD to 0 in order for the SoC to not misconfigure the ports. Volteer will need some additional changes after this is implemented to account for ports that do not have a retimers this setting is in the process of being documented in the TGL EDS and we can update once it is fully understood what this setting is changing on the SOC side.
BUG=b:145943811 BRANCH=none TEST=Boot to OS and check TypeC port1 Display, Connecting type-c display should work regardless of type-c cable orientation.
Change-Id: I29eb0513299126ad8d1ee11ded2c771f28ad13f3 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 3 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39460/2
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG@9 PS1, Line 9: Type-C ports on all the Tiger Lake platforms have retimers
I will reword this... […]
Done
https://review.coreboot.org/c/coreboot/+/39460/1//COMMIT_MSG@14 PS1, Line 14: BUG=b:145943811
Doesn't seem like the right partner bug?
the bug was used for internal patch which was merged
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 2: Code-Review+2
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 2: Code-Review+2
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation ......................................................................
Patch Set 2: Code-Review-1
(6 comments)
Please improve the commit message.
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@7 PS2, Line 7: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation Please look through `git log --oneline` what prefixes are common.
mb: Configure retimer config aux orientation on TGL platforms
The prefix does not need to be a path.
tgl boards: Configure retimer config aux orientation
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@12 PS2, Line 12: a retimers a retimer?
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@14 PS2, Line 14: this setting is changing on the SOC side. Please re-flow for 72/75 characters, and add one blank line between paragraphs.
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@12 PS2, Line 12: account for ports that do not have a retimers this setting is in the process of : being documented in the TGL EDS and we can update once it is fully understood what : this setting is changing on the SOC side. New sentence starting with *this setting*?
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@19 PS2, Line 19: type-c Please spell it consisently.
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@20 PS2, Line 20: orientation. On which board?
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Paul Menzel, John Zhao, Nick Vaccaro, Srinidhi N Kaushik,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39460
to look at the new patch set (#3).
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
tgl boards: Configure retimer Aux orientation
In order to create a working baseline all ports are being set to have retimers. Setting the TcssAuxOri UPD to 0 in order for the SoC to not misconfigure the ports. Volteer will need some additional changes after this is implemented to account for ports that do not have a retimer.
This setting is in the process of being documented in the TGL EDS and we can update once it is fully understood what this setting is changing on the SOC side.
BUG=b:145943811 BRANCH=none TEST=Boot to OS and check Type-C port1 Display on Volteer, Connecting Type-c display should work regardless of Type-c cable orientation.
Change-Id: I29eb0513299126ad8d1ee11ded2c771f28ad13f3 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 3 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39460/3
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@7 PS2, Line 7: mainboard: Set Tiger Lake platforms to have retimer config Aux orientation
Please look through `git log --oneline` what prefixes are common. […]
Done
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@12 PS2, Line 12: a retimers
a retimer?
Done
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@14 PS2, Line 14: this setting is changing on the SOC side.
Please re-flow for 72/75 characters, and add one blank line between paragraphs.
Done
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@12 PS2, Line 12: account for ports that do not have a retimers this setting is in the process of : being documented in the TGL EDS and we can update once it is fully understood what : this setting is changing on the SOC side.
New sentence starting with *this setting*?
Done
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@19 PS2, Line 19: type-c
Please spell it consisently.
Done
https://review.coreboot.org/c/coreboot/+/39460/2//COMMIT_MSG@20 PS2, Line 20: orientation.
On which board?
Done
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4: Code-Review+2
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4:
Patch Set 4: Code-Review+2
Brandon, Can you clarify what the expected behavour on Port 0 (the MLB port will be). FWIU for that port we *do* want the SBU orientation to be done by the SOC (since that one doesn't have a retimer).
Does this patch accomplish that?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4: Code-Review+2
Brandon, Can you clarify what the expected behavour on Port 0 (the MLB port will be). FWIU for that port we *do* want the SBU orientation to be done by the SOC (since that one doesn't have a retimer).
Does this patch accomplish that?
the GPIO settings are taking care of that for now...We might need to do some additional changes to this setting to have it fully correct, however with the current GPIO settings and this setting set to 0 the MLB port should be functioning correctly
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... PS4, Line 130: register "TcssAuxOri" = "0" How is the register value locked? i.e how is IOM_TYPEC_SW_CONFIG_3.LOCK (bit 31) set?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... PS4, Line 130: register "TcssAuxOri" = "0"
How is the register value locked? […]
I am not positive on the answer there let me find out but I assume it is controlled by FSP before writing the values
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... PS4, Line 130: register "TcssAuxOri" = "0"
I am not positive on the answer there let me find out but I assume it is controlled by FSP before wr […]
Great, thanks. While you're at it, could you also check on how the register behavior is handled coming out of S3/S5? According to Aline this register gets reset when in S3/S5, so even if IOM_TYPEC_SW_CONFIG_3 is set for Port 0 to do SBU lane orientation in the SoC, that setting would get reset when we exit from S3/S5 (and the reset value looks to be to be 0, which means "retimer present". Is there a way to restore settings on reset?
If not, we may not be able to rely on this solution for SBU orientation configuration on the various ports in Volteer.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39460/4/src/mainboard/google/voltee... PS4, Line 130: register "TcssAuxOri" = "0"
Great, thanks. […]
From my knowledge of setting this the GPIO takes care of the resume case. Since this setting is set every time FSPS is ran there shouldn't be an issue coming out of S5. I find it strange that the FSP setting would get reset on S3 resume, I'm still looking into this some more as to the lock bit being set once I have that information I will share what needs to be done for that
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39460 )
Change subject: tgl boards: Configure retimer Aux orientation ......................................................................
tgl boards: Configure retimer Aux orientation
In order to create a working baseline all ports are being set to have retimers. Setting the TcssAuxOri UPD to 0 in order for the SoC to not misconfigure the ports. Volteer will need some additional changes after this is implemented to account for ports that do not have a retimer.
This setting is in the process of being documented in the TGL EDS and we can update once it is fully understood what this setting is changing on the SOC side.
BUG=b:145943811 BRANCH=none TEST=Boot to OS and check Type-C port1 Display on Volteer, Connecting Type-c display should work regardless of Type-c cable orientation.
Change-Id: I29eb0513299126ad8d1ee11ded2c771f28ad13f3 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39460 Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 3 files changed, 7 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Wonkyu Kim: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 6be69fe..361e563 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -127,6 +127,7 @@
# TCSS USB3 register "TcssXhciEn" = "1" + register "TcssAuxOri" = "0"
# DP port register "DdiPortAConfig" = "1" # eDP diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index 8fd9087..6cef4f8 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -103,6 +103,9 @@ [PchSerialIoIndexUART2] = PchSerialIoPci, }"
+ # TCSS USB3 + register "TcssAuxOri" = "0" + #HD Audio register "PchHdaDspEnable" = "1" register "PchHdaAudioLinkHdaEnable" = "0" diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb index 4ff35cc..c5cc800 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb @@ -99,6 +99,9 @@ [PchSerialIoIndexUART2] = PchSerialIoPci, }"
+ # TCSS USB3 + register "TcssAuxOri" = "0" + #HD Audio register "PchHdaDspEnable" = "1" register "PchHdaAudioLinkHdaEnable" = "0"
-
Aaron Durbin (Code Review) -
Brandon Breitenstein (Code Review)
-
Divya S Sasidharan (Code Review)
-
John Zhao (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Prashant Malani (Code Review)
-
Srinidhi N Kaushik (Code Review)
-
Wonkyu Kim (Code Review)