Patchwork DRAM self refresh check in src/northbridge/i945/raminit.c

login
register
about
Submitter Sven Schnelle
Date 2011-03-21 20:52:40
Message ID <87sjugci1z.fsf@begreifnix.stackframe.org>
Download mbox | patch
Permalink /patch/2801/
State Not Applicable
Headers show

Comments

Sven Schnelle - 2011-03-21 20:52:40
Hi List,

one of the last remaining problems with the ThinkPad X60 port is that
coreboot does a full reset on ACPI S3 wakeup. There's an explicit check
that the DRAM self refresh bits in PMSTS (MCHBAR offset 0xf14) are set.

For (yet) unknown reasons, those bits are not set on my Thinkpad after
Wakeup, even though suspending works for several days, without the DRAM
being refreshed by the Chipset, so i guess Self Refresh is working. I've
commented the check out, with the following diff:

Does anyone have an idea why that check fails?
Stefan Reinauer - 2011-03-21 21:46:30
* Sven Schnelle <svens@stackframe.org> [110321 21:52]:
> Hi List,
> 
> one of the last remaining problems with the ThinkPad X60 port is that
> coreboot does a full reset on ACPI S3 wakeup. There's an explicit check
> that the DRAM self refresh bits in PMSTS (MCHBAR offset 0xf14) are set.
> 
> For (yet) unknown reasons, those bits are not set on my Thinkpad after
> Wakeup, even though suspending works for several days, without the DRAM
> being refreshed by the Chipset, so i guess Self Refresh is working. I've
> commented the check out, with the following diff:
> 
> Does anyone have an idea why that check fails?
 
Can you try setting the bits instead? (Maybe in SMM?) 

I am very hesitant to let the below patch go in, as certain ram
misconfigurations can only be corrected with a cold boot if it goes in.

Stefan



> diff --git a/src/northbridge/intel/i945/raminit.c
> b/src/northbridge/intel/i945/raminit.c
> index 8b7ffa1..e6990f7 100644
> --- a/src/northbridge/intel/i945/raminit.c
> +++ b/src/northbridge/intel/i945/raminit.c
> @@ -312,10 +312,12 @@ static void sdram_detect_errors(struct sys_info
> *sysinfo)
>         }
>  
>         if (do_reset) {
> -               printk(BIOS_DEBUG, "Reset required.\n");
> +               printk(BIOS_DEBUG, "Reset required. (ignored)\n");
> +#if 0
>                 outb(0x00, 0xcf9);
>                 outb(0x0e, 0xcf9);
>                 for (;;) asm("hlt"); /* Wait for reset! */
> +#endif
>         }
>  }
> 
> 
> -- 
> coreboot mailing list: coreboot@coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>
Sven Schnelle - 2011-03-22 06:15:53
Hi Stefan,

Stefan Reinauer <stefan.reinauer@coreboot.org> writes:

>> one of the last remaining problems with the ThinkPad X60 port is that
>> coreboot does a full reset on ACPI S3 wakeup. There's an explicit check
>> that the DRAM self refresh bits in PMSTS (MCHBAR offset 0xf14) are set.
>> 
>> For (yet) unknown reasons, those bits are not set on my Thinkpad after
>> Wakeup, even though suspending works for several days, without the DRAM
>> being refreshed by the Chipset, so i guess Self Refresh is working. I've
>> commented the check out, with the following diff:
>> 
>> Does anyone have an idea why that check fails?
>  
> Can you try setting the bits instead? (Maybe in SMM?) 

According to the datasheets, this register is readonly, and if you write
1 to it, the self refresh status bits will be cleared. So it seems like
there's no way to force those registers to 1.

My guess is that there are some other registers which needs to be setup
to have that working, but i don't know which registers, as the public
datasheets available from Intel doesn't say much about it.

> I am very hesitant to let the below patch go in, as certain ram
> misconfigurations can only be corrected with a cold boot if it goes in.

Of course. It wasn't my intention to have this patch in, it was just for
clarification what the check is i'm talking about. :)

Regards,

Sven.

Patch

diff --git a/src/northbridge/intel/i945/raminit.c
b/src/northbridge/intel/i945/raminit.c
index 8b7ffa1..e6990f7 100644
--- a/src/northbridge/intel/i945/raminit.c
+++ b/src/northbridge/intel/i945/raminit.c
@@ -312,10 +312,12 @@  static void sdram_detect_errors(struct sys_info
*sysinfo)
        }
 
        if (do_reset) {
-               printk(BIOS_DEBUG, "Reset required.\n");
+               printk(BIOS_DEBUG, "Reset required. (ignored)\n");
+#if 0
                outb(0x00, 0xcf9);
                outb(0x0e, 0xcf9);
                for (;;) asm("hlt"); /* Wait for reset! */
+#endif
        }
 }