Change in coreboot[master]: soc/amd/picasso: Add speed measurement for spi

[Eric Peers (Code Review)]

Eric Peers has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44229 )

Change subject: soc/amd/picasso: Add speed measurement for spi
......................................................................

soc/amd/picasso: Add speed measurement for spi

* Still a WIP and likely not merge-worthy *

Adds speed measurement for spi prior and after espi enablmenet including
DMA.

BUG=b:149014878
TEST=Compile & Run on Trembyle
Signed-off-by: Eric PEers 
Change-Id: I6a27c2153e6f258ef2f6a2f6c7de4f6965d535f5
---
M src/soc/amd/picasso/southbridge.c
1 file changed, 122 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/44229/1

diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c
index 54d7640..86cf9e1 100644
--- a/src/soc/amd/picasso/southbridge.c
+++ b/src/soc/amd/picasso/southbridge.c
@@ -28,6 +28,7 @@
 #include <soc/pci_devs.h>
 #include <soc/nvs.h>
 #include <types.h>
+#include <timestamp.h>
 #include "chip.h"

/*
@@ -132,6 +133,118 @@
    	lpc_enable_port80();
 }

+#if !defined(__SIMPLE_DEVICE__)
+#define _LPCB_DEV pcidev_on_root(0x14, 0x3)
+#else
+#define _LPCB_DEV PCI_DEV(0, 0x14, 0x3)
+#endif
+
+static void measure_spi(void)
+{
+	//set timer
+	uint8_t byte;
+	uint32_t dword;
+	uint64_t qword;
+	uint64_t total;
+	uint32_t *ptr;
+	uint64_t *qword_ptr;
+	uint8_t *byte_ptr;
+	uint32_t start, finish, diff;
+	int i;
+
+	printk(BIOS_DEBUG, "Measuring SPI\n");
+	if (0) {
+		printk(BIOS_DEBUG, "8b writes\n");
+		//-------------- 8b writes
+		start = get_us_since_boot();
+		byte = 0;
+		//we get 0x14000 worth of space.
+		for (byte_ptr=(uint8_t *)0xff76c000;byte_ptr<(uint8_t *)0xff76d000;byte_ptr++) {
+			*byte_ptr = byte;
+			byte++;
+		}
+		finish = get_us_since_boot();
+		diff = finish-start;
+		printk(BIOS_DEBUG, "0x1000 bytes, in %d usec\n", diff);
+	}
+	
+	//--------------- 8b reads
+	printk(BIOS_DEBUG, "8b reads\n");
+	start = get_us_since_boot();
+	total = 0;
+	for (byte_ptr = (uint8_t *)0xff76c000; byte_ptr < (uint8_t *)0xff76d000; byte_ptr++) {
+		byte = *byte_ptr;
+		total+=byte;
+	}
+	finish = get_us_since_boot();
+	diff = finish-start;
+	printk(BIOS_DEBUG, "0x1000 bytes, in %d usec\n", diff);
+	printk(BIOS_DEBUG, "TOTAL: %lld\n", total);
+
+
+	if (0) {
+		printk(BIOS_DEBUG, "32b writes\n");
+		//---------------- 32b writes
+		start = get_us_since_boot();
+		dword = 0;
+		for (ptr=(uint32_t *)0xff76d000;ptr<(uint32_t *)0xff76e000;ptr+=4) {
+			*ptr = dword;
+			dword++;
+		}
+		finish = get_us_since_boot();
+		diff = finish-start;
+		printk(BIOS_DEBUG, "0x1000 bytes, in %d usec\n", diff);
+	}
+
+	//--------------- 32b reads
+	printk(BIOS_DEBUG, "32b reads\n");
+	start = get_us_since_boot();
+	total = 0;
+	for (ptr=(uint32_t *)0xff76d000;ptr<(uint32_t *)0xff76e000;ptr+=4) {
+		dword = *ptr;
+		total+=dword;
+	}
+	finish = get_us_since_boot();
+	diff = finish-start;
+	printk(BIOS_DEBUG, "0x1000 bytes, in %d usec\n", diff);
+	printk(BIOS_DEBUG, "TOTAL: %lld\n", total);
+
+	//------------- 64b reads. Should prefetch
+	printk(BIOS_DEBUG, "64b reads\n");
+	start = get_us_since_boot();
+	total = 0;
+	for (qword_ptr=(uint64_t *)0xff76e000;qword_ptr<(uint64_t *)0xff76f000;qword_ptr+=8) {
+		qword = *qword_ptr;
+		total+=qword;
+	}
+	finish = get_us_since_boot();
+	diff = finish-start;
+	printk(BIOS_DEBUG, "0x1000 bytes, in %d usec\n", diff);
+	printk(BIOS_DEBUG, "TOTAL: %lld\n", total);
+
+	//------------ burst read
+	printk(BIOS_DEBUG, "burst read\n");
+	start = 0;
+	pci_write_config32(_LPCB_DEV, 0xb0, 0xff76f000); //src addr for DMA
+	pci_write_config32(_LPCB_DEV, 0xb4, 0x40000000); //dest addr - put it at 1GB
+	dword = pci_read_config32(_LPCB_DEV, LPC_ROM_DMA_EC_HOST_CONTROL);
+	dword |= 0x1; //tell it GO GO GO
+	dword |= 0x0FC0; //pull this many cachelines+1 pf 64B which... should be be 0x1000. [15:6] 0x1000
+	pci_write_config32(_LPCB_DEV, LPC_ROM_DMA_EC_HOST_CONTROL, dword);
+	for (i=0;i<1000;i++) {
+		dword = pci_read_config32(_LPCB_DEV, LPC_ROM_DMA_EC_HOST_CONTROL);
+		if ((dword & 0x1) == 0) break;
+		udelay(10);
+	}
+	if (i>=100) {
+		printk(BIOS_ERR, "Took too long to run DMA\n");
+	}
+	finish = get_us_since_boot();
+	diff = finish-start;
+	printk(BIOS_DEBUG, "0x1000 bytes, in %d usec\n", diff);
+
+}
+
 /* Before console init */
 void fch_pre_init(void)
 {
@@ -141,6 +254,8 @@
    	lpc_configure_decodes();

fch_spi_early_init();
+
+
    enable_acpimmio_decode_pm04();
    fch_smbus_init();
    sb_enable_cf9_io();
@@ -156,6 +271,7 @@

if (CONFIG(PICASSO_CONSOLE_UART))
    	set_uart_config(CONFIG_UART_FOR_CONSOLE);
+
 }

static void print_num_status_bits(int num_bits, uint32_t status,
@@ -216,6 +332,9 @@
    sb_print_pmxc0_status();
    i2c_soc_early_init();

+	printk(BIOS_DEBUG, "Measure spi before espi setup\n");
+	measure_spi();
+
    if (CONFIG(DISABLE_SPI_FLASH_ROM_SHARING))
    	lpc_disable_spi_rom_sharing();

@@ -223,6 +342,9 @@
    	espi_setup();
    	espi_configure_decodes();
    }
+
+	printk(BIOS_DEBUG, "Measure spi after espi setup\n");
+	measure_spi();
 }

void sb_enable(struct device *dev)

[Eric Peers (Code Review)]

Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44229 )

Change subject: soc/amd/picasso: Add speed measurement for spi
......................................................................

Patch Set 1:

(2 comments)

This change is ready for review.

https://review.coreboot.org/c/coreboot/+/44229/1/src/soc/amd/picasso/southbr... 
File src/soc/amd/picasso/southbridge.c:

https://review.coreboot.org/c/coreboot/+/44229/1/src/soc/amd/picasso/southbr... 
PS1, Line 162: 		for (byte_ptr=(uint8_t *)0xff76c000;byte_ptr<(uint8_t *)0xff76d000;byte_ptr++) {
Raul suggested using RWA as a measurement point and to pull in a megabyte at a time as well.

https://review.coreboot.org/c/coreboot/+/44229/1/src/soc/amd/picasso/southbr... 
PS1, Line 190: 		for (ptr=(uint32_t *)0xff76d000;ptr<(uint32_t *)0xff76e000;ptr+=4) {
ptr math here is likely wrong.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44229 )

Change subject: soc/amd/picasso: Add speed measurement for spi
......................................................................

Patch Set 1:

(3 comments)

I’d be nice, if you fixed the coding style, so it can be properly reviewed.

https://review.coreboot.org/c/coreboot/+/44229/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44229/1//COMMIT_MSG@11 
PS1, Line 11: enablmenet
enablement

https://review.coreboot.org/c/coreboot/+/44229/1/src/soc/amd/picasso/southbr... 
File src/soc/amd/picasso/southbridge.c:

https://review.coreboot.org/c/coreboot/+/44229/1/src/soc/amd/picasso/southbr... 
PS1, Line 195: 		diff = finish-start;
Please add spaces around the operator.

https://review.coreboot.org/c/coreboot/+/44229/1/src/soc/amd/picasso/southbr... 
PS1, Line 274: 
Remove.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44229 )

Change subject: soc/amd/picasso: Add speed measurement for spi
......................................................................

Patch Set 1:
> Patch Set 1:
>
> (3 comments)
>
> I’d be nice, if you fixed the coding style, so it can be properly reviewed.

It still says WIP in the commit message. Eric probably pressed the big button and accidentally marked this as ready for review.

[Eric Peers (Code Review)]

Eric Peers has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44229 )

Change subject: soc/amd/picasso: Add speed measurement for spi
......................................................................

Abandoned

thiswas a WIP, and as Angel points out accidentally submitted for review.