Bug 198507

Created attachment 273687 [details]
possible patch

the attached patch does solve the issue even tough intel-vbtn feels the wrong place for a dell specific thing

Reviewing the DSDT I don't think that the 0xCC / 0xCD events are likely Dell specific.  The events for switching modes come from the Dell EC (which that part of the implementation are Dell specific).

This is not a Dell driver that works with this INT33D6 interface on Windows, it's an Intel driver.

So after looking this through I think your patch is on the right track, but I'd recommend a few changes.

1) Don't generically set the capability for tablet mode during initialization.
I'm not sure if it's possible to query in advance if tablet mode will be supported.  It might be encoded in a bit for the return of method VGBS.
It should be set when the first 0xCC or 0xCD event comes through. 

2) To initialize the right value of this (tablet mode or not) I believe you can check for the existence of VGBS method and look at it's return.  You can see in _Q1E and _Q1F that UPBT method is called which updates the value of VDBS and can be queried from VGBS

3) I think the looking at the two special tablet cases 0xCC/0xCD separate from sparse_keymap_report is fine, but please add some sort of #define so it's clear what 0xCC means and what 0xCD means.

Created attachment 273727 [details]
query of the initial state

The attached patch follows your advices (if i understood them correctly:)
now the initial state is initialized correctly, I tried to start in both modes. Tough I don't have a machine which has intel-vbtn and not a tablet mode switch to test that case to be handled correctly.

Overall looking better.

Minor feedback:
1) When looking at this I found that you might be able to use KE_VSW and then you wouldn't have to special case the handling (sparse key map should handle KE_VSW properly if you put the entries in the keymap table).
Having an #define might not be as important when you change this. A comment in sparse keymap entry would be sufficient to distinguish which ones are entry and which are exit.

2) You have a warning to fix.
drivers/platform/x86/intel-vbtn.c: In function ‘intel_vbtn_probe’:
drivers/platform/x86/intel-vbtn.c:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   union acpi_object *obj = vgbs_output.pointer;

3) Use ACPI_SUCCESS(status) rather than !ACPI_FAILURE(status)
4) You should put the input_report switch into a larger block that only runs if ACPI_TYPE_INTEGER.  In case VGBS returns wrong type for a machine.
For example like this:

diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index 6b38ade..487b301 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -130,11 +130,12 @@ static int intel_vbtn_probe(struct platform_device *device)
        if (!ACPI_FAILURE(status)) {
                union acpi_object *obj = vgbs_output.pointer;
                input_set_capability(priv->input_dev, EV_SW, SW_TABLET_MODE);
-               if (obj && obj->type == ACPI_TYPE_INTEGER && (obj->integer.value & TABLET_MODE_FLAG))
-                       input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
-               else
-                       input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
-               
+               if (obj && obj->type == ACPI_TYPE_INTEGER) {
+                       if (obj->integer.value & TABLET_MODE_FLAG)
+                               input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
+                       else
+                               input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
+               }
        }
 
        status = acpi_install_notify_handler(handle,

5) Patch appears to have trailing whitespace.

$ git am ~/Public/0001-Support-intel-vbtn-based-tablet-mode-switch.patch
Applying: Support intel-vbtn based tablet mode switch
.git/rebase-apply/patch:58: trailing whitespace.

warning: 1 line adds whitespace errors.

6) Checkpatch shows patch to not be clean, clean those up.


I checked the HW I have, XPS 9350 and XPS 9370 don't use vbtn, but XPS 9365 does.
It does report tablet mode properly with your patch too.

Once you clean up the stuff above I think you should resubmit this to mailing list.

In the mean time a similar patch has already been merged upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1c828496228de

So this bug can be closed now, closing.