Re: [PATCH v2 05/11] platform/x86: asus-wmi: Support queued WMI event codes
On 12.04.19 09:48, Daniel Drake wrote: > On Thu, Apr 11, 2019 at 1:44 PM Yurii Pavlovskyi > wrote: >> Event codes are expected to be polled from a queue on at least some >> models. > > Maybe avoid the word "poll" since it suggests something else (at least to me). Ok, will change this in the code as well. >>> The fix flushes the old key codes out of the queue on load and after >> receiving event the queue is read until either .. or 1 is encountered. >> >> It might be considered a minor issue and no normal user would likely to >> observe this (there is little reason unloading the driver), but it does >> significantly frustrate a developer who is unlucky enough to encounter >> this. >> >> Introduce functionality for flushing and processing queued codes, which is >> enabled via quirk flag for ASUS7000. It might be considered if it is >> reasonable to enable it everywhere (might introduce regressions) or always >> try to flush the queue on module load and try to detect if this quirk is >> present in the future. >> >> This patch limits the effect to the specific hardware defined by ASUS7000 >> device that is used for driver detection by vendor driver of Fx505. The >> fallback is also implemented in case initial flush fails. > > Which vendor driver are you referring to here? The ASUS Keyboard hotkeys Driver V2.0.5 which is available to download for FX505 has this in his INF file: [ATKP.ntamd64] %DeviceDesc1% = NO_DRV64, ACPI\ASUS7000 But this driver is not generic. It is limited to very specific hardware, I'd guess gaming series (for instance K54C does not have this device). It was rather a way to avoid breaking anything. I think your suggestion below is much better. > > Researching more: > In our database of 144 Asus models, 142 of them have GANQ. > > Of those 142, 9 of them return One in the empty-queue case. The other > 133 match your FX505GM device exactly. So it seems valid to interpret > both 0x and 0x1 as queue-end markers. > > The 2 devices that do not have GANQ are not laptops. They also do not > have the _UID "ATK" WMI device, they only have "ASUSWMI" and their WMI > _WED method does not use a queue. > There are a few more units that have both ASUSWMI and ATK devices, and > the ASUSWMI device appears to never be queued. > Another observation is that the ASUSWMI device works with notify code > 0xD2, and the ATK device works with 0xFF. > > Nailing this down a bit further, I found a DSDT for EEEPC X101H: that > one only has ASUSWMI and it is also not queued. > > So I think you should make this queue behaviour applied more > generically, but either avoid it when the WMI device _UID is not "ATK" > (as discussed in the DCTS/DSTS thread), or avoid it when the notify > code is not 0x> > Thanks > Daniel Thanks a lot for your research, it's much appreciated! That seems to confirm that these two quirks are actually connected with ATK device. I guess it makes sense to combine the detection for both of them. Also to flush the queue we need to know the notify code beforehand, because it is checked in _WED so checking for ATK seems reasonable to me. Best regards, Yurii
Re: [PATCH v2 05/11] platform/x86: asus-wmi: Support queued WMI event codes
On Thu, Apr 11, 2019 at 1:44 PM Yurii Pavlovskyi wrote: > Event codes are expected to be polled from a queue on at least some > models. Maybe avoid the word "poll" since it suggests something else (at least to me). > The fix flushes the old key codes out of the queue on load and after > receiving event the queue is read until either .. or 1 is encountered. > > It might be considered a minor issue and no normal user would likely to > observe this (there is little reason unloading the driver), but it does > significantly frustrate a developer who is unlucky enough to encounter > this. > > Introduce functionality for flushing and processing queued codes, which is > enabled via quirk flag for ASUS7000. It might be considered if it is > reasonable to enable it everywhere (might introduce regressions) or always > try to flush the queue on module load and try to detect if this quirk is > present in the future. > > This patch limits the effect to the specific hardware defined by ASUS7000 > device that is used for driver detection by vendor driver of Fx505. The > fallback is also implemented in case initial flush fails. Which vendor driver are you referring to here? Researching more: In our database of 144 Asus models, 142 of them have GANQ. Of those 142, 9 of them return One in the empty-queue case. The other 133 match your FX505GM device exactly. So it seems valid to interpret both 0x and 0x1 as queue-end markers. The 2 devices that do not have GANQ are not laptops. They also do not have the _UID "ATK" WMI device, they only have "ASUSWMI" and their WMI _WED method does not use a queue. There are a few more units that have both ASUSWMI and ATK devices, and the ASUSWMI device appears to never be queued. Another observation is that the ASUSWMI device works with notify code 0xD2, and the ATK device works with 0xFF. Nailing this down a bit further, I found a DSDT for EEEPC X101H: that one only has ASUSWMI and it is also not queued. So I think you should make this queue behaviour applied more generically, but either avoid it when the WMI device _UID is not "ATK" (as discussed in the DCTS/DSTS thread), or avoid it when the notify code is not 0xFF. Thanks Daniel
[PATCH v2 05/11] platform/x86: asus-wmi: Support queued WMI event codes
Event codes are expected to be polled from a queue on at least some
models.
The WMI event codes are pushed into queue based on circular buffer. After
INIT method is called ACPI code is allowed to push events into this buffer
the INIT method can not be reverted. If the module is unloaded and an
event (such as hotkey press) gets emitted before inserting it back the
events get processed delayed by one or, if the queue overflows,
additionally delayed by about 3 seconds.
Patch was tested on a newer TUF Gaming FX505GM and older K54C model.
FX505GM
Device (ATKD)
{ ..
Name (ATKQ, Package (0x10)
{
0x, ..
}
Method (IANQ, 1, Serialized)
{
If ((AQNO >= 0x10))
{
Local0 = 0x64
While ((Local0 && (AQNO >= 0x10)))
{
Local0--
Sleep (0x0A)
}
...
..
AQTI++
AQTI &= 0x0F
ATKQ [AQTI] = Arg0
...
}
Method (GANQ, 0, Serialized)
{
..
If (AQNO)
{
...
Local0 = DerefOf (ATKQ [AQHI])
AQHI++
AQHI &= 0x0F
Return (Local0)
}
Return (One)
}
This code is almost identical to K54C, which does return Ones on empty
queue.
K54C:
Method (GANQ, 0, Serialized)
{
If (AQNO)
{
...
Return (Local0)
}
Return (Ones)
}
The fix flushes the old key codes out of the queue on load and after
receiving event the queue is read until either .. or 1 is encountered.
It might be considered a minor issue and no normal user would likely to
observe this (there is little reason unloading the driver), but it does
significantly frustrate a developer who is unlucky enough to encounter
this.
Introduce functionality for flushing and processing queued codes, which is
enabled via quirk flag for ASUS7000. It might be considered if it is
reasonable to enable it everywhere (might introduce regressions) or always
try to flush the queue on module load and try to detect if this quirk is
present in the future.
This patch limits the effect to the specific hardware defined by ASUS7000
device that is used for driver detection by vendor driver of Fx505. The
fallback is also implemented in case initial flush fails.
Signed-off-by: Yurii Pavlovskyi
---
drivers/platform/x86/asus-nb-wmi.c | 1 +
drivers/platform/x86/asus-wmi.c| 122 ++---
drivers/platform/x86/asus-wmi.h| 2 +
3 files changed, 97 insertions(+), 28 deletions(-)
diff --git a/drivers/platform/x86/asus-nb-wmi.c
b/drivers/platform/x86/asus-nb-wmi.c
index cc5f0765a8d9..357d273ed336 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -438,6 +438,7 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver
*driver)
if (acpi_dev_found("ASUS7000")) {
driver->quirks->force_dsts = true;
+ driver->quirks->wmi_event_queue = true;
}
}
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 80f3447734fc..5aa30f8a2a38 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -80,6 +80,12 @@ MODULE_LICENSE("GPL");
#define USB_INTEL_XUSB2PR 0xD0
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
+#define WMI_EVENT_QUEUE_SIZE 0x10
+#define WMI_EVENT_QUEUE_END0x1
+#define WMI_EVENT_MASK 0x
+/* The event value is always the same. */
+#define WMI_EVENT_VALUE0xFF
+
static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
static bool ashs_present(void)
@@ -143,6 +149,7 @@ struct asus_wmi {
int dsts_id;
int spec;
int sfun;
+ bool wmi_event_queue;
struct input_dev *inputdev;
struct backlight_device *backlight_device;
@@ -1637,77 +1644,126 @@ static int is_display_toggle(int code)
return 0;
}
-static void asus_wmi_notify(u32 value, void *context)
+static int asus_poll_wmi_event(u32 value)
{
- struct asus_wmi *asus = context;
- struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+ struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *obj;
acpi_status status;
- int code;
- int orig_code;
- unsigned int key_value = 1;
- bool autorelease = 1;
+ int code = -EIO;
- status = wmi_get_event_data(value, );
- if (status != AE_OK) {
- pr_err("bad event status 0x%x\n", status);
- return;
+ status = wmi_get_event_data(value, );
+ if (ACPI_FAILURE(status)) {
+ pr_warn("Failed to get WMI event code: %s\n",
+ acpi_format_exception(status));
+ return code;
}
- obj = (union acpi_object *)response.pointer;
+ obj = (union acpi_object *)output.pointer;
-
