Olaf Hering
2011-Mar-10 17:58 UTC
[Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
A virtualized display device is usually viewed with the vncviewer application, either by ''xm vnc domU'' or with vncviewer localhost:port. vncviewer and the RFB protocol provides absolute coordinates to the virtual display. These coordinates are either passed through to a PV guest or converted to relative coordinates for a HVM guest. A PV guest receives these coordinates and passes them to the kernels evdev driver. There it can be picked up by applications such as the xorg-input drivers. Using absolute coordinates avoids issues such as guest mouse pointer not tracking host mouse pointer due to wrong mouse acceleration settings in the guests X display. Advertise either absolute or relative coordinates to the input system and the evdev driver, depending on what dom0 provides. The xorg-input driver prefers relative coordinates even if a devices provides both. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- drivers/xen/fbfront/xenkbd.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) Index: linux-2.6.18-xen.hg/drivers/xen/fbfront/xenkbd.c ==================================================================--- linux-2.6.18-xen.hg.orig/drivers/xen/fbfront/xenkbd.c +++ linux-2.6.18-xen.hg/drivers/xen/fbfront/xenkbd.c @@ -104,7 +104,7 @@ static irqreturn_t input_handler(int rq, int __devinit xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { - int ret, i; + int ret, i, abs; struct xenkbd_info *info; struct input_dev *kbd, *ptr; @@ -123,6 +123,11 @@ int __devinit xenkbd_probe(struct xenbus info->page->in_cons = info->page->in_prod = 0; info->page->out_cons = info->page->out_prod = 0; + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-abs-pointer", "%d", &abs) < 0) + abs = 0; + if (abs) + xenbus_printf(XBT_NIL, dev->nodename, "request-abs-pointer", "1"); + /* keyboard */ kbd = input_allocate_device(); if (!kbd) @@ -155,10 +160,15 @@ int __devinit xenkbd_probe(struct xenbus ptr->id.bustype = BUS_PCI; ptr->id.vendor = 0x5853; ptr->id.product = 0xfffe; - ptr->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS); + ptr->evbit[0] = BIT(EV_KEY); + if (abs) + ptr->evbit[0] |= BIT(EV_ABS); + else { + ptr->evbit[0] |= BIT(EV_REL); + ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y) | BIT(REL_WHEEL); + } for (i = BTN_LEFT; i <= BTN_TASK; i++) set_bit(i, ptr->keybit); - ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y) | BIT(REL_WHEEL); input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0); input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0); @@ -263,7 +273,7 @@ static void xenkbd_backend_changed(struc enum xenbus_state backend_state) { struct xenkbd_info *info = dev->dev.driver_data; - int ret, val; + int val; switch (backend_state) { case XenbusStateInitialising: @@ -276,16 +286,6 @@ static void xenkbd_backend_changed(struc case XenbusStateInitWait: InitWait: - ret = xenbus_scanf(XBT_NIL, info->xbdev->otherend, - "feature-abs-pointer", "%d", &val); - if (ret < 0) - val = 0; - if (val) { - ret = xenbus_printf(XBT_NIL, info->xbdev->nodename, - "request-abs-pointer", "1"); - if (ret) - ; /* FIXME */ - } xenbus_switch_state(dev, XenbusStateConnected); break; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Stefano Stabellini
2011-Mar-10 18:09 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On Thu, 10 Mar 2011, Olaf Hering wrote:> A virtualized display device is usually viewed with the vncviewer > application, either by ''xm vnc domU'' or with vncviewer localhost:port. > vncviewer and the RFB protocol provides absolute coordinates to the > virtual display. These coordinates are either passed through to a PV > guest or converted to relative coordinates for a HVM guest. > > A PV guest receives these coordinates and passes them to the kernels > evdev driver. There it can be picked up by applications such as the > xorg-input drivers. Using absolute coordinates avoids issues such as > guest mouse pointer not tracking host mouse pointer due to wrong mouse > acceleration settings in the guests X display. > > Advertise either absolute or relative coordinates to the input system > and the evdev driver, depending on what dom0 provides. The xorg-input > driver prefers relative coordinates even if a devices provides both. > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > > --- > drivers/xen/fbfront/xenkbd.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > Index: linux-2.6.18-xen.hg/drivers/xen/fbfront/xenkbd.c > ==================================================================> --- linux-2.6.18-xen.hg.orig/drivers/xen/fbfront/xenkbd.c > +++ linux-2.6.18-xen.hg/drivers/xen/fbfront/xenkbd.c > @@ -104,7 +104,7 @@ static irqreturn_t input_handler(int rq, > int __devinit xenkbd_probe(struct xenbus_device *dev, > const struct xenbus_device_id *id) > { > - int ret, i; > + int ret, i, abs; > struct xenkbd_info *info; > struct input_dev *kbd, *ptr; > > @@ -123,6 +123,11 @@ int __devinit xenkbd_probe(struct xenbus > info->page->in_cons = info->page->in_prod = 0; > info->page->out_cons = info->page->out_prod = 0; > > + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-abs-pointer", "%d", &abs) < 0) > + abs = 0; > + if (abs) > + xenbus_printf(XBT_NIL, dev->nodename, "request-abs-pointer", "1"); > + > /* keyboard */ > kbd = input_allocate_device(); > if (!kbd) > @@ -155,10 +160,15 @@ int __devinit xenkbd_probe(struct xenbus > ptr->id.bustype = BUS_PCI; > ptr->id.vendor = 0x5853; > ptr->id.product = 0xfffe; > - ptr->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS); > + ptr->evbit[0] = BIT(EV_KEY); > + if (abs) > + ptr->evbit[0] |= BIT(EV_ABS); > + else { > + ptr->evbit[0] |= BIT(EV_REL); > + ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y) | BIT(REL_WHEEL); > + } > for (i = BTN_LEFT; i <= BTN_TASK; i++) > set_bit(i, ptr->keybit); > - ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y) | BIT(REL_WHEEL); > input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0); > input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0); >does this mean that we are not getting any wheel events when we are using absolute coordinates? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Olaf Hering
2011-Mar-10 18:14 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On Thu, Mar 10, Stefano Stabellini wrote:> does this mean that we are not getting any wheel events when we are > using absolute coordinates?Do you get them now? In my testing vncviewer does not pass them and qemu-dm does not receive them. Or whatever gets send to qemu-dm gets converted to abs_x+abs_y, if I read the code correctly. Olaf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Stefano Stabellini
2011-Mar-10 19:15 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On Thu, 10 Mar 2011, Olaf Hering wrote:> On Thu, Mar 10, Stefano Stabellini wrote: > > > does this mean that we are not getting any wheel events when we are > > using absolute coordinates? > > Do you get them now?Yes, if I disable absolute coordinates. In order to fix the wheel problem it is sufficient to add: + input_set_capability(ptr, EV_REL, REL_WHEEL); The following is a rebased version of your patch plus the small correction on the latest 2.6.38 RC, I am going to send it upstream if you don''t mind. --- From: Olaf Hering <olaf@aepfle.de> A virtualized display device is usually viewed with the vncviewer application, either by ''xm vnc domU'' or with vncviewer localhost:port. vncviewer and the RFB protocol provides absolute coordinates to the virtual display. These coordinates are either passed through to a PV guest or converted to relative coordinates for a HVM guest. A PV guest receives these coordinates and passes them to the kernels evdev driver. There it can be picked up by applications such as the xorg-input drivers. Using absolute coordinates avoids issues such as guest mouse pointer not tracking host mouse pointer due to wrong mouse acceleration settings in the guests X display. Advertise either absolute or relative coordinates to the input system and the evdev driver, depending on what dom0 provides. The xorg-input driver prefers relative coordinates even if a devices provides both. Signed-off-by: Olaf Hering <olaf@aepfle.de> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c index 7f85a86..24fc38b 100644 --- a/drivers/input/xen-kbdfront.c +++ b/drivers/input/xen-kbdfront.c @@ -110,7 +110,7 @@ static irqreturn_t input_handler(int rq, void *dev_id) static int __devinit xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { - int ret, i; + int ret, i, abs; struct xenkbd_info *info; struct input_dev *kbd, *ptr; @@ -128,6 +128,11 @@ static int __devinit xenkbd_probe(struct xenbus_device *dev, if (!info->page) goto error_nomem; + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-abs-pointer", "%d", &abs) < 0) + abs = 0; + if (abs) + xenbus_printf(XBT_NIL, dev->nodename, "request-abs-pointer", "1"); + /* keyboard */ kbd = input_allocate_device(); if (!kbd) @@ -160,10 +165,16 @@ static int __devinit xenkbd_probe(struct xenbus_device *dev, ptr->id.bustype = BUS_PCI; ptr->id.vendor = 0x5853; ptr->id.product = 0xfffe; - ptr->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS); + ptr->evbit[0] = BIT(EV_KEY); + if (abs) + ptr->evbit[0] |= BIT(EV_ABS); + else { + ptr->evbit[0] |= BIT(EV_REL); + ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y); + } + input_set_capability(ptr, EV_REL, REL_WHEEL); for (i = BTN_LEFT; i <= BTN_TASK; i++) set_bit(i, ptr->keybit); - ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y) | BIT(REL_WHEEL); input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0); input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0); @@ -272,7 +283,7 @@ static void xenkbd_backend_changed(struct xenbus_device *dev, enum xenbus_state backend_state) { struct xenkbd_info *info = dev_get_drvdata(&dev->dev); - int ret, val; + int val; switch (backend_state) { case XenbusStateInitialising: @@ -285,16 +296,6 @@ static void xenkbd_backend_changed(struct xenbus_device *dev, case XenbusStateInitWait: InitWait: - ret = xenbus_scanf(XBT_NIL, info->xbdev->otherend, - "feature-abs-pointer", "%d", &val); - if (ret < 0) - val = 0; - if (val) { - ret = xenbus_printf(XBT_NIL, info->xbdev->nodename, - "request-abs-pointer", "1"); - if (ret) - pr_warning("can''t request abs-pointer\n"); - } xenbus_switch_state(dev, XenbusStateConnected); break; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Eamon Walsh
2011-Mar-10 19:58 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
We have had an issue here where the "request-abs-pointer" key is appearing in XenStore _after_ the backend has gone to the Connected state. Qemu xenfb.c checks for this key in the "connect()" callback, causing it to miss the key and default to relative coordinates. xen be: vkbd-0: frontend update: page-ref xen be: vkbd-0: frontend not ready (yet) xen be: vkbd-0: frontend update: page-gref xen be: vkbd-0: frontend not ready (yet) xen be: vkbd-0: frontend update: event-channel xen be: vkbd-0: frontend not ready (yet) xen be: vkbd-0: frontend state: Initialising -> Initialised xen be: vkbd-0: frontend update: state xen be: vkbd-0: bind evtchn port 53 xen be: vkbd-0: ring ref 13, remote-port 11, local-port 53 xen be: vkbd-0: backend state: InitWait -> Connected <---- xen be: vkbd-0: backend update: state xen be: vkbd-0: frontend update: request-abs-pointer <---- xen be: vkbd-0: frontend state: Initialised -> Connected xen be: vkbd-0: frontend update: state This could be a problem specific to our display server, and it only happens sometimes. But we have been carrying the patch below in our xenfb.c. It makes sense to respond to a direct watch on the key itself instead of looking for it at a state change. This may be suitable for upstream to Qemu. I also note that the check in Qemu was previously made even before the connected state. It was changed in July, see: web.archiveorange.com/archive/v/1XS1v5z9uIQLGujGBDhT diff --git a/src/xenfb.c b/src/xenfb.c index a822057..5aade4c 100644 --- a/src/xenfb.c +++ b/src/xenfb.c @@ -181,10 +181,6 @@ static int input_connect(struct XenDevice *xendev) struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); int rc; - if (xenstore_read_fe_int(xendev, "request-abs-pointer", - &in->abs_pointer_wanted) == -1) - in->abs_pointer_wanted = 0; - rc = common_bind(&in->c); if (rc != 0) return rc; @@ -199,6 +195,17 @@ static void input_disconnect(struct XenDevice *xendev) xen_linpicker_input_disconnect(in); } +static void input_frontend_changed(struct XenDevice *xendev, const char *node) +{ + struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); + + if (strcmp(node, "request-abs-pointer") == 0) { + if (xenstore_read_fe_int(xendev, "request-abs-pointer", + &in->abs_pointer_wanted) < 0) + in->abs_pointer_wanted = 0; + } +} + static void input_event(struct XenDevice *xendev) { struct XenInput *xenfb = container_of(xendev, struct XenInput, c.xendev); @@ -754,6 +761,7 @@ struct XenDevOps xen_kbdmouse_ops = { .connect = input_connect, .disconnect = input_disconnect, .event = input_event, + .frontend_changed = input_frontend_changed, }; struct XenDevOps xen_framebuffer_ops = { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Stefano Stabellini
2011-Mar-11 11:58 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On Thu, 10 Mar 2011, Eamon Walsh wrote:> We have had an issue here where the "request-abs-pointer" key is appearing in XenStore _after_ the backend has gone to the Connected state. Qemu xenfb.c checks for this key in the "connect()" callback, causing it to miss the key and default to relative coordinates. > > xen be: vkbd-0: frontend update: page-ref > xen be: vkbd-0: frontend not ready (yet) > xen be: vkbd-0: frontend update: page-gref > xen be: vkbd-0: frontend not ready (yet) > xen be: vkbd-0: frontend update: event-channel > xen be: vkbd-0: frontend not ready (yet) > xen be: vkbd-0: frontend state: Initialising -> Initialised > xen be: vkbd-0: frontend update: state > xen be: vkbd-0: bind evtchn port 53 > xen be: vkbd-0: ring ref 13, remote-port 11, local-port 53 > xen be: vkbd-0: backend state: InitWait -> Connected <---- > xen be: vkbd-0: backend update: state > xen be: vkbd-0: frontend update: request-abs-pointer <---- > xen be: vkbd-0: frontend state: Initialised -> Connected > xen be: vkbd-0: frontend update: state > > > This could be a problem specific to our display server, and it only happens sometimes. But we have been carrying the patch below in our xenfb.c. It makes sense to respond to a direct watch on the key itself instead of looking for it at a state change. This may be suitable for upstream to Qemu. > > I also note that the check in Qemu was previously made even before the connected state. It was changed in July, see: > web.archiveorange.com/archive/v/1XS1v5z9uIQLGujGBDhT >The patch looks sane. Could you please rebase it on upstream qemu and send it to qemu-devel? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
John Haxby
2011-Mar-11 12:43 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On 10/03/11 19:58, Eamon Walsh wrote:> We have had an issue here where the "request-abs-pointer" key is appearing in XenStore _after_ the backend has gone to the Connected state. Qemu xenfb.c checks for this key in the "connect()" callback, causing it to miss the key and default to relative coordinates.This was submitted to qemu-devel ages ago -- 27-Aug-2010 according to my archives. That was in fact the second time it was submitted (by Stefano) but it was never picked up despite a couple of reminders :-( This is the original thread on xen-devel: markmail.org/thread/fikfzaxgaj4ay6oe And the resend-thread in qemu-devel: lists.gnu.org/archive/html/qemu-devel/2010-08/msg01515.html I think the patch you proposed, Eamon, is the same as the one that didn''t work properly -- it''s certainly a lot simpler than the two-part patch that I wrote (and that patch has been in production for OracleVM for a while, it seems solid). jch _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
John Haxby
2011-Mar-11 12:44 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On 11/03/11 11:58, Stefano Stabellini wrote:> The patch looks sane. > Could you please rebase it on upstream qemu and send it to qemu-devel? > >Please hold off until we find out what happened to the earlier patch. jch _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel
Stefano Stabellini
2011-Mar-11 12:54 UTC
Re: [Xen-devel] [PATCH] fbfront: advertise either absolute or relative coordinates
On Fri, 11 Mar 2011, John Haxby wrote:> On 11/03/11 11:58, Stefano Stabellini wrote: > > The patch looks sane. > > Could you please rebase it on upstream qemu and send it to qemu-devel? > > > Please hold off until we find out what happened to the earlier patch.You are right, sorry I forgot about those :P I think your two patches were a good fix, also I like the distinction between the connected and initialise state in xenstore. However they were lost somewhere in qemu-devel and never applied. I''ll try to prod the qemu maintainers again. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com lists.xensource.com/xen-devel