Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40379 )
Change subject: soc/intel: Update SA common code based on CONFIG_PCI_SEGMENT_GROUPS ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/apollolake/sy... File src/soc/intel/apollolake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/apollolake/sy... PS4, Line 34: PCIEXBAR_LENGTH_MIB
This value is supposed to be in bytes, right?
very good catch.
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/common/block/... PS4, Line 44: CONFIG_MMCONF_BASE_ADDRESS
Is this supposed to be the same for all segments?
Yes please refer to PCIEXBAR_0_0_0_PCI
The address used to access the PCI Express configuration space for a specific device can be determined as follows: PCI Express Base Address +Segment Number*256MB+ Bus Number * 1MB + Device Number * 32KB + Function Number * 4KB
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/common/block/... PS4, Line 30: switch (CONFIG_PCI_SEGMENT_GROUPS) { : case 1: : pciexbar_length = PCIEXBAR_LENGTH_256MB; : break;
I am a little confused here. Does the PCI segment group number determine the length?
Its not likely that, but if i take an example of IA SoC where mostly 256 bus are referring to 1 segment.
if Bus is > 256, example: 257 then shouldn’t it like 257/256 = 1 refers to Segment 1 and Bus number is 257- (segment_no * 256) = 1
but there could be lesser buses also in a segment like 64/128.
In this process we are removing those unused 64/128 bus programming which was anyway not selected by common code
Can you please point me to some reference to understand this better?
You can refer to PCIEXBAR_0_0_0_PCI register as well.
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/skylake/acpi.... File src/soc/intel/skylake/acpi.c:
https://review.coreboot.org/c/coreboot/+/40379/4/src/soc/intel/skylake/acpi.... PS4, Line 207: CONFIG_PCI_SEGMENT_GROUPS
Does skylake support multiple segments?
no.
in symmetry with common code (acpi.c) i have made this but eventually CONFIG_PCI_SEGMENT_GROUPS = 1 for all unless specified