Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
soc/intel/common: Add ASL for TCSS PCI segment
PCI1 device been created based on TCSS_PCIE_SEGMENT selection from MB Kconfig
extracted build/dsdt.aml
Device (PCI1) { Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID Name (_SEG, One) // _SEG: PCI Segment Name (_UID, One) // _UID: Unique ID Name (_ADR, Zero) // _ADR: Address .... }
Change-Id: I43924a3a34173ba3531079ef848f1935c59bb74a Signed-off-by: Subrata Banik subrata.banik@intel.com --- A src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl A src/soc/intel/common/block/acpi/acpi/pcisegment.asl 2 files changed, 128 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/41011/1
diff --git a/src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl b/src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl new file mode 100644 index 0000000..77c0248 --- /dev/null +++ b/src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl @@ -0,0 +1,74 @@ +/* + * This file is part of the coreboot project. + * + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Method (_CRS, 0, Serialized) +{ + Name (MCRS, ResourceTemplate () + { + /* Bus Numbers */ + WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, + 0x0000, 0x0000, 0x00ff, 0x0000, 0x0100) + + /* IO Region 0 */ + DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, + EntireRange, + 0x0000, 0x0000, 0x0cf7, 0x0000, 0x0cf8) + + /* IO Region 1 */ + DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, + EntireRange, + 0x0000, 0x0d00, 0xffff, 0x0000, 0xf300) + + /* PCI Memory Region (TLUD - 0xdfffffff) */ + DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, + NonCacheable, ReadWrite, + 0x00000000, 0x00000000, 0xdfffffff, 0x00000000, + 0xE0000000,,, PM01) + + /* PCI Memory Region (TUUD - (TUUD + ABOVE_4G_MMIO_SIZE)) */ + QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, + NonCacheable, ReadWrite, + 0x00000000, 0x10000, 0x1ffff, 0x00000000, + 0x10000,,, PM02) + }) + + /* Find PCI resource area in MCRS */ + CreateDwordField (MCRS, PM01._MIN, PMIN) + CreateDwordField (MCRS, PM01._MAX, PMAX) + CreateDwordField (MCRS, PM01._LEN, PLEN) + + /* + * Fix up PCI memory region + * Start with Top of Lower Usable DRAM + */ + Store (_SB.PCI0.MCHC.TLUD, PMIN) + Add (Subtract (PMAX, PMIN), 1, PLEN) + + /* Patch PM02 range based on Memory Size */ + If (LEqual (A4GS, 0)) { + CreateQwordField (MCRS, PM02._LEN, MSEN) + Store (0, MSEN) + } Else { + CreateQwordField (MCRS, PM02._MIN, MMIN) + CreateQwordField (MCRS, PM02._MAX, MMAX) + CreateQwordField (MCRS, PM02._LEN, MLEN) + /* Set 64bit MMIO resource base and length */ + Store (A4GS, MLEN) + Store (A4GB, MMIN) + Subtract (Add (MMIN, MLEN), 1, MMAX) + } + + Return (MCRS) +} diff --git a/src/soc/intel/common/block/acpi/acpi/pcisegment.asl b/src/soc/intel/common/block/acpi/acpi/pcisegment.asl new file mode 100644 index 0000000..4efaf20 --- /dev/null +++ b/src/soc/intel/common/block/acpi/acpi/pcisegment.asl @@ -0,0 +1,54 @@ +/* + * This file is part of the coreboot project. + * + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#define TBT_PCIe0_IRQ 16 +#define TBT_PCIe1_IRQ 17 +#define TBT_PCIe2_IRQ 18 +#define TBT_PCIe3_IRQ 19 + +Device (PCI1) +{ + Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID + Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID + Name (_SEG, 1) // _SEG: PCI Segment + Name (_UID, 1) // _UID: Unique ID + Name (_ADR, 0x00000000) + + Name (PICN, Package () { + /* SEG1: PCIe Root Port*/ + Package(){0x0007FFFF, 0, 0, 11 }, + Package(){0x0007FFFF, 1, 0, 10 }, + Package(){0x0007FFFF, 2, 0, 11 }, + Package(){0x0007FFFF, 3, 0, 11 }, + }) + + Name (PICP, Package () { + /* SEG1: PCIe Root Port*/ + Package(){0x0007FFFF, 0, 0, TBT_PCIe0_IRQ }, + Package(){0x0007FFFF, 1, 0, TBT_PCIe1_IRQ }, + Package(){0x0007FFFF, 2, 0, TBT_PCIe2_IRQ }, + Package(){0x0007FFFF, 3, 0, TBT_PCIe3_IRQ }, + }) + + Method (_PRT) + { + If (PICM) { + Return (PICP) + } Else { + Return (PICN) + } + } + #include "extrahostbridge.asl" +}
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
it should fail. why it compile ??
we shouldn't have both of _ADR and _HID at the same time.
https://review.coreboot.org/c/coreboot/+/41011/1/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi/pcisegment.asl:
https://review.coreboot.org/c/coreboot/+/41011/1/src/soc/intel/common/block/... PS1, Line 27: Name (_ADR, 0x00000000) ? "ACPI system firmware must supply an _ADR object for each device to enable this. A device object must contain either an _HID object or an _ADR object, but should not contain both."
Advanced Configuration and Power Interface (ACPI) Specification - Version 6.3 - January 2019
page 341 & 342
Hello build bot (Jenkins), Patrick Georgi, Patrick Rudolph, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41011
to look at the new patch set (#2).
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
soc/intel/common: Add ASL for TCSS PCI segment
PCI1 device been created based on TCSS_PCIE_SEGMENT selection from MB Kconfig
extracted build/dsdt.aml
Device (PCI1) { Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID Name (_SEG, One) // _SEG: PCI Segment Name (_UID, One) // _UID: Unique ID .... }
Change-Id: I43924a3a34173ba3531079ef848f1935c59bb74a Signed-off-by: Subrata Banik subrata.banik@intel.com --- A src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl A src/soc/intel/common/block/acpi/acpi/pcisegment.asl 2 files changed, 127 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/41011/2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 2:
Patch Set 1: Code-Review-1
(1 comment)
it should fail. why it compile ??
we shouldn't have both of _ADR and _HID at the same time.
You are right that _ADR = PCI device typically where _HID = ACPI Soft device But i have seen many instances where both exist together. didn't follow latest ACPI spec 6.3 if there is any hard rule to avoid that.
But noted your point and make the code changes
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41011/1/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi/pcisegment.asl:
https://review.coreboot.org/c/coreboot/+/41011/1/src/soc/intel/common/block/... PS1, Line 27: Name (_ADR, 0x00000000)
? […]
Ack
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 1: Code-Review-1
(1 comment)
it should fail. why it compile ??
we shouldn't have both of _ADR and _HID at the same time.
You are right that _ADR = PCI device typically where _HID = ACPI Soft device But i have seen many instances where both exist together. didn't follow latest ACPI spec 6.3 if there is any hard rule to avoid that.
But noted your point and make the code changes
thank you
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41011/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41011/2//COMMIT_MSG@9 PS2, Line 9: PCI1 device been created based on TCSS_PCIE_SEGMENT selection : from MB Kconfig I would like to know more about the motivation and reasoning behind this change. Also, how was this tested?
https://review.coreboot.org/c/coreboot/+/41011/2/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl:
https://review.coreboot.org/c/coreboot/+/41011/2/src/soc/intel/common/block/... PS2, Line 34: /* PCI Memory Region (TLUD - 0xdfffffff) */ : DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, : NonCacheable, ReadWrite, : 0x00000000, 0x00000000, 0xdfffffff, 0x00000000, : 0xE0000000,,, PM01) : : /* PCI Memory Region (TUUD - (TUUD + ABOVE_4G_MMIO_SIZE)) */ : QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, : NonCacheable, ReadWrite, : 0x00000000, 0x10000, 0x1ffff, 0x00000000, : 0x10000,,, PM02) These are exported by both the northbridge.asl MCHC device and PCI1 device?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41011/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41011/2//COMMIT_MSG@9 PS2, Line 9: PCI1 device been created based on TCSS_PCIE_SEGMENT selection : from MB Kconfig
I would like to know more about the motivation and reasoning behind this change.
To add support for multiple segment present in TGL SOC for iTBT
https://github.com/otcshare/CCG-TGL-Generic-SiC/blob/master/ClientOneSilicon...
Also, how was this tested?
ITBT root port should enumerated when multi seg function enable
https://review.coreboot.org/c/coreboot/+/41011/2/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl:
https://review.coreboot.org/c/coreboot/+/41011/2/src/soc/intel/common/block/... PS2, Line 34: /* PCI Memory Region (TLUD - 0xdfffffff) */ : DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, : NonCacheable, ReadWrite, : 0x00000000, 0x00000000, 0xdfffffff, 0x00000000, : 0xE0000000,,, PM01) : : /* PCI Memory Region (TUUD - (TUUD + ABOVE_4G_MMIO_SIZE)) */ : QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, : NonCacheable, ReadWrite, : 0x00000000, 0x10000, 0x1ffff, 0x00000000, : 0x10000,,, PM02)
These are exported by both the northbridge. […]
Refer to https://github.com/otcshare/CCG-TGL-Generic-SiC/blob/master/ClientOneSilicon...
Hello build bot (Jenkins), Patrick Georgi, Patrick Rudolph, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41011
to look at the new patch set (#3).
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
soc/intel/common: Add ASL for TCSS PCI segment
Refer from Intel RC: https://github.com/otcshare/CCG-TGL-Generic-SiC/tree/master/ClientOneSilicon...
PCI1 device been created based on TCSS_PCIE_SEGMENT selection from MB Kconfig
extracted build/dsdt.aml
Device (PCI1) { Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID Name (_SEG, One) // _SEG: PCI Segment Name (_UID, One) // _UID: Unique ID .... }
Change-Id: I43924a3a34173ba3531079ef848f1935c59bb74a Signed-off-by: Subrata Banik subrata.banik@intel.com --- A src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl A src/soc/intel/common/block/acpi/acpi/pcisegment.asl 2 files changed, 127 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/41011/3
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 3: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41011/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41011/3//COMMIT_MSG@10 PS3, Line 10: https://github.com/otcshare/CCG-TGL-Generic-SiC/tree/master/ClientOneSilicon... 404
And even if it would be available, that somebody put it on GitHub doesn't mean they are on solid ground.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41011/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41011/3//COMMIT_MSG@10 PS3, Line 10: https://github.com/otcshare/CCG-TGL-Generic-SiC/tree/master/ClientOneSilicon...
404
i see, looks like its restricted access for distribution.
And even if it would be available, that somebody put it on GitHub doesn't mean they are on solid ground.
Its Intel validated and distributed Reference code for BIOS vendors hence i assume (ensure) those code been tested but that doesn't mean that its the best code. i will try to follow the design you have suggested here based on my discussion with RC team (your feedback https://review.coreboot.org/c/coreboot/+/40722)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41011/3/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi/extrahostbridge.asl:
https://review.coreboot.org/c/coreboot/+/41011/3/src/soc/intel/common/block/... PS3, Line 16: Method (_CRS, 0, Serialized) Can't we generate the AML at runtime?
https://review.coreboot.org/c/coreboot/+/41011/3/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi/pcisegment.asl:
https://review.coreboot.org/c/coreboot/+/41011/3/src/soc/intel/common/block/... PS3, Line 19: #define TBT_PCIe3_IRQ 19 These are SoC specific, no?
Attention is currently required from: Nico Huber, Subrata Banik. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/41011/comment/0dc047f0_b7f8b36d PS3, Line 10: https://github.com/otcshare/CCG-TGL-Generic-SiC/tree/master/ClientOneSilicon...
And even if it would be available, that somebody put it on GitHub doesn't mean they are on solid ground.
^ this
Please tell, why/what for it's needed
Attention is currently required from: Nico Huber, Subrata Banik. HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41011 )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3: maybe we can use ASL 2.0 syntax (for all new asl files) ?
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41011?usp=email )
Change subject: soc/intel/common: Add ASL for TCSS PCI segment ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
HAOUAS Elyes (Code Review)
-
Martin L Roth (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)
-
Subrata Banik (Code Review)
-
Wonkyu Kim (Code Review)