Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
vc/amd/fsp/picasso: update UPD header
BUG=b:161152720 TEST=SATA on Mandolin works.
Change-Id: I1e27597e22af4df65d206a38b67c4920298b30b2 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/vendorcode/amd/fsp/picasso/FspmUpd.h M src/vendorcode/amd/fsp/picasso/FspsUpd.h 2 files changed, 26 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/43659/1
diff --git a/src/vendorcode/amd/fsp/picasso/FspmUpd.h b/src/vendorcode/amd/fsp/picasso/FspmUpd.h index 3be69c3..c1766e8 100644 --- a/src/vendorcode/amd/fsp/picasso/FspmUpd.h +++ b/src/vendorcode/amd/fsp/picasso/FspmUpd.h @@ -61,7 +61,10 @@ /** Offset 0x00CE**/ uint8_t unused8; /** Offset 0x00CF**/ uint8_t unused9; /** Offset 0x00D0**/ uint32_t bert_size; - /** Offset 0x00D4**/ uint8_t UnusedUpdSpace0[44]; + /** Offset 0x00D4**/ uint8_t UnusedUpdSpace0; + /** Offset 0x00D5**/ uint8_t ccx_down_core_mode; + /** Offset 0x00D6**/ uint8_t ccx_disable_smt; + /** Offset 0x00D7**/ uint8_t UnusedUpdSpace1[41]; /** Offset 0x0100**/ uint16_t Reserved100; /** Offset 0x0102**/ uint16_t UpdTerminator; } FSP_M_CONFIG; diff --git a/src/vendorcode/amd/fsp/picasso/FspsUpd.h b/src/vendorcode/amd/fsp/picasso/FspsUpd.h index 11e77bd..4298b11 100644 --- a/src/vendorcode/amd/fsp/picasso/FspsUpd.h +++ b/src/vendorcode/amd/fsp/picasso/FspsUpd.h @@ -9,36 +9,34 @@
#include <FspUpd.h>
-#define FSPS_UPD_DXIO_DESCRIPTOR_COUNT 6 +#define FSPS_UPD_DXIO_DESCRIPTOR_COUNT 8 #define FSPS_UPD_DDI_DESCRIPTOR_COUNT 4
typedef struct __packed { /** Offset 0x0020**/ uint32_t emmc0_mode; /** Offset 0x0024**/ uint8_t unused0[12]; /** Offset 0x0030**/ uint8_t dxio_descriptor[FSPS_UPD_DXIO_DESCRIPTOR_COUNT][16]; - /** Offset 0x0090**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT]; - /** Offset 0x00A0**/ uint32_t unused1; - /** Offset 0x00A4**/ uint32_t unused2; - /** Offset 0x00A8**/ uint32_t unused3; - /** Offset 0x00AC**/ uint32_t unused4; - /** Offset 0x00B0**/ uint8_t fch_usb_version_major; - /** Offset 0x00B1**/ uint8_t fch_usb_version_minor; - /** Offset 0x00B2**/ uint8_t fch_usb_2_port0_phy_tune[9]; - /** Offset 0x00BB**/ uint8_t fch_usb_2_port1_phy_tune[9]; - /** Offset 0x00C4**/ uint8_t fch_usb_2_port2_phy_tune[9]; - /** Offset 0x00CD**/ uint8_t fch_usb_2_port3_phy_tune[9]; - /** Offset 0x00D6**/ uint8_t fch_usb_2_port4_phy_tune[9]; - /** Offset 0x00DF**/ uint8_t fch_usb_2_port5_phy_tune[9]; - /** Offset 0x00E8**/ uint8_t fch_usb_device_removable; - /** Offset 0x00E9**/ uint8_t fch_usb_3_port_force_gen1; - /** Offset 0x00EA**/ uint8_t fch_usb_u3_rx_det_wa_enable; - /** Offset 0x00EB**/ uint8_t fch_usb_u3_rx_det_wa_portmap; - /** Offset 0x00EC**/ uint8_t fch_usb_early_debug_select_enable; - /** Offset 0x00ED**/ uint8_t unused8; - /** Offset 0x00EE**/ uint32_t xhci_oc_pin_select; - /** Offset 0x00F2**/ uint8_t xhci0_force_gen1; - /** Offset 0x00F3**/ uint8_t UnusedUpdSpace0[45]; - /** Offset 0x0120**/ uint16_t UpdTerminator; + /** Offset 0x00B0**/ uint8_t unused1[16]; + /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT]; + /** Offset 0x00D0**/ uint8_t unused2[16]; + /** Offset 0x00E0**/ uint8_t fch_usb_version_major; + /** Offset 0x00E1**/ uint8_t fch_usb_version_minor; + /** Offset 0x00E2**/ uint8_t fch_usb_2_port0_phy_tune[9]; + /** Offset 0x00EB**/ uint8_t fch_usb_2_port1_phy_tune[9]; + /** Offset 0x00F4**/ uint8_t fch_usb_2_port2_phy_tune[9]; + /** Offset 0x00FD**/ uint8_t fch_usb_2_port3_phy_tune[9]; + /** Offset 0x0106**/ uint8_t fch_usb_2_port4_phy_tune[9]; + /** Offset 0x010F**/ uint8_t fch_usb_2_port5_phy_tune[9]; + /** Offset 0x0118**/ uint8_t fch_usb_device_removable; + /** Offset 0x0119**/ uint8_t fch_usb_3_port_force_gen1; + /** Offset 0x011A**/ uint8_t fch_usb_u3_rx_det_wa_enable; + /** Offset 0x011B**/ uint8_t fch_usb_u3_rx_det_wa_portmap; + /** Offset 0x011C**/ uint8_t fch_usb_early_debug_select_enable; + /** Offset 0x011D**/ uint8_t unused3; + /** Offset 0x011E**/ uint32_t xhci_oc_pin_select; + /** Offset 0x0122**/ uint8_t xhci0_force_gen1; + /** Offset 0x0123**/ uint8_t UnusedUpdSpace0[45]; + /** Offset 0x0150**/ uint16_t UpdTerminator; } FSP_S_CONFIG;
/** Fsp S UPD Configuration
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 1:
this needs to be coordinated with the merge of the corresponding FSP patch
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43659/1//COMMIT_MSG@8 PS1, Line 8: Please give more details, where you got the update from.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43659/1//COMMIT_MSG@8 PS1, Line 8:
Please give more details, where you got the update from.
I'm not really sure what you want me to document in more details here. The headers get generated when building the FSP blob; I took those new ones and basically reapplied the changes I made to the upstream ones to the new ones
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43659
to look at the new patch set (#2).
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
vc/amd/fsp/picasso: update UPD header
A new version of UPD headers generated from the FSP tree. This adds UPDs for downcoring and increases the number of DXIO descriptor slots.
BUG=b:161152720 TEST=SATA on Mandolin works.
Change-Id: I1e27597e22af4df65d206a38b67c4920298b30b2 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/vendorcode/amd/fsp/picasso/FspmUpd.h M src/vendorcode/amd/fsp/picasso/FspsUpd.h 2 files changed, 26 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/43659/2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... File src/vendorcode/amd/fsp/picasso/FspsUpd.h:
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... PS2, Line 20: /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT]; Where is the corresponding private CL for changing this ABI?
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... File src/vendorcode/amd/fsp/picasso/FspsUpd.h:
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... PS2, Line 20: /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT];
Where is the corresponding private CL for changing this ABI?
https://chrome-internal-review.googlesource.com/c/chromeos/third_party/amd-f... should i add that to the commit message?
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... File src/vendorcode/amd/fsp/picasso/FspsUpd.h:
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... PS2, Line 20: /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT];
my plan with these two patches is to hand them over to Raul after I've looked into how to properly do the binary interface version checking and added the changes needed for that on the FSP side, so that he can make sure that the chromebook firmware images that are automatically built will never contain mismatched versions
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... File src/vendorcode/amd/fsp/picasso/FspsUpd.h:
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... PS2, Line 20: /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT];
my plan with these two patches is to hand them over to Raul after I've looked into how to properly d […]
You'll need to add a Cq-Depend line on this CL's commit description.
https://chromium.googlesource.com/chromiumos/docs/+/master/contributing.md#C...
When this CL comes over to chromium, we'd update the internal CL to have a Cq-Depend on this patch as a chromium CL. Then we push them through the CQ together.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43659
to look at the new patch set (#3).
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
vc/amd/fsp/picasso: update UPD header
A new version of UPD headers generated from the FSP tree. This adds UPDs for downcoring and increases the number of DXIO descriptor slots.
BUG=b:161152720 TEST=SATA on Mandolin works now.
Cq-Depend: chrome-internal:3175393
Change-Id: I1e27597e22af4df65d206a38b67c4920298b30b2 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/vendorcode/amd/fsp/picasso/FspmUpd.h M src/vendorcode/amd/fsp/picasso/FspsUpd.h 2 files changed, 26 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/43659/3
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 3: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... File src/vendorcode/amd/fsp/picasso/FspsUpd.h:
https://review.coreboot.org/c/coreboot/+/43659/2/src/vendorcode/amd/fsp/pica... PS2, Line 20: /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT];
You'll need to add a Cq-Depend line on this CL's commit description. […]
added the Cq-Depend for the FSP-side patch
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43659/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43659/1//COMMIT_MSG@8 PS1, Line 8:
I'm not really sure what you want me to document in more details here. […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
Patch Set 4: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43659 )
Change subject: vc/amd/fsp/picasso: update UPD header ......................................................................
vc/amd/fsp/picasso: update UPD header
A new version of UPD headers generated from the FSP tree. This adds UPDs for downcoring and increases the number of DXIO descriptor slots.
BUG=b:161152720 TEST=SATA on Mandolin works now.
Cq-Depend: chrome-internal:3175393
Change-Id: I1e27597e22af4df65d206a38b67c4920298b30b2 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/43659 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/vendorcode/amd/fsp/picasso/FspmUpd.h M src/vendorcode/amd/fsp/picasso/FspsUpd.h 2 files changed, 26 insertions(+), 25 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/vendorcode/amd/fsp/picasso/FspmUpd.h b/src/vendorcode/amd/fsp/picasso/FspmUpd.h index 3be69c3..c1766e8 100644 --- a/src/vendorcode/amd/fsp/picasso/FspmUpd.h +++ b/src/vendorcode/amd/fsp/picasso/FspmUpd.h @@ -61,7 +61,10 @@ /** Offset 0x00CE**/ uint8_t unused8; /** Offset 0x00CF**/ uint8_t unused9; /** Offset 0x00D0**/ uint32_t bert_size; - /** Offset 0x00D4**/ uint8_t UnusedUpdSpace0[44]; + /** Offset 0x00D4**/ uint8_t UnusedUpdSpace0; + /** Offset 0x00D5**/ uint8_t ccx_down_core_mode; + /** Offset 0x00D6**/ uint8_t ccx_disable_smt; + /** Offset 0x00D7**/ uint8_t UnusedUpdSpace1[41]; /** Offset 0x0100**/ uint16_t Reserved100; /** Offset 0x0102**/ uint16_t UpdTerminator; } FSP_M_CONFIG; diff --git a/src/vendorcode/amd/fsp/picasso/FspsUpd.h b/src/vendorcode/amd/fsp/picasso/FspsUpd.h index 11e77bd..4298b11 100644 --- a/src/vendorcode/amd/fsp/picasso/FspsUpd.h +++ b/src/vendorcode/amd/fsp/picasso/FspsUpd.h @@ -9,36 +9,34 @@
#include <FspUpd.h>
-#define FSPS_UPD_DXIO_DESCRIPTOR_COUNT 6 +#define FSPS_UPD_DXIO_DESCRIPTOR_COUNT 8 #define FSPS_UPD_DDI_DESCRIPTOR_COUNT 4
typedef struct __packed { /** Offset 0x0020**/ uint32_t emmc0_mode; /** Offset 0x0024**/ uint8_t unused0[12]; /** Offset 0x0030**/ uint8_t dxio_descriptor[FSPS_UPD_DXIO_DESCRIPTOR_COUNT][16]; - /** Offset 0x0090**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT]; - /** Offset 0x00A0**/ uint32_t unused1; - /** Offset 0x00A4**/ uint32_t unused2; - /** Offset 0x00A8**/ uint32_t unused3; - /** Offset 0x00AC**/ uint32_t unused4; - /** Offset 0x00B0**/ uint8_t fch_usb_version_major; - /** Offset 0x00B1**/ uint8_t fch_usb_version_minor; - /** Offset 0x00B2**/ uint8_t fch_usb_2_port0_phy_tune[9]; - /** Offset 0x00BB**/ uint8_t fch_usb_2_port1_phy_tune[9]; - /** Offset 0x00C4**/ uint8_t fch_usb_2_port2_phy_tune[9]; - /** Offset 0x00CD**/ uint8_t fch_usb_2_port3_phy_tune[9]; - /** Offset 0x00D6**/ uint8_t fch_usb_2_port4_phy_tune[9]; - /** Offset 0x00DF**/ uint8_t fch_usb_2_port5_phy_tune[9]; - /** Offset 0x00E8**/ uint8_t fch_usb_device_removable; - /** Offset 0x00E9**/ uint8_t fch_usb_3_port_force_gen1; - /** Offset 0x00EA**/ uint8_t fch_usb_u3_rx_det_wa_enable; - /** Offset 0x00EB**/ uint8_t fch_usb_u3_rx_det_wa_portmap; - /** Offset 0x00EC**/ uint8_t fch_usb_early_debug_select_enable; - /** Offset 0x00ED**/ uint8_t unused8; - /** Offset 0x00EE**/ uint32_t xhci_oc_pin_select; - /** Offset 0x00F2**/ uint8_t xhci0_force_gen1; - /** Offset 0x00F3**/ uint8_t UnusedUpdSpace0[45]; - /** Offset 0x0120**/ uint16_t UpdTerminator; + /** Offset 0x00B0**/ uint8_t unused1[16]; + /** Offset 0x00C0**/ uint32_t ddi_descriptor[FSPS_UPD_DDI_DESCRIPTOR_COUNT]; + /** Offset 0x00D0**/ uint8_t unused2[16]; + /** Offset 0x00E0**/ uint8_t fch_usb_version_major; + /** Offset 0x00E1**/ uint8_t fch_usb_version_minor; + /** Offset 0x00E2**/ uint8_t fch_usb_2_port0_phy_tune[9]; + /** Offset 0x00EB**/ uint8_t fch_usb_2_port1_phy_tune[9]; + /** Offset 0x00F4**/ uint8_t fch_usb_2_port2_phy_tune[9]; + /** Offset 0x00FD**/ uint8_t fch_usb_2_port3_phy_tune[9]; + /** Offset 0x0106**/ uint8_t fch_usb_2_port4_phy_tune[9]; + /** Offset 0x010F**/ uint8_t fch_usb_2_port5_phy_tune[9]; + /** Offset 0x0118**/ uint8_t fch_usb_device_removable; + /** Offset 0x0119**/ uint8_t fch_usb_3_port_force_gen1; + /** Offset 0x011A**/ uint8_t fch_usb_u3_rx_det_wa_enable; + /** Offset 0x011B**/ uint8_t fch_usb_u3_rx_det_wa_portmap; + /** Offset 0x011C**/ uint8_t fch_usb_early_debug_select_enable; + /** Offset 0x011D**/ uint8_t unused3; + /** Offset 0x011E**/ uint32_t xhci_oc_pin_select; + /** Offset 0x0122**/ uint8_t xhci0_force_gen1; + /** Offset 0x0123**/ uint8_t UnusedUpdSpace0[45]; + /** Offset 0x0150**/ uint16_t UpdTerminator; } FSP_S_CONFIG;
/** Fsp S UPD Configuration
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Felix Held (Code Review)
-
Paul Menzel (Code Review)