Change in coreboot[master]: amd/stoneyridge,picasso: Open TSEG earlier

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34775 )

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

amd/stoneyridge,picasso: Open TSEG earlier

Don't make assumptions about which subregion will
be accessed first.

Change-Id: I558fa4acc5068014b3748be6fc1bc34999054c0a
Signed-off-by: Kyösti Mälkki 
---
M src/soc/amd/picasso/ramtop.c
M src/soc/amd/stoneyridge/ramtop.c
2 files changed, 12 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/34775/1

diff --git a/src/soc/amd/picasso/ramtop.c b/src/soc/amd/picasso/ramtop.c
index 6b28ec7..672fdd8 100644
--- a/src/soc/amd/picasso/ramtop.c
+++ b/src/soc/amd/picasso/ramtop.c
@@ -111,6 +111,7 @@

int smm_subregion(int sub, uintptr_t *start, size_t *size)
 {
+	static int once;
    uintptr_t sub_base;
    size_t sub_size;
    const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
@@ -118,6 +119,11 @@
    smm_region(&sub_base, &sub_size);
    assert(sub_size > CONFIG_SMM_RESERVED_SIZE);

+	if (!once) {
+		clear_tvalid();
+		once = 1;
+	}
+
    switch (sub) {
    case SMM_SUBREGION_HANDLER:
    	/* Handler starts at the base of TSEG. */
@@ -127,7 +133,6 @@
    	/* External cache is in the middle of TSEG. */
    	sub_base += sub_size - cache_size;
    	sub_size = cache_size;
-		clear_tvalid();
    	break;
    default:
    	*start = 0;
diff --git a/src/soc/amd/stoneyridge/ramtop.c b/src/soc/amd/stoneyridge/ramtop.c
index 26d84ce..e2b0f7d 100644
--- a/src/soc/amd/stoneyridge/ramtop.c
+++ b/src/soc/amd/stoneyridge/ramtop.c
@@ -111,6 +111,7 @@

int smm_subregion(int sub, uintptr_t *start, size_t *size)
 {
+	static int once;
    uintptr_t sub_base;
    size_t sub_size;
    const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
@@ -118,6 +119,11 @@
    smm_region(&sub_base, &sub_size);
    assert(sub_size > CONFIG_SMM_RESERVED_SIZE);

+	if (!once) {
+		clear_tvalid();
+		once = 1;
+	}
+
    switch (sub) {
    case SMM_SUBREGION_HANDLER:
    	/* Handler starts at the base of TSEG. */
@@ -127,7 +133,6 @@
    	/* External cache is in the middle of TSEG. */
    	sub_base += sub_size - cache_size;
    	sub_size = cache_size;
-		clear_tvalid();
    	break;
    default:
    	*start = 0;

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34775 )

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

Patch Set 1: Code-Review+2

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34775 )

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

Patch Set 1: Code-Review+2

[Paul Menzel (Code Review)]

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

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/34775/2/src/soc/amd/picasso/ramtop.... 
File src/soc/amd/picasso/ramtop.c:

https://review.coreboot.org/c/coreboot/+/34775/2/src/soc/amd/picasso/ramtop.... 
PS2, Line 114: int once
Use bool?

[Furquan Shaikh (Code Review)]

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

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

Patch Set 2: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34775 )

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34775/2/src/soc/amd/picasso/ramtop.... 
File src/soc/amd/picasso/ramtop.c:

https://review.coreboot.org/c/coreboot/+/34775/2/src/soc/amd/picasso/ramtop.... 
PS2, Line 114: int once
> Use bool?

Maybe later, like after CB:34776.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34775 )

Change subject: amd/stoneyridge,picasso: Open TSEG earlier
......................................................................

amd/stoneyridge,picasso: Open TSEG earlier

Don't make assumptions about which subregion will
be accessed first.

Change-Id: I558fa4acc5068014b3748be6fc1bc34999054c0a
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34775
Reviewed-by: Paul Menzel 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Marshall Dawson 
Reviewed-by: Richard Spiegel 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/picasso/ramtop.c
M src/soc/amd/stoneyridge/ramtop.c
2 files changed, 12 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Furquan Shaikh: Looks good to me, approved
  Marshall Dawson: Looks good to me, approved
  Richard Spiegel: Looks good to me, approved

diff --git a/src/soc/amd/picasso/ramtop.c b/src/soc/amd/picasso/ramtop.c
index 6b28ec7..672fdd8 100644
--- a/src/soc/amd/picasso/ramtop.c
+++ b/src/soc/amd/picasso/ramtop.c
@@ -111,6 +111,7 @@

int smm_subregion(int sub, uintptr_t *start, size_t *size)
 {
+	static int once;
    uintptr_t sub_base;
    size_t sub_size;
    const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
@@ -118,6 +119,11 @@
    smm_region(&sub_base, &sub_size);
    assert(sub_size > CONFIG_SMM_RESERVED_SIZE);

+	if (!once) {
+		clear_tvalid();
+		once = 1;
+	}
+
    switch (sub) {
    case SMM_SUBREGION_HANDLER:
    	/* Handler starts at the base of TSEG. */
@@ -127,7 +133,6 @@
    	/* External cache is in the middle of TSEG. */
    	sub_base += sub_size - cache_size;
    	sub_size = cache_size;
-		clear_tvalid();
    	break;
    default:
    	*start = 0;
diff --git a/src/soc/amd/stoneyridge/ramtop.c b/src/soc/amd/stoneyridge/ramtop.c
index 26d84ce..e2b0f7d 100644
--- a/src/soc/amd/stoneyridge/ramtop.c
+++ b/src/soc/amd/stoneyridge/ramtop.c
@@ -111,6 +111,7 @@

int smm_subregion(int sub, uintptr_t *start, size_t *size)
 {
+	static int once;
    uintptr_t sub_base;
    size_t sub_size;
    const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
@@ -118,6 +119,11 @@
    smm_region(&sub_base, &sub_size);
    assert(sub_size > CONFIG_SMM_RESERVED_SIZE);

+	if (!once) {
+		clear_tvalid();
+		once = 1;
+	}
+
    switch (sub) {
    case SMM_SUBREGION_HANDLER:
    	/* Handler starts at the base of TSEG. */
@@ -127,7 +133,6 @@
    	/* External cache is in the middle of TSEG. */
    	sub_base += sub_size - cache_size;
    	sub_size = cache_size;
-		clear_tvalid();
    	break;
    default:
    	*start = 0;