Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37640 )
Change subject: superio/common: Add more ACPI methods ......................................................................
superio/common: Add more ACPI methods
* Make use of introduced SSDT config mode access. * Provide ACPI functions to safely access SIO config space * Use introduced functions to implement _DIS and _STA in the device
Change-Id: I520b29de925f368cd71ff8f1f58d2d57d72eff8d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/common/generic.c M src/superio/common/ssdt.c 2 files changed, 104 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/37640/1
diff --git a/src/superio/common/generic.c b/src/superio/common/generic.c index 429ee51..2e5bf88 100644 --- a/src/superio/common/generic.c +++ b/src/superio/common/generic.c @@ -153,6 +153,85 @@ acpigen_write_indexfield("INDX", "DATA", i, ARRAY_SIZE(i), FIELD_BYTEACC | FIELD_NOLOCK | FIELD_PRESERVE);
+ const char *mutex = "MTX0"; + + acpigen_write_mutex(mutex, 0); + /* Backup LDN */ + acpigen_write_name_integer("BLDN", 0); + + /* Acquire mutex - Enter config mode */ + acpigen_write_method("AMTX", 1); + { + acpigen_write_acquire(mutex, 0xffff); + + pnp_ssdt_enter_conf_mode(dev, "^INDX", "^DATA"); + + /* Backup LDN */ + acpigen_write_store(); + acpigen_emit_namestring("^LDN"); + acpigen_emit_namestring("^BLDN"); + } + acpigen_pop_len(); /* Method */ + + /* Release mutex - Exit config mode */ + acpigen_write_method("RMTX", 1); + { + /* Restore LDN */ + acpigen_write_store(); + acpigen_emit_namestring("^BLDN"); + acpigen_emit_namestring("^LDN"); + + pnp_ssdt_exit_conf_mode(dev, "^INDX", "^DATA"); + + acpigen_write_release(mutex); + } + acpigen_pop_len(); /* Method */ + + /* Select a LDN */ + acpigen_write_method("SLDN", 1); + { + acpigen_write_store(); + acpigen_emit_byte(ARG0_OP); + acpigen_emit_namestring("^LDN"); + } + acpigen_pop_len(); /* Method */ + + /* Disable a LDN */ + acpigen_write_method("DLDN", 1); + { + acpigen_emit_namestring("AMTX"); + + acpigen_emit_namestring("SLDN"); + acpigen_emit_byte(ARG0_OP); + + acpigen_write_store(); + acpigen_emit_byte(ZERO_OP); + acpigen_emit_namestring("^ACTR"); + + acpigen_emit_namestring("RMTX"); + } + acpigen_pop_len(); /* Method */ + + /* Query LDN enable state. Returns 1 if LDN is enabled. */ + acpigen_write_method("QLDN", 1); + { + acpigen_emit_namestring("AMTX"); + + acpigen_emit_namestring("SLDN"); + acpigen_emit_byte(ARG0_OP); + + acpigen_write_store(); + acpigen_emit_namestring("^ACTR"); + acpigen_emit_byte(LOCAL0_OP); + + acpigen_write_and(LOCAL0_OP, ONE_OP, LOCAL0_OP); + + acpigen_emit_namestring("RMTX"); + + acpigen_write_return_byte(LOCAL0_OP); + } + acpigen_pop_len(); /* Method */ + acpigen_pop_len(); /* Device */ acpigen_pop_len(); /* Scope */ } diff --git a/src/superio/common/ssdt.c b/src/superio/common/ssdt.c index a919aa5..bc5d394 100644 --- a/src/superio/common/ssdt.c +++ b/src/superio/common/ssdt.c @@ -200,7 +200,24 @@ acpigen_write_name_byte("LDN", ldn); acpigen_write_name_byte("VLDN", vldn);
- acpigen_write_STA(dev->enabled ? 0xf : 0); + acpigen_write_method("_STA", 0); + { + acpigen_write_store(); + acpigen_emit_namestring("^^QLDN"); + acpigen_write_integer(ldn); + acpigen_emit_byte(LOCAL0_OP); + + /* Multiply (Local0, 0xf, Local0) */ + acpigen_emit_byte(MULTIPLY_OP); + acpigen_emit_byte(LOCAL0_OP); + acpigen_write_integer(0xf); + acpigen_emit_byte(LOCAL0_OP); + + acpigen_emit_byte(RETURN_OP); + acpigen_emit_byte(LOCAL0_OP); + + } + acpigen_pop_len(); /* Method */
if (!dev->enabled) { acpigen_pop_len(); /* Device */ @@ -242,6 +259,13 @@ acpigen_write_name_string("_HID", hid); acpigen_write_name_string("_DDN", name_from_hid(hid));
+ acpigen_write_method("_DIS", 0); + { + acpigen_emit_namestring("^^DLDN"); + acpigen_write_integer(ldn); + } + acpigen_pop_len(); /* Method */ + acpigen_pop_len(); /* Device */ acpigen_pop_len(); /* Scope */ }
Hello Felix Held, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37640
to look at the new patch set (#2).
Change subject: superio/common: Add more ACPI methods ......................................................................
superio/common: Add more ACPI methods
* Make use of introduced SSDT config mode access * Make use of introduced SSDT mutex * Provide ACPI functions to safely access SIO config space * Implement method to query LDN enable state * Implement method to set LDN enable state * Use introduced functions to implement _DIS and _STA in the device
Tested on Aspeed AST2500 and Linux 5.2. Manually verified ACPI code that generates no errors in Linux.
Change-Id: I520b29de925f368cd71ff8f1f58d2d57d72eff8d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/common/generic.c M src/superio/common/ssdt.c 2 files changed, 165 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/37640/2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37640 )
Change subject: superio/common: Add more ACPI methods ......................................................................
Patch Set 2: Code-Review+2
is it intended that there is no functionality implemented to enable a LDN?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37640 )
Change subject: superio/common: Add more ACPI methods ......................................................................
Patch Set 2:
Patch Set 2: Code-Review+2
is it intended that there is no functionality implemented to enable a LDN?
Yes, there's no such ACPI method. There's only _DIS to disable (conflicting) devices.
Hello Felix Held, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37640
to look at the new patch set (#3).
Change subject: superio/common: Add more ACPI methods ......................................................................
superio/common: Add more ACPI methods
* Make use of introduced SSDT config mode access * Make use of introduced SSDT mutex * Provide ACPI functions to safely access SIO config space * Implement method to query LDN enable state * Implement method to set LDN enable state * Use introduced functions to implement _DIS and _STA in the device * Update documentation
Tested on Aspeed AST2500 and Linux 5.2. Manually verified ACPI code that generates no errors in Linux.
Change-Id: I520b29de925f368cd71ff8f1f58d2d57d72eff8d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M Documentation/superio/common/ssdt.md M src/superio/common/generic.c M src/superio/common/ssdt.c 3 files changed, 190 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/37640/3
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37640 )
Change subject: superio/common: Add more ACPI methods ......................................................................
Patch Set 3: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37640 )
Change subject: superio/common: Add more ACPI methods ......................................................................
superio/common: Add more ACPI methods
* Make use of introduced SSDT config mode access * Make use of introduced SSDT mutex * Provide ACPI functions to safely access SIO config space * Implement method to query LDN enable state * Implement method to set LDN enable state * Use introduced functions to implement _DIS and _STA in the device * Update documentation
Tested on Aspeed AST2500 and Linux 5.2. Manually verified ACPI code that generates no errors in Linux.
Change-Id: I520b29de925f368cd71ff8f1f58d2d57d72eff8d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37640 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M Documentation/superio/common/ssdt.md M src/superio/common/generic.c M src/superio/common/ssdt.c 3 files changed, 190 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/Documentation/superio/common/ssdt.md b/Documentation/superio/common/ssdt.md index f2bb334..2f4049e 100644 --- a/Documentation/superio/common/ssdt.md +++ b/Documentation/superio/common/ssdt.md @@ -45,12 +45,33 @@ end ```
+## Automatically generated methods + +The following methods are generated for each SuperIO: +## AMTX() +Acquire the global mutex and enter config mode. +It's called this at the begining of an atomic operation to make sure +no other ACPI code messes with the config space while working on it. + +## RMTX() +Exit config mode and release the global mutex. +It's called at the end of an atomic operation. + +## SLDN(Arg0) +Selects the (virtual) LDN given as Arg0. +This method isn't guarded with the global mutex. + +## DLDN(Arg0) +Disables the (virtual) LDN given as Arg0. +This method aquires the global mutex. + +## QLDN(Arg0) +Queries the state of the (virtual) LDN given as Arg0. +This method quires the global mutex. + ## TODO
1) Add ACPI HIDs to every SuperIO driver -2) Don't guess ACPI HID of LDNs if it's known -3) Add "enter config" and "exit config" bytes -4) Generate support methods that allow +2) Generate support methods that allow * Setting resource settings at runtime * Getting resource settings at runtime - * Disabling LDNs at runtime diff --git a/src/superio/common/generic.c b/src/superio/common/generic.c index 7ac1f83..85b70df 100644 --- a/src/superio/common/generic.c +++ b/src/superio/common/generic.c @@ -156,6 +156,146 @@ acpigen_write_indexfield("INDX", "DATA", i, ARRAY_SIZE(i), FIELD_BYTEACC | FIELD_NOLOCK | FIELD_PRESERVE);
+ const char *mutex = "MTX0"; + + acpigen_write_mutex(mutex, 0); + /* Backup LDN */ + acpigen_write_name_integer("BLDN", 0); + + /* Acquire mutex - Enter config mode */ + acpigen_write_method("AMTX", 0); + { + acpigen_write_acquire(mutex, 0xffff); + + /* Pick one of the children as the generic SIO doesn't have config mode */ + if (dev->link_list && dev->link_list->children) + pnp_ssdt_enter_conf_mode(dev->link_list->children, "^INDX", "^DATA"); + + /* Backup LDN */ + acpigen_write_store(); + acpigen_emit_namestring("^LDN"); + acpigen_emit_namestring("^BLDN"); + } + acpigen_pop_len(); /* Method */ + + /* Release mutex - Exit config mode */ + acpigen_write_method("RMTX", 0); + { + /* Restore LDN */ + acpigen_write_store(); + acpigen_emit_namestring("^BLDN"); + acpigen_emit_namestring("^LDN"); + + /* Pick one of the children as the generic SIO doesn't have config mode */ + if (dev->link_list && dev->link_list->children) + pnp_ssdt_exit_conf_mode(dev->link_list->children, "^INDX", "^DATA"); + + acpigen_write_release(mutex); + } + acpigen_pop_len(); /* Method */ + + /* Select a LDN */ + acpigen_write_method("SLDN", 1); + { + /* Local0 = Arg0 & 0xff */ + acpigen_emit_byte(AND_OP); + acpigen_write_integer(0xff); + acpigen_emit_byte(ARG0_OP); + acpigen_emit_byte(LOCAL0_OP); + + /* LDN = LOCAL0_OP */ + acpigen_write_store(); + acpigen_emit_byte(LOCAL0_OP); + acpigen_emit_namestring("^LDN"); + } + acpigen_pop_len(); /* Method */ + + /* Disable a LDN/VLDN */ + acpigen_write_method("DLDN", 1); + { + /* AMTX() */ + acpigen_emit_namestring("AMTX"); + + /* SLDN (Local0) */ + acpigen_emit_namestring("SLDN"); + acpigen_emit_byte(ARG0_OP); + + /* Local0 = Arg0 >> 8 */ + acpigen_emit_byte(SHIFT_RIGHT_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_integer(8); + acpigen_emit_byte(LOCAL0_OP); + + /* Local0 = Local0 & 0x7 */ + acpigen_emit_byte(AND_OP); + acpigen_write_integer(0x7); + acpigen_emit_byte(LOCAL0_OP); + acpigen_emit_byte(LOCAL0_OP); + + for (int j = 0; j < 8; j++) { + char act[6] = "^ACT0"; + act[4] += j; + + /* If (Local0 == j) { */ + acpigen_write_if_lequal_op_int(LOCAL0_OP, j); + + /* ACT[j] = 0 */ + acpigen_write_store(); + acpigen_emit_byte(ZERO_OP); + acpigen_emit_namestring(act); + + acpigen_pop_len(); /* } */ + } + + /* RMTX() */ + acpigen_emit_namestring("RMTX"); + } + acpigen_pop_len(); /* Method */ + + /* Query LDN enable state. Returns 1 if LDN/VLDN is enabled. */ + acpigen_write_method("QLDN", 1); + { + acpigen_emit_namestring("AMTX"); + + /* SLDN (Local0) */ + acpigen_emit_namestring("SLDN"); + acpigen_emit_byte(ARG0_OP); + + /* Local0 = Arg0 >> 8 */ + acpigen_emit_byte(SHIFT_RIGHT_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_integer(8); + acpigen_emit_byte(LOCAL0_OP); + + /* Local0 = Local0 & 0x7 */ + acpigen_emit_byte(AND_OP); + acpigen_write_integer(0x7); + acpigen_emit_byte(LOCAL0_OP); + acpigen_emit_byte(LOCAL0_OP); + + for (int j = 0; j < 8; j++) { + char act[6] = "^ACT0"; + act[4] += j; + /* If (Local0 == j) { */ + acpigen_write_if_lequal_op_int(LOCAL0_OP, j); + + /* Local1 = ACT[j] */ + acpigen_write_store(); + acpigen_emit_namestring(act); + acpigen_emit_byte(LOCAL1_OP); + + acpigen_pop_len(); /* } */ + } + + /* RMTX() */ + acpigen_emit_namestring("RMTX"); + + /* Return (Local1) */ + acpigen_emit_byte(RETURN_OP); + acpigen_emit_byte(LOCAL1_OP); + } + acpigen_pop_len(); /* Method */ + acpigen_pop_len(); /* Device */ acpigen_pop_len(); /* Scope */ } diff --git a/src/superio/common/ssdt.c b/src/superio/common/ssdt.c index a919aa5..bc5d394 100644 --- a/src/superio/common/ssdt.c +++ b/src/superio/common/ssdt.c @@ -200,7 +200,24 @@ acpigen_write_name_byte("LDN", ldn); acpigen_write_name_byte("VLDN", vldn);
- acpigen_write_STA(dev->enabled ? 0xf : 0); + acpigen_write_method("_STA", 0); + { + acpigen_write_store(); + acpigen_emit_namestring("^^QLDN"); + acpigen_write_integer(ldn); + acpigen_emit_byte(LOCAL0_OP); + + /* Multiply (Local0, 0xf, Local0) */ + acpigen_emit_byte(MULTIPLY_OP); + acpigen_emit_byte(LOCAL0_OP); + acpigen_write_integer(0xf); + acpigen_emit_byte(LOCAL0_OP); + + acpigen_emit_byte(RETURN_OP); + acpigen_emit_byte(LOCAL0_OP); + + } + acpigen_pop_len(); /* Method */
if (!dev->enabled) { acpigen_pop_len(); /* Device */ @@ -242,6 +259,13 @@ acpigen_write_name_string("_HID", hid); acpigen_write_name_string("_DDN", name_from_hid(hid));
+ acpigen_write_method("_DIS", 0); + { + acpigen_emit_namestring("^^DLDN"); + acpigen_write_integer(ldn); + } + acpigen_pop_len(); /* Method */ + acpigen_pop_len(); /* Device */ acpigen_pop_len(); /* Scope */ }
-
Felix Held (Code Review) -
Patrick Rudolph (Code Review)