Matt Papageorge has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
vc/amd/fsp/picasso: update pci descriptor comments
Update fsp_dxio_descriptor comments to be more comprehensive of the currently available data fields. Most of these are not currently utilized with Zork but may be in future projects.
BUG=b:161218965 TEST=Build test Trembyle
Change-Id: I8eb79fa7807dcf5b28b7b0ec60953ef857d51972 Signed-off-by: Matt Papageorge matthewpapa07@gmail.com --- M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 3 files changed, 26 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/44554/1
diff --git a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c index c6e63ad..4be866d 100644 --- a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c @@ -27,7 +27,6 @@ .link_aspm_L1_2 = true, .turn_off_unused_lanes = true, .clk_req = CLK_REQ2, - .clk_pm_support = true, }, { // WLAN @@ -42,7 +41,6 @@ .link_aspm_L1_2 = true, .turn_off_unused_lanes = true, .clk_req = CLK_REQ0, - .clk_pm_support = true, }, { // SD Reader diff --git a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c index 26a5d33..49d8ade 100644 --- a/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c @@ -46,7 +46,6 @@ .link_aspm_L1_2 = true, .turn_off_unused_lanes = true, .clk_req = CLK_REQ0, - .clk_pm_support = true, }, { // SD Reader @@ -78,7 +77,6 @@ .link_aspm_L1_2 = true, .turn_off_unused_lanes = true, .clk_req = CLK_REQ4, - .clk_pm_support = true, }, { // WLAN @@ -93,7 +91,6 @@ .link_aspm_L1_2 = true, .turn_off_unused_lanes = true, .clk_req = CLK_REQ0, - .clk_pm_support = true, }, { // SD Reader diff --git a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h index d5977ef..08c1990 100644 --- a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h +++ b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h @@ -28,6 +28,14 @@ GEN_INVALID // Max Gen for boundary check } dxio_link_speed_cap;
+/* Upstream Auto Speed Change Allowed */ +typedef enum { + SPDC_DEFAULT = 0, + SPDC_DISBLED, + SPDC_ENABLED, + SPDC_INVALID +} dxio_upstream_auto_speed_change; + /* SATA ChannelType initialization */ typedef enum { SATA_CHANNEL_OTHER = 0, // Default Channel Type @@ -150,31 +158,31 @@ * GPP[3:2] | [5:4] | PCIe */ typedef struct __packed { - uint8_t engine_type; + uint8_t engine_type; // Descriptor type, see dxio_engine_type uint8_t start_logical_lane; // Start lane of the pci device uint8_t end_logical_lane; // End lane of the pci device - uint8_t gpio_group_id; // FCH reset number. 0 is global reset + uint8_t gpio_group_id; // Currently unused by FSP uint32_t port_present :1; // Should be TRUE if train link uint32_t reserved_3 :7; uint32_t device_number :5; // Desired root port device number uint32_t function_number :3; // Desired root port function number - uint32_t link_speed_capability :2; - uint32_t auto_spd_change :2; - uint32_t eq_preset :4; - uint32_t link_aspm :2; - uint32_t link_aspm_L1_1 :1; - uint32_t link_aspm_L1_2 :1; - uint32_t clk_req :4; - uint8_t link_hotplug; - uint8_t slot_power_limit; - uint32_t slot_power_limit_scale :2; + uint32_t link_speed_capability :2; // See dxio_link_speed_cap + uint32_t auto_spd_change :2; // See dxio_upstream_auto_speed_change + uint32_t eq_preset :4; // Gen3 equalization preset + uint32_t link_aspm :2; // See dxio_aspm_type + uint32_t link_aspm_L1_1 :1; // Set to 1 if link should be L1.1 capable, otherwise 0 + uint32_t link_aspm_L1_2 :1; // Set to 1 if link should be L1.2 capable, otherwise 0 + uint32_t clk_req :4; // See cpm_clk_req + uint8_t link_hotplug; // Currently unused by FSP + uint8_t slot_power_limit; // Currently unused by FSP + uint32_t slot_power_limit_scale :2; // Currently unused by FSP uint32_t reserved_4 :6; - uint32_t link_compliance_mode :1; - uint32_t link_safe_mode :1; - uint32_t sb_link :1; - uint32_t clk_pm_support :1; - uint32_t channel_type :3; - uint32_t turn_off_unused_lanes :1; + uint32_t link_compliance_mode :1; // Currently unused by FSP + uint32_t link_safe_mode :1; // Currently unused by FSP + uint32_t sb_link :1; // Currently unused by FSP + uint32_t clk_pm_support :1; // Currently unused by FSP + uint32_t channel_type :3; // See dxio_sata_channel_type + uint32_t turn_off_unused_lanes :1; // Power down lanes if device not present uint8_t reserved[4]; } fsp_dxio_descriptor;
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44554
to look at the new patch set (#2).
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
vc/amd/fsp/picasso: update pci descriptor comments
Update fsp_dxio_descriptor comments to be more comprehensive of the currently available data fields. Most of these are not currently utilized with Zork but may be in future projects.
BUG=b:161218965 TEST=Build test Trembyle
Change-Id: I8eb79fa7807dcf5b28b7b0ec60953ef857d51972 Signed-off-by: Matt Papageorge matthewpapa07@gmail.com --- M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 3 files changed, 26 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/44554/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44554
to look at the new patch set (#3).
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
vc/amd/fsp/picasso: update pci descriptor comments
Update fsp_dxio_descriptor comments to be more comprehensive of the currently available data fields. Most of these are not currently utilized with Zork but may be in future projects.
BUG=b:161218965 TEST=Build test Trembyle
Change-Id: I8eb79fa7807dcf5b28b7b0ec60953ef857d51972 Signed-off-by: Matt Papageorge matthewpapa07@gmail.com --- M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/fsps_baseboard_trembyle.c M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 3 files changed, 26 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/44554/3
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
Patch Set 3: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
Patch Set 3: Code-Review+1
hm, splitting the mainboard changes from the vendorcode ones might be a good idea. those don't end up changing FSP behaviour, but are a separate logical change
Hello Justin Frodsham, build bot (Jenkins), Furquan Shaikh, Martin Roth, Marshall Dawson, Paul Menzel, Nikolai Vyssotski, Aaron Durbin, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44554
to look at the new patch set (#4).
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
vc/amd/fsp/picasso: update pci descriptor comments
Update fsp_dxio_descriptor comments to be more comprehensive of the currently available data fields. Most of these are not currently utilized with Zork but may be in future projects.
BUG=b:161218965 TEST=Build test Trembyle
Change-Id: I8eb79fa7807dcf5b28b7b0ec60953ef857d51972 Signed-off-by: Matt Papageorge matthewpapa07@gmail.com --- M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 1 file changed, 26 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/44554/4
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
Patch Set 4: Code-Review+2
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
Patch Set 4: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
Patch Set 4: Code-Review+2
Marshall Dawson has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
vc/amd/fsp/picasso: update pci descriptor comments
Update fsp_dxio_descriptor comments to be more comprehensive of the currently available data fields. Most of these are not currently utilized with Zork but may be in future projects.
BUG=b:161218965 TEST=Build test Trembyle
Change-Id: I8eb79fa7807dcf5b28b7b0ec60953ef857d51972 Signed-off-by: Matt Papageorge matthewpapa07@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44554 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-by: Felix Held felix-coreboot@felixheld.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/vendorcode/amd/fsp/picasso/platform_descriptors.h 1 file changed, 26 insertions(+), 18 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Felix Held: Looks good to me, approved Marshall Dawson: Looks good to me, approved
diff --git a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h index d5977ef..c3a09cc 100644 --- a/src/vendorcode/amd/fsp/picasso/platform_descriptors.h +++ b/src/vendorcode/amd/fsp/picasso/platform_descriptors.h @@ -28,6 +28,14 @@ GEN_INVALID // Max Gen for boundary check } dxio_link_speed_cap;
+/* Upstream Auto Speed Change Allowed */ +typedef enum { + SPDC_DEFAULT = 0, // Enabled for Gen2 and Gen3 + SPDC_DISBLED, + SPDC_ENABLED, + SPDC_INVALID +} dxio_upstream_auto_speed_change; + /* SATA ChannelType initialization */ typedef enum { SATA_CHANNEL_OTHER = 0, // Default Channel Type @@ -150,31 +158,31 @@ * GPP[3:2] | [5:4] | PCIe */ typedef struct __packed { - uint8_t engine_type; + uint8_t engine_type; // See dxio_engine_type uint8_t start_logical_lane; // Start lane of the pci device uint8_t end_logical_lane; // End lane of the pci device - uint8_t gpio_group_id; // FCH reset number. 0 is global reset + uint8_t gpio_group_id; // Currently unused by FSP uint32_t port_present :1; // Should be TRUE if train link uint32_t reserved_3 :7; uint32_t device_number :5; // Desired root port device number uint32_t function_number :3; // Desired root port function number - uint32_t link_speed_capability :2; - uint32_t auto_spd_change :2; - uint32_t eq_preset :4; - uint32_t link_aspm :2; - uint32_t link_aspm_L1_1 :1; - uint32_t link_aspm_L1_2 :1; - uint32_t clk_req :4; - uint8_t link_hotplug; - uint8_t slot_power_limit; - uint32_t slot_power_limit_scale :2; + uint32_t link_speed_capability :2; // See dxio_link_speed_cap + uint32_t auto_spd_change :2; // See dxio_upstream_auto_speed_change + uint32_t eq_preset :4; // Gen3 equalization preset + uint32_t link_aspm :2; // See dxio_aspm_type + uint32_t link_aspm_L1_1 :1; // En/Dis root port capabilities for L1.1 + uint32_t link_aspm_L1_2 :1; // En/Dis root port capabilities for L1.2 + uint32_t clk_req :4; // See cpm_clk_req + uint8_t link_hotplug; // Currently unused by FSP + uint8_t slot_power_limit; // Currently unused by FSP + uint32_t slot_power_limit_scale :2; // Currently unused by FSP uint32_t reserved_4 :6; - uint32_t link_compliance_mode :1; - uint32_t link_safe_mode :1; - uint32_t sb_link :1; - uint32_t clk_pm_support :1; - uint32_t channel_type :3; - uint32_t turn_off_unused_lanes :1; + uint32_t link_compliance_mode :1; // Currently unused by FSP + uint32_t link_safe_mode :1; // Currently unused by FSP + uint32_t sb_link :1; // Currently unused by FSP + uint32_t clk_pm_support :1; // Currently unused by FSP + uint32_t channel_type :3; // See dxio_sata_channel_type + uint32_t turn_off_unused_lanes :1; // Power down lanes if device not present uint8_t reserved[4]; } fsp_dxio_descriptor;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44554 )
Change subject: vc/amd/fsp/picasso: update pci descriptor comments ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8 "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/15868 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/15867 "QEMU x86 i440fx/piix4" using payload SeaBIOS : FAIL : https://lava.9esec.io/r/15866 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/15865 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/15864 "HP Z220 SFF Workstation" using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/15871 "HP Compaq 8200 Elite SFF PC" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/15870 "HP Compaq 8200 Elite SFF PC" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/15869
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Felix Held (Code Review)
-
Marshall Dawson (Code Review)
-
Matt Papageorge (Code Review)