Attention is currently required from: Felix Singer, Kapil Porwal, Sean Rhodes, Tarun Tuli, Tim Crawford.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75481?usp=email )
Change subject: soc/intel/{alderlake, meteorlake}: Properly assign the FSP ASPM UPD ......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/75481/comment/afa74fdd_a35daf40 : PS1, Line 899: if (rp_cfg->pcie_rp_aspm) : s_cfg->PcieRpAspm[i] = get_aspm_control(rp_cfg->pcie_rp_aspm); : else if (CONFIG(PCIEXP_ASPM)) /* use auto (FSP default) if PCIEXP_ASPM is enabled */ : s_cfg->PcieRpAspm[i] = ASPM_AUTO; : else : s_cfg->PcieRpAspm[i] = ASPM_DISABLE;
Those aren't disjoint cases. I want both of them. ASPM should be disabled for a *specific device* BUT be "Auto" for *everything else*.
we are not relying anywhere on the FSP default value and trying to make sure that coreboot overrides the FSP default as applicable. can you please elaborate
"Auto" has been the default value used by every version of the FSP I've used for porting boards. So I mean setting it to "Auto" is the same thing as setting it to the value already used by the FSP.
It seems like to do what I want, I now have to override `PCIEXP_ASPM` to disable it and specify `pcie_rp_aspm` for *every* device (does that even work if the config is disabled; does FSP do it instead?). Compared to the previous behavior where I only needed to specify the device I want to disable.
I don't understand the concern here, are you saying that you don't want to make "PCIEXP_ASPM" kconfig default enabled on your platform ? because you want few devices to have ASPM disable vs others enabled ?
wondering how having FSP default set to `Auto` was helpful in your case where you wished to make some device ASPM disabled but others enabled ?