Attention is currently required from: Angel Pons, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
30 comments:
Commit Message:
Likewise, I can't wrap my head around PCI-E AERs I'm getting if I boot
the machine without `pcie_aspm=off` parameter:
- BadTLP
- BadDLLP
- Timeout
- Rollover
RP5, PEG0, PEG1. Yes, it's enabled in stock firmware: […]
Done
File src/mainboard/erying/tgl/Kconfig:
config PCIEXP_ASPM
default n
config PCIEXP_CLK_PM
default n
config PCIEXP_L1_SUB_STATE
default n
It does enable/disable ASPM based on Kconfig setting. […]
I've set values to "0" in ramstage which would have the same result, but added this as preventive measure. It does make a difference, new patch series fixes PCI-E instability.
Patch Set #8, Line 39: default y
Why?
Done
File src/mainboard/erying/tgl/Kconfig.name:
Patch Set #8, Line 4: bool "erying-tgl"
There are no mITX versions of TGL platform, so it would be safe to leave it as-it. […]
Done
File src/mainboard/erying/tgl/gpio.h:
Patch Set #8, Line 168: PAD_CFG_NF(GPD9, NONE, PWROK, NF1), /* SLP_WLAN# */
> `please, no space before tabs` […]
Done
File src/mainboard/erying/tgl/ramstage.c:
Patch Set #8, Line 21: params->CpuPcieRpAdvancedErrorReporting[0] = 1;
Sure, but then AERs will still happen - we simply won't know about it.
Done
Patch Set #8, Line 24: params->CpuPcieRpPtmEnabled[0] = 1;
Why?
Done
Patch Set #8, Line 45: params->PcieRpPmSci[4] = 1;
Why?
Done
Patch Set #8, Line 71: params->ITbtPcieTunnelingForUsb4 = 0;
Now that I think about it, probably no. […]
Done
Patch Set #8, Line 72: params->PchUsbOverCurrentEnable = 0;
It was left there for debugging and I forgot about it. […]
Done
File src/mainboard/erying/tgl/romstage_fsp_params.c:
Patch Set #8, Line 24: mupd->FspmConfig.EnableAbove4GBMmio = 1;
Shouldn't this be configurable?
Technically yes, but I don't see a Kconfig option for it in coreboot (maybe I'm just missing something).
mupd->FspmConfig.OcSupport = 1;
mupd->FspmConfig.OcLock = 0;
Should be turned into a config option at SoC level.
Likewise, though I will check if it's necessary in the first place.
// iGPU
mupd->FspmConfig.GttSize = 3; // 8MB
mupd->FspmConfig.ApertureSize = 3; // 512MB
mupd->FspmConfig.GtPsmiSupport = 0;
mupd->FspmConfig.IgdDvmt50PreAlloc = 2; // 64MB
Should be configurable, if set at all. […]
Yes, without configuring those options, iGPU would crash spectacularly with framebuffer corruption or GPU reset.
mupd->FspmConfig.CpuPcieRpPcieSpeed[0] = 0; // Auto
mupd->FspmConfig.CpuPcieRpPcieSpeed[1] = 0; // Auto
There should be no need to set these.
Acknowledged
mupd->FspmConfig.CpuPcieRpCdrRelock[0] = 1;
mupd->FspmConfig.CpuPcieRpCdrRelock[1] = 1;
Why?
Acknowledged
mupd->FspmConfig.CpuPcieNewFom[0] = 1;
mupd->FspmConfig.CpuPcieNewFom[1] = 1;
Why? I don't know what exactly this does, but the default is 0. […]
Acknowledged
Patch Set #8, Line 43: mupd->FspmConfig.DmiMaxLinkSpeed = 3;
No need
Without setting this parameter, DMI behaves... erratically, slowly dropping link speed
mupd->FspmConfig.DmiAspm = 0;
mupd->FspmConfig.DmiAspmCtrl = 0;
DMI ASPM is independent from PCIe ASPM. Please remove.
Yes, I've set this on purpose. System is so unstable without this option being set that it doesn't make it to loading the kernel from NVMe drive.
It's necessary, so leaving as-is.
Patch Set #8, Line 48: mupd->FspmConfig.SpdProfileSelected = 0; // Default profile
Any reason to set this?
Like with RefClk, I'm leaving an option for users to easily enable XMP.
Tested, working and stable (seemingly first DDR4 mainboard in coreboot tree with working XMP). I'm planning to move from SMMSTORE to CMOS in the future and configure it based on CMOS value, which users will be able to configure by setting it in UiApp in EDK2 or some other mean.
Patch Set #8, Line 49: mupd->FspmConfig.RefClk = 0; // 133MHz
Any reason to set this?
Not sure which one FSP picks as default, RefClk 100MHz is less stable than 133MHz (for whatever reason).
Patch Set #8, Line 50: mupd->FspmConfig.VddVoltage = 1350; // 1.35V (Applicable only with XMP Profile 1)
Does your board have the means to change the DIMM voltage? If not, this is just lying to MRC.
Yes, based on XMP profile.
mupd->FspmConfig.Ratio = 0; // Auto
mupd->FspmConfig.RingDownBin = 0;
mupd->FspmConfig.GearRatio = 0; // Auto
Aren't these the defaults?
Changed those in recent patch.
Patch Set #8, Line 54: mupd->FspmConfig.ECT = 1;
Early Command Training is usually disabled for DDR4, any reason to enable it here?
AFAIK necessary for XMP to work?
mupd->FspmConfig.WRVC1D = 1;
mupd->FspmConfig.WRVC1D = 1;
This is set twice, shouldn't be set at all anyway.
Keen eye, thanks!
mupd->FspmConfig.ECT = 1;
mupd->FspmConfig.LCT = 1;
mupd->FspmConfig.SOT = 1;
mupd->FspmConfig.ERDMPRTC2D = 0;
mupd->FspmConfig.RDMPRT = 1;
mupd->FspmConfig.RCVET = 1;
mupd->FspmConfig.JWRL = 1;
mupd->FspmConfig.EWRTC2D = 1;
mupd->FspmConfig.ERDTC2D = 1;
mupd->FspmConfig.WRTC2D = 1;
mupd->FspmConfig.WRVC1D = 1;
mupd->FspmConfig.WRVC1D = 1;
mupd->FspmConfig.DIMMODTT = 1;
mupd->FspmConfig.DIMMRONT = 1;
mupd->FspmConfig.WRDSEQT = 1;
mupd->FspmConfig.WRSRT = 0;
mupd->FspmConfig.RDODTT = 1;
mupd->FspmConfig.RDEQT = 1;
mupd->FspmConfig.RDAPT = 1;
mupd->FspmConfig.RDTC2D = 1;
mupd->FspmConfig.WRVC2D = 1;
mupd->FspmConfig.RDVC2D = 1;
mupd->FspmConfig.CMDVC = 1;
mupd->FspmConfig.MrcSafeConfig = 0;
mupd->FspmConfig.LpDdrDqDqsReTraining = 1;
mupd->FspmConfig.SafeMode = 0;
mupd->FspmConfig.OverrideDowngradeForMixedMemory = 0;
mupd->FspmConfig.MemTestOnWarmBoot = 1;
Why change this? Changing which MRC training steps get executed is very risky as it can increase the […]
Acknowledged
Patch Set #8, Line 82: mupd->FspmConfig.DdrFreqLimit = 2933; // Setting to auto for now.
That doesn't look like "auto"
Yes, that was a mistake on my part.
I've been blaming my code for unstable memory, while in reality... my RAM became faulty.
Both D0 and D1 work perfectly with 3200MHz XMP enabled now, changed in recent patch.
Patch Set #8, Line 83: mupd->FspmConfig.SaGv = 0;
Does it matter for TGL-H?
Likely not
Patch Set #8, Line 85: mupd->FspmConfig.NModeSupport = 1; // Board type is 1N (1DPC).
N-Mode (command rate) is not bound to DPC (the number of DIMMs per channel). […]
I see, thank you.
Patch Set #8, Line 86: mupd->FspmConfig.RhPrevention = 0;
Please don't force disable Row Hammer prevention upon others. […]
Acknowledged
mupd->FspmConfig.RefreshWm = 1;
mupd->FspmConfig.ExitOnFailure = 1;
Why set these?
Copied from vendor's firmware for debugging purposes, I suppose it's safe to remove them.
To view, visit change 80853. To unsubscribe, or for help writing mail filters, visit settings.