[L] Change in coreboot[main]: mb/erying: Add Erying Polestar G613 Pro (TGL-H)

[Angel Pons (Code Review)]

Attention is currently required from: Alicja Michalska, Felix Singer, Maxim, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.

Angel Pons has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email )

Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................

Patch Set 12: Code-Review+1

(12 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/80853/comment/defebb90_d693c428?usp... :
PS8, Line 23: broken
> Broken, as it either causes VM to hang, or the entire host 😊

Good grief

https://review.coreboot.org/c/coreboot/+/80853/comment/fe4b1dca_c3a78b34?usp... :
PS8, Line 31: - Automatic fan control (IT8613E can't read CPU_TIN at the moment)
> That was my suspicion as well, but it doesn't seem to be the case.

Given my issues with the IT8613E on the ODROID-H4, figuring this one out won't be as easy

File src/mainboard/erying/tgl/Kconfig:

https://review.coreboot.org/c/coreboot/+/80853/comment/cd9c22da_361fb74b?usp... :
PS12, Line 14: 	select HAVE_ACPI_RESUME
If S3 resume is broken, I would comment out this line

File src/mainboard/erying/tgl/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/80853/comment/7d53f2a0_de653b3d?usp... :
PS12, Line 43: device ref peg1 on 	# SoC x16 (Gen4)
             : 			register "PcieClkSrcUsage[0]" = "0x41"
             : 			register "PcieClkSrcClkReq[0]" = "PCIE_CLK_NOTUSED"
             : 		end
nit: move below peg0

File src/mainboard/erying/tgl/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/80853/comment/eada445c_8c5c5c2e?usp... :
PS12, Line 27: Scope (_SB.PCI0.LPCB)
             : 	{
             : 	}
Still here from PS8

File src/mainboard/erying/tgl/hda_verb.c:

https://review.coreboot.org/c/coreboot/+/80853/comment/ce6792d7_43430e28?usp... :
PS12, Line 26: #if (!CONFIG_SOC_INTEL_DISABLE_IGD)
             : 		/* Tigerlake HDMI */
             : 		0x80862812, /* Vendor ID */
             : 		0x80860101, /* Subsystem ID */
             : 		2, /* Number of entries */
             : 		AZALIA_SUBVENDOR(2, 0x80860101),
             : 		AZALIA_PIN_CFG(2, 0x04, 0x18560010),
             : 	#endif
We don't indent preprocessor in coreboot:

```suggestion
#if (!CONFIG_SOC_INTEL_DISABLE_IGD)
    /* Tigerlake HDMI */
    0x80862812, /* Vendor ID */
    0x80860101, /* Subsystem ID */
    2, /* Number of entries */
    AZALIA_SUBVENDOR(2, 0x80860101),
    AZALIA_PIN_CFG(2, 0x04, 0x18560010),
#endif
```

File src/mainboard/erying/tgl/romstage_fsp_params.c:

https://review.coreboot.org/c/coreboot/+/80853/comment/8f615a0c_287c5646?usp... :
PS8, Line 24: 	mupd->FspmConfig.EnableAbove4GBMmio = 1;
> Technically yes, but I don't see a Kconfig option for it in coreboot (maybe I'm just missing somethi […]

Eh, it's not a big deal (hooking this up in a follow-up is easy)

https://review.coreboot.org/c/coreboot/+/80853/comment/22e4fa91_d42dedad?usp... :
PS8, Line 25: mupd->FspmConfig.OcSupport = 1;
            : 	mupd->FspmConfig.OcLock = 0;
> Likewise, though I will check if it's necessary in the first place.

OcSupport may be needed for XMP to be used, OcLock might be to allow changing settings at runtime somehow but I'm not sure

https://review.coreboot.org/c/coreboot/+/80853/comment/9eb30cca_65fdd334?usp... :
PS8, Line 28: // iGPU
            : 	mupd->FspmConfig.GttSize = 3; // 8MB
            : 	mupd->FspmConfig.ApertureSize = 3; // 512MB
            : 	mupd->FspmConfig.GtPsmiSupport = 0;
            : 	mupd->FspmConfig.IgdDvmt50PreAlloc = 2; // 64MB
> Yes, without configuring those options, iGPU would crash spectacularly with framebuffer corruption o […]

Whew, that's nasty. Let's keep it as-is for now

https://review.coreboot.org/c/coreboot/+/80853/comment/28243e81_3b35a8ec?usp... :
PS8, Line 43: 	mupd->FspmConfig.DmiMaxLinkSpeed = 3;
> Without setting this parameter, DMI behaves... […]

Interesting... This board is cursed

https://review.coreboot.org/c/coreboot/+/80853/comment/01be5f2d_ca242908?usp... :
PS8, Line 54: 	mupd->FspmConfig.ECT = 1;
> AFAIK necessary for XMP to work?

Weird, I feel manually specifying which training steps to run shouldn't be necessary. But if it works, let's leave it like this.

File src/mainboard/erying/tgl/romstage_fsp_params.c:

https://review.coreboot.org/c/coreboot/+/80853/comment/5baef651_63190233?usp... :
PS12, Line 40: 	mupd->FspmConfig.VddVoltage = 1350; // 1.35V
Note: this won't actually change the voltage going to the DIMMs