Brandon Breitenstein has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATUON_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/fsp_params_tgl.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/1
diff --git a/src/soc/intel/tigerlake/fsp_params_tgl.c b/src/soc/intel/tigerlake/fsp_params_tgl.c index 14997c5..dced634 100644 --- a/src/soc/intel/tigerlake/fsp_params_tgl.c +++ b/src/soc/intel/tigerlake/fsp_params_tgl.c @@ -86,6 +86,7 @@ else params->PeiGraphicsPeimInit = 0;
+ params->TcssAuxOri = 0; for (i = 0; i < 8; i++) params->IomTypeCPortPadCfg[i] = 0x09000000;
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/1/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_tgl.c:
https://review.coreboot.org/c/coreboot/+/39459/1/src/soc/intel/tigerlake/fsp... PS1, Line 89: 0 Can we use config variable rather than hard code value? This can be changed dependent on board design.
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 1:
Can you also check pin mux for AUX pins and create patch if we need in upstream baseline?
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATUON_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/2
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/1/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_tgl.c:
https://review.coreboot.org/c/coreboot/+/39459/1/src/soc/intel/tigerlake/fsp... PS1, Line 89: 0
Can we use config variable rather than hard code value? […]
Done
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2: Code-Review+2
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@10 PS2, Line 10: port 2 shouldn't this be configurable independently on each port?
what about the preceding 2 TCSS parameters?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@9 PS2, Line 9: CONFIGURATUON nit: CONFIGURATION? (also title line)
Also, the naming scheme makes it look like a #define, but that name is never used in the patch - what gives?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@9 PS2, Line 9: CONFIGURATUON
nit: CONFIGURATION? (also title line) […]
The EDS referenced at the end of the commit message explains this configuration
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@10 PS2, Line 10: port 2
shouldn't this be […]
I will update the commit message this is able to control all the type-c port retimer enabled or disabled control
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@10 PS2, Line 10: port 2
I will update the commit message this is able to control all the type-c port retimer enabled or disa […]
so, is this a global control bit that applies to all the ports as a group? wouldn't we need to control the retimer on each port indepentlenly to match the requirements of the system?
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@10 PS2, Line 10: port 2
so, is this a global control bit that applies to all the ports as a group? wouldn't we need to cont […]
ah, reading CB:39460, this is a bitmask. we definitely need to better way to communicate that to the reader of devicetree.cb.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports the TGL EDS describes this setting and what each bit represents
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/3
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@9 PS2, Line 9: CONFIGURATUON
The EDS referenced at the end of the commit message explains this configuration
I was referring to the typo (TUON instead of TION)
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@9 PS2, Line 9: CONFIGURATUON
I was referring to the typo (TUON instead of TION)
I was responding to the #define comment after :)
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@9 PS2, Line 9: CONFIGURATUON
I was responding to the #define comment after :)
Ah, my bad. Thanks!
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/2//COMMIT_MSG@10 PS2, Line 10: port 2
ah, reading CB:39460, this is a bitmask. […]
Done
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3: Code-Review+2
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39459/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/3//COMMIT_MSG@16 PS3, Line 16: setting and what each bit represents Please add a dot/period at the end of sentences.
https://review.coreboot.org/c/coreboot/+/39459/3//COMMIT_MSG@23 PS3, Line 23: orientation. On what board?
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 237: * this is a bitfield that corresponds to up to 5 TCSS ports Dot/period or colon at the end.
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 237: this This …
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 241: * of the physical aux lines on the motherboard Format the items as a list?
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 242: */ New textwidth is 96 characters.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, John Zhao, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports the TGL EDS describes this setting and what each bit represents.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/4
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 4:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39459/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/3//COMMIT_MSG@16 PS3, Line 16: setting and what each bit represents
Please add a dot/period at the end of sentences.
Done
https://review.coreboot.org/c/coreboot/+/39459/3//COMMIT_MSG@23 PS3, Line 23: orientation.
On what board?
Done
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 237: * this is a bitfield that corresponds to up to 5 TCSS ports
Dot/period or colon at the end.
Done
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 237: this
This …
Done
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 241: * of the physical aux lines on the motherboard
Format the items as a list?
Done
https://review.coreboot.org/c/coreboot/+/39459/3/src/soc/intel/tigerlake/chi... PS3, Line 242: */
New textwidth is 96 characters.
Done
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 236: 6 is this the right number for TGL?
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 237: 10 how does that work with a uint8_t?
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, John Zhao, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#5).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports the TGL EDS describes this setting and what each bit represents.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/5
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 236: 6
is this the right number for TGL?
IOM has the capability to control up to 6 Type-C ports so this is correct
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 237: 10
how does that work with a uint8_t?
I messed up this should be a uint16 fixing
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 5: Code-Review+1
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 236: 6
IOM has the capability to control up to 6 Type-C ports so this is correct
Can you check EDS? Even if IOM register support up to 6 ports. UP3/UP4 support up to 4 according to EDS(#575683) • TGL-UP3 processor supports a maximum of four USB-C* ports • TGL-UP4 processor supports a maximum of three USB-C* ports
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, John Zhao, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#6).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports the TGL EDS describes this setting and what each bit represents.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/6
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG@21 PS5, Line 21: Boot to OS and check Type-C port1 Display on volteer, : Connecting Type-c display should work regardless of Type-c cable : orientation. Can you clarify whether this sets the MLB port (port 0, or, port 1 if we're talking about this in terms of IOM_TYPEC_SW_CONFIGURATION) to have the SBU lane-reversal be done on the SOC?
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 236: 6
Can you check EDS? Even if IOM register support up to 6 ports. UP3/UP4 support up to 4 […]
Done
https://review.coreboot.org/c/coreboot/+/39459/4/src/soc/intel/tigerlake/chi... PS4, Line 237: 10
I messed up this should be a uint16 fixing
Done
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATUON_3 ......................................................................
Patch Set 6: Code-Review+1
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, John Zhao, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3 Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports the TGL EDS describes this setting and what each bit represents.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/7
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG@21 PS5, Line 21: Boot to OS and check Type-C port1 Display on volteer, : Connecting Type-c display should work regardless of Type-c cable : orientation.
Can you clarify whether this sets the MLB port (port 0, or, port 1 if we're talking about this in te […]
This is setting the SOC to do nothing unless told to. The GPIO settings for port 0 are allowing the SOC to do the swapping in this case.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7: Code-Review+1
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG@21 PS5, Line 21: Boot to OS and check Type-C port1 Display on volteer, : Connecting Type-c display should work regardless of Type-c cable : orientation.
This is setting the SOC to do nothing unless told to. […]
Can you kindly clarify what is meant by "nothing unless told to"? My understanding is as follows: - If IOM_TYPEC_SW_CONFIGURATION_3 register even bits are set to 0, then for the corresponding ports, the SOC *will not* do lane reversal of the SBU lines, even if the SCU IPC / Mux agent driver program the IOM to do so. - If IOM_TYPEC_SW_CONFIGURATION_3 register even bits are set to 1, then for the corresponding ports, the SOC *will* do lane reversal of the SBU lines, when the SCU IPC / Mux agent driver program the IOM to do so.
Is the above correct?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/5//COMMIT_MSG@21 PS5, Line 21: Boot to OS and check Type-C port1 Display on volteer, : Connecting Type-c display should work regardless of Type-c cable : orientation.
Can you kindly clarify what is meant by "nothing unless told to"? My understanding is as follows: […]
correct that is the expected behavior
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7: Code-Review+2
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@18 PS7, Line 18: 3.6.5 3.6.6?
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7: Code-Review+2
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@18 PS7, Line 18: 3.6.5
3.6. […]
Ack
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@9 PS7, Line 9: FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3 Please add a dot/period at the end of sentences.
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@15 PS7, Line 15: ports the … ports. The …
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@22 PS7, Line 22: Type-c Type-C
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@22 PS7, Line 22: Type-c Type-C
https://review.coreboot.org/c/coreboot/+/39459/7/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/7/src/soc/intel/tigerlake/chi... PS7, Line 239: * on the motherboard. Please format this as a list.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, John Zhao, Nick Vaccaro, Srinidhi N Kaushik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39459
to look at the new patch set (#8).
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3. Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports. The TGL EDS describes this setting and what each bit represents.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39459/8
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
Patch Set 8:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@9 PS7, Line 9: FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3
Please add a dot/period at the end of sentences.
Done
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@15 PS7, Line 15: ports the
… ports. […]
Done
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@22 PS7, Line 22: Type-c
Type-C
Done
https://review.coreboot.org/c/coreboot/+/39459/7//COMMIT_MSG@22 PS7, Line 22: Type-c
Type-C
Done
https://review.coreboot.org/c/coreboot/+/39459/7/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39459/7/src/soc/intel/tigerlake/chi... PS7, Line 239: * on the motherboard.
Please format this as a list.
what kind of list are you thinking there is no point listing out the entire bitfield here as it is described in the EDS
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39459 )
Change subject: soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3 ......................................................................
soc/intel/tigerlake: Configure IOM_TYPEC_SW_CONFIGURATION_3
FSP UPD TcssAuxOri is used for setting the IOM_TYPEC_SW_CONFIGURATION_3. Configure TcssAuxOri to retimer enabled on the port 2 Type-C port. This setting informs the SoC that a retimer is taking care of SBU orientation therefore it does not need to do any flipping.
The IOM_TYPEC_SW_CONFIGURATION_3 is a bitfield that controls the aux orientation settings for the Type-C ports. The TGL EDS describes this setting and what each bit represents.
Reference section 3.6.5 in TGL EDS #575681 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: Iae356113cbdc72983f800060b1ebebe3c66b9daf Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39459 Reviewed-by: Caveh Jalali caveh@chromium.org Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Reviewed-by: Nick Vaccaro nvaccaro@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c 2 files changed, 10 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nick Vaccaro: Looks good to me, approved 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 1d4bd5f..64c13ce 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -232,6 +232,15 @@ uint8_t TcssXdciEn;
/* + * SOC Aux orientation override: + * This is a bitfield that corresponds to up to 4 TCSS ports on TGL. + * Even numbered bits (0, 2, 4, 6) control the retimer being handled by SOC. + * Odd numbered bits (1, 3, 5, 7) control the orientation of the physical aux lines + * on the motherboard. + */ + uint16_t TcssAuxOri; + + /* * Override GPIO PM configuration: * 0: Use FSP default GPIO PM program, * 1: coreboot to override GPIO PM program diff --git a/src/soc/intel/tigerlake/fsp_params_tgl.c b/src/soc/intel/tigerlake/fsp_params_tgl.c index a8be407..8e9787b 100644 --- a/src/soc/intel/tigerlake/fsp_params_tgl.c +++ b/src/soc/intel/tigerlake/fsp_params_tgl.c @@ -104,6 +104,7 @@ else params->PeiGraphicsPeimInit = 0;
+ params->TcssAuxOri = config->TcssAuxOri; for (i = 0; i < 8; i++) params->IomTypeCPortPadCfg[i] = 0x09000000;
-
Brandon Breitenstein (Code Review) -
Caveh Jalali (Code Review)
-
John Zhao (Code Review)
-
Nick Vaccaro (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Prashant Malani (Code Review)
-
Srinidhi N Kaushik (Code Review)
-
Wonkyu Kim (Code Review)