Re: [coreboot] ASRock E350M1: Suspend to RAM: Assertion in `F14NbPstateInit` because MemClk is invalid

[Dave Frodin]

Paul,

I built a Persimmon ROM off of the latest coreboot.org code.
The s3 resume works fine using the power button.

I'm attaching two log files that I captured from the COM port.
One is for a cold boot, one is for resuming from S3.

I used the power button for the resume signal.
My Ubuntu 12.04 (I believe) reports...
uname -a
   Linux Sage 3.2.0-37-generic #58-Ubuntu SMP Thu Jan 24 15:28:10 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

Ubuntu is running off of a SATA drive.
In the past I've tried S3 resume after booting off of a USB thumbdrive and it would not work.

I've also seen problems (on other platforms) with the code that saves the h/w settings to the flash device.
This is what Persimmon reports...
SF: Detected W25Q64 with page size 1000, total 800000
SF: Successfully erased 4096 bytes @ 0xffff7000
SF: Detected W25Q64 with page size 1000, total 800000
SF: Successfully erased 24576 bytes @ 0xffff0000
SF: Detected W25Q64 with page size 1000, total 800000
SF: Successfully erased 4096 bytes @ 0xffff6000

There isn't any error checking in any of the s3 save code.
So it's possible for the code to either incorrectly
erase sectors and/or not actually write the s3 data to flash.
I found that it was useful to ...
    1) write a new coreboot image to the SPI flash
    2) boot the system to the SeaBIOS "Hit F12" message
    3) read the contents of the SPI flash
    4) compare the before and after ROM images

This revealed several s3 save issues on a different platform.

I evidently don't have the h/w to capture the logs with readserial.
I don't see a delay between hitting the power or reset buttons before
I see the COM port debug log begin to show up. There is at least a 3
second delay when the SPI device is erased and updated with the s3 save
data.

Thanks,
Dave

----- Original Message -----
> From: "Paul Menzel" 
> To: [e-mail]
> Sent: Monday, April 22, 2013 8:48:08 AM
> Subject: [coreboot] ASRock E350M1: Suspend to RAM: Assertion in `F14NbPstateInit` because MemClk is invalid
>
> (was: Looking for Linux messages (`dmesg`) and coreboot log of AMD Persimmon
> for comparison with ASRock E350M1)
>
> Dear coreboot folks,
>
> could you please do two tests on AMD Persimmon for me? Long version
> below.
>
> Am Sonntag, den 20.01.2013, 11:38 +0100 schrieb Paul Menzel:
> > I am planning on working on the ASRock E350M1 a bit. As it is similar to
> > the AMD Persimmon board, it would be awesome to have the Persimmon logs
> > for comparison. Could you please attach them to your reply with a note
> > what GNU/Linux distribution you use and what Linux version.
>
> I got some logs privately. Thanks.
>
> Now I am trying to get suspend to RAM working on the ASRock E350M1
> basically by copying what has been done for AMD Persimmon.
>
> Currently it’s not working because during resume in
>     $ nl -ba
>     src/vendorcode/amd/agesa/f14/Proc/CPU/Family/0x14/cpuCommonF14Utilities.c
>     […]
>        282
>        	/*---------------------------------------------------------------------------------------*/
>        283	/**
>        284	 * Sets up a valid set of NB P-states based on the value of
>        MEMCLK, transitions
>        285	 * to the desired NB P-state, and returns the current NB
>        frequency in megahertz.
>        286	 *
>        287	 * @param[in]     TargetMemclk            The target MEMCLK in
>        megahertz, or zero to
>        288	 *                                        indicate NB P-state
>        change only.
>        289	 * @param[in]     TargetMemclkEncoded     The target MEMCLK's
>        register encoding.
>        290	 * @param[in]     TargetNbPstate          The NB P-state to
>        exit in.
>        291	 * @param[in]     CurrentNbFreq           Current NB operating
>        frequency in megahertz.
>        292	 * @param[in]     StdHeader               Handle of Header for
>        calling lib functions and services.
>        293	 *
>        294	 * @retval        TRUE                    Transition to
>        TargetNbPstate was successful.
>        295	 * @retval        FALSE                   Transition to
>        TargetNbPstate was unsuccessful.
>        296	 */
>        297	BOOLEAN
>        298	F14NbPstateInit (
>        299	  IN       UINT32             TargetMemclk,
>        300	  IN       UINT32             TargetMemclkEncoded,
>        301	  IN       UINT32             TargetNbPstate,
>        302	     OUT   UINT32             *CurrentNbFreq,
>        303	  IN       AMD_CONFIG_PARAMS  *StdHeader
>        304	  )
>        305	{
>        306	  UINT32                EncodedNbPs1Vid;
>        307	  UINT32                EncodedNbPs0NclkDiv;
>        308	  UINT32                EncodedNbPs1NclkDiv;
>        309	  UINT32                NbP0Cof;
>        310	  UINT32                NbP1Cof;
>        311	  UINT32                NbPstateNumerator;
>        312	  UINT32                TargetNumerator;
>        313	  UINT32                TargetDenominator;
>        314	  BOOLEAN               ReturnStatus;
>        315	  BOOLEAN               WaitForTransition;
>        316	  PCI_ADDR              PciAddress;
>        317	  D18F3xD4_STRUCT       Cptc0;
>        318	  D18F3xDC_STRUCT       Cptc2;
>        319	  D18F6x90_STRUCT       NbPsCfgLow;
>        320	  D18F6x98_STRUCT       NbPsCtrlSts;
>        321	  FCRxFE00_6000_STRUCT  FCRxFE00_6000;
>        322	  FCRxFE00_6002_STRUCT  FCRxFE00_6002;
>        323	  FCRxFE00_7006_STRUCT  FCRxFE00_7006;
>        324	  FCRxFE00_7009_STRUCT  FCRxFE00_7009;
>        325
>        326	  // F14 only supports NB P0 and NB P1
>        327	  ASSERT (TargetNbPstate < 2);
>        328
>        329	  WaitForTransition = FALSE;
>        330	  ReturnStatus = TRUE;
>        331
>        332	  // Get D18F3xD4[MainPllOpFreqId] frequency
>        333	  PciAddress.AddressValue = CPTC0_PCI_ADDR;
>        334	  LibAmdPciRead (AccessWidth32, PciAddress, &Cptc0.Value,
>        StdHeader);
>        335
>        336	  // Calculate the numerator to be used for NB P-state
>        calculations
>        337	  NbPstateNumerator = (UINT32) (4 *
>        ((Cptc0.Field.MainPllOpFreqId + 0x10) * 100));
>        338
>        339	  if (TargetMemclk != 0) {
>        340	    // Determine the appropriate numerator / denominator of
>        the target memclk
>        341	    switch (TargetMemclk) {
>        342	    case DDR800_FREQUENCY:
>        343	      TargetNumerator = 400;
>        344	      TargetDenominator = 1;
>        345	      break;
>        346	    case DDR1066_FREQUENCY:
>        347	      TargetNumerator = 1600;
>        348	      TargetDenominator = 3;
>        349	      break;
>        350	    case DDR1333_FREQUENCY:
>        351	      TargetNumerator = 2000;
>        352	      TargetDenominator = 3;
>        353	      break;
>        354	    default:
>        355	      // An invalid memclk has been passed in.
>
> I hit the following assertion. No idea why. The error log unfortunately
> does not contain what it read.
>        356	      ASSERT (FALSE);
>        357	      TargetNumerator = TargetMemclk;
>        358	      TargetDenominator = 1;
>        359	      break;
>        360	    }
>     […]
>
> This is what I get on the serial line.
>     ý
>
>     coreboot-4.0-4085-geff3dd3 Mon Apr 22 00:52:48 CEST 2013 starting...
>     BSP Family_Model: 00500f10
>     cpu_init_detectedx = 00000000
>     agesawrapper_amdinitmmio passed.
>     agesawrapper_amdinitreset passed.
>     agesawrapper_amdinitearly BSP Family_Model: 00500f10
>     cpu_init_detectedx = 00000001
>     agesawrapper_amdinitmmio passed.
>     agesawrapper_amdinitreset passed.
>     agesawrapper_amdinitearly passed.
>     SLP_TYP type was 3
>     S3 detected
>     agesawrapper_amdinitresume ASSERTION FAILED: file
>     'src/vendorcode/amd/agesa/f14/Proc/CPU/Family/0x14/cpuCommonF14Utilities.c',
>     line 356
>     ASSERTION FAILED: file
>     'src/vendorcode/amd/agesa/f14/Proc/CPU/Family/0x14/cpuCommonF14Utilities.c',
>     line 386
>
> Searching for `F14NbPstateInit`, I assume during resume this function is
> called from the following code.
>     $ nl -ba src/vendorcode/amd/agesa/f14/Proc/Mem/NB/ON/mnS3on.c
>     […]
>        557	/*
>        -----------------------------------------------------------------------------*/
>        558	/**
>        559	 *
>        560	 *   This function is a wrapper to call a CPU routine to
>        change NB P-state and
>        561	 *   update NB frequency.
>        562	 *
>        563	 *     @param[in,out]   *NBPtr   - Pointer to the MEM_NB_BLOCK
>        564	 *     @param[in]  *NBPstate - NB Pstate
>        565	 *
>        566	 *     @return          TRUE - Succeed
>        567	 *     @return          FALSE - Fail
>        568	 */
>        569
>        570	BOOLEAN
>        571	STATIC
>        572	MemNS3ChangeNbFrequencyWrapON (
>        573	  IN OUT   MEM_NB_BLOCK *NBPtr,
>        574	  IN       UINT32 NBPstate
>        575	  )
>        576	{
>        577	  BOOLEAN Status;
>        578	  UINT32  NBFreq;
>        579	  UINT32  Speed;
>        580
>        581	  Speed = MemNGetBitFieldNb (NBPtr, BFMemClkFreq);
>        582	  Status = F14NbPstateInit (((Speed + 6) * 3335) / 100,
>        583	                            Speed,
>        584	                            NBPstate,
>        585	                            &NBFreq,
>        586	                            &(NBPtr->MemPtr->StdHeader));
>        587
>        588	  return Status;
>        589	}
>     […]
>
> Just to be sure, could you please confirm that on AMD Persimmon suspend
> to RAM (and resume) still works.
>
> Additionally I noticed that with the local changes, it takes three
> seconds until anything is displayed on the serial line.
>     ======= Mon Apr 22 09:00:58 2013 (adjust=86.8us)
>     00.000: <00>
>     01.019: <00>
>     02.723: <00>
>     03.997: <fd>
>     03.997:
>     03.997: coreboot-4.0-4085-geff3dd3 Mon Apr 22 00:52:48 CEST 2013
>     starting...
>     […]
>
> I guess this is related to enabling more options in `buildOpts.c`, but I
> cannot say for sure.
>
> Could you please capture the coreboot log on AMD Persimmon with
> `readserial` from the SeaBIOS repository [1] for comparison? That would
> be great.
>     readserial.py /dev/ttyUSB0 115200
>
>
> Thanks,
>
> Paul
>
> [1]
> http://review.coreboot.org/gitweb?p=seabios.git;a=tree;f=tools;h=2d6cbb520ec...