Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63198 )
Change subject: soc/intel/common: implement ioc driver ......................................................................
Patch Set 13:
(11 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63198/comment/d7ee9402_af86a5b0 PS11, Line 11: https://github.com/otcshare/CCG-MTL-Generic-PSS/blob/master/
IMO, starting with MTL, the GPMR access registers are migrated to IOC (I/O cache) instead DMI over PCR in previous generation, hence, added the required support in IA common code so that the SoC can select the required interface while programming the GPMR register
DMI can be still used using PCH case. MTL-S still using PCH while MTL-M&P are not using PCH. IOC is used for die to die communication while DMI is used for PCH interface.
@will, don't refer which is not in public in commit msg, you can explain the design change in SoC, you don't need to refer to UEFI BIOS code. I'll remove this in commit message.
As there is no external document which has all offsets for IOC, I provided as reference.
https://review.coreboot.org/c/coreboot/+/63198/comment/840da6b4_63fe3729 PS11, Line 14: TEST=Build and boot to OS for TGL and MTL
On what boards exactly?
TGL RVP and MTL PSS(pre-Silicon system). Will add info in commit message.
File src/soc/intel/common/block/gpmr/gpmr.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/fc474b55_fb048557 PS12, Line 27: uint32_t data32; : : data32 = gpmr_read32(offset); : data32 |= ordata; : gpmr_write32(offset, data32);
you mean ioc_or32?
Ack
File src/soc/intel/common/block/ioc/Kconfig:
https://review.coreboot.org/c/coreboot/+/63198/comment/9283a5e1_ccb71ed1 PS5, Line 1: SOC_INTEL_COMMON_BLOCK_IOC
Do we have KConfig for this? […]
use SOC_INTEL_COMMON_BLOCK_SA
File src/soc/intel/common/block/ioc/Kconfig:
https://review.coreboot.org/c/coreboot/+/63198/comment/d26996ba_7afa103b PS11, Line 4: Intel Processor common IO Cache (IOC)
Please elaborate, what it does, and since what generation it’s available.
Ack
https://review.coreboot.org/c/coreboot/+/63198/comment/860c756c_4de9ab42 PS11, Line 9: ioc
IOC
Ack
https://review.coreboot.org/c/coreboot/+/63198/comment/eaef2bf2_554d0fe6 PS11, Line 9: ioc register offset override
Please elaborate, what the default is, and when it should be changed.
Will remove this for now as there is one version of SOC to support IOC and add later if new SOC has different offsets.
File src/soc/intel/common/block/ioc/ioc.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/b8186eeb_f4127aa8 PS5, Line 9: MCH_BASE_ADDRESS
MCHBAR should be enabled very early (in bootblock) […]
I think it'll be safe to add. I'll add condition check and print out error message.
File src/soc/intel/common/block/ioc/ioc.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/90441961_3b0f9482 PS12, Line 15: read32
Ack […]
Done
File src/soc/intel/common/pch/lockdown/lockdown.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/e70f2494_e87742b3 PS11, Line 31: #if !CONFIG(SOC_INTEL_COMMON_BLOCK_IOC) : gpmr_or32(GPMR_DMICTL, GPMR_DMICTL_SRLOCK); : #endif
Please use C code and not the preprocessor.
Ack
File src/soc/intel/common/pch/lockdown/lockdown.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/70db5bc1_de57517f PS12, Line 32: GPMR_DMICTL
Yes, With IOC interface, there is no CTL register and it does not need to do it. […]
Done