> -----Original Message----- > From: rjwysocki@xxxxxxxxx [mailto:rjwysocki@xxxxxxxxx] On Behalf Of Rafael J. > Wysocki > Sent: Monday, June 12, 2017 2:12 PM > To: Jérôme de Bretagne <jerome.debretagne@xxxxxxxxx> > Cc: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>; ACPI Devel Maling List > <linux-acpi@xxxxxxxxxxxxxxx>; Linux PM <linux-pm@xxxxxxxxxxxxxxx>; Rafael J. > Wysocki <rjw@xxxxxxxxxxxxx>; Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx>; Darren Hart <dvhart@xxxxxxxxxxxxx>; > Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>; Mika Westerberg > <mika.westerberg@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH 0/3] ACPI / sleep: Support power button wakeup from S2I on > recent Dell laptops > > On Mon, Jun 12, 2017 at 12:01 AM, Jérôme de Bretagne > <jerome.debretagne@xxxxxxxxx> wrote: > > Hi Mario, Hi Rafael, > > > >>> Some assumptions now: either the SCI is ignored erroneously or it's > >>> not interpreted correctly by the expected driver? I guess that's the 2 > >>> possibilities I'll try to investigate. > >> > >> Well that's too bad. Yes, you're correct that the EC has changed between > >> those versions. Are you testing on or off AC adapter? > > > > I haven't detected a pattern specific to the state of the AC adapter. > > > > Good news on another front. By adding some more logs in the > > s2idle-dell-test branch, I've been able to check and confirm that > > notify_handler() from intel-hid.c is actually called upon short power key > > press during s2idle on my Latitude 7275. > > > > However, the system with BIOS 1.1.31 doesn't match (anymore?) the > > current criteria to call pm_wakeup_hard_event introduced in commit: > > > > platform: x86: intel-hid: Wake up the system from suspend-to-idle > > 7871dc61497a71be93c4f80d43ac109152510e7e > > > > This ugly 3-line modification I've added on the actual patch here: > > > > + if (priv->wakeup_mode) { > > + > > + /* Wake up Dell Latitude 7275 BIOS 1.1.31. */ > > + if (event == 0xce) > > + pm_wakeup_hard_event(&device->dev); > > + > > + /* Wake up on 5-button array events only. */ > > + if (event == 0xc0 || !priv->array) > > + return; > > + > > + if (sparse_keymap_entry_from_scancode(priv->array, event)) > > + pm_wakeup_hard_event(&device->dev); > > + else > > + dev_info(&device->dev, "unknown event 0x%x\n", event); > > + > > + return; > > + } > > > > make the system wake up on a standard short press finally! > > > > Is the "Wake up on 5-button array events only." assumption broad > > enough to cover the various Intel systems that need this behavior? > > > > Or am I just detecting another ACPI issue in fact that prevents this > > system from setting up properly the 5 button array with BIOS 1.1.31? > > It looks like priv->array is not set on your system with the new BIOS, > which means that intel_button_array_input_setup() is not called any > more, so either evaluating the HEBC method fails entirely, or its > return value is not as expected. > > > How would you recommend me to test that 2nd assumption? > > Log the status and event_cap right after the > > status = acpi_evaluate_integer(handle, "HEBC", NULL, &event_cap); > > statement in intel_hid_probe(). > > Thanks, > Rafael I don't see HEBC in the ASL for the 7275. It would be good to provide this information across both of the BIOS versions (working and non-working). As well as what status = acpi_evaluate_integer(handle, "BTNC", NULL, &button_cap); evaluates as on both too. Since this platform shipped with Win 8.1 initially rather than Win10, It's possible 5 button array was not part of the INT33D5 spec at that time and wasn't used by the Windows Intel HID filter driver. My suspicion: Win 8.1 didn't support 5 button array definition yet and those events that are normally part of 5 button array (as detected by bit 17) were coming across as regular HID event codes (like you were seeing 0xCE and 0xCF unknown events from debug log when OS was running). Perhaps someone from Intel can confirm that (the spec is not public). If that's correct, the intel-hid driver would need some changing to not check HEBC, but just check BTNC directly every time and Rafael's patch would need some changing too since it's only looking at button array event not HID event. ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
