[v3] HID: wacom: generic: add 5 tablet touch keys

Message ID	1481930680-6023-1-git-send-email-pingc@wacom.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-input-owner@kernel.org> From: Ping Cheng <pinglinux@gmail.com> To: linux-input@vger.kernel.org Cc: jikos@kernel.org, jason.gerecke@wacom.com, dmitry.torokhov@gmail.com, benjamin.tissoires@redhat.com, Ping Cheng <pingc@wacom.com>, Ping Cheng <ping.cheng@wacom.com> Subject: [PATCH v3] HID: wacom: generic: add 5 tablet touch keys Date: Fri, 16 Dec 2016 15:24:40 -0800 Message-Id: <1481930680-6023-1-git-send-email-pingc@wacom.com> Sender: linux-input-owner@vger.kernel.org Precedence: bulk

Ping Cheng Dec. 16, 2016, 11:24 p.m. UTC

Wacom Cintiq Pro [1] is a series of display tablets. They support
5 touch keys on the tablet. Those keys represent specific functions.
They turn display off; bring up OSD; bring up On Screen Keyboard;
bring up desktop control panel; and turn touch on/off.

The most left touch key, that turns display on/off, is controlled by
firmware. When the display is on, the mode is set. Otherwise, the
mode is off. So, it works like a switch. That's why we introduced a
new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
was a request from useland, more specifically Gnome, developers.

Other four touch keys are true software keys. We use the existing
KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
since none of the existing keys support those two actions.

[1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro

Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
---
v3: Since no one has followed up with v2, let's add some more comments for
SW_INDIRECT so we keep the offlist decision public ;).
---
 drivers/hid/wacom_wac.c                | 25 +++++++++++++++++++++++++
 drivers/hid/wacom_wac.h                |  5 +++++
 include/linux/mod_devicetable.h        |  2 +-
 include/uapi/linux/input-event-codes.h |  6 +++++-
 4 files changed, 36 insertions(+), 2 deletions(-)

Gerecke, Jason Dec. 18, 2016, 5:59 p.m. UTC | #1

My apologies for this second message. I had an unexpected HTML mail
bounce from the list. My Reviewed-by still stands.

On 12/18/16, Jason Gerecke <killertofu@gmail.com> wrote:
> Thanks for the patch, Ping. This version looks good to me.
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
>
>
> --
> Jason
> ---
> Now instead of four in the eights place /
> you’ve got three, ‘Cause you added one /
> (That is to say, eight) to the two, /
> But you can’t take seven from three, /
> So you look at the sixty-fours....
>
> On Dec 16, 2016 3:26 PM, "Ping Cheng" <pinglinux@gmail.com> wrote:
>
>> Wacom Cintiq Pro [1] is a series of display tablets. They support
>> 5 touch keys on the tablet. Those keys represent specific functions.
>> They turn display off; bring up OSD; bring up On Screen Keyboard;
>> bring up desktop control panel; and turn touch on/off.
>>
>> The most left touch key, that turns display on/off, is controlled by
>> firmware. When the display is on, the mode is set. Otherwise, the
>> mode is off. So, it works like a switch. That's why we introduced a
>> new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
>> was a request from useland, more specifically Gnome, developers.
>>
>> Other four touch keys are true software keys. We use the existing
>> KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
>> we have to request two new keys: KEY_ONSCREEN_KEYBOARD and
>> KEY_MUTE_DEVICE
>> since none of the existing keys support those two actions.
>>
>> [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
>>
>> Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
>> ---
>> v3: Since no one has followed up with v2, let's add some more comments
>> for
>> SW_INDIRECT so we keep the offlist decision public ;).
>> ---
>>  drivers/hid/wacom_wac.c                | 25 +++++++++++++++++++++++++
>>  drivers/hid/wacom_wac.h                |  5 +++++
>>  include/linux/mod_devicetable.h        |  2 +-
>>  include/uapi/linux/input-event-codes.h |  6 +++++-
>>  4 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
>> index b1a9a3c..739c592 100644
>> --- a/drivers/hid/wacom_wac.c
>> +++ b/drivers/hid/wacom_wac.c
>> @@ -1578,6 +1578,26 @@ static void wacom_wac_pad_usage_mapping(struct
>> hid_device *hdev,
>>                 wacom_map_usage(input, usage, field, EV_ABS, ABS_WHEEL,
>> 0);
>>                 features->device_type |= WACOM_DEVICETYPE_PAD;
>>                 break;
>> +       case WACOM_HID_WD_DIRECT:
>> +               wacom_map_usage(input, usage, field, EV_SW, SW_INDIRECT,
>> 0);
>> +               features->device_type |= WACOM_DEVICETYPE_PAD;
>> +               break;
>> +       case WACOM_HID_WD_BUTTONCONFIG:
>> +               wacom_map_usage(input, usage, field, EV_KEY,
>> KEY_BUTTONCONFIG, 0);
>> +               features->device_type |= WACOM_DEVICETYPE_PAD;
>> +               break;
>> +       case WACOM_HID_WD_ONSCREEN_KEYBOARD:
>> +               wacom_map_usage(input, usage, field, EV_KEY,
>> KEY_ONSCREEN_KEYBOARD, 0);
>> +               features->device_type |= WACOM_DEVICETYPE_PAD;
>> +               break;
>> +       case WACOM_HID_WD_CONTROLPANEL:
>> +               wacom_map_usage(input, usage, field, EV_KEY,
>> KEY_CONTROLPANEL, 0);
>> +               features->device_type |= WACOM_DEVICETYPE_PAD;
>> +               break;
>> +       case WACOM_HID_WD_MUTE_DEVICE:
>> +               wacom_map_usage(input, usage, field, EV_KEY,
>> KEY_MUTE_DEVICE, 0);
>> +               features->device_type |= WACOM_DEVICETYPE_PAD;
>> +               break;
>>         }
>>
>>         switch (equivalent_usage & 0xfffffff0) {
>> @@ -1629,6 +1649,11 @@ static void wacom_wac_pad_event(struct hid_device
>> *hdev, struct hid_field *field
>>         case WACOM_HID_WD_TOUCHRINGSTATUS:
>>                 break;
>>
>> +       case WACOM_HID_WD_DIRECT:
>> +               features->input_event_flag = true;
>> +               input_event(input, usage->type, usage->code, !value);
>> +               break;
>> +
>>         default:
>>                 features->input_event_flag = true;
>>                 input_event(input, usage->type, usage->code, value);
>> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
>> index fb0e50a..7a7b0f1 100644
>> --- a/drivers/hid/wacom_wac.h
>> +++ b/drivers/hid/wacom_wac.h
>> @@ -105,6 +105,11 @@
>>  #define WACOM_HID_WD_ACCELEROMETER_Z    (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0403)
>>  #define WACOM_HID_WD_BATTERY_CHARGING   (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0404)
>>  #define WACOM_HID_WD_BATTERY_LEVEL      (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x043b)
>> +#define WACOM_HID_WD_DIRECT             (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0980)
>> +#define WACOM_HID_WD_MUTE_DEVICE        (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0981)
>> +#define WACOM_HID_WD_CONTROLPANEL       (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0982)
>> +#define WACOM_HID_WD_ONSCREEN_KEYBOARD  (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0983)
>> +#define WACOM_HID_WD_BUTTONCONFIG       (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0986)
>>  #define WACOM_HID_WD_EXPRESSKEY00       (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0910)
>>  #define WACOM_HID_WD_EXPRESSKEYCAP00    (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0950)
>>  #define WACOM_HID_WD_BUTTONHOME         (WACOM_HID_UP_WACOMDIGITIZER |
>> 0x0990)
>> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_
>> devicetable.h
>> index ed84c07..342ddd6 100644
>> --- a/include/linux/mod_devicetable.h
>> +++ b/include/linux/mod_devicetable.h
>> @@ -291,7 +291,7 @@ struct pcmcia_device_id {
>>  #define INPUT_DEVICE_ID_LED_MAX                0x0f
>>  #define INPUT_DEVICE_ID_SND_MAX                0x07
>>  #define INPUT_DEVICE_ID_FF_MAX         0x7f
>> -#define INPUT_DEVICE_ID_SW_MAX         0x0f
>> +#define INPUT_DEVICE_ID_SW_MAX         0x1f
>>
>>  #define INPUT_DEVICE_ID_MATCH_BUS      1
>>  #define INPUT_DEVICE_ID_MATCH_VENDOR   2
>> diff --git a/include/uapi/linux/input-event-codes.h
>> b/include/uapi/linux/input-event-codes.h
>> index d6d071f..32ef894 100644
>> --- a/include/uapi/linux/input-event-codes.h
>> +++ b/include/uapi/linux/input-event-codes.h
>> @@ -641,6 +641,9 @@
>>   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
>>   */
>>  #define KEY_DATA                       0x275
>> +/* same as SW_MUTE_DEVICE but triggered by a key */
>> +#define KEY_MUTE_DEVICE                0x278
>> +#define KEY_ONSCREEN_KEYBOARD          0x279
>>
>>  #define BTN_TRIGGER_HAPPY              0x2c0
>>  #define BTN_TRIGGER_HAPPY1             0x2c0
>> @@ -781,7 +784,8 @@
>>  #define SW_LINEIN_INSERT       0x0d  /* set = inserted */
>>  #define SW_MUTE_DEVICE         0x0e  /* set = device disabled */
>>  #define SW_PEN_INSERTED                0x0f  /* set = pen inserted */
>> -#define SW_MAX                 0x0f
>> +#define SW_INDIRECT            0x10  /* set = not a direct input device
>> */
>> +#define SW_MAX                 0x1f
>>  #define SW_CNT                 (SW_MAX+1)
>>
>>  /*
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

Jiri Kosina Dec. 19, 2016, 10:30 a.m. UTC | #2

On Fri, 16 Dec 2016, Ping Cheng wrote:

> Wacom Cintiq Pro [1] is a series of display tablets. They support
> 5 touch keys on the tablet. Those keys represent specific functions.
> They turn display off; bring up OSD; bring up On Screen Keyboard;
> bring up desktop control panel; and turn touch on/off.
> 
> The most left touch key, that turns display on/off, is controlled by
> firmware. When the display is on, the mode is set. Otherwise, the
> mode is off. So, it works like a switch. That's why we introduced a
> new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> was a request from useland, more specifically Gnome, developers.
> 
> Other four touch keys are true software keys. We use the existing
> KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> since none of the existing keys support those two actions.
> 
> [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> 
> Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> ---
> v3: Since no one has followed up with v2, let's add some more comments for
> SW_INDIRECT so we keep the offlist decision public ;).

[ ... snip ... ]

> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index d6d071f..32ef894 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -641,6 +641,9 @@
>   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
>   */
>  #define KEY_DATA			0x275
> +/* same as SW_MUTE_DEVICE but triggered by a key */
> +#define KEY_MUTE_DEVICE 		0x278
> +#define KEY_ONSCREEN_KEYBOARD		0x279
>  
>  #define BTN_TRIGGER_HAPPY		0x2c0
>  #define BTN_TRIGGER_HAPPY1		0x2c0
> @@ -781,7 +784,8 @@
>  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
>  #define SW_MUTE_DEVICE		0x0e  /* set = device disabled */
>  #define SW_PEN_INSERTED		0x0f  /* set = pen inserted */
> -#define SW_MAX			0x0f
> +#define SW_INDIRECT		0x10  /* set = not a direct input device */
> +#define SW_MAX			0x1f

I'd like to have explicit Ack from Dmitry on these ... Dmitry?

Thanks,

Dmitry Torokhov Dec. 23, 2016, 1:13 a.m. UTC | #3

On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> On Fri, 16 Dec 2016, Ping Cheng wrote:
> 
> > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > 5 touch keys on the tablet. Those keys represent specific functions.
> > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > bring up desktop control panel; and turn touch on/off.
> > 
> > The most left touch key, that turns display on/off, is controlled by
> > firmware. When the display is on, the mode is set. Otherwise, the
> > mode is off. So, it works like a switch. That's why we introduced a
> > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > was a request from useland, more specifically Gnome, developers.
> > 
> > Other four touch keys are true software keys. We use the existing
> > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > since none of the existing keys support those two actions.
> > 
> > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > 
> > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > ---
> > v3: Since no one has followed up with v2, let's add some more comments for
> > SW_INDIRECT so we keep the offlist decision public ;).
> 
> [ ... snip ... ]
> 
> > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > index d6d071f..32ef894 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -641,6 +641,9 @@
> >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> >   */
> >  #define KEY_DATA			0x275
> > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > +#define KEY_MUTE_DEVICE 		0x278
> > +#define KEY_ONSCREEN_KEYBOARD		0x279
> >  
> >  #define BTN_TRIGGER_HAPPY		0x2c0
> >  #define BTN_TRIGGER_HAPPY1		0x2c0
> > @@ -781,7 +784,8 @@
> >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> >  #define SW_MUTE_DEVICE		0x0e  /* set = device disabled */
> >  #define SW_PEN_INSERTED		0x0f  /* set = pen inserted */
> > -#define SW_MAX			0x0f
> > +#define SW_INDIRECT		0x10  /* set = not a direct input device */
> > +#define SW_MAX			0x1f
> 
> I'd like to have explicit Ack from Dmitry on these ... Dmitry?

Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.

What Wacom folks seem to need is way to re-classfiy the device (i.e.
update its properties) and let userspace know somehow. We can't keep
adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
that device configuration changed and that userspace needs to
interrogate the device again. We can emit this code both when we change
properties as well as when we change ABS limits and changing keymaps and
so forth.

Benjamin, Peter, any opinion on the above?

I'm OK with the other 2 new keycodes.

Thanks.

Benjamin Tissoires Jan. 3, 2017, 9:30 a.m. UTC | #4

On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > 
> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > > bring up desktop control panel; and turn touch on/off.
> > > 
> > > The most left touch key, that turns display on/off, is controlled by
> > > firmware. When the display is on, the mode is set. Otherwise, the
> > > mode is off. So, it works like a switch. That's why we introduced a
> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > > was a request from useland, more specifically Gnome, developers.
> > > 
> > > Other four touch keys are true software keys. We use the existing
> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > > since none of the existing keys support those two actions.
> > > 
> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > > 
> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > > ---
> > > v3: Since no one has followed up with v2, let's add some more comments for
> > > SW_INDIRECT so we keep the offlist decision public ;).
> > 
> > [ ... snip ... ]
> > 
> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > index d6d071f..32ef894 100644
> > > --- a/include/uapi/linux/input-event-codes.h
> > > +++ b/include/uapi/linux/input-event-codes.h
> > > @@ -641,6 +641,9 @@
> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > >   */
> > >  #define KEY_DATA			0x275
> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > > +#define KEY_MUTE_DEVICE 		0x278
> > > +#define KEY_ONSCREEN_KEYBOARD		0x279
> > >  
> > >  #define BTN_TRIGGER_HAPPY		0x2c0
> > >  #define BTN_TRIGGER_HAPPY1		0x2c0
> > > @@ -781,7 +784,8 @@
> > >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> > >  #define SW_MUTE_DEVICE		0x0e  /* set = device disabled */
> > >  #define SW_PEN_INSERTED		0x0f  /* set = pen inserted */
> > > -#define SW_MAX			0x0f
> > > +#define SW_INDIRECT		0x10  /* set = not a direct input device */
> > > +#define SW_MAX			0x1f
> > 
> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> 
> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> 
> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> update its properties) and let userspace know somehow. We can't keep
> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> that device configuration changed and that userspace needs to
> interrogate the device again. We can emit this code both when we change
> properties as well as when we change ABS limits and changing keymaps and
> so forth.
> 
> Benjamin, Peter, any opinion on the above?

Oooh, so that's the purpose of this event :) (the documentation says
"TBD"). I am fine with that. We would need to adapt the documentation in
Documentation/input/event-codes.txt and make sure the handlers are
properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
the events in userspace).

On the userspace tool, I guess there will be some tweaks to do in
libevdev and libinput, but these are basically what would need to be
done with the creation of the new switches.

Thanks for the suggestion. I'll let the Wacom engineers work on the
kernel code now :)

Cheers,
Benjamin

> 
> I'm OK with the other 2 new keycodes.
> 
> Thanks.
> 
> -- 
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Peter Hutterer Jan. 3, 2017, 9:55 a.m. UTC | #5

On Thu, Dec 22, 2016 at 05:13:23PM -0800, Dmitry Torokhov wrote:
> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > 
> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > > bring up desktop control panel; and turn touch on/off.
> > > 
> > > The most left touch key, that turns display on/off, is controlled by
> > > firmware. When the display is on, the mode is set. Otherwise, the
> > > mode is off. So, it works like a switch. That's why we introduced a
> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > > was a request from useland, more specifically Gnome, developers.
> > > 
> > > Other four touch keys are true software keys. We use the existing
> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > > since none of the existing keys support those two actions.
> > > 
> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > > 
> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > > ---
> > > v3: Since no one has followed up with v2, let's add some more comments for
> > > SW_INDIRECT so we keep the offlist decision public ;).
> > 
> > [ ... snip ... ]
> > 
> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > index d6d071f..32ef894 100644
> > > --- a/include/uapi/linux/input-event-codes.h
> > > +++ b/include/uapi/linux/input-event-codes.h
> > > @@ -641,6 +641,9 @@
> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > >   */
> > >  #define KEY_DATA			0x275
> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > > +#define KEY_MUTE_DEVICE 		0x278
> > > +#define KEY_ONSCREEN_KEYBOARD		0x279
> > >  
> > >  #define BTN_TRIGGER_HAPPY		0x2c0
> > >  #define BTN_TRIGGER_HAPPY1		0x2c0
> > > @@ -781,7 +784,8 @@
> > >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> > >  #define SW_MUTE_DEVICE		0x0e  /* set = device disabled */
> > >  #define SW_PEN_INSERTED		0x0f  /* set = pen inserted */
> > > -#define SW_MAX			0x0f
> > > +#define SW_INDIRECT		0x10  /* set = not a direct input device */
> > > +#define SW_MAX			0x1f
> > 
> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> 
> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> 
> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> update its properties) and let userspace know somehow. We can't keep
> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> that device configuration changed and that userspace needs to
> interrogate the device again. We can emit this code both when we change
> properties as well as when we change ABS limits and changing keymaps and
> so forth.
> 
> Benjamin, Peter, any opinion on the above?

hmm, I quite like the idea. provided we can define the semantics correctly
(mostly to figure out what happens with ongoing events during SYN_CONFIG)
this should work, at least for new clients.

Existing clients will largely treat SYN_CONFIG as noop or like SYN_REPORT.

Cheers,
   Peter

> 
> I'm OK with the other 2 new keycodes.
> 
> Thanks.
> 
> -- 
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ping Cheng Jan. 3, 2017, 10:33 p.m. UTC | #6

Hi Dmitry,

On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
>> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
>> > On Fri, 16 Dec 2016, Ping Cheng wrote:
>> >
>> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
>> > > 5 touch keys on the tablet. Those keys represent specific functions.
>> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
>> > > bring up desktop control panel; and turn touch on/off.
>> > >
>> > > The most left touch key, that turns display on/off, is controlled by
>> > > firmware. When the display is on, the mode is set. Otherwise, the
>> > > mode is off. So, it works like a switch. That's why we introduced a
>> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
>> > > was a request from useland, more specifically Gnome, developers.
>> > >
>> > > Other four touch keys are true software keys. We use the existing
>> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
>> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
>> > > since none of the existing keys support those two actions.
>> > >
>> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
>> > >
>> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
>> > > ---
>> > > v3: Since no one has followed up with v2, let's add some more comments for
>> > > SW_INDIRECT so we keep the offlist decision public ;).
>> >
>> > [ ... snip ... ]
>> >
>> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
>> > > index d6d071f..32ef894 100644
>> > > --- a/include/uapi/linux/input-event-codes.h
>> > > +++ b/include/uapi/linux/input-event-codes.h
>> > > @@ -641,6 +641,9 @@
>> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
>> > >   */
>> > >  #define KEY_DATA                 0x275
>> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
>> > > +#define KEY_MUTE_DEVICE          0x278
>> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
>> > >
>> > >  #define BTN_TRIGGER_HAPPY                0x2c0
>> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
>> > > @@ -781,7 +784,8 @@
>> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
>> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
>> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
>> > > -#define SW_MAX                   0x0f
>> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
>> > > +#define SW_MAX                   0x1f
>> >
>> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
>>
>> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
>>
>> What Wacom folks seem to need is way to re-classfiy the device (i.e.
>> update its properties) and let userspace know somehow. We can't keep
>> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
>> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
>> that device configuration changed and that userspace needs to
>> interrogate the device again. We can emit this code both when we change
>> properties as well as when we change ABS limits and changing keymaps and
>> so forth.

By "to interrogate the device again",  do you mean once we report
EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?

If not, how do we tell userspace which feature/info has been changed?

>>
>> Benjamin, Peter, any opinion on the above?
>
> Oooh, so that's the purpose of this event :) (the documentation says
> "TBD"). I am fine with that. We would need to adapt the documentation in
> Documentation/input/event-codes.txt and make sure the handlers are
> properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> the events in userspace).

Can you update the "TBD" in Documentation/input/event-codes.txt so we
have an agreed description to follow?

Thanks,

Ping

> On the userspace tool, I guess there will be some tweaks to do in
> libevdev and libinput, but these are basically what would need to be
> done with the creation of the new switches.
>
> Thanks for the suggestion. I'll let the Wacom engineers work on the
> kernel code now :)
>
> Cheers,
> Benjamin
>
>>
>> I'm OK with the other 2 new keycodes.
>>
>> Thanks.
>>
>> --
>> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Benjamin Tissoires Jan. 3, 2017, 11:02 p.m. UTC | #7

On Jan 03 2017 or thereabouts, Ping Cheng wrote:
> Hi Dmitry,
> 
> On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> >> >
> >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> >> > > bring up desktop control panel; and turn touch on/off.
> >> > >
> >> > > The most left touch key, that turns display on/off, is controlled by
> >> > > firmware. When the display is on, the mode is set. Otherwise, the
> >> > > mode is off. So, it works like a switch. That's why we introduced a
> >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> >> > > was a request from useland, more specifically Gnome, developers.
> >> > >
> >> > > Other four touch keys are true software keys. We use the existing
> >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> >> > > since none of the existing keys support those two actions.
> >> > >
> >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> >> > >
> >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> >> > > ---
> >> > > v3: Since no one has followed up with v2, let's add some more comments for
> >> > > SW_INDIRECT so we keep the offlist decision public ;).
> >> >
> >> > [ ... snip ... ]
> >> >
> >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> >> > > index d6d071f..32ef894 100644
> >> > > --- a/include/uapi/linux/input-event-codes.h
> >> > > +++ b/include/uapi/linux/input-event-codes.h
> >> > > @@ -641,6 +641,9 @@
> >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> >> > >   */
> >> > >  #define KEY_DATA                 0x275
> >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> >> > > +#define KEY_MUTE_DEVICE          0x278
> >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> >> > >
> >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> >> > > @@ -781,7 +784,8 @@
> >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> >> > > -#define SW_MAX                   0x0f
> >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> >> > > +#define SW_MAX                   0x1f
> >> >
> >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> >>
> >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> >>
> >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> >> update its properties) and let userspace know somehow. We can't keep
> >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> >> that device configuration changed and that userspace needs to
> >> interrogate the device again. We can emit this code both when we change
> >> properties as well as when we change ABS limits and changing keymaps and
> >> so forth.
> 
> By "to interrogate the device again",  do you mean once we report
> EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?

Something along those lines, yes. However, there is one thing which I am
not so sure after second thoughts. Devices are tagged by udev first
before they are handed to libinput. If we request a reconfigure, it's
possible that a mouse becomes a touchpad and the udev tags will be off.

It won't be an issue in our case (direct or indirect is not a udev
prop), but I wonder if this will be an issue in the future.

> 
> If not, how do we tell userspace which feature/info has been changed?

I guess the userspace is knowledgeable enough to detect them. If the
events are somewhat rare, it doesn't matter if we redo a full init.

> 
> >>
> >> Benjamin, Peter, any opinion on the above?
> >
> > Oooh, so that's the purpose of this event :) (the documentation says
> > "TBD"). I am fine with that. We would need to adapt the documentation in
> > Documentation/input/event-codes.txt and make sure the handlers are
> > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > the events in userspace).
> 
> Can you update the "TBD" in Documentation/input/event-codes.txt so we
> have an agreed description to follow?

I guess that will be the result of this thread and the re-spin of the
series that makes use of SYN_CONFIG :)

Cheers,
Benjamin

> 
> Thanks,
> 
> Ping
> 
> > On the userspace tool, I guess there will be some tweaks to do in
> > libevdev and libinput, but these are basically what would need to be
> > done with the creation of the new switches.
> >
> > Thanks for the suggestion. I'll let the Wacom engineers work on the
> > kernel code now :)
> >
> > Cheers,
> > Benjamin
> >
> >>
> >> I'm OK with the other 2 new keycodes.
> >>
> >> Thanks.
> >>
> >> --
> >> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Peter Hutterer Jan. 3, 2017, 11:29 p.m. UTC | #8

On Tue, Jan 03, 2017 at 02:33:36PM -0800, Ping Cheng wrote:
> Hi Dmitry,
> 
> On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> >> >
> >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> >> > > bring up desktop control panel; and turn touch on/off.
> >> > >
> >> > > The most left touch key, that turns display on/off, is controlled by
> >> > > firmware. When the display is on, the mode is set. Otherwise, the
> >> > > mode is off. So, it works like a switch. That's why we introduced a
> >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> >> > > was a request from useland, more specifically Gnome, developers.
> >> > >
> >> > > Other four touch keys are true software keys. We use the existing
> >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> >> > > since none of the existing keys support those two actions.
> >> > >
> >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> >> > >
> >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> >> > > ---
> >> > > v3: Since no one has followed up with v2, let's add some more comments for
> >> > > SW_INDIRECT so we keep the offlist decision public ;).
> >> >
> >> > [ ... snip ... ]
> >> >
> >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> >> > > index d6d071f..32ef894 100644
> >> > > --- a/include/uapi/linux/input-event-codes.h
> >> > > +++ b/include/uapi/linux/input-event-codes.h
> >> > > @@ -641,6 +641,9 @@
> >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> >> > >   */
> >> > >  #define KEY_DATA                 0x275
> >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> >> > > +#define KEY_MUTE_DEVICE          0x278
> >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> >> > >
> >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> >> > > @@ -781,7 +784,8 @@
> >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> >> > > -#define SW_MAX                   0x0f
> >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> >> > > +#define SW_MAX                   0x1f
> >> >
> >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> >>
> >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> >>
> >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> >> update its properties) and let userspace know somehow. We can't keep
> >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> >> that device configuration changed and that userspace needs to
> >> interrogate the device again. We can emit this code both when we change
> >> properties as well as when we change ABS limits and changing keymaps and
> >> so forth.
> 
> By "to interrogate the device again",  do you mean once we report
> EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?
> 
> If not, how do we tell userspace which feature/info has been changed?
> 
> >>
> >> Benjamin, Peter, any opinion on the above?
> >
> > Oooh, so that's the purpose of this event :) (the documentation says
> > "TBD"). I am fine with that. We would need to adapt the documentation in
> > Documentation/input/event-codes.txt and make sure the handlers are
> > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > the events in userspace).
> 
> Can you update the "TBD" in Documentation/input/event-codes.txt so we
> have an agreed description to follow?

Here's a first attempt:

 * SYN_CONFIG:
   - Used to indicate that the device's static description has changed. This
     can happen when
     - at least one event type or code has been added or removed, or
     - at least one device property has been added or removed, or
     - at least one absolute axis has changed properties, or
     - the keycode has changed, or
     - the name, id, phys or unique identifier of the device has changed,
     A client should re-query the device once a SYN_CONFIG has been received
     as if the device was newly added. SYN_CONFIG does not indicate which
     information changed, a client is required to re-read the full 
     state. A SYN_CONFIG may be triggered even by types/codes/... that the
     client is not aware of, i.e. the state before and after may look
     identical to any given client.
     Dynamic values such as key state, switch state, absolute axis value,
     force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
     trigger a SYN_DROPPED to terminate ongoing event sequences.

We should also add a blurb to the EVIOC*MASK ioctls about SYN_CONFIG.

Cheers,
   Peter


> 
> > On the userspace tool, I guess there will be some tweaks to do in
> > libevdev and libinput, but these are basically what would need to be
> > done with the creation of the new switches.
> >
> > Thanks for the suggestion. I'll let the Wacom engineers work on the
> > kernel code now :)
> >
> > Cheers,
> > Benjamin
> >
> >>
> >> I'm OK with the other 2 new keycodes.
> >>
> >> Thanks.
> >>
> >> --
> >> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Peter Hutterer Jan. 4, 2017, 12:20 a.m. UTC | #9

On Wed, Jan 04, 2017 at 12:02:08AM +0100, Benjamin Tissoires wrote:
> On Jan 03 2017 or thereabouts, Ping Cheng wrote:
> > Hi Dmitry,
> > 
> > On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> > >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > >> >
> > >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > >> > > bring up desktop control panel; and turn touch on/off.
> > >> > >
> > >> > > The most left touch key, that turns display on/off, is controlled by
> > >> > > firmware. When the display is on, the mode is set. Otherwise, the
> > >> > > mode is off. So, it works like a switch. That's why we introduced a
> > >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > >> > > was a request from useland, more specifically Gnome, developers.
> > >> > >
> > >> > > Other four touch keys are true software keys. We use the existing
> > >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > >> > > since none of the existing keys support those two actions.
> > >> > >
> > >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > >> > >
> > >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > >> > > ---
> > >> > > v3: Since no one has followed up with v2, let's add some more comments for
> > >> > > SW_INDIRECT so we keep the offlist decision public ;).
> > >> >
> > >> > [ ... snip ... ]
> > >> >
> > >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > >> > > index d6d071f..32ef894 100644
> > >> > > --- a/include/uapi/linux/input-event-codes.h
> > >> > > +++ b/include/uapi/linux/input-event-codes.h
> > >> > > @@ -641,6 +641,9 @@
> > >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > >> > >   */
> > >> > >  #define KEY_DATA                 0x275
> > >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > >> > > +#define KEY_MUTE_DEVICE          0x278
> > >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> > >> > >
> > >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> > >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> > >> > > @@ -781,7 +784,8 @@
> > >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> > >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> > >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> > >> > > -#define SW_MAX                   0x0f
> > >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> > >> > > +#define SW_MAX                   0x1f
> > >> >
> > >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> > >>
> > >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> > >>
> > >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> > >> update its properties) and let userspace know somehow. We can't keep
> > >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> > >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> > >> that device configuration changed and that userspace needs to
> > >> interrogate the device again. We can emit this code both when we change
> > >> properties as well as when we change ABS limits and changing keymaps and
> > >> so forth.
> > 
> > By "to interrogate the device again",  do you mean once we report
> > EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?
> 
> Something along those lines, yes. However, there is one thing which I am
> not so sure after second thoughts. Devices are tagged by udev first
> before they are handed to libinput. If we request a reconfigure, it's
> possible that a mouse becomes a touchpad and the udev tags will be off.
> 
> It won't be an issue in our case (direct or indirect is not a udev
> prop), but I wonder if this will be an issue in the future.

hmm, good point. this would also affect anything that changes the struct
input_absinfo because we export the device size via udev properties. 
come to think of it this is already buggy because the axis corrections are
triggered *after* ID_INPUT_HEIGHT_MM is set. For reference:
https://github.com/systemd/systemd/issues/5020

Cheers,
   Peter

> 
> > 
> > If not, how do we tell userspace which feature/info has been changed?
> 
> I guess the userspace is knowledgeable enough to detect them. If the
> events are somewhat rare, it doesn't matter if we redo a full init.
> 
> > 
> > >>
> > >> Benjamin, Peter, any opinion on the above?
> > >
> > > Oooh, so that's the purpose of this event :) (the documentation says
> > > "TBD"). I am fine with that. We would need to adapt the documentation in
> > > Documentation/input/event-codes.txt and make sure the handlers are
> > > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > > the events in userspace).
> > 
> > Can you update the "TBD" in Documentation/input/event-codes.txt so we
> > have an agreed description to follow?
> 
> I guess that will be the result of this thread and the re-spin of the
> series that makes use of SYN_CONFIG :)
> 
> Cheers,
> Benjamin
> 
> > 
> > Thanks,
> > 
> > Ping
> > 
> > > On the userspace tool, I guess there will be some tweaks to do in
> > > libevdev and libinput, but these are basically what would need to be
> > > done with the creation of the new switches.
> > >
> > > Thanks for the suggestion. I'll let the Wacom engineers work on the
> > > kernel code now :)
> > >
> > > Cheers,
> > > Benjamin
> > >
> > >>
> > >> I'm OK with the other 2 new keycodes.
> > >>
> > >> Thanks.
> > >>
> > >> --
> > >> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Benjamin Tissoires Jan. 4, 2017, 9:27 a.m. UTC | #10

On Jan 04 2017 or thereabouts, Peter Hutterer wrote:
> On Tue, Jan 03, 2017 at 02:33:36PM -0800, Ping Cheng wrote:
> > Hi Dmitry,
> > 
> > On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> > >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > >> >
> > >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > >> > > bring up desktop control panel; and turn touch on/off.
> > >> > >
> > >> > > The most left touch key, that turns display on/off, is controlled by
> > >> > > firmware. When the display is on, the mode is set. Otherwise, the
> > >> > > mode is off. So, it works like a switch. That's why we introduced a
> > >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > >> > > was a request from useland, more specifically Gnome, developers.
> > >> > >
> > >> > > Other four touch keys are true software keys. We use the existing
> > >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > >> > > since none of the existing keys support those two actions.
> > >> > >
> > >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > >> > >
> > >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > >> > > ---
> > >> > > v3: Since no one has followed up with v2, let's add some more comments for
> > >> > > SW_INDIRECT so we keep the offlist decision public ;).
> > >> >
> > >> > [ ... snip ... ]
> > >> >
> > >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > >> > > index d6d071f..32ef894 100644
> > >> > > --- a/include/uapi/linux/input-event-codes.h
> > >> > > +++ b/include/uapi/linux/input-event-codes.h
> > >> > > @@ -641,6 +641,9 @@
> > >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > >> > >   */
> > >> > >  #define KEY_DATA                 0x275
> > >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > >> > > +#define KEY_MUTE_DEVICE          0x278
> > >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> > >> > >
> > >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> > >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> > >> > > @@ -781,7 +784,8 @@
> > >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> > >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> > >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> > >> > > -#define SW_MAX                   0x0f
> > >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> > >> > > +#define SW_MAX                   0x1f
> > >> >
> > >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> > >>
> > >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> > >>
> > >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> > >> update its properties) and let userspace know somehow. We can't keep
> > >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> > >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> > >> that device configuration changed and that userspace needs to
> > >> interrogate the device again. We can emit this code both when we change
> > >> properties as well as when we change ABS limits and changing keymaps and
> > >> so forth.
> > 
> > By "to interrogate the device again",  do you mean once we report
> > EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?
> > 
> > If not, how do we tell userspace which feature/info has been changed?
> > 
> > >>
> > >> Benjamin, Peter, any opinion on the above?
> > >
> > > Oooh, so that's the purpose of this event :) (the documentation says
> > > "TBD"). I am fine with that. We would need to adapt the documentation in
> > > Documentation/input/event-codes.txt and make sure the handlers are
> > > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > > the events in userspace).
> > 
> > Can you update the "TBD" in Documentation/input/event-codes.txt so we
> > have an agreed description to follow?
> 
> Here's a first attempt:
> 
>  * SYN_CONFIG:
>    - Used to indicate that the device's static description has changed. This

Should we indicate the obvious fact that this is only after the device has
been registered and presented to userspace? I can imagine drivers adding
those notifications after each declaration while preparing the input
node :)

>      can happen when
>      - at least one event type or code has been added or removed, or
>      - at least one device property has been added or removed, or
>      - at least one absolute axis has changed properties, or
>      - the keycode has changed, or
>      - the name, id, phys or unique identifier of the device has changed,
>      A client should re-query the device once a SYN_CONFIG has been received
>      as if the device was newly added. SYN_CONFIG does not indicate which
>      information changed, a client is required to re-read the full 
>      state. A SYN_CONFIG may be triggered even by types/codes/... that the
>      client is not aware of, i.e. the state before and after may look
>      identical to any given client.
>      Dynamic values such as key state, switch state, absolute axis value,
>      force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
>      trigger a SYN_DROPPED to terminate ongoing event sequences.

I think we should enforce this last sentence a little bit more:
"A SYN_CONFIG is basically a reset of the device so it should be
considered as a SYN_DROPPED from the client perspective. For backward
compatibility with clients not supporting SYN_CONFIG, a SYN_DROPPED
event is sent right before SYN_CONFIG."

Also, maybe we should add a warning about the udev properties:
"If a SYN_CONFIG allows to change the static description of a device, it
should be used with care. Some userspace pieces (udev) rely on the
device creation to tag it properly. Completely changing the device type
from a mouse to a touchpad is better handled through the destruction and
then creation of a new input node than relying on SYN_CONFIG"

(please fix typos/grammar/jibberish)

> 
> We should also add a blurb to the EVIOC*MASK ioctls about SYN_CONFIG.

Saying that it can't be masked and that it will invalidate the current
masks? Or this is the responsibility of the client?


Cheers,
Benjamin

> > 
> > > On the userspace tool, I guess there will be some tweaks to do in
> > > libevdev and libinput, but these are basically what would need to be
> > > done with the creation of the new switches.
> > >
> > > Thanks for the suggestion. I'll let the Wacom engineers work on the
> > > kernel code now :)
> > >
> > > Cheers,
> > > Benjamin
> > >
> > >>
> > >> I'm OK with the other 2 new keycodes.
> > >>
> > >> Thanks.
> > >>
> > >> --
> > >> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Peter Hutterer Jan. 6, 2017, 4:23 a.m. UTC | #11

On Wed, Jan 04, 2017 at 10:27:15AM +0100, Benjamin Tissoires wrote:
> On Jan 04 2017 or thereabouts, Peter Hutterer wrote:
> > On Tue, Jan 03, 2017 at 02:33:36PM -0800, Ping Cheng wrote:
> > > Hi Dmitry,
> > > 
> > > On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> > > <benjamin.tissoires@redhat.com> wrote:
> > > > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> > > >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > > >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > > >> >
> > > >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > > >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > > >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > > >> > > bring up desktop control panel; and turn touch on/off.
> > > >> > >
> > > >> > > The most left touch key, that turns display on/off, is controlled by
> > > >> > > firmware. When the display is on, the mode is set. Otherwise, the
> > > >> > > mode is off. So, it works like a switch. That's why we introduced a
> > > >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > > >> > > was a request from useland, more specifically Gnome, developers.
> > > >> > >
> > > >> > > Other four touch keys are true software keys. We use the existing
> > > >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > > >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > > >> > > since none of the existing keys support those two actions.
> > > >> > >
> > > >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > > >> > >
> > > >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > > >> > > ---
> > > >> > > v3: Since no one has followed up with v2, let's add some more comments for
> > > >> > > SW_INDIRECT so we keep the offlist decision public ;).
> > > >> >
> > > >> > [ ... snip ... ]
> > > >> >
> > > >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > >> > > index d6d071f..32ef894 100644
> > > >> > > --- a/include/uapi/linux/input-event-codes.h
> > > >> > > +++ b/include/uapi/linux/input-event-codes.h
> > > >> > > @@ -641,6 +641,9 @@
> > > >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > > >> > >   */
> > > >> > >  #define KEY_DATA                 0x275
> > > >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > > >> > > +#define KEY_MUTE_DEVICE          0x278
> > > >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> > > >> > >
> > > >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> > > >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> > > >> > > @@ -781,7 +784,8 @@
> > > >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> > > >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> > > >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> > > >> > > -#define SW_MAX                   0x0f
> > > >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> > > >> > > +#define SW_MAX                   0x1f
> > > >> >
> > > >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> > > >>
> > > >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> > > >>
> > > >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> > > >> update its properties) and let userspace know somehow. We can't keep
> > > >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> > > >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> > > >> that device configuration changed and that userspace needs to
> > > >> interrogate the device again. We can emit this code both when we change
> > > >> properties as well as when we change ABS limits and changing keymaps and
> > > >> so forth.
> > > 
> > > By "to interrogate the device again",  do you mean once we report
> > > EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?
> > > 
> > > If not, how do we tell userspace which feature/info has been changed?
> > > 
> > > >>
> > > >> Benjamin, Peter, any opinion on the above?
> > > >
> > > > Oooh, so that's the purpose of this event :) (the documentation says
> > > > "TBD"). I am fine with that. We would need to adapt the documentation in
> > > > Documentation/input/event-codes.txt and make sure the handlers are
> > > > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > > > the events in userspace).
> > > 
> > > Can you update the "TBD" in Documentation/input/event-codes.txt so we
> > > have an agreed description to follow?
> > 
> > Here's a first attempt:
> > 
> >  * SYN_CONFIG:
> >    - Used to indicate that the device's static description has changed. This
> 
> Should we indicate the obvious fact that this is only after the device has
> been registered and presented to userspace? I can imagine drivers adding
> those notifications after each declaration while preparing the input
> node :)

added, see below

> 
> >      can happen when
> >      - at least one event type or code has been added or removed, or
> >      - at least one device property has been added or removed, or
> >      - at least one absolute axis has changed properties, or
> >      - the keycode has changed, or
> >      - the name, id, phys or unique identifier of the device has changed,
> >      A client should re-query the device once a SYN_CONFIG has been received
> >      as if the device was newly added. SYN_CONFIG does not indicate which
> >      information changed, a client is required to re-read the full 
> >      state. A SYN_CONFIG may be triggered even by types/codes/... that the
> >      client is not aware of, i.e. the state before and after may look
> >      identical to any given client.
> >      Dynamic values such as key state, switch state, absolute axis value,
> >      force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
> >      trigger a SYN_DROPPED to terminate ongoing event sequences.
> 
> I think we should enforce this last sentence a little bit more:
> "A SYN_CONFIG is basically a reset of the device so it should be
> considered as a SYN_DROPPED from the client perspective. For backward
> compatibility with clients not supporting SYN_CONFIG, a SYN_DROPPED
> event is sent right before SYN_CONFIG."

Are we planning to force a SYN_DROPPED every time? Even when the event queue
is empty? I think it wouldn't do much other than keep the client busy. 

Come to think of it, there's a chance that this *reduces* backwards
compatibility. Until a SYN_DROPPED occurs, a client has no reason to ever
check bits again. But re-fetching all states during SYN_DROPPED is normal,
so if one axis disappears, the client may now get confused.

> Also, maybe we should add a warning about the udev properties:
> "If a SYN_CONFIG allows to change the static description of a device, it
> should be used with care. Some userspace pieces (udev) rely on the
> device creation to tag it properly. Completely changing the device type
> from a mouse to a touchpad is better handled through the destruction and
> then creation of a new input node than relying on SYN_CONFIG"
> 
> (please fix typos/grammar/jibberish)
> 
> > 
> > We should also add a blurb to the EVIOC*MASK ioctls about SYN_CONFIG.
> 
> Saying that it can't be masked and that it will invalidate the current
> masks? Or this is the responsibility of the client?

I would just leave the masks as-is, less work and more predictability. This
means that a client has to re-apply them on SYN_CONFIG to avoid events from
new axes (if any) but that seems like the most straightforward approach. We
already allow masking out axes a device doesn't have, so invalidating
doesn't really give us any benefits. 

Ok, let's have a v2 draft:

* SYN_CONFIG:
  - Used to indicate that the device's static description has changed, after
    the device has been registered and presented to userspace.
    This can happen when
    - at least one event type or code has been added or removed, or
    - at least one device property has been added or removed, or
    - at least one absolute axis has changed properties, or
    - the keycode has changed, or
    - the name, id, phys or unique identifier of the device has changed,
    Do not send this event during the initial device initialization process,
    this event is exclusively for changes at runtime.

    A client should re-query the device once a SYN_CONFIG has been received
    as if the device was newly added. SYN_CONFIG does not indicate which
    information changed, a client is required to re-read the full 
    state. A SYN_CONFIG may be triggered even by types/codes/... that the
    client is not aware of, i.e. the state before and after may look
    identical to any given client.

    Dynamic values such as key state, switch state, absolute axis value,
    force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
    trigger a SYN_DROPPED to terminate ongoing event sequences.

    SYN_CONFIG should be used with extreme care. There is no guarantee that
    a SYN_CONFIG event is handled correctly by a client, or even noticed.
    For example, udev relies on the original device description only to
    assign a device type. If a SYN_CONFIG were to change the device
    sufficiently to warrant a new device type tag, this may never be
    detected. Changing a device type (e.g. from mouse to touchpad) is better
    handled through the destruction and recreation of the input node.

Cheers,
   Peter
 
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Benjamin Tissoires Jan. 10, 2017, 8:47 a.m. UTC | #12

On Jan 06 2017 or thereabouts, Peter Hutterer wrote:
> On Wed, Jan 04, 2017 at 10:27:15AM +0100, Benjamin Tissoires wrote:
> > On Jan 04 2017 or thereabouts, Peter Hutterer wrote:
> > > On Tue, Jan 03, 2017 at 02:33:36PM -0800, Ping Cheng wrote:
> > > > Hi Dmitry,
> > > > 
> > > > On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> > > > <benjamin.tissoires@redhat.com> wrote:
> > > > > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> > > > >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > > > >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > > > >> >
> > > > >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > > > >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > > > >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > > > >> > > bring up desktop control panel; and turn touch on/off.
> > > > >> > >
> > > > >> > > The most left touch key, that turns display on/off, is controlled by
> > > > >> > > firmware. When the display is on, the mode is set. Otherwise, the
> > > > >> > > mode is off. So, it works like a switch. That's why we introduced a
> > > > >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > > > >> > > was a request from useland, more specifically Gnome, developers.
> > > > >> > >
> > > > >> > > Other four touch keys are true software keys. We use the existing
> > > > >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > > > >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > > > >> > > since none of the existing keys support those two actions.
> > > > >> > >
> > > > >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > > > >> > >
> > > > >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > > > >> > > ---
> > > > >> > > v3: Since no one has followed up with v2, let's add some more comments for
> > > > >> > > SW_INDIRECT so we keep the offlist decision public ;).
> > > > >> >
> > > > >> > [ ... snip ... ]
> > > > >> >
> > > > >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > > >> > > index d6d071f..32ef894 100644
> > > > >> > > --- a/include/uapi/linux/input-event-codes.h
> > > > >> > > +++ b/include/uapi/linux/input-event-codes.h
> > > > >> > > @@ -641,6 +641,9 @@
> > > > >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > > > >> > >   */
> > > > >> > >  #define KEY_DATA                 0x275
> > > > >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > > > >> > > +#define KEY_MUTE_DEVICE          0x278
> > > > >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> > > > >> > >
> > > > >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> > > > >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> > > > >> > > @@ -781,7 +784,8 @@
> > > > >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> > > > >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> > > > >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> > > > >> > > -#define SW_MAX                   0x0f
> > > > >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> > > > >> > > +#define SW_MAX                   0x1f
> > > > >> >
> > > > >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> > > > >>
> > > > >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> > > > >>
> > > > >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> > > > >> update its properties) and let userspace know somehow. We can't keep
> > > > >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> > > > >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> > > > >> that device configuration changed and that userspace needs to
> > > > >> interrogate the device again. We can emit this code both when we change
> > > > >> properties as well as when we change ABS limits and changing keymaps and
> > > > >> so forth.
> > > > 
> > > > By "to interrogate the device again",  do you mean once we report
> > > > EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?
> > > > 
> > > > If not, how do we tell userspace which feature/info has been changed?
> > > > 
> > > > >>
> > > > >> Benjamin, Peter, any opinion on the above?
> > > > >
> > > > > Oooh, so that's the purpose of this event :) (the documentation says
> > > > > "TBD"). I am fine with that. We would need to adapt the documentation in
> > > > > Documentation/input/event-codes.txt and make sure the handlers are
> > > > > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > > > > the events in userspace).
> > > > 
> > > > Can you update the "TBD" in Documentation/input/event-codes.txt so we
> > > > have an agreed description to follow?
> > > 
> > > Here's a first attempt:
> > > 
> > >  * SYN_CONFIG:
> > >    - Used to indicate that the device's static description has changed. This
> > 
> > Should we indicate the obvious fact that this is only after the device has
> > been registered and presented to userspace? I can imagine drivers adding
> > those notifications after each declaration while preparing the input
> > node :)
> 
> added, see below
> 
> > 
> > >      can happen when
> > >      - at least one event type or code has been added or removed, or
> > >      - at least one device property has been added or removed, or
> > >      - at least one absolute axis has changed properties, or
> > >      - the keycode has changed, or
> > >      - the name, id, phys or unique identifier of the device has changed,
> > >      A client should re-query the device once a SYN_CONFIG has been received
> > >      as if the device was newly added. SYN_CONFIG does not indicate which
> > >      information changed, a client is required to re-read the full 
> > >      state. A SYN_CONFIG may be triggered even by types/codes/... that the
> > >      client is not aware of, i.e. the state before and after may look
> > >      identical to any given client.
> > >      Dynamic values such as key state, switch state, absolute axis value,
> > >      force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
> > >      trigger a SYN_DROPPED to terminate ongoing event sequences.
> > 
> > I think we should enforce this last sentence a little bit more:
> > "A SYN_CONFIG is basically a reset of the device so it should be
> > considered as a SYN_DROPPED from the client perspective. For backward
> > compatibility with clients not supporting SYN_CONFIG, a SYN_DROPPED
> > event is sent right before SYN_CONFIG."
> 
> Are we planning to force a SYN_DROPPED every time? Even when the event queue
> is empty? I think it wouldn't do much other than keep the client busy. 
> 
> Come to think of it, there's a chance that this *reduces* backwards
> compatibility. Until a SYN_DROPPED occurs, a client has no reason to ever
> check bits again. But re-fetching all states during SYN_DROPPED is normal,
> so if one axis disappears, the client may now get confused.

Ouch, very much ouch.

Maybe SYN_CONFIG should be limited to only props, axis ranges and new
axis?

> 
> > Also, maybe we should add a warning about the udev properties:
> > "If a SYN_CONFIG allows to change the static description of a device, it
> > should be used with care. Some userspace pieces (udev) rely on the
> > device creation to tag it properly. Completely changing the device type
> > from a mouse to a touchpad is better handled through the destruction and
> > then creation of a new input node than relying on SYN_CONFIG"
> > 
> > (please fix typos/grammar/jibberish)
> > 
> > > 
> > > We should also add a blurb to the EVIOC*MASK ioctls about SYN_CONFIG.
> > 
> > Saying that it can't be masked and that it will invalidate the current
> > masks? Or this is the responsibility of the client?
> 
> I would just leave the masks as-is, less work and more predictability. This
> means that a client has to re-apply them on SYN_CONFIG to avoid events from
> new axes (if any) but that seems like the most straightforward approach. We
> already allow masking out axes a device doesn't have, so invalidating
> doesn't really give us any benefits. 
> 
> Ok, let's have a v2 draft:
> 
> * SYN_CONFIG:
>   - Used to indicate that the device's static description has changed, after
>     the device has been registered and presented to userspace.
>     This can happen when
>     - at least one event type or code has been added or removed, or
>     - at least one device property has been added or removed, or
>     - at least one absolute axis has changed properties, or
>     - the keycode has changed, or
>     - the name, id, phys or unique identifier of the device has changed,
>     Do not send this event during the initial device initialization process,
>     this event is exclusively for changes at runtime.
> 
>     A client should re-query the device once a SYN_CONFIG has been received
>     as if the device was newly added. SYN_CONFIG does not indicate which
>     information changed, a client is required to re-read the full 
>     state. A SYN_CONFIG may be triggered even by types/codes/... that the
>     client is not aware of, i.e. the state before and after may look
>     identical to any given client.
> 
>     Dynamic values such as key state, switch state, absolute axis value,
>     force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
>     trigger a SYN_DROPPED to terminate ongoing event sequences.
> 
>     SYN_CONFIG should be used with extreme care. There is no guarantee that
>     a SYN_CONFIG event is handled correctly by a client, or even noticed.
>     For example, udev relies on the original device description only to
>     assign a device type. If a SYN_CONFIG were to change the device
>     sufficiently to warrant a new device type tag, this may never be
>     detected. Changing a device type (e.g. from mouse to touchpad) is better
>     handled through the destruction and recreation of the input node.

I like it. Besides the legacy breakage we are talking about above.

Cheers,
Benjamin

> 
> Cheers,
>    Peter
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Peter Hutterer Jan. 12, 2017, 4:25 a.m. UTC | #13

On Tue, Jan 10, 2017 at 09:47:24AM +0100, Benjamin Tissoires wrote:
> On Jan 06 2017 or thereabouts, Peter Hutterer wrote:
> > On Wed, Jan 04, 2017 at 10:27:15AM +0100, Benjamin Tissoires wrote:
> > > On Jan 04 2017 or thereabouts, Peter Hutterer wrote:
> > > > On Tue, Jan 03, 2017 at 02:33:36PM -0800, Ping Cheng wrote:
> > > > > Hi Dmitry,
> > > > > 
> > > > > On Tue, Jan 3, 2017 at 1:30 AM, Benjamin Tissoires
> > > > > <benjamin.tissoires@redhat.com> wrote:
> > > > > > On Dec 22 2016 or thereabouts, Dmitry Torokhov wrote:
> > > > > >> On Mon, Dec 19, 2016 at 11:30:13AM +0100, Jiri Kosina wrote:
> > > > > >> > On Fri, 16 Dec 2016, Ping Cheng wrote:
> > > > > >> >
> > > > > >> > > Wacom Cintiq Pro [1] is a series of display tablets. They support
> > > > > >> > > 5 touch keys on the tablet. Those keys represent specific functions.
> > > > > >> > > They turn display off; bring up OSD; bring up On Screen Keyboard;
> > > > > >> > > bring up desktop control panel; and turn touch on/off.
> > > > > >> > >
> > > > > >> > > The most left touch key, that turns display on/off, is controlled by
> > > > > >> > > firmware. When the display is on, the mode is set. Otherwise, the
> > > > > >> > > mode is off. So, it works like a switch. That's why we introduced a
> > > > > >> > > new switch: SW_INDIRECT. The switch defauts to INDIRECT instead of DIRECT
> > > > > >> > > was a request from useland, more specifically Gnome, developers.
> > > > > >> > >
> > > > > >> > > Other four touch keys are true software keys. We use the existing
> > > > > >> > > KEY_BUTTONCONFIG and KEY_CONTROLPANEL for OSD and control panel. But,
> > > > > >> > > we have to request two new keys: KEY_ONSCREEN_KEYBOARD and KEY_MUTE_DEVICE
> > > > > >> > > since none of the existing keys support those two actions.
> > > > > >> > >
> > > > > >> > > [1] http://www.wacom.com/en-us/products/pen-displays/wacom-cintiq-pro
> > > > > >> > >
> > > > > >> > > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > > > > >> > > ---
> > > > > >> > > v3: Since no one has followed up with v2, let's add some more comments for
> > > > > >> > > SW_INDIRECT so we keep the offlist decision public ;).
> > > > > >> >
> > > > > >> > [ ... snip ... ]
> > > > > >> >
> > > > > >> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > > > >> > > index d6d071f..32ef894 100644
> > > > > >> > > --- a/include/uapi/linux/input-event-codes.h
> > > > > >> > > +++ b/include/uapi/linux/input-event-codes.h
> > > > > >> > > @@ -641,6 +641,9 @@
> > > > > >> > >   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
> > > > > >> > >   */
> > > > > >> > >  #define KEY_DATA                 0x275
> > > > > >> > > +/* same as SW_MUTE_DEVICE but triggered by a key */
> > > > > >> > > +#define KEY_MUTE_DEVICE          0x278
> > > > > >> > > +#define KEY_ONSCREEN_KEYBOARD            0x279
> > > > > >> > >
> > > > > >> > >  #define BTN_TRIGGER_HAPPY                0x2c0
> > > > > >> > >  #define BTN_TRIGGER_HAPPY1               0x2c0
> > > > > >> > > @@ -781,7 +784,8 @@
> > > > > >> > >  #define SW_LINEIN_INSERT 0x0d  /* set = inserted */
> > > > > >> > >  #define SW_MUTE_DEVICE           0x0e  /* set = device disabled */
> > > > > >> > >  #define SW_PEN_INSERTED          0x0f  /* set = pen inserted */
> > > > > >> > > -#define SW_MAX                   0x0f
> > > > > >> > > +#define SW_INDIRECT              0x10  /* set = not a direct input device */
> > > > > >> > > +#define SW_MAX                   0x1f
> > > > > >> >
> > > > > >> > I'd like to have explicit Ack from Dmitry on these ... Dmitry?
> > > > > >>
> > > > > >> Sorry for the delay, but I think adding SW_INDIRECT is actually wrong.
> > > > > >>
> > > > > >> What Wacom folks seem to need is way to re-classfiy the device (i.e.
> > > > > >> update its properties) and let userspace know somehow. We can't keep
> > > > > >> adding SW_INDIRECT and SW_NOTPOINTER and SW_NOTBUTTONPAD and so forth.
> > > > > >> We however have EV_SYN/SYN_CONFIG that we may use to let userspace know
> > > > > >> that device configuration changed and that userspace needs to
> > > > > >> interrogate the device again. We can emit this code both when we change
> > > > > >> properties as well as when we change ABS limits and changing keymaps and
> > > > > >> so forth.
> > > > > 
> > > > > By "to interrogate the device again",  do you mean once we report
> > > > > EV_SYN/SYN_CONFIG, userspace needs to reinitialize the device?
> > > > > 
> > > > > If not, how do we tell userspace which feature/info has been changed?
> > > > > 
> > > > > >>
> > > > > >> Benjamin, Peter, any opinion on the above?
> > > > > >
> > > > > > Oooh, so that's the purpose of this event :) (the documentation says
> > > > > > "TBD"). I am fine with that. We would need to adapt the documentation in
> > > > > > Documentation/input/event-codes.txt and make sure the handlers are
> > > > > > properly adapted too (*maybe* add a SYN_DROP event to empty the queue of
> > > > > > the events in userspace).
> > > > > 
> > > > > Can you update the "TBD" in Documentation/input/event-codes.txt so we
> > > > > have an agreed description to follow?
> > > > 
> > > > Here's a first attempt:
> > > > 
> > > >  * SYN_CONFIG:
> > > >    - Used to indicate that the device's static description has changed. This
> > > 
> > > Should we indicate the obvious fact that this is only after the device has
> > > been registered and presented to userspace? I can imagine drivers adding
> > > those notifications after each declaration while preparing the input
> > > node :)
> > 
> > added, see below
> > 
> > > 
> > > >      can happen when
> > > >      - at least one event type or code has been added or removed, or
> > > >      - at least one device property has been added or removed, or
> > > >      - at least one absolute axis has changed properties, or
> > > >      - the keycode has changed, or
> > > >      - the name, id, phys or unique identifier of the device has changed,
> > > >      A client should re-query the device once a SYN_CONFIG has been received
> > > >      as if the device was newly added. SYN_CONFIG does not indicate which
> > > >      information changed, a client is required to re-read the full 
> > > >      state. A SYN_CONFIG may be triggered even by types/codes/... that the
> > > >      client is not aware of, i.e. the state before and after may look
> > > >      identical to any given client.
> > > >      Dynamic values such as key state, switch state, absolute axis value,
> > > >      force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
> > > >      trigger a SYN_DROPPED to terminate ongoing event sequences.
> > > 
> > > I think we should enforce this last sentence a little bit more:
> > > "A SYN_CONFIG is basically a reset of the device so it should be
> > > considered as a SYN_DROPPED from the client perspective. For backward
> > > compatibility with clients not supporting SYN_CONFIG, a SYN_DROPPED
> > > event is sent right before SYN_CONFIG."
> > 
> > Are we planning to force a SYN_DROPPED every time? Even when the event queue
> > is empty? I think it wouldn't do much other than keep the client busy. 
> > 
> > Come to think of it, there's a chance that this *reduces* backwards
> > compatibility. Until a SYN_DROPPED occurs, a client has no reason to ever
> > check bits again. But re-fetching all states during SYN_DROPPED is normal,
> > so if one axis disappears, the client may now get confused.
> 
> Ouch, very much ouch.
> 
> Maybe SYN_CONFIG should be limited to only props, axis ranges and new
> axis?
> 
> > 
> > > Also, maybe we should add a warning about the udev properties:
> > > "If a SYN_CONFIG allows to change the static description of a device, it
> > > should be used with care. Some userspace pieces (udev) rely on the
> > > device creation to tag it properly. Completely changing the device type
> > > from a mouse to a touchpad is better handled through the destruction and
> > > then creation of a new input node than relying on SYN_CONFIG"
> > > 
> > > (please fix typos/grammar/jibberish)
> > > 
> > > > 
> > > > We should also add a blurb to the EVIOC*MASK ioctls about SYN_CONFIG.
> > > 
> > > Saying that it can't be masked and that it will invalidate the current
> > > masks? Or this is the responsibility of the client?
> > 
> > I would just leave the masks as-is, less work and more predictability. This
> > means that a client has to re-apply them on SYN_CONFIG to avoid events from
> > new axes (if any) but that seems like the most straightforward approach. We
> > already allow masking out axes a device doesn't have, so invalidating
> > doesn't really give us any benefits. 
> > 
> > Ok, let's have a v2 draft:
> > 
> > * SYN_CONFIG:
> >   - Used to indicate that the device's static description has changed, after
> >     the device has been registered and presented to userspace.
> >     This can happen when
> >     - at least one event type or code has been added or removed, or
> >     - at least one device property has been added or removed, or
> >     - at least one absolute axis has changed properties, or
> >     - the keycode has changed, or
> >     - the name, id, phys or unique identifier of the device has changed,
> >     Do not send this event during the initial device initialization process,
> >     this event is exclusively for changes at runtime.
> > 
> >     A client should re-query the device once a SYN_CONFIG has been received
> >     as if the device was newly added. SYN_CONFIG does not indicate which
> >     information changed, a client is required to re-read the full 
> >     state. A SYN_CONFIG may be triggered even by types/codes/... that the
> >     client is not aware of, i.e. the state before and after may look
> >     identical to any given client.
> > 
> >     Dynamic values such as key state, switch state, absolute axis value,
> >     force feedback, etc. do not trigger a SYN_CONFIG. A SYN_CONFIG may
> >     trigger a SYN_DROPPED to terminate ongoing event sequences.
> > 
> >     SYN_CONFIG should be used with extreme care. There is no guarantee that
> >     a SYN_CONFIG event is handled correctly by a client, or even noticed.
> >     For example, udev relies on the original device description only to
> >     assign a device type. If a SYN_CONFIG were to change the device
> >     sufficiently to warrant a new device type tag, this may never be
> >     detected. Changing a device type (e.g. from mouse to touchpad) is better
> >     handled through the destruction and recreation of the input node.
> 
> I like it. Besides the legacy breakage we are talking about above.

For the archives: we're currently looking into a workaround for this issue
on this device. if that works, the discussion on whether and how to use
SYN_CONFIG can be pushed off to the next device that really needs it.

Cheers,
   Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[v3] HID: wacom: generic: add 5 tablet touch keys

Commit Message

Comments

Patch