Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46757 )
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint
Drop unnecessary smbus.asl in favor of southbridge common code.
Tested with BUILD_TIMELESS=1, Purism Librem 13 v1 does not change.
Change-Id: I13b35d2155a2cede0a56846b8bf8a79d4ebfc7b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/acpi/ctdp.asl M src/soc/intel/broadwell/pch/acpi/pch.asl D src/soc/intel/broadwell/pch/acpi/smbus.asl 3 files changed, 40 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/46757/1
diff --git a/src/soc/intel/broadwell/acpi/ctdp.asl b/src/soc/intel/broadwell/acpi/ctdp.asl index 8391482..b18ec78 100644 --- a/src/soc/intel/broadwell/acpi/ctdp.asl +++ b/src/soc/intel/broadwell/acpi/ctdp.asl @@ -9,8 +9,7 @@ Name (CTCU, 2) /* CTDP Up Select */ Name (SPL1, 0) /* Saved PL1 value */
- OperationRegion (MCHB, SystemMemory, - Add (MCH_BASE_ADDRESS, 0x5000), 0x1000) + OperationRegion (MCHB, SystemMemory, MCH_BASE_ADDRESS + 0x5000, 0x1000) Field (MCHB, DWordAcc, Lock, Preserve) { Offset (0x930), /* PACKAGE_POWER_SKU */ @@ -51,7 +50,7 @@ }
/* - * Search CPU0 _PSS looking for control=arg0 and then + * Search CPU0 _PSS looking for control = arg0 and then * return previous P-state entry number for new _PPC * * Format of _PSS: @@ -62,17 +61,16 @@ External (_SB.CP00._PSS) Method (PSSS, 1, NotSerialized) { - Store (One, Local0) /* Start at P1 */ - Store (SizeOf (_SB.CP00._PSS), Local1) + Local0 = 1 /* Start at P1 */ + Local1 = SizeOf (_SB.CP00._PSS)
- While (LLess (Local0, Local1)) { + While (Local0 < Local1) { /* Store _PSS entry Control value to Local2 */ - ShiftRight (DeRefOf (Index (DeRefOf (Index - (_SB.CP00._PSS, Local0)), 4)), 8, Local2) - If (LEqual (Local2, Arg0)) { - Return (Subtract (Local0, 1)) + Local2 = DeRefOf (Index (DeRefOf (Index (_SB.CP00._PSS, Local0)), 4)) >> 8 + If (Local2 == Arg0) { + Return (Local0 - 1) } - Increment (Local0) + Local0++ }
Return (0) @@ -83,7 +81,7 @@ { /* Haswell ULT PL2 = 25W */ /* FIXME: update for broadwell */ - Return (Multiply (25, 8)) + Return (25 * 8) }
/* Set Config TDP Down */ @@ -92,31 +90,31 @@ If (Acquire (CTCM, 100)) { Return (0) } - If (LEqual (CTCD, CTCC)) { + If (CTCD == CTCC) { Release (CTCM) Return (0) }
- Store ("Set TDP Down", Debug) + Debug = "Set TDP Down"
/* Set CTC */ - Store (CTCD, CTCS) + CTCS = CTCD
/* Set TAR */ - Store (TARD, TARS) + TARS = TARD
/* Set PPC limit and notify OS */ - Store (PSSS (TARD), PPCM) + PPCM = PSSS (TARD) PPCN ()
/* Set PL2 */ - Store (CPL2 (CTDD), PL2V) + PL2V = CPL2 (CTDD)
/* Set PL1 */ - Store (CTDD, PL1V) + PL1V = CTDD
/* Store the new TDP Down setting */ - Store (CTCD, CTCC) + CTCC = CTCD
Release (CTCM) Return (1) @@ -128,31 +126,31 @@ If (Acquire (CTCM, 100)) { Return (0) } - If (LEqual (CTCN, CTCC)) { + If (CTCN == CTCC) { Release (CTCM) Return (0) }
- Store ("Set TDP Nominal", Debug) + Debug = "Set TDP Nominal"
/* Set PL1 */ - Store (CTDN, PL1V) + PL1V = CTDN
/* Set PL2 */ - Store (CPL2 (CTDN), PL2V) + PL2V = CPL2 (CTDN)
/* Set PPC limit and notify OS */ - Store (PSSS (TARN), PPCM) + PPCM = PSSS (TARN) PPCN ()
/* Set TAR */ - Store (TARN, TARS) + TARS = TARN
/* Set CTC */ - Store (CTCN, CTCS) + CTCS = CTCN
/* Store the new TDP Nominal setting */ - Store (CTCN, CTCC) + CTCC = CTCN
Release (CTCM) Return (1) @@ -161,7 +159,7 @@ /* Calculate PL1 value based on requested TDP */ Method (TDPP, 1, NotSerialized) { - Return (Multiply (ShiftLeft (Subtract (PUNI, 1), 2), Arg0)) + Return (((PUNI - 1) << 2) * Arg0) }
/* Enable Controllable TDP to limit PL1 to requested value */ @@ -171,22 +169,22 @@ Return (0) }
- Store ("Enable PL1 Limit", Debug) + Debug = "Enable PL1 Limit"
/* Set _PPC to LFM */ - Store (PSSS (LFM_), Local0) - Add (Local0, 1, PPCM) + Local0 = PSSS (LFM_) + PPCM = Local0 + 1 \PPCN ()
/* Set TAR to LFM-1 */ - Subtract (LFM_, 1, TARS) + TARS = LFM_ - 1
/* Set PL1 to desired value */ - Store (PL1V, SPL1) - Store (TDPP (Arg0), PL1V) + SPL1 = PL1V + PL1V = TDPP (Arg0)
/* Set PL1 CLAMP bit */ - Store (One, PL1C) + PL1C = 1
Release (CTCM) Return (1) @@ -199,19 +197,19 @@ Return (0) }
- Store ("Disable PL1 Limit", Debug) + Debug = "Disable PL1 Limit"
/* Clear PL1 CLAMP bit */ - Store (Zero, PL1C) + PL1C = 0
/* Set PL1 to normal value */ - Store (SPL1, PL1V) + PL1V = SPL1
/* Set TAR to 0 */ - Store (Zero, TARS) + TARS = 0
/* Set _PPC to 0 */ - Store (Zero, PPCM) + PPCM = 0 \PPCN ()
Release (CTCM) diff --git a/src/soc/intel/broadwell/pch/acpi/pch.asl b/src/soc/intel/broadwell/pch/acpi/pch.asl index 07db9f7..5a94bca 100644 --- a/src/soc/intel/broadwell/pch/acpi/pch.asl +++ b/src/soc/intel/broadwell/pch/acpi/pch.asl @@ -60,7 +60,7 @@ #include "sata.asl"
// SMBus 0:1f.3 -#include "smbus.asl" +#include <southbridge/intel/common/acpi/smbus.asl>
// Serial IO #include "serialio.asl" diff --git a/src/soc/intel/broadwell/pch/acpi/smbus.asl b/src/soc/intel/broadwell/pch/acpi/smbus.asl deleted file mode 100644 index 32b0b9c..0000000 --- a/src/soc/intel/broadwell/pch/acpi/smbus.asl +++ /dev/null @@ -1,8 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -// Intel SMBus Controller 0:1f.3 - -Device (SBUS) -{ - Name (_ADR, 0x001f0003) -}
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46757
to look at the new patch set (#2).
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint
Drop unnecessary smbus.asl in favor of southbridge common code.
Tested with BUILD_TIMELESS=1, Purism Librem 13 v1 does not change.
Change-Id: I13b35d2155a2cede0a56846b8bf8a79d4ebfc7b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/acpi/ctdp.asl M src/soc/intel/broadwell/pch/acpi/lpc.asl M src/soc/intel/broadwell/pch/acpi/pch.asl D src/soc/intel/broadwell/pch/acpi/smbus.asl 4 files changed, 42 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/46757/2
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46757
to look at the new patch set (#5).
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint
Drop unnecessary smbus.asl in favor of southbridge common code.
Tested with BUILD_TIMELESS=1, Purism Librem 13 v1 does not change.
Change-Id: I13b35d2155a2cede0a56846b8bf8a79d4ebfc7b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/acpi/ctdp.asl A src/soc/intel/broadwell/acpi/hostbridge.asl M src/soc/intel/broadwell/pch/acpi/lpc.asl M src/soc/intel/broadwell/pch/acpi/pch.asl D src/soc/intel/broadwell/pch/acpi/smbus.asl 5 files changed, 239 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/46757/5
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46757
to look at the new patch set (#6).
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint
Drop unnecessary smbus.asl in favor of southbridge common code.
Tested with BUILD_TIMELESS=1, Purism Librem 13 v1 does not change.
Change-Id: I13b35d2155a2cede0a56846b8bf8a79d4ebfc7b3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/acpi/ctdp.asl M src/soc/intel/broadwell/acpi/systemagent.asl M src/soc/intel/broadwell/pch/acpi/lpc.asl M src/soc/intel/broadwell/pch/acpi/pch.asl D src/soc/intel/broadwell/pch/acpi/smbus.asl 5 files changed, 46 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/46757/6
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46757 )
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46757/8/src/soc/intel/broadwell/acp... File src/soc/intel/broadwell/acpi/ctdp.asl:
PS8: Is that just conversion to ASL 2 syntax?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46757 )
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46757/8/src/soc/intel/broadwell/acp... File src/soc/intel/broadwell/acpi/ctdp.asl:
PS8:
Is that just conversion to ASL 2 syntax?
Maybe, but I made this patch with a diff tool, so it may be more than just converting to ASL 2.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46757 )
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46757/8/src/soc/intel/broadwell/acp... File src/soc/intel/broadwell/acpi/ctdp.asl:
PS8:
Maybe, but I made this patch with a diff tool, so it may be more than just converting to ASL 2.
Ack
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46757 )
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
Patch Set 10: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46757 )
Change subject: soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint ......................................................................
soc/intel/broadwell: Align ACPI with Haswell/Lynxpoint
Drop unnecessary smbus.asl in favor of southbridge common code.
Tested with BUILD_TIMELESS=1, Purism Librem 13 v1 does not change.
Change-Id: I13b35d2155a2cede0a56846b8bf8a79d4ebfc7b3 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46757 Reviewed-by: Michael Niewöhner foss@mniewoehner.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/broadwell/acpi/ctdp.asl M src/soc/intel/broadwell/acpi/systemagent.asl M src/soc/intel/broadwell/pch/acpi/lpc.asl M src/soc/intel/broadwell/pch/acpi/pch.asl D src/soc/intel/broadwell/pch/acpi/smbus.asl 5 files changed, 46 insertions(+), 56 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/acpi/ctdp.asl b/src/soc/intel/broadwell/acpi/ctdp.asl index 8391482..b18ec78 100644 --- a/src/soc/intel/broadwell/acpi/ctdp.asl +++ b/src/soc/intel/broadwell/acpi/ctdp.asl @@ -9,8 +9,7 @@ Name (CTCU, 2) /* CTDP Up Select */ Name (SPL1, 0) /* Saved PL1 value */
- OperationRegion (MCHB, SystemMemory, - Add (MCH_BASE_ADDRESS, 0x5000), 0x1000) + OperationRegion (MCHB, SystemMemory, MCH_BASE_ADDRESS + 0x5000, 0x1000) Field (MCHB, DWordAcc, Lock, Preserve) { Offset (0x930), /* PACKAGE_POWER_SKU */ @@ -51,7 +50,7 @@ }
/* - * Search CPU0 _PSS looking for control=arg0 and then + * Search CPU0 _PSS looking for control = arg0 and then * return previous P-state entry number for new _PPC * * Format of _PSS: @@ -62,17 +61,16 @@ External (_SB.CP00._PSS) Method (PSSS, 1, NotSerialized) { - Store (One, Local0) /* Start at P1 */ - Store (SizeOf (_SB.CP00._PSS), Local1) + Local0 = 1 /* Start at P1 */ + Local1 = SizeOf (_SB.CP00._PSS)
- While (LLess (Local0, Local1)) { + While (Local0 < Local1) { /* Store _PSS entry Control value to Local2 */ - ShiftRight (DeRefOf (Index (DeRefOf (Index - (_SB.CP00._PSS, Local0)), 4)), 8, Local2) - If (LEqual (Local2, Arg0)) { - Return (Subtract (Local0, 1)) + Local2 = DeRefOf (Index (DeRefOf (Index (_SB.CP00._PSS, Local0)), 4)) >> 8 + If (Local2 == Arg0) { + Return (Local0 - 1) } - Increment (Local0) + Local0++ }
Return (0) @@ -83,7 +81,7 @@ { /* Haswell ULT PL2 = 25W */ /* FIXME: update for broadwell */ - Return (Multiply (25, 8)) + Return (25 * 8) }
/* Set Config TDP Down */ @@ -92,31 +90,31 @@ If (Acquire (CTCM, 100)) { Return (0) } - If (LEqual (CTCD, CTCC)) { + If (CTCD == CTCC) { Release (CTCM) Return (0) }
- Store ("Set TDP Down", Debug) + Debug = "Set TDP Down"
/* Set CTC */ - Store (CTCD, CTCS) + CTCS = CTCD
/* Set TAR */ - Store (TARD, TARS) + TARS = TARD
/* Set PPC limit and notify OS */ - Store (PSSS (TARD), PPCM) + PPCM = PSSS (TARD) PPCN ()
/* Set PL2 */ - Store (CPL2 (CTDD), PL2V) + PL2V = CPL2 (CTDD)
/* Set PL1 */ - Store (CTDD, PL1V) + PL1V = CTDD
/* Store the new TDP Down setting */ - Store (CTCD, CTCC) + CTCC = CTCD
Release (CTCM) Return (1) @@ -128,31 +126,31 @@ If (Acquire (CTCM, 100)) { Return (0) } - If (LEqual (CTCN, CTCC)) { + If (CTCN == CTCC) { Release (CTCM) Return (0) }
- Store ("Set TDP Nominal", Debug) + Debug = "Set TDP Nominal"
/* Set PL1 */ - Store (CTDN, PL1V) + PL1V = CTDN
/* Set PL2 */ - Store (CPL2 (CTDN), PL2V) + PL2V = CPL2 (CTDN)
/* Set PPC limit and notify OS */ - Store (PSSS (TARN), PPCM) + PPCM = PSSS (TARN) PPCN ()
/* Set TAR */ - Store (TARN, TARS) + TARS = TARN
/* Set CTC */ - Store (CTCN, CTCS) + CTCS = CTCN
/* Store the new TDP Nominal setting */ - Store (CTCN, CTCC) + CTCC = CTCN
Release (CTCM) Return (1) @@ -161,7 +159,7 @@ /* Calculate PL1 value based on requested TDP */ Method (TDPP, 1, NotSerialized) { - Return (Multiply (ShiftLeft (Subtract (PUNI, 1), 2), Arg0)) + Return (((PUNI - 1) << 2) * Arg0) }
/* Enable Controllable TDP to limit PL1 to requested value */ @@ -171,22 +169,22 @@ Return (0) }
- Store ("Enable PL1 Limit", Debug) + Debug = "Enable PL1 Limit"
/* Set _PPC to LFM */ - Store (PSSS (LFM_), Local0) - Add (Local0, 1, PPCM) + Local0 = PSSS (LFM_) + PPCM = Local0 + 1 \PPCN ()
/* Set TAR to LFM-1 */ - Subtract (LFM_, 1, TARS) + TARS = LFM_ - 1
/* Set PL1 to desired value */ - Store (PL1V, SPL1) - Store (TDPP (Arg0), PL1V) + SPL1 = PL1V + PL1V = TDPP (Arg0)
/* Set PL1 CLAMP bit */ - Store (One, PL1C) + PL1C = 1
Release (CTCM) Return (1) @@ -199,19 +197,19 @@ Return (0) }
- Store ("Disable PL1 Limit", Debug) + Debug = "Disable PL1 Limit"
/* Clear PL1 CLAMP bit */ - Store (Zero, PL1C) + PL1C = 0
/* Set PL1 to normal value */ - Store (SPL1, PL1V) + PL1V = SPL1
/* Set TAR to 0 */ - Store (Zero, TARS) + TARS = 0
/* Set _PPC to 0 */ - Store (Zero, PPCM) + PPCM = 0 \PPCN ()
Release (CTCM) diff --git a/src/soc/intel/broadwell/acpi/systemagent.asl b/src/soc/intel/broadwell/acpi/systemagent.asl index 258e6e7..3e7ced0 100644 --- a/src/soc/intel/broadwell/acpi/systemagent.asl +++ b/src/soc/intel/broadwell/acpi/systemagent.asl @@ -141,9 +141,9 @@ Method (_CRS, 0, Serialized) { // Find PCI resource area in MCRS - CreateDwordField(MCRS, ^PM01._MIN, PMIN) - CreateDwordField(MCRS, ^PM01._MAX, PMAX) - CreateDwordField(MCRS, ^PM01._LEN, PLEN) + 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 @@ -170,7 +170,7 @@ /* PCI Device Resource Consumption */ Device (PDRC) { - Name (_HID, EISAID("PNP0C02")) + Name (_HID, EISAID ("PNP0C02")) Name (_UID, 1)
Name (PDRS, ResourceTemplate() { diff --git a/src/soc/intel/broadwell/pch/acpi/lpc.asl b/src/soc/intel/broadwell/pch/acpi/lpc.asl index 5bdfea2..4422907 100644 --- a/src/soc/intel/broadwell/pch/acpi/lpc.asl +++ b/src/soc/intel/broadwell/pch/acpi/lpc.asl @@ -181,6 +181,6 @@
#include "gpio.asl" #include "irqlinks.asl" - #include <acpi/ec.asl> - #include <acpi/superio.asl> + #include "acpi/ec.asl" + #include "acpi/superio.asl" } diff --git a/src/soc/intel/broadwell/pch/acpi/pch.asl b/src/soc/intel/broadwell/pch/acpi/pch.asl index 07db9f7..5a94bca 100644 --- a/src/soc/intel/broadwell/pch/acpi/pch.asl +++ b/src/soc/intel/broadwell/pch/acpi/pch.asl @@ -60,7 +60,7 @@ #include "sata.asl"
// SMBus 0:1f.3 -#include "smbus.asl" +#include <southbridge/intel/common/acpi/smbus.asl>
// Serial IO #include "serialio.asl" diff --git a/src/soc/intel/broadwell/pch/acpi/smbus.asl b/src/soc/intel/broadwell/pch/acpi/smbus.asl deleted file mode 100644 index 32b0b9c..0000000 --- a/src/soc/intel/broadwell/pch/acpi/smbus.asl +++ /dev/null @@ -1,8 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -// Intel SMBus Controller 0:1f.3 - -Device (SBUS) -{ - Name (_ADR, 0x001f0003) -}
-
Angel Pons (Code Review) -
Michael Niewöhner (Code Review)
-
Paul Menzel (Code Review)