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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 11 Jan 2017, Oleksandr Andrushchenko wrote:
> 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@xxxxxxxx>
> > > > > 
> > > > > Signed-off-by: Oleksandr Andrushchenko
> > > > > <Oleksandr_Andrushchenko@xxxxxxxx>
> > > > > ---
> > > > >    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?

I am not familiar with the multitouch driver infrastructure in Linux.
Why is it easier to advertise multiple multitouch devices under a single
xenstore connection? I would have thought it would be more difficult to
handle from Linux point of view.


> > 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel