Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Felix Held, Hannah Williams, Jamie Ryu.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions ......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/84104/comment/ac0e6b42_f7771df6?usp... : PS11, Line 241: gpe0_mask
Subrata,
Your point of decoupling GPE1 from GPE0 makes sense. However, I am not quite followed your suggestion, can you help me a bit more here? In fact, this is the only place that particularly disable PME_B0 event and then all GPE bits are disabled. I am not sure what is the reason behind and why we cannot just simply disable all GPE events. Going back to the doubt that I have on extending this 'pmc_disable_std_gpe(PME_B0_EN);' so that it can cover GPE1 portion. No STS bits was read in order to disable PME_B0. I don't find a clean/generic way to come out with mask array to disable GPE1 events and combine with the existing 32-bit GPE0 mask here for this pmc_disable_std_gpe function as an array argument.
static void smm_southbridge_enable(uint16_t pm1_events) { uint32_t smi_params = ENABLE_SMI_PARAMS;
printk(BIOS_DEBUG, "Enabling SMIs.\n"); /* Configure events */ pmc_enable_pm1(pm1_events); pmc_disable_std_gpe(PME_B0_EN); <- ... pmc_disable_all_gpe();
So here is my understanding, GPE1 is design to be more granular and specific to internal SoC IPs whereelse GPE0 can be more convoluted.
for example: GPE0 PME_B0_STS (GPE0_STS_127_96 bit 13) status can be a reflection of any below device
Internal devices which can set this bit: - Integrated LAN - HD Audio/Audio DSP - xHCI - CSME Maskable Host Wake
whereelse, the GPE1 (GPE1_STS_31_0 bit 18) is more granular form of GPE0 (GPE0_STS_127_96 bit 13) where we should be able to know which internal device has triggered the wake. in this case, it may be due to CNVI BT device.
Now coming to how can you implement the array, ideally you need to add below elements for GPE1 event array (which is similar to GPE0 GPE0_STS_127_96 array).
General Purpose Event 1 Status [31:0] (GPE1_STS_31_0) General Purpose Event 1 Status [63:32] (GPE1_STS_63_32) General Purpose Event 1 Status [63:32] (GPE1_STS_95_64)
during boot, we should check GPE0 by default and only check GPE1 if respective Kconfig is enabled to know the wake event. similarly while booting to OS, we shall be able to able to clear the pending status of both GPE0 and GPE1 (if kconfig is enabled)