Attention is currently required from: Tim Wawrzynczak, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50857 )
Change subject: sb/intel/common: Swap callbacks in intel_acpi_gen_def_acpi_pirq ......................................................................
Patch Set 6:
(1 comment)
File src/southbridge/intel/common/acpi_pirq_gen.c:
https://review.coreboot.org/c/coreboot/+/50857/comment/c6a4c67a_0a011bd0 PS3, Line 33: pirq - PIRQ_A
Check the newest patch train, I think it's a little cleaner... […]
Yeah, looking at the latest patches.. like you said, it is a little cleaner.. but probably we should separate them out further so that it is not too confusing for the reader.
Recapturing what we have so far (and thinking out loud):
* IOAPIC route information is exposed in ACPI using GSI#. - Older platforms (prior to SKL) routed INT pins for a slot using PIRQ to an IOAPIC IRQ. - Starting with SKL, it is possible to route INT pins either using PIRQ to an IOAPIC IRQ or directly to IOAPIC IRQ as well. In both the above cases, information exposed in _PRT is a GSI#.
* PIC route again has two variants. Either a static mapping (on recent Intel platforms) or dynamic mapping (on older platforms). - Dynamic mapping is provided by LNKx devices under the LPC device in .asl files. Thus, IRQ information is exposed using source path/index. - On the other hand, static mapping uses GSI#. In both the above cases for PIC route, INT pin is routed to PIRQ which then uses the static or dynamic mapping.
Given that, we really just need two helper functions in acpigen_pci.c: * acpigen_write_PRT_GSI_entry(dev, pin, gsi) * acpigen_write_PRT_source_entry(dev, pin, source_path, index)
This matches what ACPI spec says. Each route entry can be either a GSI# or a Source path/index. It doesn't really matter if that entry is being added to IOAPIC or PIC.
Now, the information that is required for each slot-pin combination is: - Which PIRQ pin the slot-pin is routed to (if any)? - What is the GSI# used by the slot-pin (if any)?
Thus, instead of obtaining a table `pci_int_mapping[][]`, I think we need the caller to provide following information:
struct slot_pin_irq_map { int pirq; /* PIRQ # the pin is routed to (if any) */ int gsi; /* GSI # the pin is routed to (if any) */ };
This can be used to encode PIN->PIRQ/GSI information for each slot-pin (i.e. [32][4]).
enum pirq_map_type { PIRQ_GSI, /* PIRQ uses static mapping (assigned to a particular GSI#) */ PIRQ_SOURCE_PATH, /* PIRQ uses dynamic mapping (uses source path/index) */ };
This can be used to encode the PIRQ mapping information by the caller i.e. how the PIRQ # maps to GSI# or a source path. Probably something like:
struct pirq_map { enum pirq_map_type type; int gsi[MAX_PIRQS]; /* Used for PIRQ_GSI */ char source_path[MAX_PIRQS][DEVICE_PATH_MAX]; /* Used for PIRQ_SOURCE_PATH */ };
The above PIRQ information and the PIN->PIRQ/GSI mapping can be put together into an input buffer by the caller. I think that should avoid all the callbacks/Kconfigs that are required as well. We can add a new function here `intel_write_pci0_PRT()`.
Older platforms using intel_acpi_gen_def_acpi_pirq: - These will still call intel_acpi_gen_def_acpi_pirq that will call intel_write_pci0_PRT providing PIRQ # and PIRQ_SOURCE_PATH.
Newer platforms: - Called from intel/common/block - these will call intel_write_pci0_PRT directly providing PIRQ # (if applicable), GSI # and PIRQ_GSI.
What`intel_write_pci0_PRT()` will have to do is: - For PIC route, it will have to use `pirq` from slot_pin_irq_map and pirq_map to add appropriate GSI# or source path. - For APIC route, it will have to use `gsi` if available or convert `pirq` to GSI # and use that to add _PRT entry.