David Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/var/terrador: Enable CA Mirror ......................................................................
mb/google/volteer/var/terrador: Enable CA Mirror
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/romstage/fsp_params.c 3 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/42274/1
diff --git a/src/mainboard/google/volteer/variants/terrador/overridetree.cb b/src/mainboard/google/volteer/variants/terrador/overridetree.cb index 75422d8..8236890 100644 --- a/src/mainboard/google/volteer/variants/terrador/overridetree.cb +++ b/src/mainboard/google/volteer/variants/terrador/overridetree.cb @@ -1,4 +1,5 @@ chip soc/intel/tigerlake + register "CmdMirror" = "0x00000033" device domain 0 on end end diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index e693699..2c7b604 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -83,6 +83,9 @@ /* Rank Margin Tool. 1:Enable, 0:Disable */ uint8_t RMT;
+ /* LPDDR4x Command Pins Mirrored */ + uint32_t CmdMirror; + /* USB related */ struct usb2_port_config usb2_ports[16]; struct usb3_port_config usb3_ports[10]; diff --git a/src/soc/intel/tigerlake/romstage/fsp_params.c b/src/soc/intel/tigerlake/romstage/fsp_params.c index f7956c8..b77bb66 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params.c @@ -196,6 +196,9 @@
/* Change VmxEnable UPD value according to ENABLE_VMX Kconfig */ m_cfg->VmxEnable = CONFIG(ENABLE_VMX); + + /* LPDDR4x Command Pins Mirrored */ + m_cfg->CmdMirror[0] = config->CmdMirror; }
void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42274
to look at the new patch set (#2).
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
mb/google/volteer/variants/terrador: Enable CA Mirror
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/romstage/fsp_params.c 3 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/42274/2
Kane Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
Patch Set 2:
i think you need to split this to 2 commits. one for UPD and another one for the dev tree.
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
Patch Set 3:
Patch Set 2:
i think you need to split this to 2 commits. one for UPD and another one for the dev tree.
Done. Thanks
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
Patch Set 3: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG@7 PS3, Line 7: CA Cmd?
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG@7 PS3, Line 7: mb/google/volteer/variants/terrador: Enable CA Mirror Please summarize, why CmdMirror is good to have, and what it improves.
Derek Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG@7 PS3, Line 7: CA
Cmd?
There are different terms, let's use Cmd Mirror to align
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG@7 PS3, Line 7: mb/google/volteer/variants/terrador: Enable CA Mirror
Please summarize, why CmdMirror is good to have, and what it improves.
Can you add below in commit message? Enable CmdMirror for Terrador to achieve optimum routing from SoC to DRAMs
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro, Derek Huang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42274
to look at the new patch set (#4).
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
mb/google/volteer/variants/terrador: Enable CA Mirror
Enable CmdMirror for Terrador to achieve optimum routing from SoC to DRAMs
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/42274/4
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable CA Mirror ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG@7 PS3, Line 7: mb/google/volteer/variants/terrador: Enable CA Mirror
Can you add below in commit message? […]
Done
https://review.coreboot.org/c/coreboot/+/42274/3//COMMIT_MSG@7 PS3, Line 7: CA
There are different terms, let's use Cmd Mirror to align
Thanks.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro, Derek Huang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42274
to look at the new patch set (#5).
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
mb/google/volteer/variants/terrador: Enable Cmd Mirror
Enable CmdMirror for Terrador to achieve optimum routing from SoC to DRAMs
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/42274/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 6: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... PS6, Line 2: 0x00000033 What does this value mean?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... PS6, Line 2: 0x00000033
What does this value mean?
Look in the description for the UPD. "BitMask where bits [3:0] are Controller 0 Channel [3:0] and bits [7:4] are Controller 1 Channel [3:0]."
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... PS6, Line 2: 0x00000033
Look in the description for the UPD. […]
Can we add a comment here for the same?
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro, Derek Huang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42274
to look at the new patch set (#7).
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
mb/google/volteer/variants/terrador: Enable Cmd Mirror
Enable CmdMirror for Terrador to achieve optimum routing from SoC to DRAMs
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/42274/7
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/6/src/mainboard/google/voltee... PS6, Line 2: 0x00000033
Can we add a comment here for the same?
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... PS7, Line 2: # BitMask where bits [3:0] are Controller 0 Channel [3:0] and : # bits [7:4] are Controller 1 Channel [3:0]. What about making this more specific to what the bits means here? e.g., "Enable command mirroring for controller 0 channel 3, and controller 1 channel 3" ?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... PS7, Line 2: # BitMask where bits [3:0] are Controller 0 Channel [3:0] and : # bits [7:4] are Controller 1 Channel [3:0].
What about making this more specific to what the bits means here? e.g., […]
+1. channels 0 and 3, right?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... PS7, Line 2: # BitMask where bits [3:0] are Controller 0 Channel [3:0] and : # bits [7:4] are Controller 1 Channel [3:0].
+1. […]
Oops, I can read bitmasks 😄
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... PS7, Line 2: # BitMask where bits [3:0] are Controller 0 Channel [3:0] and : # bits [7:4] are Controller 1 Channel [3:0].
Oops, I can read bitmasks 😄
Woops me too :P... Channel 0 and 1.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro, Derek Huang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42274
to look at the new patch set (#8).
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
mb/google/volteer/variants/terrador: Enable Cmd Mirror
Enable CmdMirror for Terrador to achieve optimum routing from SoC to DRAMs
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/42274/8
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 8:
(1 comment)
Thanks.
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/terrador/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42274/7/src/mainboard/google/voltee... PS7, Line 2: # BitMask where bits [3:0] are Controller 0 Channel [3:0] and : # bits [7:4] are Controller 1 Channel [3:0].
Woops me too :P... Channel 0 and 1.
Done.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
Patch Set 8: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42274 )
Change subject: mb/google/volteer/variants/terrador: Enable Cmd Mirror ......................................................................
mb/google/volteer/variants/terrador: Enable Cmd Mirror
Enable CmdMirror for Terrador to achieve optimum routing from SoC to DRAMs
BUG=b:156435028 BRANCH=none TEST=FW_NAME=terrador emerge-volteer coreboot chromeos-bootimage
Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Change-Id: I0db9fff0dddf35c99a6cb2a90d40886ed8e18686 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42274 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/volteer/variants/terrador/overridetree.cb 1 file changed, 5 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/variants/terrador/overridetree.cb b/src/mainboard/google/volteer/variants/terrador/overridetree.cb index 4bff10d..81da6bd 100644 --- a/src/mainboard/google/volteer/variants/terrador/overridetree.cb +++ b/src/mainboard/google/volteer/variants/terrador/overridetree.cb @@ -1,4 +1,9 @@ chip soc/intel/tigerlake + # BitMask where bits [3:0] are Controller 0 Channel [3:0] and + # bits [7:4] are Controller 1 Channel [3:0]. + # Enable Command Mirroring for controller 0 channel 0 and 1, + # and controller 1 channel 0 and 1. + register "CmdMirror" = "0x00000033" device domain 0 on device pci 15.1 on chip drivers/i2c/hid
-
David Wu (Code Review) -
Derek Huang (Code Review)
-
Furquan Shaikh (Code Review)
-
Kane Chen (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Fagerburg (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)