James has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
nb/intel/snb: Add PCI routing table for PEG root ports
Previously the PRTs were defined in southbridge code (#13612), but this was lost when southbridge PRTs became autogenerated. Add the proper PRTs for the PCI express for graphics root ports.
This (again) fixes warnings issued by Linux for interrupts on secondary functions of devices on the PEG ports, such as the HDMI audio controller on graphics cards.
pcieport 0000:00:01.0: can't derive routing for PCI INT B snd_hda_intel 0000:01:00.1: PCI INT B: no GSI
Tested with GIGABYTE P67A-UD3R (#31363) with Radeon HD 5670.
Change-Id: Ic429ec2fdeadb9dab1c03916974e173004d6cd16 Signed-off-by: James Ye jye836@gmail.com --- M src/northbridge/intel/sandybridge/acpi/peg.asl 1 file changed, 76 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/39021/1
diff --git a/src/northbridge/intel/sandybridge/acpi/peg.asl b/src/northbridge/intel/sandybridge/acpi/peg.asl index fcec00e..b7881ea 100644 --- a/src/northbridge/intel/sandybridge/acpi/peg.asl +++ b/src/northbridge/intel/sandybridge/acpi/peg.asl @@ -27,6 +27,25 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + If (PICM) { + Return (Package() { + Package() { 0x0000ffff, 0, 0, 16 }, + Package() { 0x0000ffff, 1, 0, 17 }, + Package() { 0x0000ffff, 2, 0, 18 }, + Package() { 0x0000ffff, 3, 0, 19 } + }) + } Else { + Return (Package() { + Package() { 0x0000ffff, 0, _SB.PCI0.LPCB.LNKA, 0 }, + Package() { 0x0000ffff, 1, _SB.PCI0.LPCB.LNKB, 0 }, + Package() { 0x0000ffff, 2, _SB.PCI0.LPCB.LNKC, 0 }, + Package() { 0x0000ffff, 3, _SB.PCI0.LPCB.LNKD, 0 } + }) + } + } }
Device (PEG1) @@ -42,6 +61,25 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + If (PICM) { + Return (Package() { + Package() { 0x0000ffff, 0, 0, 17 }, + Package() { 0x0000ffff, 1, 0, 18 }, + Package() { 0x0000ffff, 2, 0, 19 }, + Package() { 0x0000ffff, 3, 0, 16 } + }) + } Else { + Return (Package() { + Package() { 0x0000ffff, 0, _SB.PCI0.LPCB.LNKB, 0 }, + Package() { 0x0000ffff, 1, _SB.PCI0.LPCB.LNKC, 0 }, + Package() { 0x0000ffff, 2, _SB.PCI0.LPCB.LNKD, 0 }, + Package() { 0x0000ffff, 3, _SB.PCI0.LPCB.LNKA, 0 } + }) + } + } }
Device (PEG2) @@ -57,6 +95,25 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + If (PICM) { + Return (Package() { + Package() { 0x0000ffff, 0, 0, 18 }, + Package() { 0x0000ffff, 1, 0, 19 }, + Package() { 0x0000ffff, 2, 0, 16 }, + Package() { 0x0000ffff, 3, 0, 17 } + }) + } Else { + Return (Package() { + Package() { 0x0000ffff, 0, _SB.PCI0.LPCB.LNKC, 0 }, + Package() { 0x0000ffff, 1, _SB.PCI0.LPCB.LNKD, 0 }, + Package() { 0x0000ffff, 2, _SB.PCI0.LPCB.LNKA, 0 }, + Package() { 0x0000ffff, 3, _SB.PCI0.LPCB.LNKB, 0 } + }) + } + } }
Device (PEG6) @@ -72,4 +129,23 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + If (PICM) { + Return (Package() { + Package() { 0x0000ffff, 0, 0, 16 }, + Package() { 0x0000ffff, 1, 0, 17 }, + Package() { 0x0000ffff, 2, 0, 18 }, + Package() { 0x0000ffff, 3, 0, 19 } + }) + } Else { + Return (Package() { + Package() { 0x0000ffff, 0, _SB.PCI0.LPCB.LNKA, 0 }, + Package() { 0x0000ffff, 1, _SB.PCI0.LPCB.LNKB, 0 }, + Package() { 0x0000ffff, 2, _SB.PCI0.LPCB.LNKC, 0 }, + Package() { 0x0000ffff, 3, _SB.PCI0.LPCB.LNKD, 0 } + }) + } + } }
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@9 PS1, Line 9: Previously the PRTs were defined in southbridge code (#13612), but this Please at least add the commit message summary of the referenced commit/change-set. There is also the CB:13612 syntax, so Gerrit marks it up as a link.
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@18 PS1, Line 18: > snd_hda_intel 0000:01:00.1: PCI INT B: no GSI Instead of a (Markdown) citation, make it a code block by indenting it with 4 spaces.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@20 PS1, Line 20: Tested with GIGABYTE P67A-UD3R (#31363) with Radeon HD 5670. Tested with MSI (i.e. only tested that the warning goes away) or tested IRQ operation? Please try with `nomsi` in the kernel command, in case.
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/acpi/peg.asl:
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 137: Package() { 0x0000ffff, 0, 0, 16 }, In my vendor DSDT (H77 chipset), this device' table starts with 19.
If there are doubts (and ports we can't test) should we reduce this to the ports we can test?
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@9 PS1, Line 9: Previously the PRTs were defined in southbridge code (#13612), but this
Please at least add the commit message summary of the referenced commit/change-set. […]
Thanks, noted.
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@18 PS1, Line 18: > snd_hda_intel 0000:01:00.1: PCI INT B: no GSI
Instead of a (Markdown) citation, make it a code block by indenting it with 4 spaces.
Thanks, noted.
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@20 PS1, Line 20: Tested with GIGABYTE P67A-UD3R (#31363) with Radeon HD 5670.
Tested with MSI (i.e. only tested that the warning goes away) or […]
nomsi gives the same results as with MSI. HDMI audio works as tested with speaker-test, although in any case, linux produces a warning "IRQ timing workaround is activated for card #1. Suggest a bigger bdl_pos_adj", even under OEM firmware.
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/acpi/peg.asl:
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 137: Package() { 0x0000ffff, 0, 0, 16 },
In my vendor DSDT (H77 chipset), this device' table starts with 19. […]
Yes, I would avoid changing things if in doubt. I've only tested the PEG10 slot so far, and I don't have the capability to test PEG11, PEG12 or PEG60.
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/acpi/peg.asl:
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 137: Package() { 0x0000ffff, 0, 0, 16 },
Yes, I would avoid changing things if in doubt. […]
I looked at a vendor DSDT of a Z77 board (ASUS P8Z77-V LX), and it matches what you report for the PEG60 root port.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/acpi/peg.asl:
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 47: } If I'm not mistaken, we can just re-use the IRQM method of the PCH code. e.g.
Method (_PRT) { Return (_SB.PCI0.IRQM (1)) }
(and 2, 3, 4 as argument for the other ports)
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 137: Package() { 0x0000ffff, 0, 0, 16 },
I looked at a vendor DSDT of a Z77 board (ASUS P8Z77-V LX), and it matches what you report for the P […]
Let's just take 19, 16, 17, 18 / D, A, B, C then. If all sources say the same, I feel safe :)
Hello Patrick Rudolph, Stefan Reinauer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39021
to look at the new patch set (#2).
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
nb/intel/snb: Add PCI routing table for PEG root ports
Previously the PRTs were defined in southbridge code (8014714 southbridge/intel/bd82x6x/acpi: Fix IRQ warnings), but this was lost when southbridge PRTs became autogenerated. Add the proper PRTs for the PCI express for graphics root ports.
This (again) fixes warnings issued by Linux for interrupts on secondary functions of devices on the PEG ports, such as the HDMI audio controller on graphics cards.
pcieport 0000:00:01.0: can't derive routing for PCI INT B snd_hda_intel 0000:01:00.1: PCI INT B: no GSI
Tested with GIGABYTE P67A-UD3R (CB:31363) with Radeon HD 5670.
Change-Id: Ic429ec2fdeadb9dab1c03916974e173004d6cd16 Signed-off-by: James Ye jye836@gmail.com --- M src/northbridge/intel/sandybridge/acpi/peg.asl 1 file changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/39021/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 2: Code-Review+2
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@9 PS1, Line 9: Previously the PRTs were defined in southbridge code (#13612), but this
Thanks, noted.
Done
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@18 PS1, Line 18: > snd_hda_intel 0000:01:00.1: PCI INT B: no GSI
Thanks, noted.
Done
https://review.coreboot.org/c/coreboot/+/39021/1//COMMIT_MSG@20 PS1, Line 20: Tested with GIGABYTE P67A-UD3R (#31363) with Radeon HD 5670.
nomsi gives the same results as with MSI. […]
The command line should be pci=nomsi, MSIs can also be disabled in the snd_hda_intel driver with enable_msi=0. Verified with /proc/interrupts that MSIs aren't used and done.
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/acpi/peg.asl:
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 47: }
If I'm not mistaken, we can just re-use the IRQM method of the PCH code. e.g. […]
Yes, thanks. :)
https://review.coreboot.org/c/coreboot/+/39021/1/src/northbridge/intel/sandy... PS1, Line 137: Package() { 0x0000ffff, 0, 0, 16 },
Let's just take 19, 16, 17, 18 / D, A, B, C then. If all sources say the same, […]
Done.
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
nb/intel/snb: Add PCI routing table for PEG root ports
Previously the PRTs were defined in southbridge code (8014714 southbridge/intel/bd82x6x/acpi: Fix IRQ warnings), but this was lost when southbridge PRTs became autogenerated. Add the proper PRTs for the PCI express for graphics root ports.
This (again) fixes warnings issued by Linux for interrupts on secondary functions of devices on the PEG ports, such as the HDMI audio controller on graphics cards.
pcieport 0000:00:01.0: can't derive routing for PCI INT B snd_hda_intel 0000:01:00.1: PCI INT B: no GSI
Tested with GIGABYTE P67A-UD3R (CB:31363) with Radeon HD 5670.
Change-Id: Ic429ec2fdeadb9dab1c03916974e173004d6cd16 Signed-off-by: James Ye jye836@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39021 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/northbridge/intel/sandybridge/acpi/peg.asl 1 file changed, 20 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/acpi/peg.asl b/src/northbridge/intel/sandybridge/acpi/peg.asl index fcec00e..afc24df 100644 --- a/src/northbridge/intel/sandybridge/acpi/peg.asl +++ b/src/northbridge/intel/sandybridge/acpi/peg.asl @@ -27,6 +27,11 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + Return (_SB.PCI0.IRQM (1)) + } }
Device (PEG1) @@ -42,6 +47,11 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + Return (_SB.PCI0.IRQM (2)) + } }
Device (PEG2) @@ -57,6 +67,11 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + Return (_SB.PCI0.IRQM (3)) + } }
Device (PEG6) @@ -72,4 +87,9 @@ { Name(_ADR, 0x00000000) } + + Method (_PRT) + { + Return (_SB.PCI0.IRQM (4)) + } }
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39021 )
Change subject: nb/intel/snb: Add PCI routing table for PEG root ports ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/722 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/721 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/720
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
James (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)