| Message ID | 1f6ac54784b39ebb6ce02a9fb9e944c840fddb7b.1448547341.git.kernel@kempniu.pl (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thursday 26 November 2015 15:18:32 Micha? K?pie? wrote: > On some laptop models (e.g. Dell Vostro V131), pressing the Dell Instant > Launch hotkey does not raise an i8042 interrupt - only WMI event 0xe025 > is generated. As there is no flawless way to determine whether a given > machine is capable of simulating a keypress when this hotkey is pressed, > a new module parameter is added so that the user can decide whether the > WMI event should be processed or ignored. > > Signed-off-by: Micha? K?pie? <kernel@kempniu.pl> > --- > As my last message [1] in the rather lengthy thread failed to elicit any > response, I guess I might just as well post the proposed patch so that > we have something specific to discuss. > > [1] http://www.spinics.net/lists/platform-driver-x86/msg07679.html > Can you wait just a little bit? Till end of month two Dell kernel devs are on vacation, so after that they maybe answer to question about new hotkey format/support in kernel. > drivers/platform/x86/dell-wmi.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c > index 8cb0f57..e68ce3b 100644 > --- a/drivers/platform/x86/dell-wmi.c > +++ b/drivers/platform/x86/dell-wmi.c > @@ -44,6 +44,10 @@ MODULE_LICENSE("GPL"); > #define DELL_EVENT_GUID "9DBB5994-A997-11DA-B012-B622A1EF5492" > > static int acpi_video; > +static bool process_dil; > + > +module_param(process_dil, bool, 0644); > +MODULE_PARM_DESC(process_dil, "Generate an input event when the WMI event for Dell Instant Launch hotkey is received"); I do not like name "dil". It takes me few minutes to interpret it as Dell Instant Launch... Also I do not know if this is the best approach. > /* Shortcut and audio panel keys */ > - { KE_IGNORE, 0xe025, { KEY_RESERVED } }, > + { KE_KEY, 0xe025, { KEY_PROG4 } }, > { KE_IGNORE, 0xe026, { KEY_RESERVED } }, I'm trying to figure out if those two keys are really reported via keyboard controller or not. They were added 4 years ago in commit f1566f0dc07ec9b5409b348070f5a700032d7881. But from bug report http://bugs.launchpad.net/bugs/815914 there is no information if those two keys are really reported by keyboard controller or not. And if not our problem could be easier...
Hi Pali, > Can you wait just a little bit? Till end of month two Dell kernel devs > are on vacation, so after that they maybe answer to question about new > hotkey format/support in kernel. Absolutely, after all this issue is already several months old (or even years, depending on how you interpret it). I just didn't want it to fade into the void. > > diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c > > index 8cb0f57..e68ce3b 100644 > > --- a/drivers/platform/x86/dell-wmi.c > > +++ b/drivers/platform/x86/dell-wmi.c > > @@ -44,6 +44,10 @@ MODULE_LICENSE("GPL"); > > #define DELL_EVENT_GUID "9DBB5994-A997-11DA-B012-B622A1EF5492" > > > > static int acpi_video; > > +static bool process_dil; > > + > > +module_param(process_dil, bool, 0644); > > +MODULE_PARM_DESC(process_dil, "Generate an input event when the WMI event for Dell Instant Launch hotkey is received"); > > I do not like name "dil". It takes me few minutes to interpret it as > Dell Instant Launch... Yes, it's ugly. I really wanted to avoid process_dell_instant_launch. Perhaps I shouldn't have? > Also I do not know if this is the best approach. Same here, that's why I submitted this patch for discussion. > > /* Shortcut and audio panel keys */ > > - { KE_IGNORE, 0xe025, { KEY_RESERVED } }, > > + { KE_KEY, 0xe025, { KEY_PROG4 } }, > > { KE_IGNORE, 0xe026, { KEY_RESERVED } }, > > I'm trying to figure out if those two keys are really reported via > keyboard controller or not. They were added 4 years ago in commit > f1566f0dc07ec9b5409b348070f5a700032d7881. But from bug report > http://bugs.launchpad.net/bugs/815914 there is no information if those > two keys are really reported by keyboard controller or not. > > And if not our problem could be easier... That would indeed be sweet as this patch could then be shrinked to just changing the entry in the sparse keymap. Does anyone have a Dell XPS L502X handy? Also, any ideas for making sure no other model is generating that keypress?
On Thursday 26 November 2015 15:55:56 Micha? K?pie? wrote: > > > /* Shortcut and audio panel keys */ > > > > > > - { KE_IGNORE, 0xe025, { KEY_RESERVED } }, > > > + { KE_KEY, 0xe025, { KEY_PROG4 } }, > > > > > > { KE_IGNORE, 0xe026, { KEY_RESERVED } }, > > > > I'm trying to figure out if those two keys are really reported via > > keyboard controller or not. They were added 4 years ago in commit > > f1566f0dc07ec9b5409b348070f5a700032d7881. But from bug report > > http://bugs.launchpad.net/bugs/815914 there is no information if > > those two keys are really reported by keyboard controller or not. > > > > And if not our problem could be easier... > > That would indeed be sweet as this patch could then be shrinked to > just changing the entry in the sparse keymap. Does anyone have a > Dell XPS L502X handy? Also, any ideas for making sure no other > model is generating that keypress? And now I have info how keys are reported on Dell XPS L502X. Sadly it is worse as I expected :-( Here is output from Jean-Louis Dupond notebook: $ sudo /usr/bin/input-events 4 /dev/input/event4 bustype : BUS_I8042 vendor : 0x1 product : 0x1 version : 43841 name : "AT Translated Set 2 keyboard" phys : "isa0060/serio0/input0" bits ev : EV_SYN EV_KEY EV_MSC EV_LED EV_REP waiting for events 10:26:29.945739: EV_MSC MSC_SCAN 219 10:26:29.945739: EV_KEY KEY_LEFTMETA (0x7d) pressed 10:26:29.945739: EV_SYN code=0 value=0 10:26:29.946468: EV_MSC MSC_SCAN 45 10:26:29.946468: EV_KEY KEY_X (0x2d) pressed 10:26:29.946468: EV_SYN code=0 value=0 10:26:29.948469: EV_MSC MSC_SCAN 45 10:26:29.948469: EV_KEY KEY_X (0x2d) released 10:26:29.948469: EV_SYN code=0 value=0 10:26:29.951473: EV_MSC MSC_SCAN 219 10:26:29.951473: EV_KEY KEY_LEFTMETA (0x7d) released 10:26:29.951473: EV_SYN code=0 value=0 x (Press+release first key with name "Windows Mobility Center control") (key X was printed to console) 10:26:32.898689: EV_MSC MSC_SCAN 133 10:26:32.898689: EV_KEY KEY_BRIGHTNESSDOWN (0xe0) pressed 10:26:32.898689: EV_SYN code=0 value=0 10:26:32.898730: EV_MSC MSC_SCAN 133 10:26:32.898730: EV_KEY KEY_BRIGHTNESSDOWN (0xe0) released 10:26:32.898730: EV_SYN code=0 value=0 (Press+release second key with name "Instant launch control") 10:26:35.090018: EV_MSC MSC_SCAN 132 10:26:35.090018: EV_KEY KEY_NEXTSONG (0xa3) pressed 10:26:35.090018: EV_SYN code=0 value=0 10:26:35.092765: EV_MSC MSC_SCAN 132 10:26:35.092765: EV_KEY KEY_NEXTSONG (0xa3) released 10:26:35.092765: EV_SYN code=0 value=0 (Press+release third key with name "Audio control-panel control") As you can see events are send also via keyboard controller! Key codes are configured by userspace (udev/systemd) and looks like there is bug in userspace rules (reason for brightnes or nextsong), see: https://wiki.ubuntu.com/HardwareSupport/Machines/Laptops/Dell/XPS/15 So it is not easy to make both machines (Dell XPS L502X and Dell Vostro V131) works correctly :-( At least I do not see how. And that mapping "Windows Mobility Center control" key to combination of two keys (KEY_LEFTMETA + X) is some total stupid nonsense... If anybody has idea how to fix this big firmware/BIOS mess please let us know...
Hi Pali, Thanks again for your efforts. > $ sudo /usr/bin/input-events 4 > /dev/input/event4 > bustype : BUS_I8042 > vendor : 0x1 > product : 0x1 > version : 43841 > name : "AT Translated Set 2 keyboard" > phys : "isa0060/serio0/input0" > bits ev : EV_SYN EV_KEY EV_MSC EV_LED EV_REP > > waiting for events > > 10:26:29.945739: EV_MSC MSC_SCAN 219 > 10:26:29.945739: EV_KEY KEY_LEFTMETA (0x7d) pressed > 10:26:29.945739: EV_SYN code=0 value=0 > 10:26:29.946468: EV_MSC MSC_SCAN 45 > 10:26:29.946468: EV_KEY KEY_X (0x2d) pressed > 10:26:29.946468: EV_SYN code=0 value=0 > 10:26:29.948469: EV_MSC MSC_SCAN 45 > 10:26:29.948469: EV_KEY KEY_X (0x2d) released > 10:26:29.948469: EV_SYN code=0 value=0 > 10:26:29.951473: EV_MSC MSC_SCAN 219 > 10:26:29.951473: EV_KEY KEY_LEFTMETA (0x7d) released > 10:26:29.951473: EV_SYN code=0 value=0 > x > > (Press+release first key with name "Windows Mobility Center control") > (key X was printed to console) > > 10:26:32.898689: EV_MSC MSC_SCAN 133 > 10:26:32.898689: EV_KEY KEY_BRIGHTNESSDOWN (0xe0) pressed > 10:26:32.898689: EV_SYN code=0 value=0 > 10:26:32.898730: EV_MSC MSC_SCAN 133 > 10:26:32.898730: EV_KEY KEY_BRIGHTNESSDOWN (0xe0) released > 10:26:32.898730: EV_SYN code=0 value=0 > > (Press+release second key with name "Instant launch control") > > 10:26:35.090018: EV_MSC MSC_SCAN 132 > 10:26:35.090018: EV_KEY KEY_NEXTSONG (0xa3) pressed > 10:26:35.090018: EV_SYN code=0 value=0 > 10:26:35.092765: EV_MSC MSC_SCAN 132 > 10:26:35.092765: EV_KEY KEY_NEXTSONG (0xa3) released > 10:26:35.092765: EV_SYN code=0 value=0 > > (Press+release third key with name "Audio control-panel control") > > As you can see events are send also via keyboard controller! > > Key codes are configured by userspace (udev/systemd) and looks like > there is bug in userspace rules (reason for brightnes or nextsong), see: > https://wiki.ubuntu.com/HardwareSupport/Machines/Laptops/Dell/XPS/15 Well, the V131 also sends its second hotkey (Dell Support Center) as a scancode, which can be mapped in userspace. But I fail to understand why this would be problematic, see below. > So it is not easy to make both machines (Dell XPS L502X and Dell Vostro > V131) works correctly :-( At least I do not see how. As far as I understand, it's not the keyboard events that are a problem for us, it's the WMI events, because some models generate them along with the scancode (L502x) while on others the WMI event is the only notification the OS can get that the hotkey was pressed (V131). So the only sparse keymap entry which has to vary between models seems to be the 0xe025 entry (obviously that's just the current state of our knowledge, not vendor-provided information). That is why I submitted a patch attempting to resolve this conflict. If you disagree with the above, could you please elaborate? > And that mapping "Windows Mobility Center control" key to combination of > two keys (KEY_LEFTMETA + X) is some total stupid nonsense... Well, it is unfortunate, but at least I can map it to what I please in my window manager. What I am really struggling to understand is why on earth would one employ something as complicated as ACPI/WMI for handling keypresses. Not to mention requiring cryptic SMI calls for enabling the notifications in the first place.
On Monday 30 November 2015 15:14:00 Micha? K?pie? wrote: > Hi Pali, > > Thanks again for your efforts. > > > $ sudo /usr/bin/input-events 4 > > /dev/input/event4 > > bustype : BUS_I8042 > > vendor : 0x1 > > product : 0x1 > > version : 43841 > > name : "AT Translated Set 2 keyboard" > > phys : "isa0060/serio0/input0" > > bits ev : EV_SYN EV_KEY EV_MSC EV_LED EV_REP > > > > waiting for events > > > > 10:26:29.945739: EV_MSC MSC_SCAN 219 > > 10:26:29.945739: EV_KEY KEY_LEFTMETA (0x7d) pressed > > 10:26:29.945739: EV_SYN code=0 value=0 > > 10:26:29.946468: EV_MSC MSC_SCAN 45 > > 10:26:29.946468: EV_KEY KEY_X (0x2d) pressed > > 10:26:29.946468: EV_SYN code=0 value=0 > > 10:26:29.948469: EV_MSC MSC_SCAN 45 > > 10:26:29.948469: EV_KEY KEY_X (0x2d) released > > 10:26:29.948469: EV_SYN code=0 value=0 > > 10:26:29.951473: EV_MSC MSC_SCAN 219 > > 10:26:29.951473: EV_KEY KEY_LEFTMETA (0x7d) released > > 10:26:29.951473: EV_SYN code=0 value=0 > > x > > > > (Press+release first key with name "Windows Mobility Center control") > > (key X was printed to console) > > > > 10:26:32.898689: EV_MSC MSC_SCAN 133 > > 10:26:32.898689: EV_KEY KEY_BRIGHTNESSDOWN (0xe0) pressed > > 10:26:32.898689: EV_SYN code=0 value=0 > > 10:26:32.898730: EV_MSC MSC_SCAN 133 > > 10:26:32.898730: EV_KEY KEY_BRIGHTNESSDOWN (0xe0) released > > 10:26:32.898730: EV_SYN code=0 value=0 > > > > (Press+release second key with name "Instant launch control") > > > > 10:26:35.090018: EV_MSC MSC_SCAN 132 > > 10:26:35.090018: EV_KEY KEY_NEXTSONG (0xa3) pressed > > 10:26:35.090018: EV_SYN code=0 value=0 > > 10:26:35.092765: EV_MSC MSC_SCAN 132 > > 10:26:35.092765: EV_KEY KEY_NEXTSONG (0xa3) released > > 10:26:35.092765: EV_SYN code=0 value=0 > > > > (Press+release third key with name "Audio control-panel control") > > > > As you can see events are send also via keyboard controller! > > > > Key codes are configured by userspace (udev/systemd) and looks like > > there is bug in userspace rules (reason for brightnes or nextsong), see: > > https://wiki.ubuntu.com/HardwareSupport/Machines/Laptops/Dell/XPS/15 > > Well, the V131 also sends its second hotkey (Dell Support Center) as a > scancode, which can be mapped in userspace. But I fail to understand > why this would be problematic, see below. > Same reason what you wrote below... On some machines WMI event must be ignored and on other must be sent to userspace. The best would be if embedded controller could be configured to send events via i8042 bus... I'm starting to thing that blacklist or whitelist of machines is needed to collect. And via it decide if WMI event will be accepted or dropped. > > So it is not easy to make both machines (Dell XPS L502X and Dell Vostro > > V131) works correctly :-( At least I do not see how. > > As far as I understand, it's not the keyboard events that are a problem > for us, it's the WMI events, because some models generate them along > with the scancode (L502x) while on others the WMI event is the only > notification the OS can get that the hotkey was pressed (V131). So the > only sparse keymap entry which has to vary between models seems to be > the 0xe025 entry (obviously that's just the current state of our > knowledge, not vendor-provided information). That is why I submitted a > patch attempting to resolve this conflict. > > If you disagree with the above, could you please elaborate? > > > And that mapping "Windows Mobility Center control" key to combination of > > two keys (KEY_LEFTMETA + X) is some total stupid nonsense... > > Well, it is unfortunate, but at least I can map it to what I please in > my window manager. What I am really struggling to understand is why on > earth would one employ something as complicated as ACPI/WMI for handling > keypresses. Not to mention requiring cryptic SMI calls for enabling the > notifications in the first place. > I remember that on older Acer laptops was needed to send some PS/2 command to enable additional Fn keys to generate scan codes... Dell just upgraded this "feature" that OS needs to send SMI call!
> The best would be if embedded controller could be configured to send > events via i8042 bus... Yes, but if I correctly understand Mario's message back from July [1], V131's EC simply does not support generating scancodes at all. > I'm starting to thing that blacklist or whitelist of machines is needed > to collect. And via it decide if WMI event will be accepted or dropped. If you believe this is the way to go, I'm perfectly fine with that. [1] http://www.spinics.net/lists/platform-driver-x86/msg07220.html
On Mon, Nov 30, 2015 at 03:54:58PM +0100, Micha? K?pie? wrote: > > The best would be if embedded controller could be configured to send > > events via i8042 bus... > > Yes, but if I correctly understand Mario's message back from July [1], > V131's EC simply does not support generating scancodes at all. > > > I'm starting to thing that blacklist or whitelist of machines is needed > > to collect. And via it decide if WMI event will be accepted or dropped. > > If you believe this is the way to go, I'm perfectly fine with that. This is a common approach among platform drivers since we try to support multiple products with a single driver. Ideally, we could detect if this was necessary by the response of some ACPI call or another, but failing that, DMI matching is our fallback.
On Thu, Nov 26, 2015 at 03:18:32PM +0100, Micha? K?pie? wrote: > On some laptop models (e.g. Dell Vostro V131), pressing the Dell Instant > Launch hotkey does not raise an i8042 interrupt - only WMI event 0xe025 > is generated. As there is no flawless way to determine whether a given > machine is capable of simulating a keypress when this hotkey is pressed, > a new module parameter is added so that the user can decide whether the > WMI event should be processed or ignored. > > Signed-off-by: Micha? K?pie? <kernel@kempniu.pl> Module parameters are to be avoided wherever possible, especially for things like this which set precedent and don't scale well. If we cannot detect which machines use the EC and which only use WMI, then we can fall back to checking DMI for specific models. Per the above, and Pali's feedback. I'll be dropping this one in anticipation of a V2. Thanks,
> Module parameters are to be avoided wherever possible, especially for things > like this which set precedent and don't scale well. If we cannot detect which > machines use the EC and which only use WMI, then we can fall back to checking > DMI for specific models. > > Per the above, and Pali's feedback. I'll be dropping this one in anticipation of > a V2. Thanks for reviewing, I'll prepare version 2 using DMI matching.
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 8cb0f57..e68ce3b 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -44,6 +44,10 @@ MODULE_LICENSE("GPL"); #define DELL_EVENT_GUID "9DBB5994-A997-11DA-B012-B622A1EF5492" static int acpi_video; +static bool process_dil; + +module_param(process_dil, bool, 0644); +MODULE_PARM_DESC(process_dil, "Generate an input event when the WMI event for Dell Instant Launch hotkey is received"); MODULE_ALIAS("wmi:"DELL_EVENT_GUID); @@ -87,7 +91,7 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = { { KE_IGNORE, 0xe020, { KEY_MUTE } }, /* Shortcut and audio panel keys */ - { KE_IGNORE, 0xe025, { KEY_RESERVED } }, + { KE_KEY, 0xe025, { KEY_PROG4 } }, { KE_IGNORE, 0xe026, { KEY_RESERVED } }, { KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } }, @@ -183,6 +187,9 @@ static void dell_wmi_process_key(int reported_key) key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video) return; + if (key->keycode == KEY_PROG4 && !process_dil) + return; + sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true); }
On some laptop models (e.g. Dell Vostro V131), pressing the Dell Instant Launch hotkey does not raise an i8042 interrupt - only WMI event 0xe025 is generated. As there is no flawless way to determine whether a given machine is capable of simulating a keypress when this hotkey is pressed, a new module parameter is added so that the user can decide whether the WMI event should be processed or ignored. Signed-off-by: Micha? K?pie? <kernel@kempniu.pl> --- As my last message [1] in the rather lengthy thread failed to elicit any response, I guess I might just as well post the proposed patch so that we have something specific to discuss. [1] http://www.spinics.net/lists/platform-driver-x86/msg07679.html drivers/platform/x86/dell-wmi.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)