Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Kangheui Won, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52688 )
Change subject: psp_verstage: make temp_stack optional
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52688
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44196103a3531e9d01c96ab8f454c8b580fe9807
Gerrit-Change-Number: 52688
Gerrit-PatchSet: 1
Gerrit-Owner: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 29 Apr 2021 15:17:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52687 )
Change subject: psp_verstage: make get_max_workbuf_size optional
......................................................................
psp_verstage: make get_max_workbuf_size optional
From cezanne we have enough space in PSP so we don't have to worry about
workbuf size. Hence the function only exists in picasso and deprecated
for later platforms.
So wrap svc_get_max_workbuf_size and provide default weak function so
future platforms don't have to implement dumb function for it.
TEST=build and boot zork, check weak function is not called in zork
Signed-off-by: Kangheui Won <khwon(a)chromium.org>
Change-Id: I16e8edf8070aaacb3a6a6a8adc92b44a230c3139
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52687
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Martin Roth <martinroth(a)google.com>
---
M src/soc/amd/common/psp_verstage/include/psp_verstage.h
M src/soc/amd/common/psp_verstage/psp_verstage.c
M src/soc/amd/picasso/psp_verstage/svc.c
3 files changed, 18 insertions(+), 3 deletions(-)
Approvals:
build bot (Jenkins): Verified
Martin Roth: Looks good to me, approved
diff --git a/src/soc/amd/common/psp_verstage/include/psp_verstage.h b/src/soc/amd/common/psp_verstage/include/psp_verstage.h
index f7cb1c9..6589725 100644
--- a/src/soc/amd/common/psp_verstage/include/psp_verstage.h
+++ b/src/soc/amd/common/psp_verstage/include/psp_verstage.h
@@ -56,6 +56,7 @@
void verstage_soc_init(void);
uintptr_t *map_spi_rom(void);
+uint32_t get_max_workbuf_size(uint32_t *size);
uint32_t update_psp_bios_dir(uint32_t *psp_dir_offset, uint32_t *bios_dir_offset);
uint32_t save_uapp_data(void *address, uint32_t size);
diff --git a/src/soc/amd/common/psp_verstage/psp_verstage.c b/src/soc/amd/common/psp_verstage/psp_verstage.c
index 58f17e1..f6cc5e9 100644
--- a/src/soc/amd/common/psp_verstage/psp_verstage.c
+++ b/src/soc/amd/common/psp_verstage/psp_verstage.c
@@ -25,6 +25,14 @@
void __weak verstage_mainboard_early_init(void) {}
void __weak verstage_mainboard_init(void) {}
+uint32_t __weak get_max_workbuf_size(uint32_t *size)
+{
+ /* This svc only exists in picasso and deprecated for later platforms.
+ * Provide sane default function here for those platforms.
+ */
+ *size = (uint32_t)((uintptr_t)_etransfer_buffer - (uintptr_t)_transfer_buffer);
+ return 0;
+}
static void reboot_into_recovery(struct vb2_context *ctx, uint32_t subcode)
{
@@ -133,10 +141,11 @@
struct transfer_info_struct buffer_info = {0};
/*
- * This should never fail, but if it does, we should still try to
- * save the buffer. If that fails, then we should go to recovery mode.
+ * This should never fail on picasso, but if it does, we should still
+ * try to save the buffer. If that fails, then we should go to
+ * recovery mode.
*/
- if (svc_get_max_workbuf_size(&max_buffer_size)) {
+ if (get_max_workbuf_size(&max_buffer_size)) {
post_code(POSTCODE_DEFAULT_BUFFER_SIZE_NOTICE);
printk(BIOS_NOTICE, "Notice: using default transfer buffer size.\n");
max_buffer_size = MIN_TRANSFER_BUFFER_SIZE;
diff --git a/src/soc/amd/picasso/psp_verstage/svc.c b/src/soc/amd/picasso/psp_verstage/svc.c
index a20c2a6..28e6baf 100644
--- a/src/soc/amd/picasso/psp_verstage/svc.c
+++ b/src/soc/amd/picasso/psp_verstage/svc.c
@@ -149,6 +149,11 @@
return retval;
}
+uint32_t get_max_workbuf_size(uint32_t *size)
+{
+ return svc_get_max_workbuf_size(size);
+}
+
uint32_t svc_get_max_workbuf_size(uint32_t *size)
{
uint32_t retval = 0;
--
To view, visit https://review.coreboot.org/c/coreboot/+/52687
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I16e8edf8070aaacb3a6a6a8adc92b44a230c3139
Gerrit-Change-Number: 52687
Gerrit-PatchSet: 2
Gerrit-Owner: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Kangheui Won, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52687 )
Change subject: psp_verstage: make get_max_workbuf_size optional
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52687
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I16e8edf8070aaacb3a6a6a8adc92b44a230c3139
Gerrit-Change-Number: 52687
Gerrit-PatchSet: 1
Gerrit-Owner: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 29 Apr 2021 15:16:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52625 )
Change subject: soc/amd/picasso: move PSP_SRAM addrs to separate header
......................................................................
soc/amd/picasso: move PSP_SRAM addrs to separate header
These addresses will be changed in cezanne. Before start working on
cezanne, move these out to separate header as a clean-up.
TEST=emerge-zork coreboot
Signed-off-by: Kangheui Won <khwon(a)chromium.org>
Change-Id: I2499281d250aae701f86bfcc87c7681e5b684b6a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52625
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Raul Rangel <rrangel(a)chromium.org>
---
M src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld
A src/soc/amd/picasso/include/soc/psp_verstage_addr.h
2 files changed, 32 insertions(+), 27 deletions(-)
Approvals:
build bot (Jenkins): Verified
Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld b/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld
index aa27bae..cc9ebc0 100644
--- a/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld
+++ b/src/soc/amd/common/block/cpu/noncar/memlayout_psp_verstage.ld
@@ -3,33 +3,7 @@
#include <memlayout.h>
#include <soc/psp_transfer.h>
#include <fmap_config.h>
-
-/* TODO: Move defines to SoC-specific header file to allow SoC specific values if needed. */
-
-/*
- * Start of available space is 0x15000 and this is where the
- * header for the user app (verstage) must be mapped.
- * Size is 0x28000 bytes
- */
-#define PSP_SRAM_START 0x15000
-#define PSP_SRAM_SIZE 160K
-
-#define VERSTAGE_START 0x15000
-
-/*
- * The temp stack can be made much smaller if needed - even 256 bytes
- * should be sufficient. This is just for the function mapping the
- * actual stack.
- */
-#define PSP_VERSTAGE_TEMP_STACK_START 0x32000
-#define PSP_VERSTAGE_TEMP_STACK_SIZE 4K
-
-/*
- * The top of the stack must be 4k aligned, so set the bottom as 4k aligned
- * and make the size a multiple of 4k
- */
-#define PSP_VERSTAGE_STACK_START 0x33000
-#define PSP_VERSTAGE_STACK_SIZE 40K
+#include <soc/psp_verstage_addr.h>
ENTRY(_psp_vs_start)
SECTIONS
diff --git a/src/soc/amd/picasso/include/soc/psp_verstage_addr.h b/src/soc/amd/picasso/include/soc/psp_verstage_addr.h
new file mode 100644
index 0000000..61ae926
--- /dev/null
+++ b/src/soc/amd/picasso/include/soc/psp_verstage_addr.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef AMD_PICASSO_PSP_VERSTAGE_ADDR_H
+#define AMD_PICASSO_PSP_VERSTAGE_ADDR_H
+
+/*
+ * Start of available space is 0x15000 and this is where the
+ * header for the user app (verstage) must be mapped.
+ * Size is 0x28000 bytes
+ */
+#define PSP_SRAM_START 0x15000
+#define PSP_SRAM_SIZE (160K)
+#define VERSTAGE_START PSP_SRAM_START
+
+/*
+ * The temp stack can be made much smaller if needed - even 256 bytes
+ * should be sufficient. This is just for the function mapping the
+ * actual stack.
+ */
+#define PSP_VERSTAGE_TEMP_STACK_START 0x32000
+#define PSP_VERSTAGE_TEMP_STACK_SIZE (4K)
+
+/*
+ * The top of the stack must be 4k aligned, so set the bottom as 4k aligned
+ * and make the size a multiple of 4k
+ */
+
+#define PSP_VERSTAGE_STACK_START 0x33000
+#define PSP_VERSTAGE_STACK_SIZE (40K)
+
+#endif /* AMD_PICASSO_PSP_VERSTAGE_ADDR_H */
--
To view, visit https://review.coreboot.org/c/coreboot/+/52625
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2499281d250aae701f86bfcc87c7681e5b684b6a
Gerrit-Change-Number: 52625
Gerrit-PatchSet: 3
Gerrit-Owner: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Jason Glenesk, Marshall Dawson, Kangheui Won, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52625 )
Change subject: soc/amd/picasso: move PSP_SRAM addrs to separate header
......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/picasso/include/soc/psp_verstage_addr.h:
https://review.coreboot.org/c/coreboot/+/52625/comment/5bd7ba2f_314920b0
PS1, Line 12: (160K)
> lint-007-checkpatch didn't like it, probably they do less check on ld file. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/52625
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2499281d250aae701f86bfcc87c7681e5b684b6a
Gerrit-Change-Number: 52625
Gerrit-PatchSet: 2
Gerrit-Owner: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 29 Apr 2021 15:14:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Comment-In-Reply-To: Kangheui Won <khwon(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Martin Roth, Furquan Shaikh, Marshall Dawson, Mathew King, Angel Pons, Eric Peers, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52643 )
Change subject: soc/amd/common/acp: Move Audio Co-processor driver to common
......................................................................
Patch Set 3:
(2 comments)
File src/soc/amd/common/block/include/amdblocks/acp.h:
https://review.coreboot.org/c/coreboot/+/52643/comment/1d45b8af_f619ee4c
PS3, Line 22: bool acp_i2s_use_external_48mhz_osc;
> this controls if the 48 MHz output from the clock generator in the FCH gets disable, so i wouldn't p […]
Ack. Since this is urgently required to enable audio for Guybrush, I will move this out in a follow-up CL.
File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/52643/comment/8d5986df_8238f429
PS3, Line 274: bool acp_i2s_use_external_48mhz_osc;
> i'd keep this one in the soc-specific code, since it's not directly acp-related
Ack. Since this is urgently required for Cezanne, I will move this out in a follow-up CL.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I91470ff68d1c183df9a2927d71b03371b535186a
Gerrit-Change-Number: 52643
Gerrit-PatchSet: 3
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Mathew King <mathewk(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 29 Apr 2021 15:10:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment