[PATCH] Support tablet mode switch for Dell laptops

[PATCH] Support tablet mode switch for Dell laptops

[Marco Martin]

Dell laptops send events to intel-vbtn.c
0xCC when the laptop enters in tablet mode and
0xCD when the laptop goes out of it

This has been confirmed working on a Dell Inspiron 13-7352
and an Inspiron 13-7000

I'm not sure intel-vbtn is the right place for it, as it
should be dell-specific, but this is the only device driver
where those events arrive at all
also, it would need a way to query the initial state of the switch

CC:platform-driver-x86@vger.kernel.org
CC:Matthew Garrett <mjg59@srcf.ucam.org>
CC:"Pali Rohár" <pali.rohar@gmail.com>
CC:Darren Hart <dvhart@infradead.org>
CC:Andy Shevchenko <andy@infradead.org>

Signed-off-by: Marco Martin <notmart@gmail.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 146d02f..1fa63e8 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -61,6 +61,10 @@ static int intel_vbtn_input_setup(struct platform_device *device)
 	priv->input_dev->name = "Intel Virtual Button driver";
 	priv->input_dev->id.bustype = BUS_HOST;
 
+	input_set_capability(priv->input_dev, EV_SW, SW_TABLET_MODE);
+	/*TODO: query initial state (and if the switch is present*/
+	input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
+
 	ret = input_register_device(priv->input_dev);
 	if (ret)
 		goto err_free_device;
@@ -84,7 +88,13 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
 	struct platform_device *device = context;
 	struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
 
-	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
+	if (event == 0xCC) {
+		input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
+		input_sync(priv->input_dev);
+	} else if (event == 0xCD) {
+		input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
+		input_sync(priv->input_dev);
+	} else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
 		dev_info(&device->dev, "unknown event index 0x%x\n",
 			 event);
 }
-- 
2.7.4

Re: [PATCH] Support tablet mode switch for Dell laptops

[Andy Shevchenko]

On Thu, Jan 18, 2018 at 3:59 PM, Marco Martin <notmart@gmail.com> wrote:
> Dell laptops send events to intel-vbtn.c
> 0xCC when the laptop enters in tablet mode and
> 0xCD when the laptop goes out of it
>
> This has been confirmed working on a Dell Inspiron 13-7352
> and an Inspiron 13-7000
>
> I'm not sure intel-vbtn is the right place for it, as it
> should be dell-specific, but this is the only device driver
> where those events arrive at all
> also, it would need a way to query the initial state of the switch

Yeah, to me sounds like a hack for now.
So, I would like to hear from everyone: Pali, Mario, Darren?

> CC:platform-driver-x86@vger.kernel.org
> CC:Matthew Garrett <mjg59@srcf.ucam.org>
> CC:"Pali Rohár" <pali.rohar@gmail.com>
> CC:Darren Hart <dvhart@infradead.org>
> CC:Andy Shevchenko <andy@infradead.org>
>
> Signed-off-by: Marco Martin <notmart@gmail.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 146d02f..1fa63e8 100644
> --- a/drivers/platform/x86/intel-vbtn.c
> +++ b/drivers/platform/x86/intel-vbtn.c
> @@ -61,6 +61,10 @@ static int intel_vbtn_input_setup(struct platform_device *device)
>         priv->input_dev->name = "Intel Virtual Button driver";
>         priv->input_dev->id.bustype = BUS_HOST;
>
> +       input_set_capability(priv->input_dev, EV_SW, SW_TABLET_MODE);
> +       /*TODO: query initial state (and if the switch is present*/
> +       input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> +
>         ret = input_register_device(priv->input_dev);
>         if (ret)
>                 goto err_free_device;
> @@ -84,7 +88,13 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
>         struct platform_device *device = context;
>         struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
>
> -       if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> +       if (event == 0xCC) {
> +               input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> +               input_sync(priv->input_dev);
> +       } else if (event == 0xCD) {
> +               input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> +               input_sync(priv->input_dev);
> +       } else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
>                 dev_info(&device->dev, "unknown event index 0x%x\n",
>                          event);
>  }
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH] Support tablet mode switch for Dell laptops

[Mario.Limonciello]

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Andy Shevchenko
> Sent: Thursday, January 18, 2018 9:13 AM
> To: Marco Martin <notmart@gmail.com>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>
> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Matthew Garrett
> <mjg59@srcf.ucam.org>; Pali Rohár <pali.rohar@gmail.com>; Darren Hart
> <dvhart@infradead.org>; Andy Shevchenko <andy@infradead.org>;
> bhush94@gmail.com; Platform Driver <platform-driver-x86@vger.kernel.org>
> Subject: Re: [PATCH] Support tablet mode switch for Dell laptops
> 
> On Thu, Jan 18, 2018 at 3:59 PM, Marco Martin <notmart@gmail.com> wrote:
> > Dell laptops send events to intel-vbtn.c
> > 0xCC when the laptop enters in tablet mode and
> > 0xCD when the laptop goes out of it
> >
> > This has been confirmed working on a Dell Inspiron 13-7352
> > and an Inspiron 13-7000
> >
> > I'm not sure intel-vbtn is the right place for it, as it
> > should be dell-specific, but this is the only device driver
> > where those events arrive at all
> > also, it would need a way to query the initial state of the switch
> 
> Yeah, to me sounds like a hack for now.
> So, I would like to hear from everyone: Pali, Mario, Darren?

AFAIK we don't have a public specification for the ACPI interface
that intel-vbtn uses, it's all reverse engineered.

I think the appropriate thing to do would be open a kernel Bugzilla for your
issue and attach:
1) kernel log without your patch (do you get unknown event index?)
2) your DSDT there to review.

You can subscribe us to it.

Also you should subscribe Alex Hung, who has done lots of work on intel-vbtn
and intel-hid.

> 
> > CC:platform-driver-x86@vger.kernel.org
> > CC:Matthew Garrett <mjg59@srcf.ucam.org>
> > CC:"Pali Rohár" <pali.rohar@gmail.com>
> > CC:Darren Hart <dvhart@infradead.org>
> > CC:Andy Shevchenko <andy@infradead.org>
> >
> > Signed-off-by: Marco Martin <notmart@gmail.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 146d02f..1fa63e8 100644
> > --- a/drivers/platform/x86/intel-vbtn.c
> > +++ b/drivers/platform/x86/intel-vbtn.c
> > @@ -61,6 +61,10 @@ static int intel_vbtn_input_setup(struct platform_device
> *device)
> >         priv->input_dev->name = "Intel Virtual Button driver";
> >         priv->input_dev->id.bustype = BUS_HOST;
> >
> > +       input_set_capability(priv->input_dev, EV_SW, SW_TABLET_MODE);
> > +       /*TODO: query initial state (and if the switch is present*/
> > +       input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > +
> >         ret = input_register_device(priv->input_dev);
> >         if (ret)
> >                 goto err_free_device;
> > @@ -84,7 +88,13 @@ static void notify_handler(acpi_handle handle, u32 event,
> void *context)
> >         struct platform_device *device = context;
> >         struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
> >
> > -       if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > +       if (event == 0xCC) {
> > +               input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> > +               input_sync(priv->input_dev);
> > +       } else if (event == 0xCD) {
> > +               input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > +               input_sync(priv->input_dev);
> > +       } else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> >                 dev_info(&device->dev, "unknown event index 0x%x\n",
> >                          event);
> >  }
> > --
> > 2.7.4
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

RE: [PATCH] Support tablet mode switch for Dell laptops

[Mario.Limonciello]

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Andy Shevchenko
> Sent: Thursday, January 18, 2018 9:13 AM
> To: Marco Martin <notmart@gmail.com>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>
> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Matthew Garrett
> <mjg59@srcf.ucam.org>; Pali Rohár <pali.rohar@gmail.com>; Darren Hart
> <dvhart@infradead.org>; Andy Shevchenko <andy@infradead.org>;
> bhush94@gmail.com; Platform Driver <platform-driver-x86@vger.kernel.org>
> Subject: Re: [PATCH] Support tablet mode switch for Dell laptops
> 
> On Thu, Jan 18, 2018 at 3:59 PM, Marco Martin <notmart@gmail.com> wrote:
> > Dell laptops send events to intel-vbtn.c
> > 0xCC when the laptop enters in tablet mode and
> > 0xCD when the laptop goes out of it
> >
> > This has been confirmed working on a Dell Inspiron 13-7352
> > and an Inspiron 13-7000
> >
> > I'm not sure intel-vbtn is the right place for it, as it
> > should be dell-specific, but this is the only device driver
> > where those events arrive at all
> > also, it would need a way to query the initial state of the switch
> 
> Yeah, to me sounds like a hack for now.
> So, I would like to hear from everyone: Pali, Mario, Darren?

AFAIK we don't have a public specification for the ACPI interface
that intel-vbtn uses, it's all reverse engineered.

I think the appropriate thing to do would be open a kernel Bugzilla for your
issue and attach:
1) kernel log without your patch (do you get unknown event index?)
2) your DSDT there to review.

You can subscribe us to it.

Also you should subscribe Alex Hung, who has done lots of work on intel-vbtn
and intel-hid.

> 
> > CC:platform-driver-x86@vger.kernel.org
> > CC:Matthew Garrett <mjg59@srcf.ucam.org>
> > CC:"Pali Rohár" <pali.rohar@gmail.com>
> > CC:Darren Hart <dvhart@infradead.org>
> > CC:Andy Shevchenko <andy@infradead.org>
> >
> > Signed-off-by: Marco Martin <notmart@gmail.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 146d02f..1fa63e8 100644
> > --- a/drivers/platform/x86/intel-vbtn.c
> > +++ b/drivers/platform/x86/intel-vbtn.c
> > @@ -61,6 +61,10 @@ static int intel_vbtn_input_setup(struct platform_device
> *device)
> >         priv->input_dev->name = "Intel Virtual Button driver";
> >         priv->input_dev->id.bustype = BUS_HOST;
> >
> > +       input_set_capability(priv->input_dev, EV_SW, SW_TABLET_MODE);
> > +       /*TODO: query initial state (and if the switch is present*/
> > +       input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > +
> >         ret = input_register_device(priv->input_dev);
> >         if (ret)
> >                 goto err_free_device;
> > @@ -84,7 +88,13 @@ static void notify_handler(acpi_handle handle, u32 event,
> void *context)
> >         struct platform_device *device = context;
> >         struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev);
> >
> > -       if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > +       if (event == 0xCC) {
> > +               input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> > +               input_sync(priv->input_dev);
> > +       } else if (event == 0xCD) {
> > +               input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > +               input_sync(priv->input_dev);
> > +       } else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> >                 dev_info(&device->dev, "unknown event index 0x%x\n",
> >                          event);
> >  }
> > --
> > 2.7.4
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] Support tablet mode switch for Dell laptops

[Pali Rohár]

On Thursday 18 January 2018 14:59:50 Marco Martin wrote:
> -	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> +	if (event == 0xCC) {
> +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> +		input_sync(priv->input_dev);
> +	} else if (event == 0xCD) {
> +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> +		input_sync(priv->input_dev);
> +	} else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))

Is not it possible to put 0xCC and 0xCD into sparse keymap table?
Because IIRC sparse keymap was created just to avoid that big
if-elseif-elseif-else blocks.

>  		dev_info(&device->dev, "unknown event index 0x%x\n",
>  			 event);
>  }

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] Support tablet mode switch for Dell laptops

[Marco Martin]

On Thu, Jan 18, 2018 at 4:23 PM,  <Mario.Limonciello@dell.com> wrote:
> AFAIK we don't have a public specification for the ACPI interface
> that intel-vbtn uses, it's all reverse engineered.
>
> I think the appropriate thing to do would be open a kernel Bugzilla for your
> issue and attach:
> 1) kernel log without your patch (do you get unknown event index?)
> 2) your DSDT there to review.
>
> You can subscribe us to it.

done at
https://bugzilla.kernel.org/show_bug.cgi?id=198507

Re: [PATCH] Support tablet mode switch for Dell laptops

[Pali Rohár]

On Thursday 18 January 2018 16:44:08 Pali Rohár wrote:
> On Thursday 18 January 2018 14:59:50 Marco Martin wrote:
> > -	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > +	if (event == 0xCC) {
> > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> > +		input_sync(priv->input_dev);
> > +	} else if (event == 0xCD) {
> > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > +		input_sync(priv->input_dev);
> > +	} else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> 
> Is not it possible to put 0xCC and 0xCD into sparse keymap table?
> Because IIRC sparse keymap was created just to avoid that big
> if-elseif-elseif-else blocks.

Dmitry, can you comment above part? I think that there should be better
way how to handle above switches and sparse keymap via one input device.

> >  		dev_info(&device->dev, "unknown event index 0x%x\n",
> >  			 event);
> >  }
> 

-- 
Pali Rohár
pali.rohar@gmail.com

RE: [PATCH] Support tablet mode switch for Dell laptops

[Mario.Limonciello]

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Pali Rohár
> Sent: Friday, January 19, 2018 10:01 AM
> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-kernel@vger.kernel.org; mjg59@srcf.ucam.org; dvhart@infradead.org;
> andy@infradead.org; bhush94@gmail.com; platform-driver-x86@vger.kernel.org;
> Marco Martin <notmart@gmail.com>
> Subject: Re: [PATCH] Support tablet mode switch for Dell laptops
> 
> On Thursday 18 January 2018 16:44:08 Pali Rohár wrote:
> > On Thursday 18 January 2018 14:59:50 Marco Martin wrote:
> > > -	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > > +	if (event == 0xCC) {
> > > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> > > +		input_sync(priv->input_dev);
> > > +	} else if (event == 0xCD) {
> > > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > > +		input_sync(priv->input_dev);
> > > +	} else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> >
> > Is not it possible to put 0xCC and 0xCD into sparse keymap table?
> > Because IIRC sparse keymap was created just to avoid that big
> > if-elseif-elseif-else blocks.
> 
> Dmitry, can you comment above part? I think that there should be better
> way how to handle above switches and sparse keymap via one input device.
> 
> > >  		dev_info(&device->dev, "unknown event index 0x%x\n",
> > >  			 event);
> > >  }
> >
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

We discussed a little more on the bug he filed with attached DSDT.  I think that in 
sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.

Assuming that works I'm expecting him to resubmit soon with that and some other
changes I recommended on the bug.

RE: [PATCH] Support tablet mode switch for Dell laptops

[Mario.Limonciello]

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Pali Rohár
> Sent: Friday, January 19, 2018 10:01 AM
> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-kernel@vger.kernel.org; mjg59@srcf.ucam.org; dvhart@infradead.org;
> andy@infradead.org; bhush94@gmail.com; platform-driver-x86@vger.kernel.org;
> Marco Martin <notmart@gmail.com>
> Subject: Re: [PATCH] Support tablet mode switch for Dell laptops
> 
> On Thursday 18 January 2018 16:44:08 Pali Rohár wrote:
> > On Thursday 18 January 2018 14:59:50 Marco Martin wrote:
> > > -	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > > +	if (event == 0xCC) {
> > > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> > > +		input_sync(priv->input_dev);
> > > +	} else if (event == 0xCD) {
> > > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > > +		input_sync(priv->input_dev);
> > > +	} else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> >
> > Is not it possible to put 0xCC and 0xCD into sparse keymap table?
> > Because IIRC sparse keymap was created just to avoid that big
> > if-elseif-elseif-else blocks.
> 
> Dmitry, can you comment above part? I think that there should be better
> way how to handle above switches and sparse keymap via one input device.
> 
> > >  		dev_info(&device->dev, "unknown event index 0x%x\n",
> > >  			 event);
> > >  }
> >
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

We discussed a little more on the bug he filed with attached DSDT.  I think that in 
sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.

Assuming that works I'm expecting him to resubmit soon with that and some other
changes I recommended on the bug.

Re: [PATCH] Support tablet mode switch for Dell laptops

[Dmitry Torokhov]

On Fri, Jan 19, 2018 at 04:09:09PM +0000, 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 Pali Rohár
> > Sent: Friday, January 19, 2018 10:01 AM
> > To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: linux-kernel@vger.kernel.org; mjg59@srcf.ucam.org; dvhart@infradead.org;
> > andy@infradead.org; bhush94@gmail.com; platform-driver-x86@vger.kernel.org;
> > Marco Martin <notmart@gmail.com>
> > Subject: Re: [PATCH] Support tablet mode switch for Dell laptops
> > 
> > On Thursday 18 January 2018 16:44:08 Pali Rohár wrote:
> > > On Thursday 18 January 2018 14:59:50 Marco Martin wrote:
> > > > -	if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > > > +	if (event == 0xCC) {
> > > > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 1);
> > > > +		input_sync(priv->input_dev);
> > > > +	} else if (event == 0xCD) {
> > > > +		input_report_switch(priv->input_dev, SW_TABLET_MODE, 0);
> > > > +		input_sync(priv->input_dev);
> > > > +	} else if (!sparse_keymap_report_event(priv->input_dev, event, 1, true))
> > >
> > > Is not it possible to put 0xCC and 0xCD into sparse keymap table?
> > > Because IIRC sparse keymap was created just to avoid that big
> > > if-elseif-elseif-else blocks.
> > 
> > Dmitry, can you comment above part? I think that there should be better
> > way how to handle above switches and sparse keymap via one input device.
> > 
> > > >  		dev_info(&device->dev, "unknown event index 0x%x\n",
> > > >  			 event);
> > > >  }
> > >
> > 
> > --
> > Pali Rohár
> > pali.rohar@gmail.com
> 
> We discussed a little more on the bug he filed with attached DSDT.  I think that in 
> sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.

No it should be KE_SW I believe, as the "keycode" encodes the state.
See example in intel-hid.c.

So you probably want:

	...
	{ KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Press */
	{ KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Release */

Thanks.

-- 
Dmitry

Re: [PATCH] Support tablet mode switch for Dell laptops

[Marco Martin]

On Fri, Jan 19, 2018 at 6:29 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> We discussed a little more on the bug he filed with attached DSDT.  I think that in
>> sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.
>
> No it should be KE_SW I believe, as the "keycode" encodes the state.
> See example in intel-hid.c.
>
> So you probably want:
>
>         ...
>         { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Press */
>         { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Release */
>
> Thanks.

If I add those lines to intel_vbtn_keymap and remove the two special
cases for 0xCC and 0xCD, it does work, but only if I explicitly call
input_sync(priv->input_dev) after sparse_keymap_report_event()
otherwise i don't get the event (cheching for instance with evtest)
until a sync gets called (i tried to put the sync only for 0xcd and in
that case i get both 0xcc and 0xcd when i go out of tablet mode)
--
Marco Martin

Re: [PATCH] Support tablet mode switch for Dell laptops

[Marco Martin]

On Fri, Jan 19, 2018 at 6:29 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> We discussed a little more on the bug he filed with attached DSDT.  I think that in
>> sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.
>
> No it should be KE_SW I believe, as the "keycode" encodes the state.
> See example in intel-hid.c.
>
> So you probably want:
>
>         ...
>         { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Press */
>         { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Release */
>
> Thanks.

If I add those lines to intel_vbtn_keymap and remove the two special
cases for 0xCC and 0xCD, it does work, but only if I explicitly call
input_sync(priv->input_dev) after sparse_keymap_report_event()
otherwise i don't get the event (cheching for instance with evtest)
until a sync gets called (i tried to put the sync only for 0xcd and in
that case i get both 0xcc and 0xcd when i go out of tablet mode)

Re: [PATCH] Support tablet mode switch for Dell laptops

[Marco Martin]

On Mon, Jan 22, 2018 at 12:59 PM, Marco Martin <notmart@gmail.com> wrote:
> On Fri, Jan 19, 2018 at 6:29 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>>> We discussed a little more on the bug he filed with attached DSDT.  I think that in
>>> sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.
>>
>> No it should be KE_SW I believe, as the "keycode" encodes the state.
>> See example in intel-hid.c.
>>
>> So you probably want:
>>
>>         ...
>>         { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Press */
>>         { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Release */
>>
>> Thanks.
>
> If I add those lines to intel_vbtn_keymap and remove the two special
> cases for 0xCC and 0xCD, it does work, but only if I explicitly call
> input_sync(priv->input_dev) after sparse_keymap_report_event()
> otherwise i don't get the event (cheching for instance with evtest)
> until a sync gets called (i tried to put the sync only for 0xcd and in
> that case i get both 0xcc and 0xcd when i go out of tablet mode)

with the caveat of an input_sync every event, new patch as a new
thread with Mario's
 and Dmitry's suggestions integrated in, and formatted to conform
checkpatch controls

--
Marco Martin

Re: [PATCH] Support tablet mode switch for Dell laptops

[Marco Martin]

On Mon, Jan 22, 2018 at 12:59 PM, Marco Martin <notmart@gmail.com> wrote:
> On Fri, Jan 19, 2018 at 6:29 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>>> We discussed a little more on the bug he filed with attached DSDT.  I think that in
>>> sparse keymap he can use KE_VSW for the entries and sparse keymap can handle.
>>
>> No it should be KE_SW I believe, as the "keycode" encodes the state.
>> See example in intel-hid.c.
>>
>> So you probably want:
>>
>>         ...
>>         { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Press */
>>         { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Release */
>>
>> Thanks.
>
> If I add those lines to intel_vbtn_keymap and remove the two special
> cases for 0xCC and 0xCD, it does work, but only if I explicitly call
> input_sync(priv->input_dev) after sparse_keymap_report_event()
> otherwise i don't get the event (cheching for instance with evtest)
> until a sync gets called (i tried to put the sync only for 0xcd and in
> that case i get both 0xcc and 0xcd when i go out of tablet mode)

with the caveat of an input_sync every event, new patch as a new
thread with Mario's
 and Dmitry's suggestions integrated in, and formatted to conform
checkpatch controls