* [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices
@ 2020-05-02 18:29 Hans de Goede
2020-05-02 18:29 ` [PATCH 1/5] platform/x86: intel-vbtn: Use acpi_evaluate_integer() Hans de Goede
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Hans de Goede @ 2020-05-02 18:29 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Mario Limonciello
Cc: Hans de Goede, linux-acpi, platform-driver-x86, linux-kernel
Hi All,
Here is a series of fixes, mostly aimed at fixing commit: de9647efeaa9
("platform/x86: intel-vbtn: Only activate tablet mode switch on 2-in-1's")
causing the driver to not bind on some devices where it could and
should report SW_TABLET_MODE.
The last commit makes the driver also work on some devices where it
previously would not work because they lack a VBDL method.
Mario, can you test this on a Dell XPS 9360 (for which you wrote the
de9647efeaa9 commit) to ensure that this series does not cause a
regression there? Also I have a question for you about using the DMI
chassis-type for this / a proposal for dealing with this differently
below the '---' of the commit msg of the 4th patch.
Regards,
Hans
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] platform/x86: intel-vbtn: Use acpi_evaluate_integer()
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
@ 2020-05-02 18:29 ` Hans de Goede
2020-05-02 18:29 ` [PATCH 2/5] platform/x86: intel-vbtn: Split keymap into buttons and switches parts Hans de Goede
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Hans de Goede @ 2020-05-02 18:29 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Mario Limonciello
Cc: Hans de Goede, linux-acpi, platform-driver-x86, linux-kernel
Use acpi_evaluate_integer() instead of open-coding it.
This is a preparation patch for adding a intel_vbtn_has_switches()
helper function.
Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode switch on 2-in-1's")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/intel-vbtn.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index b5880936d785..191894d648bb 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -119,28 +119,21 @@ static void detect_tablet_mode(struct platform_device *device)
const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
acpi_handle handle = ACPI_HANDLE(&device->dev);
- struct acpi_buffer vgbs_output = { ACPI_ALLOCATE_BUFFER, NULL };
- union acpi_object *obj;
+ unsigned long long vgbs;
acpi_status status;
int m;
if (!(chassis_type && strcmp(chassis_type, "31") == 0))
- goto out;
+ return;
- status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output);
+ status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
if (ACPI_FAILURE(status))
- goto out;
-
- obj = vgbs_output.pointer;
- if (!(obj && obj->type == ACPI_TYPE_INTEGER))
- goto out;
+ return;
- m = !(obj->integer.value & TABLET_MODE_FLAG);
+ m = !(vgbs & TABLET_MODE_FLAG);
input_report_switch(priv->input_dev, SW_TABLET_MODE, m);
- m = (obj->integer.value & DOCK_MODE_FLAG) ? 1 : 0;
+ m = (vgbs & DOCK_MODE_FLAG) ? 1 : 0;
input_report_switch(priv->input_dev, SW_DOCK, m);
-out:
- kfree(vgbs_output.pointer);
}
static int intel_vbtn_probe(struct platform_device *device)
--
2.26.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] platform/x86: intel-vbtn: Split keymap into buttons and switches parts
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
2020-05-02 18:29 ` [PATCH 1/5] platform/x86: intel-vbtn: Use acpi_evaluate_integer() Hans de Goede
@ 2020-05-02 18:29 ` Hans de Goede
2020-05-02 18:29 ` [PATCH 3/5] platform/x86: intel-vbtn: Do not advertise switches to userspace if they are not there Hans de Goede
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Hans de Goede @ 2020-05-02 18:29 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Mario Limonciello
Cc: Hans de Goede, linux-acpi, platform-driver-x86, linux-kernel
Split the sparse keymap into 2 separate keymaps, a buttons and a switches
keymap and combine the 2 to a single map again in intel_vbtn_input_setup().
This is a preparation patch for not telling userspace that we have switches
when we do not have them (and for doing the same for the buttons).
Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode switch on 2-in-1's")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/intel-vbtn.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index 191894d648bb..634096cef21a 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -40,14 +40,20 @@ static const struct key_entry intel_vbtn_keymap[] = {
{ KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
{ KE_KEY, 0xC8, { KEY_ROTATE_LOCK_TOGGLE } }, /* rotate-lock key press */
{ KE_KEY, 0xC9, { KEY_ROTATE_LOCK_TOGGLE } }, /* rotate-lock key release */
+};
+
+static const struct key_entry intel_vbtn_switchmap[] = {
{ KE_SW, 0xCA, { .sw = { SW_DOCK, 1 } } }, /* Docked */
{ KE_SW, 0xCB, { .sw = { SW_DOCK, 0 } } }, /* Undocked */
{ KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Tablet */
{ KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Laptop */
- { KE_END },
};
+#define KEYMAP_LEN \
+ (ARRAY_SIZE(intel_vbtn_keymap) + ARRAY_SIZE(intel_vbtn_switchmap) + 1)
+
struct intel_vbtn_priv {
+ struct key_entry keymap[KEYMAP_LEN];
struct input_dev *input_dev;
bool wakeup_mode;
};
@@ -55,13 +61,29 @@ struct intel_vbtn_priv {
static int intel_vbtn_input_setup(struct platform_device *device)
{
struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
- int ret;
+ int ret, keymap_len = 0;
+
+ if (true) {
+ memcpy(&priv->keymap[keymap_len], intel_vbtn_keymap,
+ ARRAY_SIZE(intel_vbtn_keymap) *
+ sizeof(struct key_entry));
+ keymap_len += ARRAY_SIZE(intel_vbtn_keymap);
+ }
+
+ if (true) {
+ memcpy(&priv->keymap[keymap_len], intel_vbtn_switchmap,
+ ARRAY_SIZE(intel_vbtn_switchmap) *
+ sizeof(struct key_entry));
+ keymap_len += ARRAY_SIZE(intel_vbtn_switchmap);
+ }
+
+ priv->keymap[keymap_len].type = KE_END;
priv->input_dev = devm_input_allocate_device(&device->dev);
if (!priv->input_dev)
return -ENOMEM;
- ret = sparse_keymap_setup(priv->input_dev, intel_vbtn_keymap, NULL);
+ ret = sparse_keymap_setup(priv->input_dev, priv->keymap, NULL);
if (ret)
return ret;
--
2.26.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] platform/x86: intel-vbtn: Do not advertise switches to userspace if they are not there
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
2020-05-02 18:29 ` [PATCH 1/5] platform/x86: intel-vbtn: Use acpi_evaluate_integer() Hans de Goede
2020-05-02 18:29 ` [PATCH 2/5] platform/x86: intel-vbtn: Split keymap into buttons and switches parts Hans de Goede
@ 2020-05-02 18:29 ` Hans de Goede
2020-05-02 18:29 ` [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types Hans de Goede
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Hans de Goede @ 2020-05-02 18:29 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Mario Limonciello
Cc: Hans de Goede, linux-acpi, platform-driver-x86, linux-kernel
Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
reporting SW_TABLET_MODE = 1 to userspace on laptops (specifically on the
Dell XPS 9360), to avoid e.g. userspace ignoring touchpad events because
userspace thought the device was in tablet-mode.
But if we are not getting the initial status of the switch because the
device does not have a tablet mode, then we really should not advertise
the presence of a tablet-mode switch to userspace at all, as userspace may
use the mere presence of this switch for certain heuristics.
Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode switch on 2-in-1's")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/intel-vbtn.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index 634096cef21a..500fae82e12c 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -55,6 +55,7 @@ static const struct key_entry intel_vbtn_switchmap[] = {
struct intel_vbtn_priv {
struct key_entry keymap[KEYMAP_LEN];
struct input_dev *input_dev;
+ bool has_switches;
bool wakeup_mode;
};
@@ -70,7 +71,7 @@ static int intel_vbtn_input_setup(struct platform_device *device)
keymap_len += ARRAY_SIZE(intel_vbtn_keymap);
}
- if (true) {
+ if (priv->has_switches) {
memcpy(&priv->keymap[keymap_len], intel_vbtn_switchmap,
ARRAY_SIZE(intel_vbtn_switchmap) *
sizeof(struct key_entry));
@@ -138,16 +139,12 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
static void detect_tablet_mode(struct platform_device *device)
{
- const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
acpi_handle handle = ACPI_HANDLE(&device->dev);
unsigned long long vgbs;
acpi_status status;
int m;
- if (!(chassis_type && strcmp(chassis_type, "31") == 0))
- return;
-
status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
if (ACPI_FAILURE(status))
return;
@@ -158,6 +155,19 @@ static void detect_tablet_mode(struct platform_device *device)
input_report_switch(priv->input_dev, SW_DOCK, m);
}
+static bool intel_vbtn_has_switches(acpi_handle handle)
+{
+ const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
+ unsigned long long vgbs;
+ acpi_status status;
+
+ if (!(chassis_type && strcmp(chassis_type, "31") == 0))
+ return false;
+
+ status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
+ return ACPI_SUCCESS(status);
+}
+
static int intel_vbtn_probe(struct platform_device *device)
{
acpi_handle handle = ACPI_HANDLE(&device->dev);
@@ -176,13 +186,16 @@ static int intel_vbtn_probe(struct platform_device *device)
return -ENOMEM;
dev_set_drvdata(&device->dev, priv);
+ priv->has_switches = intel_vbtn_has_switches(handle);
+
err = intel_vbtn_input_setup(device);
if (err) {
pr_err("Failed to setup Intel Virtual Button\n");
return err;
}
- detect_tablet_mode(device);
+ if (priv->has_switches)
+ detect_tablet_mode(device);
status = acpi_install_notify_handler(handle,
ACPI_DEVICE_NOTIFY,
--
2.26.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
` (2 preceding siblings ...)
2020-05-02 18:29 ` [PATCH 3/5] platform/x86: intel-vbtn: Do not advertise switches to userspace if they are not there Hans de Goede
@ 2020-05-02 18:29 ` Hans de Goede
2020-05-04 15:37 ` Mario.Limonciello
2020-05-02 18:29 ` [PATCH 5/5] platform/x86: intel-vbtn: Fix probe failure on devices with only switches Hans de Goede
2020-05-04 15:38 ` [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Mario.Limonciello
5 siblings, 1 reply; 12+ messages in thread
From: Hans de Goede @ 2020-05-02 18:29 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Mario Limonciello
Cc: Hans de Goede, linux-acpi, platform-driver-x86, linux-kernel
Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
reporting SW_TABLET_MODE = 1 to userspace on laptops.
Some devices with a detachable keyboard and using the intel-vbnt (INT33D6)
interface to report if they are in tablet mode (keyboard detached) or not,
report 32 / "Detachable" as chassis-type, e.g. the HP Pavilion X2 series.
Other devices with a detachable keyboard and using the intel-vbnt (INT33D6)
interface to report SW_TABLET_MODE, report 8 / "Portable" as chassis-type.
The Dell Venue 11 Pro 7130 is an example of this.
Extend the DMI chassis-type check to also accept Portables and Detachables
so that the intel-vbtn driver will report SW_TABLET_MODE on these devices.
Note the chassis-type check was originally added to avoid a false-positive
tablet-mode report on the Dell XPS 9360 laptop. To the best of my knowledge
that laptop is using a chassis-type of 9 / "Laptop", so after this commit
we still ignore the tablet-switch for that chassis-type.
Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode switch on 2-in-1's")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Looking at the Microsoft Windows documentation for tablet-mode reporting:
https://docs.microsoft.com/en-us/windows-hardware/drivers/gpiobtn/button-implementation
Then the presence of a tablet-mode switch is indicated by the presence
of a PNP0C60 compatible ACPI devices. There are 2 ways in which this device
can report the tablet-mode. 1. Directly providing a GpioInt resource inside
the PNP0C60 device, 2. Through injecting events from a Windows driver.
It seems that the intel-vbtn / the INT33D6 ACPI device is the ACPI side
of Intel's generic solution for the case where the tablet-mode comes from
the embedded-controller and needs to be "injected".
This all suggests that it might be better to replace the chassis-type
check with a acpi_dev_present("PNP0C60", NULL, -1) check.
Mario, can you provide an acpidump and alsa-info.sh output for the
Dell XPS 9360, so that I can check if that might help with the issue
there, and thus is a potential candidate to replace the chassis-type
check?
---
drivers/platform/x86/intel-vbtn.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index 500fae82e12c..4921fc15dc6c 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -158,12 +158,22 @@ static void detect_tablet_mode(struct platform_device *device)
static bool intel_vbtn_has_switches(acpi_handle handle)
{
const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
+ unsigned long chassis_type_int;
unsigned long long vgbs;
acpi_status status;
- if (!(chassis_type && strcmp(chassis_type, "31") == 0))
+ if (kstrtoul(chassis_type, 10, &chassis_type_int))
return false;
+ switch (chassis_type_int) {
+ case 8: /* Portable */
+ case 31: /* Convertible */
+ case 32: /* Detachable */
+ break;
+ default:
+ return false;
+ }
+
status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
return ACPI_SUCCESS(status);
}
--
2.26.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] platform/x86: intel-vbtn: Fix probe failure on devices with only switches
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
` (3 preceding siblings ...)
2020-05-02 18:29 ` [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types Hans de Goede
@ 2020-05-02 18:29 ` Hans de Goede
2020-05-04 15:38 ` [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Mario.Limonciello
5 siblings, 0 replies; 12+ messages in thread
From: Hans de Goede @ 2020-05-02 18:29 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Mario Limonciello
Cc: Hans de Goede, linux-acpi, platform-driver-x86, linux-kernel
On some devices the INT33D6 vbtn device is only used to report
tablet-mode / docked status (switches) and there are no vbtn managed
buttons.
On these devices there is no VBDL object.
Move the VBDL check to a intel_vbtn_has_buttons() helper and only exit
from intel_vbtn_probe() with -ENODEV when there are both no buttons and
no switches. Also only report the buttons being present to userspace if
the has_buttons check has succeeded.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/intel-vbtn.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index 4921fc15dc6c..4efc70b693a7 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -55,6 +55,7 @@ static const struct key_entry intel_vbtn_switchmap[] = {
struct intel_vbtn_priv {
struct key_entry keymap[KEYMAP_LEN];
struct input_dev *input_dev;
+ bool has_buttons;
bool has_switches;
bool wakeup_mode;
};
@@ -64,7 +65,7 @@ static int intel_vbtn_input_setup(struct platform_device *device)
struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
int ret, keymap_len = 0;
- if (true) {
+ if (priv->has_buttons) {
memcpy(&priv->keymap[keymap_len], intel_vbtn_keymap,
ARRAY_SIZE(intel_vbtn_keymap) *
sizeof(struct key_entry));
@@ -155,6 +156,14 @@ static void detect_tablet_mode(struct platform_device *device)
input_report_switch(priv->input_dev, SW_DOCK, m);
}
+static bool intel_vbtn_has_buttons(acpi_handle handle)
+{
+ acpi_status status;
+
+ status = acpi_evaluate_object(handle, "VBDL", NULL, NULL);
+ return ACPI_SUCCESS(status);
+}
+
static bool intel_vbtn_has_switches(acpi_handle handle)
{
const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
@@ -181,12 +190,15 @@ static bool intel_vbtn_has_switches(acpi_handle handle)
static int intel_vbtn_probe(struct platform_device *device)
{
acpi_handle handle = ACPI_HANDLE(&device->dev);
+ bool has_buttons, has_switches;
struct intel_vbtn_priv *priv;
acpi_status status;
int err;
- status = acpi_evaluate_object(handle, "VBDL", NULL, NULL);
- if (ACPI_FAILURE(status)) {
+ has_buttons = intel_vbtn_has_buttons(handle);
+ has_switches = intel_vbtn_has_switches(handle);
+
+ if (!has_buttons && !has_switches) {
dev_warn(&device->dev, "failed to read Intel Virtual Button driver\n");
return -ENODEV;
}
@@ -196,7 +208,8 @@ static int intel_vbtn_probe(struct platform_device *device)
return -ENOMEM;
dev_set_drvdata(&device->dev, priv);
- priv->has_switches = intel_vbtn_has_switches(handle);
+ priv->has_buttons = has_buttons;
+ priv->has_switches = has_switches;
err = intel_vbtn_input_setup(device);
if (err) {
--
2.26.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types
2020-05-02 18:29 ` [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types Hans de Goede
@ 2020-05-04 15:37 ` Mario.Limonciello
2020-05-05 9:05 ` Hans de Goede
0 siblings, 1 reply; 12+ messages in thread
From: Mario.Limonciello @ 2020-05-04 15:37 UTC (permalink / raw)
To: hdegoede, dvhart, andy; +Cc: linux-acpi, platform-driver-x86, linux-kernel
> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Saturday, May 2, 2020 1:30 PM
> To: Darren Hart; Andy Shevchenko; Limonciello, Mario
> Cc: Hans de Goede; linux-acpi@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch
> on "Detachable" and "Portable" chassis-types
>
>
> [EXTERNAL EMAIL]
>
> Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
> switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
> reporting SW_TABLET_MODE = 1 to userspace on laptops.
>
> Some devices with a detachable keyboard and using the intel-vbnt (INT33D6)
> interface to report if they are in tablet mode (keyboard detached) or not,
> report 32 / "Detachable" as chassis-type, e.g. the HP Pavilion X2 series.
>
> Other devices with a detachable keyboard and using the intel-vbnt (INT33D6)
> interface to report SW_TABLET_MODE, report 8 / "Portable" as chassis-type.
> The Dell Venue 11 Pro 7130 is an example of this.
>
> Extend the DMI chassis-type check to also accept Portables and Detachables
> so that the intel-vbtn driver will report SW_TABLET_MODE on these devices.
>
> Note the chassis-type check was originally added to avoid a false-positive
> tablet-mode report on the Dell XPS 9360 laptop. To the best of my knowledge
> that laptop is using a chassis-type of 9 / "Laptop", so after this commit
> we still ignore the tablet-switch for that chassis-type.
Yes that's correct.
>
> Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
> switch on 2-in-1's")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Looking at the Microsoft Windows documentation for tablet-mode reporting:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/gpiobtn/button-
> implementation
>
> Then the presence of a tablet-mode switch is indicated by the presence
> of a PNP0C60 compatible ACPI devices. There are 2 ways in which this device
> can report the tablet-mode. 1. Directly providing a GpioInt resource inside
> the PNP0C60 device, 2. Through injecting events from a Windows driver.
>
> It seems that the intel-vbtn / the INT33D6 ACPI device is the ACPI side
> of Intel's generic solution for the case where the tablet-mode comes from
> the embedded-controller and needs to be "injected".
>
> This all suggests that it might be better to replace the chassis-type
> check with a acpi_dev_present("PNP0C60", NULL, -1) check.
>
> Mario, can you provide an acpidump and alsa-info.sh output for the
> Dell XPS 9360, so that I can check if that might help with the issue
> there, and thus is a potential candidate to replace the chassis-type
> check?
Unfortunately with WFH right now, I don't have access to a XPS 9630 to
double check the patch series.
However I do agree this should be a good approach.
Reviewed-by: Mario Limonciello <Mario.limonciello@dell.com>
> ---
> drivers/platform/x86/intel-vbtn.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-
> vbtn.c
> index 500fae82e12c..4921fc15dc6c 100644
> --- a/drivers/platform/x86/intel-vbtn.c
> +++ b/drivers/platform/x86/intel-vbtn.c
> @@ -158,12 +158,22 @@ static void detect_tablet_mode(struct platform_device
> *device)
> static bool intel_vbtn_has_switches(acpi_handle handle)
> {
> const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
> + unsigned long chassis_type_int;
> unsigned long long vgbs;
> acpi_status status;
>
> - if (!(chassis_type && strcmp(chassis_type, "31") == 0))
> + if (kstrtoul(chassis_type, 10, &chassis_type_int))
> return false;
>
> + switch (chassis_type_int) {
> + case 8: /* Portable */
> + case 31: /* Convertible */
> + case 32: /* Detachable */
> + break;
> + default:
> + return false;
> + }
> +
> status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
> return ACPI_SUCCESS(status);
> }
> --
> 2.26.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
` (4 preceding siblings ...)
2020-05-02 18:29 ` [PATCH 5/5] platform/x86: intel-vbtn: Fix probe failure on devices with only switches Hans de Goede
@ 2020-05-04 15:38 ` Mario.Limonciello
5 siblings, 0 replies; 12+ messages in thread
From: Mario.Limonciello @ 2020-05-04 15:38 UTC (permalink / raw)
To: hdegoede, dvhart, andy; +Cc: linux-acpi, platform-driver-x86, linux-kernel
> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Saturday, May 2, 2020 1:30 PM
> To: Darren Hart; Andy Shevchenko; Limonciello, Mario
> Cc: Hans de Goede; linux-acpi@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work
> on more devices
>
>
> [EXTERNAL EMAIL]
>
> Hi All,
>
> Here is a series of fixes, mostly aimed at fixing commit: de9647efeaa9
> ("platform/x86: intel-vbtn: Only activate tablet mode switch on 2-in-1's")
> causing the driver to not bind on some devices where it could and
> should report SW_TABLET_MODE.
>
> The last commit makes the driver also work on some devices where it
> previously would not work because they lack a VBDL method.
>
> Mario, can you test this on a Dell XPS 9360 (for which you wrote the
> de9647efeaa9 commit) to ensure that this series does not cause a
> regression there?
Unfortunately I can't double check that with WFH, I don't have access to this hardware.
> Also I have a question for you about using the DMI
> chassis-type for this / a proposal for dealing with this differently
> below the '---' of the commit msg of the 4th patch.
OK will look.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types
2020-05-04 15:37 ` Mario.Limonciello
@ 2020-05-05 9:05 ` Hans de Goede
2020-05-05 14:22 ` Mario.Limonciello
0 siblings, 1 reply; 12+ messages in thread
From: Hans de Goede @ 2020-05-05 9:05 UTC (permalink / raw)
To: Mario.Limonciello, dvhart, andy
Cc: linux-acpi, platform-driver-x86, linux-kernel
Hi,
On 5/4/20 5:37 PM, Mario.Limonciello@dell.com wrote:
>
>
>> -----Original Message-----
>> From: Hans de Goede <hdegoede@redhat.com>
>> Sent: Saturday, May 2, 2020 1:30 PM
>> To: Darren Hart; Andy Shevchenko; Limonciello, Mario
>> Cc: Hans de Goede; linux-acpi@vger.kernel.org; platform-driver-
>> x86@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch
>> on "Detachable" and "Portable" chassis-types
>>
>>
>> [EXTERNAL EMAIL]
>>
>> Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
>> switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
>> reporting SW_TABLET_MODE = 1 to userspace on laptops.
>>
>> Some devices with a detachable keyboard and using the intel-vbnt (INT33D6)
>> interface to report if they are in tablet mode (keyboard detached) or not,
>> report 32 / "Detachable" as chassis-type, e.g. the HP Pavilion X2 series.
>>
>> Other devices with a detachable keyboard and using the intel-vbnt (INT33D6)
>> interface to report SW_TABLET_MODE, report 8 / "Portable" as chassis-type.
>> The Dell Venue 11 Pro 7130 is an example of this.
>>
>> Extend the DMI chassis-type check to also accept Portables and Detachables
>> so that the intel-vbtn driver will report SW_TABLET_MODE on these devices.
>>
>> Note the chassis-type check was originally added to avoid a false-positive
>> tablet-mode report on the Dell XPS 9360 laptop. To the best of my knowledge
>> that laptop is using a chassis-type of 9 / "Laptop", so after this commit
>> we still ignore the tablet-switch for that chassis-type.
>
> Yes that's correct.
>
>>
>> Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
>> switch on 2-in-1's")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Looking at the Microsoft Windows documentation for tablet-mode reporting:
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/gpiobtn/button-
>> implementation
>>
>> Then the presence of a tablet-mode switch is indicated by the presence
>> of a PNP0C60 compatible ACPI devices. There are 2 ways in which this device
>> can report the tablet-mode. 1. Directly providing a GpioInt resource inside
>> the PNP0C60 device, 2. Through injecting events from a Windows driver.
>>
>> It seems that the intel-vbtn / the INT33D6 ACPI device is the ACPI side
>> of Intel's generic solution for the case where the tablet-mode comes from
>> the embedded-controller and needs to be "injected".
>>
>> This all suggests that it might be better to replace the chassis-type
>> check with a acpi_dev_present("PNP0C60", NULL, -1) check.
>>
>> Mario, can you provide an acpidump and alsa-info.sh output for the
>> Dell XPS 9360, so that I can check if that might help with the issue
>> there, and thus is a potential candidate to replace the chassis-type
>> check?
>
> Unfortunately with WFH right now, I don't have access to a XPS 9630 to
> double check the patch series.
>
> However I do agree this should be a good approach.
Ok, so lets stick with the chassis-type check (as amended by this patch)
for now then. Then once you are able to go to your office again, we
can examine the acpi_dev_present("PNP0C60", NULL, -1) alternative.
> Reviewed-by: Mario Limonciello <Mario.limonciello@dell.com>
Thank you.
Regards,
Hans
>> ---
>> drivers/platform/x86/intel-vbtn.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-
>> vbtn.c
>> index 500fae82e12c..4921fc15dc6c 100644
>> --- a/drivers/platform/x86/intel-vbtn.c
>> +++ b/drivers/platform/x86/intel-vbtn.c
>> @@ -158,12 +158,22 @@ static void detect_tablet_mode(struct platform_device
>> *device)
>> static bool intel_vbtn_has_switches(acpi_handle handle)
>> {
>> const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
>> + unsigned long chassis_type_int;
>> unsigned long long vgbs;
>> acpi_status status;
>>
>> - if (!(chassis_type && strcmp(chassis_type, "31") == 0))
>> + if (kstrtoul(chassis_type, 10, &chassis_type_int))
>> return false;
>>
>> + switch (chassis_type_int) {
>> + case 8: /* Portable */
>> + case 31: /* Convertible */
>> + case 32: /* Detachable */
>> + break;
>> + default:
>> + return false;
>> + }
>> +
>> status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
>> return ACPI_SUCCESS(status);
>> }
>> --
>> 2.26.0
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types
2020-05-05 9:05 ` Hans de Goede
@ 2020-05-05 14:22 ` Mario.Limonciello
2020-05-05 14:27 ` Andy Shevchenko
0 siblings, 1 reply; 12+ messages in thread
From: Mario.Limonciello @ 2020-05-05 14:22 UTC (permalink / raw)
To: hdegoede, dvhart, andy; +Cc: linux-acpi, platform-driver-x86, linux-kernel
> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org <platform-driver-x86-
> owner@vger.kernel.org> On Behalf Of Hans de Goede
> Sent: Tuesday, May 5, 2020 4:06 AM
> To: Limonciello, Mario; dvhart@infradead.org; andy@infradead.org
> Cc: linux-acpi@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode
> switch on "Detachable" and "Portable" chassis-types
>
>
> [EXTERNAL EMAIL]
>
> Hi,
>
> On 5/4/20 5:37 PM, Mario.Limonciello@dell.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Hans de Goede <hdegoede@redhat.com>
> >> Sent: Saturday, May 2, 2020 1:30 PM
> >> To: Darren Hart; Andy Shevchenko; Limonciello, Mario
> >> Cc: Hans de Goede; linux-acpi@vger.kernel.org; platform-driver-
> >> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode
> switch
> >> on "Detachable" and "Portable" chassis-types
> >>
> >>
> >> [EXTERNAL EMAIL]
> >>
> >> Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
> >> switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
> >> reporting SW_TABLET_MODE = 1 to userspace on laptops.
> >>
> >> Some devices with a detachable keyboard and using the intel-vbnt (INT33D6)
> >> interface to report if they are in tablet mode (keyboard detached) or not,
> >> report 32 / "Detachable" as chassis-type, e.g. the HP Pavilion X2 series.
> >>
> >> Other devices with a detachable keyboard and using the intel-vbnt (INT33D6)
> >> interface to report SW_TABLET_MODE, report 8 / "Portable" as chassis-type.
> >> The Dell Venue 11 Pro 7130 is an example of this.
> >>
> >> Extend the DMI chassis-type check to also accept Portables and Detachables
> >> so that the intel-vbtn driver will report SW_TABLET_MODE on these devices.
> >>
> >> Note the chassis-type check was originally added to avoid a false-positive
> >> tablet-mode report on the Dell XPS 9360 laptop. To the best of my knowledge
> >> that laptop is using a chassis-type of 9 / "Laptop", so after this commit
> >> we still ignore the tablet-switch for that chassis-type.
> >
> > Yes that's correct.
> >
> >>
> >> Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
> >> switch on 2-in-1's")
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> Looking at the Microsoft Windows documentation for tablet-mode reporting:
> >> https://docs.microsoft.com/en-us/windows-hardware/drivers/gpiobtn/button-
> >> implementation
> >>
> >> Then the presence of a tablet-mode switch is indicated by the presence
> >> of a PNP0C60 compatible ACPI devices. There are 2 ways in which this device
> >> can report the tablet-mode. 1. Directly providing a GpioInt resource inside
> >> the PNP0C60 device, 2. Through injecting events from a Windows driver.
> >>
> >> It seems that the intel-vbtn / the INT33D6 ACPI device is the ACPI side
> >> of Intel's generic solution for the case where the tablet-mode comes from
> >> the embedded-controller and needs to be "injected".
> >>
> >> This all suggests that it might be better to replace the chassis-type
> >> check with a acpi_dev_present("PNP0C60", NULL, -1) check.
> >>
> >> Mario, can you provide an acpidump and alsa-info.sh output for the
> >> Dell XPS 9360, so that I can check if that might help with the issue
> >> there, and thus is a potential candidate to replace the chassis-type
> >> check?
> >
> > Unfortunately with WFH right now, I don't have access to a XPS 9630 to
> > double check the patch series.
> >
> > However I do agree this should be a good approach.
>
> Ok, so lets stick with the chassis-type check (as amended by this patch)
> for now then. Then once you are able to go to your office again, we
> can examine the acpi_dev_present("PNP0C60", NULL, -1) alternative.
I know XPS 13's are pretty popular, perhaps someone on the mailing list who has
one can share ACPI dump in the interim.
>
> > Reviewed-by: Mario Limonciello <Mario.limonciello@dell.com>
>
> Thank you.
>
> Regards,
>
> Hans
>
>
>
>
> >> ---
> >> drivers/platform/x86/intel-vbtn.c | 12 +++++++++++-
> >> 1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-
> >> vbtn.c
> >> index 500fae82e12c..4921fc15dc6c 100644
> >> --- a/drivers/platform/x86/intel-vbtn.c
> >> +++ b/drivers/platform/x86/intel-vbtn.c
> >> @@ -158,12 +158,22 @@ static void detect_tablet_mode(struct platform_device
> >> *device)
> >> static bool intel_vbtn_has_switches(acpi_handle handle)
> >> {
> >> const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
> >> + unsigned long chassis_type_int;
> >> unsigned long long vgbs;
> >> acpi_status status;
> >>
> >> - if (!(chassis_type && strcmp(chassis_type, "31") == 0))
> >> + if (kstrtoul(chassis_type, 10, &chassis_type_int))
> >> return false;
> >>
> >> + switch (chassis_type_int) {
> >> + case 8: /* Portable */
> >> + case 31: /* Convertible */
> >> + case 32: /* Detachable */
> >> + break;
> >> + default:
> >> + return false;
> >> + }
> >> +
> >> status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
> >> return ACPI_SUCCESS(status);
> >> }
> >> --
> >> 2.26.0
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types
2020-05-05 14:22 ` Mario.Limonciello
@ 2020-05-05 14:27 ` Andy Shevchenko
2020-05-07 11:25 ` Hans de Goede
0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2020-05-05 14:27 UTC (permalink / raw)
To: Mario Limonciello
Cc: Hans de Goede, Darren Hart, Andy Shevchenko,
ACPI Devel Maling List, Platform Driver,
Linux Kernel Mailing List
On Tue, May 5, 2020 at 5:22 PM <Mario.Limonciello@dell.com> wrote:
>
> > -----Original Message-----
> > From: platform-driver-x86-owner@vger.kernel.org <platform-driver-x86-
> > owner@vger.kernel.org> On Behalf Of Hans de Goede
> > Sent: Tuesday, May 5, 2020 4:06 AM
> > To: Limonciello, Mario; dvhart@infradead.org; andy@infradead.org
> > Cc: linux-acpi@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode
> > switch on "Detachable" and "Portable" chassis-types
> >
> >
> > [EXTERNAL EMAIL]
> >
> > Hi,
> >
> > On 5/4/20 5:37 PM, Mario.Limonciello@dell.com wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Hans de Goede <hdegoede@redhat.com>
> > >> Sent: Saturday, May 2, 2020 1:30 PM
> > >> To: Darren Hart; Andy Shevchenko; Limonciello, Mario
> > >> Cc: Hans de Goede; linux-acpi@vger.kernel.org; platform-driver-
> > >> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > >> Subject: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode
> > switch
> > >> on "Detachable" and "Portable" chassis-types
> > >>
> > >>
> > >> [EXTERNAL EMAIL]
> > >>
> > >> Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
> > >> switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
> > >> reporting SW_TABLET_MODE = 1 to userspace on laptops.
> > >>
> > >> Some devices with a detachable keyboard and using the intel-vbnt (INT33D6)
> > >> interface to report if they are in tablet mode (keyboard detached) or not,
> > >> report 32 / "Detachable" as chassis-type, e.g. the HP Pavilion X2 series.
> > >>
> > >> Other devices with a detachable keyboard and using the intel-vbnt (INT33D6)
> > >> interface to report SW_TABLET_MODE, report 8 / "Portable" as chassis-type.
> > >> The Dell Venue 11 Pro 7130 is an example of this.
> > >>
> > >> Extend the DMI chassis-type check to also accept Portables and Detachables
> > >> so that the intel-vbtn driver will report SW_TABLET_MODE on these devices.
> > >>
> > >> Note the chassis-type check was originally added to avoid a false-positive
> > >> tablet-mode report on the Dell XPS 9360 laptop. To the best of my knowledge
> > >> that laptop is using a chassis-type of 9 / "Laptop", so after this commit
> > >> we still ignore the tablet-switch for that chassis-type.
> > >
> > > Yes that's correct.
> > >
> > >>
> > >> Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
> > >> switch on 2-in-1's")
> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > >> ---
> > >> Looking at the Microsoft Windows documentation for tablet-mode reporting:
> > >> https://docs.microsoft.com/en-us/windows-hardware/drivers/gpiobtn/button-
> > >> implementation
> > >>
> > >> Then the presence of a tablet-mode switch is indicated by the presence
> > >> of a PNP0C60 compatible ACPI devices. There are 2 ways in which this device
> > >> can report the tablet-mode. 1. Directly providing a GpioInt resource inside
> > >> the PNP0C60 device, 2. Through injecting events from a Windows driver.
> > >>
> > >> It seems that the intel-vbtn / the INT33D6 ACPI device is the ACPI side
> > >> of Intel's generic solution for the case where the tablet-mode comes from
> > >> the embedded-controller and needs to be "injected".
> > >>
> > >> This all suggests that it might be better to replace the chassis-type
> > >> check with a acpi_dev_present("PNP0C60", NULL, -1) check.
> > >>
> > >> Mario, can you provide an acpidump and alsa-info.sh output for the
> > >> Dell XPS 9360, so that I can check if that might help with the issue
> > >> there, and thus is a potential candidate to replace the chassis-type
> > >> check?
> > >
> > > Unfortunately with WFH right now, I don't have access to a XPS 9630 to
> > > double check the patch series.
> > >
> > > However I do agree this should be a good approach.
> >
> > Ok, so lets stick with the chassis-type check (as amended by this patch)
> > for now then. Then once you are able to go to your office again, we
> > can examine the acpi_dev_present("PNP0C60", NULL, -1) alternative.
>
> I know XPS 13's are pretty popular, perhaps someone on the mailing list who has
> one can share ACPI dump in the interim.
https://github.com/intel/dptfxtract/issues/13
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types
2020-05-05 14:27 ` Andy Shevchenko
@ 2020-05-07 11:25 ` Hans de Goede
0 siblings, 0 replies; 12+ messages in thread
From: Hans de Goede @ 2020-05-07 11:25 UTC (permalink / raw)
To: Andy Shevchenko, Mario Limonciello
Cc: Darren Hart, Andy Shevchenko, ACPI Devel Maling List,
Platform Driver, Linux Kernel Mailing List
Hi,
On 5/5/20 4:27 PM, Andy Shevchenko wrote:
> On Tue, May 5, 2020 at 5:22 PM <Mario.Limonciello@dell.com> wrote:
>>
>>> -----Original Message-----
>>> From: platform-driver-x86-owner@vger.kernel.org <platform-driver-x86-
>>> owner@vger.kernel.org> On Behalf Of Hans de Goede
>>> Sent: Tuesday, May 5, 2020 4:06 AM
>>> To: Limonciello, Mario; dvhart@infradead.org; andy@infradead.org
>>> Cc: linux-acpi@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-
>>> kernel@vger.kernel.org
>>> Subject: Re: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode
>>> switch on "Detachable" and "Portable" chassis-types
>>>
>>>
>>> [EXTERNAL EMAIL]
>>>
>>> Hi,
>>>
>>> On 5/4/20 5:37 PM, Mario.Limonciello@dell.com wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Hans de Goede <hdegoede@redhat.com>
>>>>> Sent: Saturday, May 2, 2020 1:30 PM
>>>>> To: Darren Hart; Andy Shevchenko; Limonciello, Mario
>>>>> Cc: Hans de Goede; linux-acpi@vger.kernel.org; platform-driver-
>>>>> x86@vger.kernel.org; linux-kernel@vger.kernel.org
>>>>> Subject: [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode
>>> switch
>>>>> on "Detachable" and "Portable" chassis-types
>>>>>
>>>>>
>>>>> [EXTERNAL EMAIL]
>>>>>
>>>>> Commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
>>>>> switch on 2-in-1's") added a DMI chassis-type check to avoid accidentally
>>>>> reporting SW_TABLET_MODE = 1 to userspace on laptops.
>>>>>
>>>>> Some devices with a detachable keyboard and using the intel-vbnt (INT33D6)
>>>>> interface to report if they are in tablet mode (keyboard detached) or not,
>>>>> report 32 / "Detachable" as chassis-type, e.g. the HP Pavilion X2 series.
>>>>>
>>>>> Other devices with a detachable keyboard and using the intel-vbnt (INT33D6)
>>>>> interface to report SW_TABLET_MODE, report 8 / "Portable" as chassis-type.
>>>>> The Dell Venue 11 Pro 7130 is an example of this.
>>>>>
>>>>> Extend the DMI chassis-type check to also accept Portables and Detachables
>>>>> so that the intel-vbtn driver will report SW_TABLET_MODE on these devices.
>>>>>
>>>>> Note the chassis-type check was originally added to avoid a false-positive
>>>>> tablet-mode report on the Dell XPS 9360 laptop. To the best of my knowledge
>>>>> that laptop is using a chassis-type of 9 / "Laptop", so after this commit
>>>>> we still ignore the tablet-switch for that chassis-type.
>>>>
>>>> Yes that's correct.
>>>>
>>>>>
>>>>> Fixes: de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode
>>>>> switch on 2-in-1's")
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>> Looking at the Microsoft Windows documentation for tablet-mode reporting:
>>>>> https://docs.microsoft.com/en-us/windows-hardware/drivers/gpiobtn/button-
>>>>> implementation
>>>>>
>>>>> Then the presence of a tablet-mode switch is indicated by the presence
>>>>> of a PNP0C60 compatible ACPI devices. There are 2 ways in which this device
>>>>> can report the tablet-mode. 1. Directly providing a GpioInt resource inside
>>>>> the PNP0C60 device, 2. Through injecting events from a Windows driver.
>>>>>
>>>>> It seems that the intel-vbtn / the INT33D6 ACPI device is the ACPI side
>>>>> of Intel's generic solution for the case where the tablet-mode comes from
>>>>> the embedded-controller and needs to be "injected".
>>>>>
>>>>> This all suggests that it might be better to replace the chassis-type
>>>>> check with a acpi_dev_present("PNP0C60", NULL, -1) check.
>>>>>
>>>>> Mario, can you provide an acpidump and alsa-info.sh output for the
>>>>> Dell XPS 9360, so that I can check if that might help with the issue
>>>>> there, and thus is a potential candidate to replace the chassis-type
>>>>> check?
>>>>
>>>> Unfortunately with WFH right now, I don't have access to a XPS 9630 to
>>>> double check the patch series.
>>>>
>>>> However I do agree this should be a good approach.
>>>
>>> Ok, so lets stick with the chassis-type check (as amended by this patch)
>>> for now then. Then once you are able to go to your office again, we
>>> can examine the acpi_dev_present("PNP0C60", NULL, -1) alternative.
>>
>> I know XPS 13's are pretty popular, perhaps someone on the mailing list who has
>> one can share ACPI dump in the interim.
>
> https://github.com/intel/dptfxtract/issues/13
Good one.
So this has:
Device (CIND)
{
Name (_HID, "INT33D3" /* Intel GPIO Buttons */) // _HID: Hardware I
Name (_CID, "PNP0C60" /* Display Sensor Device */) // _CID: Compati
Method (_STA, 0, Serialized) // _STA: Status
{
If ((OSYS >= 0x07DC))
{
Return (0x0F)
}
Return (Zero)
}
}
And OSYS >= 0x07DC checks for "Windows 2012" which Linux does advertise,
so despite not having a tablet-mode(switch) the XPS 9360 still has a
PNP0C60 ACPI device and will report 0xf (present) as status for it,
so a acpi_dev_present("PNP0C60", NULL, -1) check will succeed on it.
Conclusion: such a check is not a valid alternative for checking DMI
chassis-types (and from that pov this series thus is ready for merging).
Regards,
Hans
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-05-07 11:25 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-02 18:29 [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Hans de Goede
2020-05-02 18:29 ` [PATCH 1/5] platform/x86: intel-vbtn: Use acpi_evaluate_integer() Hans de Goede
2020-05-02 18:29 ` [PATCH 2/5] platform/x86: intel-vbtn: Split keymap into buttons and switches parts Hans de Goede
2020-05-02 18:29 ` [PATCH 3/5] platform/x86: intel-vbtn: Do not advertise switches to userspace if they are not there Hans de Goede
2020-05-02 18:29 ` [PATCH 4/5] platform/x86: intel-vbtn: Also handle tablet-mode switch on "Detachable" and "Portable" chassis-types Hans de Goede
2020-05-04 15:37 ` Mario.Limonciello
2020-05-05 9:05 ` Hans de Goede
2020-05-05 14:22 ` Mario.Limonciello
2020-05-05 14:27 ` Andy Shevchenko
2020-05-07 11:25 ` Hans de Goede
2020-05-02 18:29 ` [PATCH 5/5] platform/x86: intel-vbtn: Fix probe failure on devices with only switches Hans de Goede
2020-05-04 15:38 ` [PATCH 0/5] platform/x86: intel-vbtn: Fixes + rework to make it work on more devices Mario.Limonciello
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).