Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37290 )
Change subject: amd/{AGESA,binaryPI}/ramtop: use BIOSRAM to save and restore cbmem_top ......................................................................
amd/{AGESA,binaryPI}/ramtop: use BIOSRAM to save and restore cbmem_top
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I2af206d416f931a5947e7b3eff2715a0574275c4 --- M src/northbridge/amd/pi/ramtop.c M src/southbridge/amd/agesa/hudson/ramtop.c 2 files changed, 9 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/37290/1
diff --git a/src/northbridge/amd/pi/ramtop.c b/src/northbridge/amd/pi/ramtop.c index 823a15c..6d317dc 100644 --- a/src/northbridge/amd/pi/ramtop.c +++ b/src/northbridge/amd/pi/ramtop.c @@ -14,20 +14,18 @@ #define __SIMPLE_DEVICE__
#include <stdint.h> +#include <amdblocks/acpimmio.h> #include <device/pci_ops.h> #include <cbmem.h>
-#define CBMEM_TOP_SCRATCHPAD 0x78 +#define BIOSRAM_CBMEM_TOP 0xf0 /* 4 bytes */
void backup_top_of_low_cacheable(uintptr_t ramtop) { - uint16_t top_cache = ramtop >> 16; - pci_write_config16(PCI_DEV(0,0,0), CBMEM_TOP_SCRATCHPAD, top_cache); + biosram_write32(BIOSRAM_CBMEM_TOP, ramtop); }
uintptr_t restore_top_of_low_cacheable(void) { - uint16_t top_cache; - top_cache = pci_read_config16(PCI_DEV(0,0,0), CBMEM_TOP_SCRATCHPAD); - return (top_cache << 16); + return biosram_read32(BIOSRAM_CBMEM_TOP); } diff --git a/src/southbridge/amd/agesa/hudson/ramtop.c b/src/southbridge/amd/agesa/hudson/ramtop.c index 22b291d..65a7b9a 100644 --- a/src/southbridge/amd/agesa/hudson/ramtop.c +++ b/src/southbridge/amd/agesa/hudson/ramtop.c @@ -14,11 +14,14 @@ */
#include <stdint.h> +#include <amdblocks/acpimmio.h> #include <arch/io.h> #include <arch/acpi.h> #include <cbmem.h> #include "hudson.h"
+#define BIOSRAM_CBMEM_TOP 0xf0 /* 4 bytes */ + int acpi_get_sleep_type(void) { u16 tmp = inw(ACPI_PM1_CNT_BLK); @@ -28,24 +31,10 @@
void backup_top_of_low_cacheable(uintptr_t ramtop) { - u32 dword = ramtop; - int nvram_pos = 0xf8, i; /* temp */ - for (i = 0; i < 4; i++) { - outb(nvram_pos, BIOSRAM_INDEX); - outb((dword >> (8 * i)) & 0xff, BIOSRAM_DATA); - nvram_pos++; - } + biosram_write32(BIOSRAM_CBMEM_TOP, ramtop); }
uintptr_t restore_top_of_low_cacheable(void) { - uint32_t xdata = 0; - int xnvram_pos = 0xf8, xi; - for (xi = 0; xi < 4; xi++) { - outb(xnvram_pos, BIOSRAM_INDEX); - xdata &= ~(0xff << (xi * 8)); - xdata |= inb(BIOSRAM_DATA) << (xi *8); - xnvram_pos++; - } - return xdata; + return biosram_read32(BIOSRAM_CBMEM_TOP); }
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37290
to look at the new patch set (#2).
Change subject: amd/{AGESA,binaryPI}/ramtop: use BIOSRAM to save and restore cbmem top ......................................................................
amd/{AGESA,binaryPI}/ramtop: use BIOSRAM to save and restore cbmem top
TEST=boot PC Engines apu2 and launch Debian with Linux kernel 4.14.50
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I2af206d416f931a5947e7b3eff2715a0574275c4 --- M src/northbridge/amd/pi/ramtop.c M src/southbridge/amd/agesa/hudson/ramtop.c 2 files changed, 9 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/37290/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37290 )
Change subject: amd/{AGESA,binaryPI}/ramtop: use BIOSRAM to save and restore cbmem top ......................................................................
Patch Set 8:
(1 comment)
squash with CB:37330, move CB:37328 before this
The three implementations of backup/restore probably became identical with those in amd/stoneyridge,picasso. So the logical followup is to have one common implementation in amd/common/block.
The platform still has full control of what value exactly is stored.
https://review.coreboot.org/c/coreboot/+/37290/8/src/northbridge/amd/pi/ramt... File src/northbridge/amd/pi/ramtop.c:
https://review.coreboot.org/c/coreboot/+/37290/8/src/northbridge/amd/pi/ramt... PS8, Line 21: #define BIOSRAM_CBMEM_TOP 0xf0 /* 4 bytes */ These offsets should not be split across several files. Sharing the map with amd/stoneyridge would be ideal. I have no idea why they chose to start filling the space from the end.
Michał Żygowski has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37290 )
Change subject: amd/{AGESA,binaryPI}/ramtop: use BIOSRAM to save and restore cbmem top ......................................................................
Abandoned
Done in CB:37402