Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Paul Menzel, Tarun.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80663?usp=email )
Change subject: mb/google/rex/var/karis: Refactor SSD power sequencing
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80663/comment/23df4768_b67d2af8 :
PS4, Line 9: Improve SSD readiness time by enabling earlier power sequencing.
> What are the old and new timings?
i do not believe this is a valid comment, as you would have understood the benefit by now by looking at the commit message. as mentioned in the commit msg that
```
bootblock (A20/0, A19/1)
|
v
romstage (A20/1)
```
bootblock and romstage are completing the power seq now compared to ramstage in the previous case. Now everyone knows what it means by completing power seq before ramstage. I don't believe that I have to give timing justification.
marking this open resolve.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80663?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I79171a7830b75f5c20bbe30023f2814a62743a13
Gerrit-Change-Number: 80663
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 17:54:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Leah Rowe.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80717?usp=email )
Change subject: nb/haswell: Disable iGPU when dGPU is used
......................................................................
Patch Set 2:
(1 comment)
File src/northbridge/intel/haswell/gma.c:
https://review.coreboot.org/c/coreboot/+/80717/comment/09a39a9d_6d2cd45b :
PS2, Line 469: dev->enabled = 0;
Is this intentional or just copied from sandybridge? Is there a reason to mark this device as disabled?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80717?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1df0a3aa42f8475b7741007bf3e28c2e089d916b
Gerrit-Change-Number: 80717
Gerrit-PatchSet: 2
Gerrit-Owner: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 17:09:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Martin L Roth.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80730?usp=email )
Change subject: util/xcompile: Use a more complete clang target
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80730/comment/e10703fc_b3b863e8 :
PS1, Line 9: the compilers is
singular compiler
--
To view, visit https://review.coreboot.org/c/coreboot/+/80730?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie9356a2bc0f5b77e934cc16482d6ccb1961195dc
Gerrit-Change-Number: 80730
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 17:04:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80735?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified-1 by build bot (Jenkins)
Change subject: lib: Declare heap in assembly
......................................................................
lib: Declare heap in assembly
Some linkers like LLD don't set up the NOBITS flag on the .heap section
when just declaring that region in the linker script. This would have
the program loader initialize the heap, which is not desirable for
performance reasons. Also if the cbfs file would not be compressed it
would be huge as the default heap is 1M.
Change-Id: I3ca7221d10f144f608823e0b9624533780fbf335
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/lib/Makefile.mk
A src/lib/heap.S
M src/lib/program.ld
3 files changed, 19 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/80735/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80735?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3ca7221d10f144f608823e0b9624533780fbf335
Gerrit-Change-Number: 80735
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/80734?usp=email )
Change subject: lib/program.ld: Don't load the heap to memory
......................................................................
Abandoned
--
To view, visit https://review.coreboot.org/c/coreboot/+/80734?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I20ca337a49f222838c68fefcece999e2fc5a9245
Gerrit-Change-Number: 80734
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: abandon
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80700?usp=email )
Change subject: soc/amd/common/acp: use clrsetbits32p to avoid need for casts
......................................................................
soc/amd/common/acp: use clrsetbits32p to avoid need for casts
Use clrsetbits32p instead of clrsetbits32 to not need to cast the
uintptr_t address to void * in the function call.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ic29bf04866a7e1d5c831422f31803a724a41069b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80700
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
---
M src/soc/amd/common/block/acp/acp_gen1.c
M src/soc/amd/common/block/acp/acp_gen2.c
2 files changed, 2 insertions(+), 2 deletions(-)
Approvals:
Matt DeVillier: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/soc/amd/common/block/acp/acp_gen1.c b/src/soc/amd/common/block/acp/acp_gen1.c
index 0fe37f3..4bfd711 100644
--- a/src/soc/amd/common/block/acp/acp_gen1.c
+++ b/src/soc/amd/common/block/acp/acp_gen1.c
@@ -17,7 +17,7 @@
static void acp_update32(uintptr_t bar, uint32_t reg, uint32_t clear, uint32_t set)
{
- clrsetbits32((void *)(bar + reg), clear, set);
+ clrsetbits32p(bar + reg, clear, set);
}
void acp_init(struct device *dev)
diff --git a/src/soc/amd/common/block/acp/acp_gen2.c b/src/soc/amd/common/block/acp/acp_gen2.c
index 50de4b0..be24ec9 100644
--- a/src/soc/amd/common/block/acp/acp_gen2.c
+++ b/src/soc/amd/common/block/acp/acp_gen2.c
@@ -17,7 +17,7 @@
static void acp_update32(uintptr_t bar, uint32_t reg, uint32_t clear, uint32_t set)
{
- clrsetbits32((void *)(bar + reg), clear, set);
+ clrsetbits32p(bar + reg, clear, set);
}
void acp_init(struct device *dev)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80700?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic29bf04866a7e1d5c831422f31803a724a41069b
Gerrit-Change-Number: 80700
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80699?usp=email )
(
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: soc/amd/glinda: Use gpp_clk_setup_common function
......................................................................
soc/amd/glinda: Use gpp_clk_setup_common function
In follow up to commit 0452d0939e7d ("soc/amd: Factor out gpp_clk_setup function") use gpp_clk_setup_common for glinda as well.
Change-Id: If0c1cda0d36de48c7f7315a1b8203b0e53f63f75
Signed-off-by: Varshit Pandya <pandyavarshit(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80699
Reviewed-by: Anand Vaikar <a.vaikar2021(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de>
---
M src/soc/amd/glinda/Kconfig
M src/soc/amd/glinda/fch.c
M src/soc/amd/glinda/include/soc/southbridge.h
3 files changed, 2 insertions(+), 53 deletions(-)
Approvals:
build bot (Jenkins): Verified
Felix Held: Looks good to me, approved
Anand Vaikar: Looks good to me, but someone else must approve
diff --git a/src/soc/amd/glinda/Kconfig b/src/soc/amd/glinda/Kconfig
index 8a6649a..d529002 100644
--- a/src/soc/amd/glinda/Kconfig
+++ b/src/soc/amd/glinda/Kconfig
@@ -46,6 +46,7 @@
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_MULTI_PCI_SEGMENT
select SOC_AMD_COMMON_BLOCK_ESPI_EXTENDED_DECODE_RANGES # TODO: Check if this is still correct
+ select SOC_AMD_COMMON_BLOCK_GPP_CLK
select SOC_AMD_COMMON_BLOCK_GRAPHICS # TODO: Check if this is still correct
select SOC_AMD_COMMON_BLOCK_HAS_ESPI # TODO: Check if this is still correct
select SOC_AMD_COMMON_BLOCK_HAS_ESPI_ALERT_ENABLE # TODO: Check if this is still correct
diff --git a/src/soc/amd/glinda/fch.c b/src/soc/amd/glinda/fch.c
index bb03ad4..2218ce3 100644
--- a/src/soc/amd/glinda/fch.c
+++ b/src/soc/amd/glinda/fch.c
@@ -132,46 +132,7 @@
static void gpp_clk_setup(void)
{
struct soc_amd_glinda_config *cfg = config_of_soc();
-
- /* look-up table to be able to iterate over the PCIe clock output settings */
- const uint8_t gpp_clk_shift_lut[GPP_CLK_OUTPUT_COUNT] = {
- GPP_CLK0_REQ_SHIFT,
- GPP_CLK1_REQ_SHIFT,
- GPP_CLK2_REQ_SHIFT,
- GPP_CLK3_REQ_SHIFT,
- GPP_CLK4_REQ_SHIFT,
- GPP_CLK5_REQ_SHIFT,
- GPP_CLK6_REQ_SHIFT,
- };
-
- uint32_t gpp_clk_ctl = misc_read32(GPP_CLK_CNTRL);
-
- pcie_gpp_dxio_update_clk_req_config(&cfg->gpp_clk_config[0],
- ARRAY_SIZE(cfg->gpp_clk_config));
- for (int i = 0; i < GPP_CLK_OUTPUT_COUNT; i++) {
- gpp_clk_ctl &= ~GPP_CLK_REQ_MASK(gpp_clk_shift_lut[i]);
- /*
- * The remapping of values is done so that the default of the enum used for the
- * devicetree settings is the clock being enabled, so that a missing devicetree
- * configuration for this will result in an always active clock and not an
- * inactive PCIe clock output. Only the configuration for the clock outputs
- * available on the package is provided via the devicetree; the rest is
- * switched off unconditionally.
- */
- switch (i < GPP_CLK_OUTPUT_AVAILABLE ? cfg->gpp_clk_config[i] : GPP_CLK_OFF) {
- case GPP_CLK_REQ:
- gpp_clk_ctl |= GPP_CLK_REQ_EXT(gpp_clk_shift_lut[i]);
- break;
- case GPP_CLK_OFF:
- gpp_clk_ctl |= GPP_CLK_REQ_OFF(gpp_clk_shift_lut[i]);
- break;
- case GPP_CLK_ON:
- default:
- gpp_clk_ctl |= GPP_CLK_REQ_ON(gpp_clk_shift_lut[i]);
- }
- }
-
- misc_write32(GPP_CLK_CNTRL, gpp_clk_ctl);
+ gpp_clk_setup_common(&cfg->gpp_clk_config[0], ARRAY_SIZE(cfg->gpp_clk_config));
}
static void cgpll_clock_gate_init(void)
diff --git a/src/soc/amd/glinda/include/soc/southbridge.h b/src/soc/amd/glinda/include/soc/southbridge.h
index 32945e8..9cd835b 100644
--- a/src/soc/amd/glinda/include/soc/southbridge.h
+++ b/src/soc/amd/glinda/include/soc/southbridge.h
@@ -87,20 +87,7 @@
#define FCH_LEGACY_UART_DECODE (ALINK_AHB_ADDRESS + 0x20) /* 0xfedc0020 */
/* FCH MISC Registers 0xfed80e00 */
-#define GPP_CLK_CNTRL 0x00
-#define GPP_CLK0_REQ_SHIFT 0
-#define GPP_CLK1_REQ_SHIFT 2
-#define GPP_CLK4_REQ_SHIFT 4
-#define GPP_CLK2_REQ_SHIFT 6
-#define GPP_CLK3_REQ_SHIFT 8
-#define GPP_CLK5_REQ_SHIFT 10
-#define GPP_CLK6_REQ_SHIFT 12
-#define GPP_CLK_OUTPUT_COUNT 7
#define GPP_CLK_OUTPUT_AVAILABLE 4
-#define GPP_CLK_REQ_MASK(clk_shift) (0x3 << (clk_shift))
-#define GPP_CLK_REQ_ON(clk_shift) (0x3 << (clk_shift))
-#define GPP_CLK_REQ_EXT(clk_shift) (0x1 << (clk_shift))
-#define GPP_CLK_REQ_OFF(clk_shift) (0x0 << (clk_shift))
#define MISC_CLKGATEDCNTL 0x2c
#define ALINKCLK_GATEOFFEN BIT(16)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80699?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If0c1cda0d36de48c7f7315a1b8203b0e53f63f75
Gerrit-Change-Number: 80699
Gerrit-PatchSet: 5
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Anand Vaikar <a.vaikar2021(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80735?usp=email )
Change subject: lib: Declare heap in assembly
......................................................................
lib: Declare heap in assembly
Some linkers like LLD don't set up the NOBITS flag on the .heap section
when just declaring that region in the linker script. This would have
the program loader initialize the heap, which is not desirable for
performance reasons. Also if the cbfs file would not be compressed it
would be huge as the default heap is 1M.
Change-Id: I3ca7221d10f144f608823e0b9624533780fbf335
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/lib/Makefile.mk
M src/lib/program.ld
2 files changed, 4 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/80735/1
diff --git a/src/lib/Makefile.mk b/src/lib/Makefile.mk
index c5e4278..bd5363a 100644
--- a/src/lib/Makefile.mk
+++ b/src/lib/Makefile.mk
@@ -140,6 +140,7 @@
ramstage-y += memchr.c
ramstage-y += memcmp.c
ramstage-y += malloc.c
+ramstage-y += heap.S
ramstage-y += dimm_info_util.c
ramstage-y += delay.c
ramstage-y += fallback_boot.c
diff --git a/src/lib/program.ld b/src/lib/program.ld
index 6d72d9e..bdcadca 100644
--- a/src/lib/program.ld
+++ b/src/lib/program.ld
@@ -131,12 +131,11 @@
#endif
#if ENV_HAS_HEAP_SECTION
-.heap . (NOLOAD) : {
+.heap . : {
. = ALIGN(ARCH_POINTER_ALIGN_SIZE);
- _heap = .;
- . += CONFIG_HEAP_SIZE;
+ *(.heap)
+ *(.heap.*)
. = ALIGN(ARCH_POINTER_ALIGN_SIZE);
- _eheap = .;
RECORD_SIZE(heap)
}
#endif
--
To view, visit https://review.coreboot.org/c/coreboot/+/80735?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3ca7221d10f144f608823e0b9624533780fbf335
Gerrit-Change-Number: 80735
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange