Change in coreboot[master]: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45914 )

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`

Also correct comments and clean up cosmetics of this function.

Change-Id: I5f3720c02875f546f9dad4bb5582e97d91d39c0d
Signed-off-by: Angel Pons 
---
M src/northbridge/intel/i945/memmap.c
1 file changed, 5 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/45914/1

diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c
index c92e466..af32f39 100644
--- a/src/northbridge/intel/i945/memmap.c
+++ b/src/northbridge/intel/i945/memmap.c
@@ -85,11 +85,10 @@
 {
    uintptr_t top_of_ram;

-	/* Cache 8 MiB region below the top of RAM and 2 MiB above top of
-	 * RAM to cover both cbmem as the TSEG region.
-	 */
+	/* Cache 8 MiB region below the top of RAM to cover CBMEM */
    top_of_ram = (uintptr_t)cbmem_top();
-	postcar_frame_add_mtrr(pcf, top_of_ram - 8*MiB, 8*MiB, MTRR_TYPE_WRBACK);
-	postcar_frame_add_mtrr(pcf, northbridge_get_tseg_base(),
-			       northbridge_get_tseg_size(), MTRR_TYPE_WRBACK);
+	postcar_frame_add_mtrr(pcf, top_of_ram - 8 * MiB, 8 * MiB, MTRR_TYPE_WRBACK);
+
+	/* Cache the TSEG region */
+	postcar_enable_tseg_cache(pcf);
 }

[Paul Menzel (Code Review)]

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

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Patch Set 2:

Is this tested?

[Angel Pons (Code Review)]

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

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Is this tested?

Not yet but it should be functionally equivalent.

[Masanori Ogino (Code Review)]

Masanori Ogino has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45914 )

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Patch Set 2: Code-Review+1

The `postcar_enable_tseg_cache` function is essentially equivalent to the removed code block.

It compiles on top of commit a5f4781d81 with `CONFIG_TSEG_STAGE_CACHE=y` and `CONFIG_SMM_TSEG=y`, runs on Lenovo ThinkPad X60s, and boots Linux 5.4.68 with SeaBIOS 1.14.0 and GRUB 2.0.4.

It looks good to me.

[Masanori Ogino (Code Review)]

Masanori Ogino has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45914 )

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Patch Set 2:
> Patch Set 2: Code-Review+1
>
> The `postcar_enable_tseg_cache` function is essentially equivalent to the removed code block.
>
> It compiles on top of commit a5f4781d81 with `CONFIG_TSEG_STAGE_CACHE=y` and `CONFIG_SMM_TSEG=y`, runs on Lenovo ThinkPad X60s, and boots Linux 5.4.68 with SeaBIOS 1.14.0 and GRUB 2.0.4.
>
> It looks good to me.

And suspend by both closing the lid and executing `sudo systemctl suspend` works and resumes successfully. Forgot to mention. Sorry.

[Angel Pons (Code Review)]

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

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2: Code-Review+1
> >
> > The `postcar_enable_tseg_cache` function is essentially equivalent to the removed code block.
> >
> > It compiles on top of commit a5f4781d81 with `CONFIG_TSEG_STAGE_CACHE=y` and `CONFIG_SMM_TSEG=y`, runs on Lenovo ThinkPad X60s, and boots Linux 5.4.68 with SeaBIOS 1.14.0 and GRUB 2.0.4.
> >
> > It looks good to me.
>
> And suspend by both closing the lid and executing `sudo systemctl suspend` works and resumes successfully. Forgot to mention. Sorry.

Thanks for testing!

[Angel Pons (Code Review)]

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

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Patch Set 2: Code-Review-2

On hold until the purpose of caching what is marked as TSEG is clear. This might have to do with the stage cache.

[Angel Pons (Code Review)]

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45914 )

Change subject: nb/intel/i945/memmap.c: Use `postcar_enable_tseg_cache`
......................................................................

Abandoned