Re: [PATCH 2/2] xen/kbdif: add multi-touch support

[Oleksandr Andrushchenko <andr2000@gmail.com>]

On 01/11/2017 02:29 AM, Stefano Stabellini wrote:
> On Tue, 10 Jan 2017, Oleksandr Andrushchenko wrote:
>> On 01/07/2017 12:37 AM, Stefano Stabellini wrote:
>>> On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote:
>>>> From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>>>>
>>>> Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>>>> ---
>>>>    xen/include/public/io/kbdif.h | 228
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 228 insertions(+)
>>>>
>>>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>>>> index 0e19a40..1b446f9 100644
>>>> --- a/xen/include/public/io/kbdif.h
>>>> +++ b/xen/include/public/io/kbdif.h
>>>> @@ -57,6 +57,12 @@
>>>>     *      Backends, which support reporting of absolute coordinates for
>>>> pointer
>>>>     *      device should set this to 1.
>>>>     *
>>>> + * feature-multi-touch
>>>> + *      Values:         <uint>
>>>> + *
>>>> + *      Backends, which support reporting of multi-touch events
>>>> + *      should set this to 1.
>>>> + *
>>>>     *------------------------- Pointer Device Parameters
>>>> ------------------------
>>>>     *
>>>>     * width
>>>> @@ -87,6 +93,11 @@
>>>>     *      Request from backend to report absolute pointer coordinates
>>>>     *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
>>>>     *
>>>> + * request-multi-touch
>>>> + *      Values:         <uint>
>>>> + *
>>>> + *      Request from backend to report multi-touch events.
>>> I think in English should be "request backend to report multi-touch
>>> events". Same above.
>> Done
>>>> + *
>>>>     *----------------------- Request Transport Parameters
>>>> -----------------------
>>>>     *
>>>>     * event-channel
>>>> @@ -107,6 +118,43 @@
>>>>     *      OBSOLETE, not recommended for use.
>>>>     *      A unique (within the guest domain given) value identifying event
>>>>     *      channel and page reference pair.
>>>> + *
>>>> + *----------------------- Multi-touch Device Parameters
>>>> -----------------------
>>>> + *
>>>> + * Every multi-touch input device uses a dedicated event ring and is
>>> For clarity I would say "Every multi-touch input device uses a dedicated
>>> frontend/backend connection". That includes a ring, an event channel,
>>> and xenstore entries.
>>>
>> Done
>>>> + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder.
>>> I don't understand why we need a new xenstore folder. Why do we need
>>> XENKBD_PATH_MTOUCH?
>> This is just another (IMO, cleaner) approach to define
>> properties in XenStore for multiple instances.
>> Let's see what vif does on my PC:
>> /local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304"
>> ...
>> /local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306"
>> ...
>> /local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308"
>> ...
>> /local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310"
>> ...
>> /local/domain/11/device/vif/0/request-rx-copy = "1"
>>
>> We have folders "queue-%d" per queue which in my case are
>>
>> under XENKBD_PATH_MTOUCH.
>>
>> /local/domain/11/device/vkbd/0/mtouch/0/width = "3200"
>> /local/domain/11/device/vkbd/0/mtouch/0/height = "3200"
>> /local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5"
>> /local/domain/11/device/vkbd/0/mtouch/1/width = "6400"
>> /local/domain/11/device/vkbd/0/mtouch/1/height = "6400"
>> /local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10"
>>
>> Namely, instead of "mtouch-%d" I use "mtouch/%d/.
>> This is just like using "/local/domain/%d" but not
>> "/local/domain-%d", so "mtouch/%d" seem to be more
>> consistent.
> I agree that mtouch/%d is better than mtouch-%d, but it's only necessary
> if we have more than one mtouch per vkbd. I would configure it so that
> one can only have one mtouch per vkbd:
>
>    /local/domain/11/device/vkbd/0/mtouch/width = "3200"
>    /local/domain/11/device/vkbd/0/mtouch/height = "3200"
>    /local/domain/11/device/vkbd/1/mtouch/width = "6400"
>    /local/domain/11/device/vkbd/1/mtouch/height = "6400"
correct me if I'm wrong, but this notation implies
multiple drivers support, not a single driver with
multiple devices.
If so, *DISCLAIMER* *Linux* I see no simple solution to
do that in the front driver.
Could you please clarify?
> This is also what I was referring to when I wrote:
>
>> It is useful to distinguish mouse events from keyboard events, from
>> multitouch events more easily, but I don't think we should support
>> multiple keyboards or multiple mice on the same xenkbd ring. If we need
>> two mice, we can setup two xenkdb rings.
> The same applies to multitouch devices, I think.
>
> But maybe you have good reasons for supporting multiple multitouch
> devices on a single vkbd connection? How many do you expect to have on a
> single VM?
well, let me put the whole tree from xenstore:

/local/domain/0/backend/vkbd/2/0/frontend-id = "2"
/local/domain/0/backend/vkbd/2/0/frontend = "/local/domain/2/device/vkbd/0"
/local/domain/0/backend/vkbd/2/0/dev_id = "0"
/local/domain/0/backend/vkbd/2/0/state = "6"
/local/domain/0/backend/vkbd/2/0/feature-abs-pointer = "1"
/local/domain/0/backend/vkbd/2/0/width = "2048"
/local/domain/0/backend/vkbd/2/0/height = "2048"
/local/domain/0/backend/vkbd/2/0/feature-multi-touch = "1"

/local/domain/2/device/vkbd/0/backend-id = "0"
/local/domain/2/device/vkbd/0/backend = "/local/domain/0/backend/vkbd/2/0"
/local/domain/2/device/vkbd/0/state = "6"
/local/domain/2/device/vkbd/0/request-abs-pointer = "1"
/local/domain/2/device/vkbd/0/page-ref = "1248025"
/local/domain/2/device/vkbd/0/page-gref = "336"
/local/domain/2/device/vkbd/0/event-channel = "43"

/local/domain/2/device/vkbd/0/request-multi-touch = "1"

/local/domain/2/device/vkbd/0/mtouch/0/width = "3200"
/local/domain/2/device/vkbd/0/mtouch/0/height = "3200"
/local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5"
/local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386"
/local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335"
/local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44"

/local/domain/2/device/vkbd/0/mtouch/1/width = "6400"
/local/domain/2/device/vkbd/0/mtouch/1/height = "6400"
/local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10"
/local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038"
/local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337"
/local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45"

 From the above:
1. No change to the existing kbd+ptr:
1.1. They still share a dedicated (single) connection
channel as they did before multi-touch.
1.2. No multi-touch events via that connection
1.3. Old backends/frontends will see no difference
2. Each multi-touch device has its own:
2.1. Configuration (width x height, num-contacts)
2.2. Own connection channel (page-ref, event-channel)

So, for the example above: 1 kbd + 1 ptr + 2 mt devices
within a single driver, 3 connections

Note: this doesn't allow multiple kbd and ptr devices.

Hope this answers your questions,
Oleksandr

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel