RE: Regression: Dell XPS 13 9360 keyboard no longer works

[<Mario.Limonciello@dell.com>]

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Jeremy Cline
> Sent: Thursday, February 22, 2018 10:42 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>; notmart@gmail.com
> Cc: pali.rohar@gmail.com; andriy.shevchenko@linux.intel.com;
> mjg59@srcf.ucam.org; dvhart@infradead.org; platform-driver-
> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: Regression: Dell XPS 13 9360 keyboard no longer works
> 
> On 02/22/2018 11:17 AM, Mario.Limonciello@dell.com wrote:
> >> -----Original Message-----
> >> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> >> owner@vger.kernel.org] On Behalf Of Jeremy Cline
> >> Sent: Thursday, February 22, 2018 10:17 AM
> >> To: Limonciello, Mario <Mario_Limonciello@Dell.com>; notmart@gmail.com
> >> Cc: pali.rohar@gmail.com; andriy.shevchenko@linux.intel.com;
> >> mjg59@srcf.ucam.org; dvhart@infradead.org; platform-driver-
> >> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: Re: Regression: Dell XPS 13 9360 keyboard no longer works
> >>
> >> On 02/22/2018 11:14 AM, Mario.Limonciello@dell.com wrote:
> >>>> -----Original Message-----
> >>>> From: Jeremy Cline [mailto:jeremy@jcline.org]
> >>>> Sent: Thursday, February 22, 2018 9:59 AM
> >>>> To: Limonciello, Mario <Mario_Limonciello@Dell.com>; notmart@gmail.com
> >>>> Cc: pali.rohar@gmail.com; andriy.shevchenko@linux.intel.com;
> >>>> mjg59@srcf.ucam.org; dvhart@infradead.org; platform-driver-
> >>>> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> >>>> Subject: Re: Regression: Dell XPS 13 9360 keyboard no longer works
> >>>>
> >>>> On 02/22/2018 10:21 AM, Mario.Limonciello@dell.com wrote:> I guess that
> >>>> means we got this wrong and the patch should be reverted
> >>>>> until we figure this out.
> >>>>>
> >>>>> Jeremy,
> >>>>>
> >>>>> Can you please confirm what BIOS version you are on?
> >>>>> Also Is this a 9360 with 7th or 8th gen Intel CPU?
> >>>>
> >>>> Hi Mario,
> >>>>
> >>>> I've got BIOS version 2.5.0 with the 7th gen Intel CPU.
> >>>>
> >>>>
> >>>> Regards,
> >>>> Jeremy
> >>>
> >>> Jeremy,
> >>>
> >>> Thanks.  Do you have any of the Dell docks (TB16/WD15)?  If so are you
> connected
> >> to any dock
> >>> when reproducing this problem?
> >>
> >> Mario,
> >>
> >> I do have a TB16. I can reproduce this whether or not I'm connected to
> >> the dock, though.
> >>
> >>
> >> Regards,
> >> Jeremy
> >
> > Jeremy,
> >
> > Can you try booting up from a cold boot with it connected to see if it still
> happens?
> >
> 
> Mario,
> 
> Yup, it still happens from a cold boot when connected to the dock.

OK thanks for confirming.  Here's what I've concluded:

* So looking through the ACPI tables on the 9360 it initializes that status
(slate vs laptop mode) bit to "slate" mode.  The 9360 isn't a 2-in1- so that
seems wrong to me, but that's what it does.
It only gets updated based on dock status.

The 9365 (which is a 2 in 1) however seems to initialize the status properly.

So that's an impasse of what to do.
It's not clear to me what is really happening:
a) We're missing something else in this driver (eg something else that 
indicates whether to trust VGBS output)

b) Mis-interpreting the results from it (we shouldn't report the switch for 
tablet mode based on what we do)

c) 9360 has a BIOS bug (seems unlikely since Windows doesn't freak out and
show virtual keyboard at wrong time)

I'm leaning on it's probably <a>.
We should check for tablet mode should only be run if chassis type 
matches 2-in-1 (chassis type 0x1F).  

I believe that should fix the problem on the 9360, let it continue to work on
the 9365 (and other 2-in-1's).

Can you guys please test this?  If that works I'll split up the routine and submit
it.

diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index b703d6f..07bc489 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/dmi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
 #include <linux/kernel.h>
@@ -102,6 +103,7 @@ static int intel_vbtn_probe(struct platform_device *device)
        struct acpi_buffer vgbs_output = { ACPI_ALLOCATE_BUFFER, NULL };
        acpi_handle handle = ACPI_HANDLE(&device->dev);
        struct intel_vbtn_priv *priv;
+       const char *chassis_type;
        acpi_status status;
        int err;
 
@@ -123,22 +125,24 @@ static int intel_vbtn_probe(struct platform_device *device)
        }
 
        /*
-        * VGBS being present and returning something means we have
-        * a tablet mode switch.
+        * Running on 2-in-1 chassis, VGBS being present and
+        * returning something means we have a tablet mode switch.
         */
-       status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output);
-       if (ACPI_SUCCESS(status)) {
-               union acpi_object *obj = vgbs_output.pointer;
+       chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
+        if (chassis_type && strcmp(chassis_type, "31")) {
+               status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output);
+               if (ACPI_SUCCESS(status)) {
+                       union acpi_object *obj = vgbs_output.pointer;
 
-               if (obj && obj->type == ACPI_TYPE_INTEGER) {
-                       int m = !(obj->integer.value & TABLET_MODE_FLAG);
+                       if (obj && obj->type == ACPI_TYPE_INTEGER) {
+                               int m = !(obj->integer.value & TABLET_MODE_FLAG);
 
-                       input_report_switch(priv->input_dev, SW_TABLET_MODE, m);
+                               input_report_switch(priv->input_dev, SW_TABLET_MODE, m);
+                       }
                }
+               kfree(vgbs_output.pointer);
        }
-       kfree(vgbs_output.pointer);
-
        status = acpi_install_notify_handler(handle,
                                             ACPI_DEVICE_NOTIFY,
                                             notify_handler,