Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31208
Change subject: ec/google/wilco: Add virtual button support ......................................................................
ec/google/wilco: Add virtual button support
Add an ACPI device that is compatible with the the Intel Virtual Button kernel driver for reporting tablet mode state and various virtual button events that may come from the EC.
This driver is used in Windows and in the Linux kernel at drivers/platform/x86/intel-vbtn.c
Because of a check in the kernel driver it expects the board to define the SMBIOS enclosure type as convertible for the check at driver load time for tablet/laptop and dock/undock to work.
The virtual tablet mode button will proxy the tablet mode state sent from the Sensor Hub to a SW_TABLET_MODE event in the kernel.
The virtual power buton is used during S0ix for the EC to wake the system with an SCI. There are separate press and release events which are sent for completeness, although the kernel driver will ignore the release event.
BUG=b:73137291 TEST=Test that the power button can wake the system from S0ix. Also verify that the device is reported as laptop mode at boot.
Change-Id: I0d5dc985a3cfb1d01ff164c4e67f17e6b1cdd619 Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/ec/google/wilco/acpi/ec.asl M src/ec/google/wilco/acpi/ec_ram.asl M src/ec/google/wilco/acpi/event.asl A src/ec/google/wilco/acpi/vbtn.asl 4 files changed, 116 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/31208/1
diff --git a/src/ec/google/wilco/acpi/ec.asl b/src/ec/google/wilco/acpi/ec.asl index be80b2d..5aca187 100644 --- a/src/ec/google/wilco/acpi/ec.asl +++ b/src/ec/google/wilco/acpi/ec.asl @@ -164,6 +164,7 @@ #include "event.asl" #include "lid.asl" #include "platform.asl" + #include "vbtn.asl" #ifdef EC_ENABLE_DPTF #include "dptf.asl" #endif diff --git a/src/ec/google/wilco/acpi/ec_ram.asl b/src/ec/google/wilco/acpi/ec_ram.asl index 1e5d7cb..1c030af 100644 --- a/src/ec/google/wilco/acpi/ec_ram.asl +++ b/src/ec/google/wilco/acpi/ec_ram.asl @@ -115,6 +115,7 @@ Name (DRTQ, Package () { 0x38, 0xff, RD }) /* DPTF: Read Trip Query */
Name (ORST, Package () { 0x39, 0xff, RD }) /* Orientation State */ +Name (OTBL, Package () { 0x39, 0x02, RD }) /* Orientation: Tablet mode */ Name (OREV, Package () { 0x3a, 0xff, RD }) /* Orientation Events */ Name (OECH, Package () { 0x3a, 0x01, RD }) /* Event: Orientation */ Name (OERL, Package () { 0x3a, 0x02, RD }) /* Event: Rotation Lock */ diff --git a/src/ec/google/wilco/acpi/event.asl b/src/ec/google/wilco/acpi/event.asl index 754ed26..a10a388 100644 --- a/src/ec/google/wilco/acpi/event.asl +++ b/src/ec/google/wilco/acpi/event.asl @@ -81,6 +81,16 @@ Printf ("QS EVENT") Notify (^WLCO, 0x90) } + + If (EBIT (E2OR, Arg0)) { + If (R (OTBL)) { + Printf ("EC event indicates tablet mode") + Notify (^VBTN, ^VTBL) + } Else { + Printf ("EC event indicates laptop mode") + NotifY (^VBTN, ^VLAP) + } + } }
/* Handle events in PmEv3 */ @@ -88,6 +98,16 @@ { Printf ("EVT3: %o", Arg0)
+ If (EBIT (E3CP, Arg0)) { + If (R (P2PB)) { + Printf ("Power button pressed") + Notify (^VBTN, ^VPPB) + } Else { + Printf ("Power button released") + Notify (^VBTN, ^VRPB) + } + } + #ifdef EC_ENABLE_DPTF /* Theraml Events */ If (EBIT (E3TH, Arg0)) { diff --git a/src/ec/google/wilco/acpi/vbtn.asl b/src/ec/google/wilco/acpi/vbtn.asl new file mode 100644 index 0000000..201ab51 --- /dev/null +++ b/src/ec/google/wilco/acpi/vbtn.asl @@ -0,0 +1,94 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * 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; version 2 of + * the License. + * + * 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. + */ + +/* + * Intel Virtual Button driver compatible with the driver found in + * the Linux kernel at drivers/platform/x86/intel-vbtn.c + * + * For tablet/laptop and dock/undock events to work the board must + * select SYSTEM_TYPE_CONVERTIBLE for the SMBIOS enclosure type to + * indicate the device is a convertible. + */ + +Name (FLAP, 0x40) /* Flag indicating device is in laptop mode */ + +/* Virtual events */ +Name (VPPB, 0xc0) /* Power Button press */ +Name (VRPB, 0xc1) /* Power Button release */ +Name (VPSP, 0xc2) /* Special key press (LEFTMETA in Linux) */ +Name (VRSP, 0xc3) /* Special key release (LEFTMETA in Linux) */ +Name (VPVU, 0xc4) /* Volume Up press */ +Name (VRVU, 0xc5) /* Volume Up release */ +Name (VPVD, 0xc6) /* Volume Down press */ +Name (VRVD, 0xc7) /* Volume Down release */ +Name (VPRL, 0xc8) /* Rotate Lock press */ +Name (VRRL, 0xc9) /* Rotate Lock release */ +Name (VDOC, 0xca) /* Docked */ +Name (VUND, 0xcb) /* Undocked */ +Name (VTBL, 0xcc) /* Tablet Mode */ +Name (VLAP, 0xcd) /* Laptop Mode */ + +Device (VBTN) +{ + Name (_HID, "INT33D6") + Name (_UID, One) + Name (_DDN, "Intel Virtual Button Driver") + + /* + * This method is called at driver probe time and must exist or + * the driver will not load. + */ + Method (VBDL) + { + } + + /* + * This method returns flags indicating tablet and dock modes. + * It is called at driver probe time so the OS knows what the + * state of the device is at boot. + */ + Method (VGBS) + { + Local0 = Zero + + /* Check EC orientation for tablet mode flag */ + If (R (OTBL)) { + Printf ("EC reports tablet mode at boot") + } Else { + Printf ("EC reports laptop mode at boot") + Local0 |= ^^FLAP + } + Return (Local0) + } + + Method(_STA, 0) + { + Return (0xF) + } +} + +Device (VBTO) +{ + Name (_HID, "INT33D3") + Name (_CID, "PNP0C60") + Name (_UID, One) + Name (_DDN, "Laptop/tablet mode indicator driver") + + Method (_STA, 0) + { + Return (0xF) + } +}
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 1:
Jett in theory this enables the tablet mode event once the ISH asserts NB_MODE.
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 4: Code-Review+1
I primarily looked at vbtn.asl (the other files seem sane I don't have a spec to reference).
I'm pretty sure that the Windows driver doesn't bother to check the chassis type (nor do anything else besides the required core functionality), so the Linux driver probably could've used a different workaround (or avoided one entirely). Regardless, there's not much we can do about unless/until someone can get their hands on the system that caused the kernel workaround to be introduced.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl File src/ec/google/wilco/acpi/event.asl:
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl@9... PS4, Line 91: Y lowercase y?
Alex Thiessen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 4:
(3 comments)
Ran languagetool: https://languagetool.org
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG@9 PS4, Line 9: the the ENGLISH_WORD_REPEAT_RULE Message: Possible typo: you repeated a word Suggestion: the
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG@21 PS4, Line 21: a EN_A_VS_AN Message: Use 'an' instead of 'a' if the following word starts with a vowel sound, e.g. 'an article', 'an hour' Suggestion: an
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG@23 PS4, Line 23: buton MORFOLOGIK_RULE_EN_US Message: Possible spelling mistake found Suggestion: button
Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl File src/ec/google/wilco/acpi/event.asl:
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl@8... PS4, Line 85: If (EBIT (E2OR, Arg0)) { Does this work with the OEM's tristate enum for orientations that they want to use? Or will this potential change based on the final outcome of that discussion?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl File src/ec/google/wilco/acpi/event.asl:
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl@8... PS4, Line 85: If (EBIT (E2OR, Arg0)) {
Does this work with the OEM's tristate enum for orientations that they want to use? Or will this pot […]
No I ignored the other bit as it wouldn't fit in to how Chrome handles these modes. If needed we can add the rotate lock virtual button later, but there will need to be work at a lot of other layers first for it to do anything useful.
https://review.coreboot.org/#/c/31208/4/src/ec/google/wilco/acpi/event.asl@9... PS4, Line 91: Y
lowercase y?
Done
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG@9 PS4, Line 9: the the
ENGLISH_WORD_REPEAT_RULE […]
Done
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG@21 PS4, Line 21: a
EN_A_VS_AN […]
This is not correct.
https://review.coreboot.org/#/c/31208/4//COMMIT_MSG@23 PS4, Line 23: buton
MORFOLOGIK_RULE_EN_US […]
Done
Hello Jett Rink, Matt Delco, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31208
to look at the new patch set (#5).
Change subject: ec/google/wilco: Add virtual button support ......................................................................
ec/google/wilco: Add virtual button support
Add an ACPI device that is compatible with the Intel Virtual Button kernel driver for reporting tablet mode state and various virtual button events that may come from the EC.
This driver is used in Windows and in the Linux kernel at drivers/platform/x86/intel-vbtn.c
Because of a check in the kernel driver it expects the board to define the SMBIOS enclosure type as convertible for the check at driver load time for tablet/laptop and dock/undock to work.
The virtual tablet mode button will proxy the tablet mode state sent from the Sensor Hub to a SW_TABLET_MODE event in the kernel.
The virtual power button is used during S0ix for the EC to wake the system with an SCI. There are separate press and release events which are sent for completeness, although the kernel driver will ignore the release event.
BUG=b:73137291 TEST=Test that the power button can wake the system from S0ix. Also verify that the device is reported as laptop mode at boot.
Change-Id: I0d5dc985a3cfb1d01ff164c4e67f17e6b1cdd619 Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/ec/google/wilco/acpi/ec.asl M src/ec/google/wilco/acpi/ec_ram.asl M src/ec/google/wilco/acpi/event.asl A src/ec/google/wilco/acpi/vbtn.asl 4 files changed, 116 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/31208/5
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
Patch Set 5: Code-Review+2
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31208 )
Change subject: ec/google/wilco: Add virtual button support ......................................................................
ec/google/wilco: Add virtual button support
Add an ACPI device that is compatible with the Intel Virtual Button kernel driver for reporting tablet mode state and various virtual button events that may come from the EC.
This driver is used in Windows and in the Linux kernel at drivers/platform/x86/intel-vbtn.c
Because of a check in the kernel driver it expects the board to define the SMBIOS enclosure type as convertible for the check at driver load time for tablet/laptop and dock/undock to work.
The virtual tablet mode button will proxy the tablet mode state sent from the Sensor Hub to a SW_TABLET_MODE event in the kernel.
The virtual power button is used during S0ix for the EC to wake the system with an SCI. There are separate press and release events which are sent for completeness, although the kernel driver will ignore the release event.
BUG=b:73137291 TEST=Test that the power button can wake the system from S0ix. Also verify that the device is reported as laptop mode at boot.
Change-Id: I0d5dc985a3cfb1d01ff164c4e67f17e6b1cdd619 Signed-off-by: Duncan Laurie dlaurie@google.com Reviewed-on: https://review.coreboot.org/c/31208 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/ec/google/wilco/acpi/ec.asl M src/ec/google/wilco/acpi/ec_ram.asl M src/ec/google/wilco/acpi/event.asl A src/ec/google/wilco/acpi/vbtn.asl 4 files changed, 116 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/ec/google/wilco/acpi/ec.asl b/src/ec/google/wilco/acpi/ec.asl index be80b2d..5aca187 100644 --- a/src/ec/google/wilco/acpi/ec.asl +++ b/src/ec/google/wilco/acpi/ec.asl @@ -164,6 +164,7 @@ #include "event.asl" #include "lid.asl" #include "platform.asl" + #include "vbtn.asl" #ifdef EC_ENABLE_DPTF #include "dptf.asl" #endif diff --git a/src/ec/google/wilco/acpi/ec_ram.asl b/src/ec/google/wilco/acpi/ec_ram.asl index 1e5d7cb..1c030af 100644 --- a/src/ec/google/wilco/acpi/ec_ram.asl +++ b/src/ec/google/wilco/acpi/ec_ram.asl @@ -115,6 +115,7 @@ Name (DRTQ, Package () { 0x38, 0xff, RD }) /* DPTF: Read Trip Query */
Name (ORST, Package () { 0x39, 0xff, RD }) /* Orientation State */ +Name (OTBL, Package () { 0x39, 0x02, RD }) /* Orientation: Tablet mode */ Name (OREV, Package () { 0x3a, 0xff, RD }) /* Orientation Events */ Name (OECH, Package () { 0x3a, 0x01, RD }) /* Event: Orientation */ Name (OERL, Package () { 0x3a, 0x02, RD }) /* Event: Rotation Lock */ diff --git a/src/ec/google/wilco/acpi/event.asl b/src/ec/google/wilco/acpi/event.asl index d818d22..24cf268 100644 --- a/src/ec/google/wilco/acpi/event.asl +++ b/src/ec/google/wilco/acpi/event.asl @@ -81,6 +81,16 @@ Printf ("QS EVENT") Notify (^WEVT, 0x90) } + + If (EBIT (E2OR, Arg0)) { + If (R (OTBL)) { + Printf ("EC event indicates tablet mode") + Notify (^VBTN, ^VTBL) + } Else { + Printf ("EC event indicates laptop mode") + Notify (^VBTN, ^VLAP) + } + } }
/* Handle events in PmEv3 */ @@ -88,6 +98,16 @@ { Printf ("EVT3: %o", Arg0)
+ If (EBIT (E3CP, Arg0)) { + If (R (P2PB)) { + Printf ("Power button pressed") + Notify (^VBTN, ^VPPB) + } Else { + Printf ("Power button released") + Notify (^VBTN, ^VRPB) + } + } + #ifdef EC_ENABLE_DPTF /* Theraml Events */ If (EBIT (E3TH, Arg0)) { diff --git a/src/ec/google/wilco/acpi/vbtn.asl b/src/ec/google/wilco/acpi/vbtn.asl new file mode 100644 index 0000000..201ab51 --- /dev/null +++ b/src/ec/google/wilco/acpi/vbtn.asl @@ -0,0 +1,94 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * 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; version 2 of + * the License. + * + * 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. + */ + +/* + * Intel Virtual Button driver compatible with the driver found in + * the Linux kernel at drivers/platform/x86/intel-vbtn.c + * + * For tablet/laptop and dock/undock events to work the board must + * select SYSTEM_TYPE_CONVERTIBLE for the SMBIOS enclosure type to + * indicate the device is a convertible. + */ + +Name (FLAP, 0x40) /* Flag indicating device is in laptop mode */ + +/* Virtual events */ +Name (VPPB, 0xc0) /* Power Button press */ +Name (VRPB, 0xc1) /* Power Button release */ +Name (VPSP, 0xc2) /* Special key press (LEFTMETA in Linux) */ +Name (VRSP, 0xc3) /* Special key release (LEFTMETA in Linux) */ +Name (VPVU, 0xc4) /* Volume Up press */ +Name (VRVU, 0xc5) /* Volume Up release */ +Name (VPVD, 0xc6) /* Volume Down press */ +Name (VRVD, 0xc7) /* Volume Down release */ +Name (VPRL, 0xc8) /* Rotate Lock press */ +Name (VRRL, 0xc9) /* Rotate Lock release */ +Name (VDOC, 0xca) /* Docked */ +Name (VUND, 0xcb) /* Undocked */ +Name (VTBL, 0xcc) /* Tablet Mode */ +Name (VLAP, 0xcd) /* Laptop Mode */ + +Device (VBTN) +{ + Name (_HID, "INT33D6") + Name (_UID, One) + Name (_DDN, "Intel Virtual Button Driver") + + /* + * This method is called at driver probe time and must exist or + * the driver will not load. + */ + Method (VBDL) + { + } + + /* + * This method returns flags indicating tablet and dock modes. + * It is called at driver probe time so the OS knows what the + * state of the device is at boot. + */ + Method (VGBS) + { + Local0 = Zero + + /* Check EC orientation for tablet mode flag */ + If (R (OTBL)) { + Printf ("EC reports tablet mode at boot") + } Else { + Printf ("EC reports laptop mode at boot") + Local0 |= ^^FLAP + } + Return (Local0) + } + + Method(_STA, 0) + { + Return (0xF) + } +} + +Device (VBTO) +{ + Name (_HID, "INT33D3") + Name (_CID, "PNP0C60") + Name (_UID, One) + Name (_DDN, "Laptop/tablet mode indicator driver") + + Method (_STA, 0) + { + Return (0xF) + } +}
-
Alex Thiessen (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Jett Rink (Code Review)
-
Matt Delco (Code Review)