Change in coreboot[master]: soc/amd/picasso: Add UPD settings to chip.h

[Marshall Dawson (Code Review)]

Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

soc/amd/picasso: Add UPD settings to chip.h

Add values that align with UPD settings.

Signed-off-by: Marshall Dawson 
Change-Id: I6bce44a43e57ba00d2b29cfa6249cef51e9ceabb
---
M src/soc/amd/picasso/chip.h
1 file changed, 41 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/38699/1

diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h
index 4e9e18b..0681277 100644
--- a/src/soc/amd/picasso/chip.h
+++ b/src/soc/amd/picasso/chip.h
@@ -44,10 +44,51 @@
    	I2S_PINS_I2S_TDM = 4,
    	I2S_PINS_UNCONF = 7,	/* All pads will be input mode */
    } acp_pin_cfg;
+
+	/* Options for these are in src/arch/x86/include/arch/acpi.h */
+	uint8_t  fadt_pm_profile;
+	uint16_t fadt_boot_arch;
+	uint32_t fadt_flags;
+
+	/* System config index */
+	uint8_t system_config;
+
+	/* STAPM Configuration */
+	uint32_t fast_ppt_limit;
+	uint32_t slow_ppt_limit;
+	uint32_t slow_ppt_time_constant;
+	uint32_t stapm_time_constant;
+	uint32_t sustained_power_limit;
+
+	/* PROCHOT_L de-assertion Ramp Time */
+	uint32_t prochot_l_deassertion_ramp_time;
+
+	/* Lower die temperature limit */
+	uint32_t thermctl_limit;
+
+	/* FP5 Processor Voltage Supply PSI Currents. 0 indicates use SOC default */
+	uint32_t psi0_current_limit;
+	uint32_t psi0_soc_current_limit;
+	uint32_t vddcr_soc_voltage_margin;
+	uint32_t vddcr_vdd_voltage_margin;
+
+	/* VRM Limits. 0 indicates use SOC default */
+	uint32_t vrm_maximum_current_limit;
+	uint32_t vrm_soc_maximum_current_limit;
+	uint32_t vrm_current_limit;
+	uint32_t vrm_soc_current_limit;
+
+	/* Misc SMU settings */
+	uint8_t sb_tsi_alert_comparator_mode_en;
+	uint8_t core_dldo_bypass;
+	uint8_t min_soc_vid_offset;
+	uint8_t aclk_dpm0_freq_400MHz;
 };

typedef struct soc_amd_picasso_config config_t;

+const config_t *get_soc_config(void);
+
 extern struct device_operations pci_domain_ops;

#endif /* __PICASSO_CHIP_H__ */

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38699/2/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/38699/2/src/soc/amd/picasso/chip.h@... 
PS2, Line 77: 
            : const config_t *get_soc_config(void);
do this in a separate commit?

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38699/2/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/38699/2/src/soc/amd/picasso/chip.h@... 
PS2, Line 77: 
            : const config_t *get_soc_config(void);
> do this in a separate commit?

done

[Raul Rangel (Code Review)]

Raul Rangel has uploaded a new patch set (#7) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

soc/amd/picasso: Add UPD settings to chip.h

Add values that align with UPD settings.

BUG=b:153675909
TEST=Trembyle builds and boots to payload

Signed-off-by: Marshall Dawson 
Change-Id: I6bce44a43e57ba00d2b29cfa6249cef51e9ceabb
---
M src/soc/amd/picasso/chip.h
1 file changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/38699/7

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38699/7/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/38699/7/src/soc/amd/picasso/chip.h@... 
PS7, Line 36: 	/* Options for these are in src/arch/x86/include/arch/acpi.h */
Are all these really used by SoC and have to be provided by mainboard? Can we add them as required later on?

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38699/7/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/38699/7/src/soc/amd/picasso/chip.h@... 
PS7, Line 36: 	/* Options for these are in src/arch/x86/include/arch/acpi.h */
> Are all these really used by SoC and have to be provided by mainboard? Can we add them as required l […]

We do define them all: https://source.corp.google.com/chromeos_public/src/third_party/coreboot-zork...

I could go and break them all up into their own CLs, though they all get used here: https://review.coreboot.org/c/coreboot/+/34423/25/src/soc/amd/picasso/romsta...

I'm just trying to get the CLs that don't have any deps on bootblock landed to shrink the patch train.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 7: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/38699/7/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/38699/7/src/soc/amd/picasso/chip.h@... 
PS7, Line 36: 	/* Options for these are in src/arch/x86/include/arch/acpi.h */
> We do define them all: https://source.corp.google. […]

Ah okay. Sounds good. Let's keep it this way.

[Raul Rangel (Code Review)]

Raul Rangel has uploaded a new patch set (#9) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

soc/amd/picasso: Add UPD settings to chip.h

Add values that align with UPD settings.

BUG=b:153675909
TEST=Trembyle builds and boots to payload

Signed-off-by: Marshall Dawson 
Change-Id: I6bce44a43e57ba00d2b29cfa6249cef51e9ceabb
---
M src/soc/amd/picasso/chip.h
1 file changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/38699/9

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38699/2/src/soc/amd/picasso/chip.h 
File src/soc/amd/picasso/chip.h:

https://review.coreboot.org/c/coreboot/+/38699/2/src/soc/amd/picasso/chip.h@... 
PS2, Line 77: 
            : const config_t *get_soc_config(void);
> done

Done

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

soc/amd/picasso: Add UPD settings to chip.h

Add values that align with UPD settings.

BUG=b:153675909
TEST=Trembyle builds and boots to payload

Signed-off-by: Marshall Dawson 
Change-Id: I6bce44a43e57ba00d2b29cfa6249cef51e9ceabb
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38699
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/soc/amd/picasso/chip.h
1 file changed, 39 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h
index 9c756ed..4cc10ef 100644
--- a/src/soc/amd/picasso/chip.h
+++ b/src/soc/amd/picasso/chip.h
@@ -31,6 +31,45 @@
    	I2S_PINS_I2S_TDM = 4,
    	I2S_PINS_UNCONF = 7,	/* All pads will be input mode */
    } acp_pin_cfg;
+
+	/* Options for these are in src/arch/x86/include/arch/acpi.h */
+	uint8_t  fadt_pm_profile;
+	uint16_t fadt_boot_arch;
+	uint32_t fadt_flags;
+
+	/* System config index */
+	uint8_t system_config;
+
+	/* STAPM Configuration */
+	uint32_t fast_ppt_limit;
+	uint32_t slow_ppt_limit;
+	uint32_t slow_ppt_time_constant;
+	uint32_t stapm_time_constant;
+	uint32_t sustained_power_limit;
+
+	/* PROCHOT_L de-assertion Ramp Time */
+	uint32_t prochot_l_deassertion_ramp_time;
+
+	/* Lower die temperature limit */
+	uint32_t thermctl_limit;
+
+	/* FP5 Processor Voltage Supply PSI Currents. 0 indicates use SOC default */
+	uint32_t psi0_current_limit;
+	uint32_t psi0_soc_current_limit;
+	uint32_t vddcr_soc_voltage_margin;
+	uint32_t vddcr_vdd_voltage_margin;
+
+	/* VRM Limits. 0 indicates use SOC default */
+	uint32_t vrm_maximum_current_limit;
+	uint32_t vrm_soc_maximum_current_limit;
+	uint32_t vrm_current_limit;
+	uint32_t vrm_soc_current_limit;
+
+	/* Misc SMU settings */
+	uint8_t sb_tsi_alert_comparator_mode_en;
+	uint8_t core_dldo_bypass;
+	uint8_t min_soc_vid_offset;
+	uint8_t aclk_dpm0_freq_400MHz;
 };

typedef struct soc_amd_picasso_config config_t;

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38699 )

Change subject: soc/amd/picasso: Add UPD settings to chip.h
......................................................................

Patch Set 10:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4
Emulation targets:
"QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2822
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2821
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2820
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/2819

Please note: This test is under development and might not be accurate at all!