[patch] pvfb: Split mouse and keyboard into separate devices.

[patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

  Hi,

This patch creates two separate input devices for keyboard and mouse
events.  The reason for this is to separate them in the linux input
layer and allow them being routed different ways.

Use case:  Configure the X-Server like this to get the mouse
events directly from the linux input layer, which has the major
advantage that absolute coordinates work correctly:

Section "InputDevice"
  Driver       "evdev"
  Identifier   "Mouse[1]"
  Option       "Device" "/dev/input/event<nr>"
EndSection

This makes the keyboard stop working though in case mouse and
keyboard events are coming through the same input device.

please apply,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[-- Attachment #2: fix-xenkbd.diff --]
[-- Type: text/x-patch, Size: 4826 bytes --]

pvfb: Split mouse and keyboard into separate devices.

This patch creates two separate input devices for keyboard and mouse
events.  The reason for this is to separate them in the linux input
layer and allow them being routed different ways.

Use case:  Configure the X-Server like this to get the mouse
events directly from the linux input layer, which has the major
advantage that absolute coordinates work correctly:

Section "InputDevice"
  Driver       "evdev"
  Identifier   "Mouse[1]"
  Option       "Device" "/dev/input/event<nr>"
EndSection

This makes the keyboard stop working though in case mouse and
keyboard events are coming through the same input device.

Signed-off-by: Gerd Hoffmann <kraxel@suse.de>
Cc: Markus Armbruster <armbru@redhat.com>
---
 linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c |   60 +++++++++++++---------
 1 file changed, 38 insertions(+), 22 deletions(-)

Index: build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
===================================================================
--- build-64-unstable-13762.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
+++ build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
@@ -29,7 +29,8 @@
 
 struct xenkbd_info
 {
-	struct input_dev *dev;
+	struct input_dev *kbd;
+	struct input_dev *ptr;
 	struct xenkbd_page *page;
 	int irq;
 	struct xenbus_device *xbdev;
@@ -60,19 +61,21 @@ static irqreturn_t input_handler(int rq,
 
 		switch (event->type) {
 		case XENKBD_TYPE_MOTION:
-			input_report_rel(info->dev, REL_X, event->motion.rel_x);
-			input_report_rel(info->dev, REL_Y, event->motion.rel_y);
+			input_report_rel(info->ptr, REL_X, event->motion.rel_x);
+			input_report_rel(info->ptr, REL_Y, event->motion.rel_y);
+			input_sync(info->ptr);
 			break;
 		case XENKBD_TYPE_KEY:
-			input_report_key(info->dev, event->key.keycode, event->key.pressed);
+			input_report_key(info->kbd, event->key.keycode, event->key.pressed);
+			input_sync(info->kbd);
 			break;
 		case XENKBD_TYPE_POS:
-			input_report_abs(info->dev, ABS_X, event->pos.abs_x);
-			input_report_abs(info->dev, ABS_Y, event->pos.abs_y);
+			input_report_abs(info->ptr, ABS_X, event->pos.abs_x);
+			input_report_abs(info->ptr, ABS_Y, event->pos.abs_y);
+			input_sync(info->ptr);
 			break;
 		}
 	}
-	input_sync(info->dev);
 	mb();			/* ensure we got ring contents */
 	page->in_cons = cons;
 	notify_remote_via_irq(info->irq);
@@ -85,7 +88,7 @@ int __devinit xenkbd_probe(struct xenbus
 {
 	int ret, i;
 	struct xenkbd_info *info;
-	struct input_dev *input_dev;
+	struct input_dev *kbd, *ptr;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {
@@ -101,32 +104,44 @@ int __devinit xenkbd_probe(struct xenbus
 	info->page->in_cons = info->page->in_prod = 0;
 	info->page->out_cons = info->page->out_prod = 0;
 
-	input_dev = input_allocate_device();
-	if (!input_dev)
+	kbd = input_allocate_device();
+	ptr = input_allocate_device();
+	if (!kbd || !ptr)
 		goto error_nomem;
 
-	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
-	input_dev->keybit[LONG(BTN_MOUSE)]
+	ptr->evbit[0] = BIT(EV_REL) | BIT(EV_ABS);
+	ptr->keybit[LONG(BTN_MOUSE)]
 		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
 	/* TODO additional buttons */
-	input_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
+	ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
 
+	kbd->evbit[0] = BIT(EV_KEY);
 	/* FIXME not sure this is quite right */
 	for (i = 0; i < 256; i++)
-		set_bit(i, input_dev->keybit);
+		set_bit(i, kbd->keybit);
 
-	input_dev->name = "Xen Virtual Keyboard/Mouse";
+	kbd->name = "Xen Virtual Keyboard";
+	ptr->name = "Xen Virtual Touchscreen";
 
-	input_set_abs_params(input_dev, ABS_X, 0, XENFB_WIDTH, 0, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
+	input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0);
+	input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
 
-	ret = input_register_device(input_dev);
+	ret = input_register_device(kbd);
 	if (ret) {
-		input_free_device(input_dev);
-		xenbus_dev_fatal(dev, ret, "input_register_device");
+		input_free_device(kbd);
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
 		goto error;
 	}
-	info->dev = input_dev;
+	info->kbd = kbd;
+
+	ret = input_register_device(ptr);
+	if (ret) {
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
+		goto error;
+	}
+	info->ptr = ptr;
 
 	ret = xenkbd_connect_backend(dev, info);
 	if (ret < 0)
@@ -155,7 +170,8 @@ static int xenkbd_remove(struct xenbus_d
 	struct xenkbd_info *info = dev->dev.driver_data;
 
 	xenkbd_disconnect_backend(info);
-	input_unregister_device(info->dev);
+	input_unregister_device(info->kbd);
+	input_unregister_device(info->ptr);
 	free_page((unsigned long)info->page);
 	kfree(info);
 	return 0;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Markus Armbruster]

Gerd Hoffmann <kraxel@suse.de> writes:

>   Hi,
>
> This patch creates two separate input devices for keyboard and mouse
> events.  The reason for this is to separate them in the linux input
> layer and allow them being routed different ways.
>
> Use case:  Configure the X-Server like this to get the mouse
> events directly from the linux input layer, which has the major
> advantage that absolute coordinates work correctly:
>
> Section "InputDevice"
>   Driver       "evdev"
>   Identifier   "Mouse[1]"
>   Option       "Device" "/dev/input/event<nr>"
> EndSection
>
> This makes the keyboard stop working though in case mouse and
> keyboard events are coming through the same input device.

Review below.

I'll leave judging whether this is a good idea to those who know more
about X than I do.

> please apply,
>   Gerd
>
> -- 
> Gerd Hoffmann <kraxel@suse.de>
[...]
> ---
>  linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c |   60 +++++++++++++---------
>  1 file changed, 38 insertions(+), 22 deletions(-)
>
> Index: build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
> ===================================================================
> --- build-64-unstable-13762.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
> +++ build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
> @@ -29,7 +29,8 @@
>  
>  struct xenkbd_info
>  {
> -	struct input_dev *dev;
> +	struct input_dev *kbd;
> +	struct input_dev *ptr;
>  	struct xenkbd_page *page;
>  	int irq;
>  	struct xenbus_device *xbdev;
> @@ -60,19 +61,21 @@ static irqreturn_t input_handler(int rq,
>  
>  		switch (event->type) {
>  		case XENKBD_TYPE_MOTION:
> -			input_report_rel(info->dev, REL_X, event->motion.rel_x);
> -			input_report_rel(info->dev, REL_Y, event->motion.rel_y);
> +			input_report_rel(info->ptr, REL_X, event->motion.rel_x);
> +			input_report_rel(info->ptr, REL_Y, event->motion.rel_y);
> +			input_sync(info->ptr);
>  			break;
>  		case XENKBD_TYPE_KEY:
> -			input_report_key(info->dev, event->key.keycode, event->key.pressed);
> +			input_report_key(info->kbd, event->key.keycode, event->key.pressed);
> +			input_sync(info->kbd);
>  			break;
>  		case XENKBD_TYPE_POS:
> -			input_report_abs(info->dev, ABS_X, event->pos.abs_x);
> -			input_report_abs(info->dev, ABS_Y, event->pos.abs_y);
> +			input_report_abs(info->ptr, ABS_X, event->pos.abs_x);
> +			input_report_abs(info->ptr, ABS_Y, event->pos.abs_y);
> +			input_sync(info->ptr);
>  			break;
>  		}
>  	}
> -	input_sync(info->dev);
>  	mb();			/* ensure we got ring contents */
>  	page->in_cons = cons;
>  	notify_remote_via_irq(info->irq);
> @@ -85,7 +88,7 @@ int __devinit xenkbd_probe(struct xenbus
>  {
>  	int ret, i;
>  	struct xenkbd_info *info;
> -	struct input_dev *input_dev;
> +	struct input_dev *kbd, *ptr;
>  
>  	info = kzalloc(sizeof(*info), GFP_KERNEL);
>  	if (!info) {
> @@ -101,32 +104,44 @@ int __devinit xenkbd_probe(struct xenbus
>  	info->page->in_cons = info->page->in_prod = 0;
>  	info->page->out_cons = info->page->out_prod = 0;
>  
> -	input_dev = input_allocate_device();
> -	if (!input_dev)
> +	kbd = input_allocate_device();
> +	ptr = input_allocate_device();
> +	if (!kbd || !ptr)
>  		goto error_nomem;

If just one of two fails, the other is leaked, I fear.

>  
> -	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
> -	input_dev->keybit[LONG(BTN_MOUSE)]
> +	ptr->evbit[0] = BIT(EV_REL) | BIT(EV_ABS);
> +	ptr->keybit[LONG(BTN_MOUSE)]
>  		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
>  	/* TODO additional buttons */
> -	input_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
> +	ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
>  
> +	kbd->evbit[0] = BIT(EV_KEY);
>  	/* FIXME not sure this is quite right */
>  	for (i = 0; i < 256; i++)
> -		set_bit(i, input_dev->keybit);
> +		set_bit(i, kbd->keybit);
>  
> -	input_dev->name = "Xen Virtual Keyboard/Mouse";
> +	kbd->name = "Xen Virtual Keyboard";
> +	ptr->name = "Xen Virtual Touchscreen";

Touchscreen is going to confuse people.  I'd rather call it a mouse.
Or pointing device, if need be.

>  
> -	input_set_abs_params(input_dev, ABS_X, 0, XENFB_WIDTH, 0, 0);
> -	input_set_abs_params(input_dev, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
> +	input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0);
> +	input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
>  
> -	ret = input_register_device(input_dev);
> +	ret = input_register_device(kbd);
>  	if (ret) {
> -		input_free_device(input_dev);
> -		xenbus_dev_fatal(dev, ret, "input_register_device");
> +		input_free_device(kbd);
> +		input_free_device(ptr);
> +		xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
>  		goto error;
>  	}
> -	info->dev = input_dev;
> +	info->kbd = kbd;
> +
> +	ret = input_register_device(ptr);
> +	if (ret) {
> +		input_free_device(ptr);
> +		xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
> +		goto error;
> +	}
> +	info->ptr = ptr;
>  
>  	ret = xenkbd_connect_backend(dev, info);
>  	if (ret < 0)


It *might* be simpler to have something like

 error:
        if (ptr)
        	input_free_device(ptr);
        if (kbd)
        	input_free_device(kbd);

> @@ -155,7 +170,8 @@ static int xenkbd_remove(struct xenbus_d
>  	struct xenkbd_info *info = dev->dev.driver_data;
>  
>  	xenkbd_disconnect_backend(info);
> -	input_unregister_device(info->dev);
> +	input_unregister_device(info->kbd);
> +	input_unregister_device(info->ptr);
>  	free_page((unsigned long)info->page);
>  	kfree(info);
>  	return 0;

Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

Markus Armbruster wrote:
> Review below.
>> +	kbd = input_allocate_device();
>> +	ptr = input_allocate_device();
>> +	if (!kbd || !ptr)
>>  		goto error_nomem;
> 
> If just one of two fails, the other is leaked, I fear.

Yep, you are right, fixed version attached.

> It *might* be simpler to have something like
> 
>  error:
>         if (ptr)
>         	input_free_device(ptr);
>         if (kbd)
>         	input_free_device(kbd);

I don't think it would simplify the code because you'll have to take
care then that the xenkbd_remove() call in the error path doesn't result
in a double-free ...

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[-- Attachment #2: fix-xenkbd.diff --]
[-- Type: text/x-patch, Size: 5063 bytes --]

pvfb: Split mouse and keyboard into separate devices.

This patch creates two separate input devices for keyboard and mouse
events.  The reason for this is to separate them in the linux input
layer and allow them being routed different ways.

Use case:  Configure the X-Server like this to get the mouse
events directly from the linux input layer, which has the major
advantage that absolute coordinates work correctly:

Section "InputDevice"
  Driver       "evdev"
  Identifier   "Mouse[1]"
  Option       "Device" "/dev/input/event<nr>"
EndSection

This makes the keyboard stop working though in case mouse and
keyboard events are coming through the same input device.

Signed-off-by: Gerd Hoffmann <kraxel@suse.de>
Cc: Markus Armbruster <armbru@redhat.com>
---
 linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c |   64 ++++++++++++++--------
 1 file changed, 42 insertions(+), 22 deletions(-)

Index: build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
===================================================================
--- build-64-unstable-13762.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
+++ build-64-unstable-13762/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
@@ -29,7 +29,8 @@
 
 struct xenkbd_info
 {
-	struct input_dev *dev;
+	struct input_dev *kbd;
+	struct input_dev *ptr;
 	struct xenkbd_page *page;
 	int irq;
 	struct xenbus_device *xbdev;
@@ -60,19 +61,21 @@ static irqreturn_t input_handler(int rq,
 
 		switch (event->type) {
 		case XENKBD_TYPE_MOTION:
-			input_report_rel(info->dev, REL_X, event->motion.rel_x);
-			input_report_rel(info->dev, REL_Y, event->motion.rel_y);
+			input_report_rel(info->ptr, REL_X, event->motion.rel_x);
+			input_report_rel(info->ptr, REL_Y, event->motion.rel_y);
+			input_sync(info->ptr);
 			break;
 		case XENKBD_TYPE_KEY:
-			input_report_key(info->dev, event->key.keycode, event->key.pressed);
+			input_report_key(info->kbd, event->key.keycode, event->key.pressed);
+			input_sync(info->kbd);
 			break;
 		case XENKBD_TYPE_POS:
-			input_report_abs(info->dev, ABS_X, event->pos.abs_x);
-			input_report_abs(info->dev, ABS_Y, event->pos.abs_y);
+			input_report_abs(info->ptr, ABS_X, event->pos.abs_x);
+			input_report_abs(info->ptr, ABS_Y, event->pos.abs_y);
+			input_sync(info->ptr);
 			break;
 		}
 	}
-	input_sync(info->dev);
 	mb();			/* ensure we got ring contents */
 	page->in_cons = cons;
 	notify_remote_via_irq(info->irq);
@@ -85,7 +88,7 @@ int __devinit xenkbd_probe(struct xenbus
 {
 	int ret, i;
 	struct xenkbd_info *info;
-	struct input_dev *input_dev;
+	struct input_dev *kbd, *ptr;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {
@@ -101,32 +104,46 @@ int __devinit xenkbd_probe(struct xenbus
 	info->page->in_cons = info->page->in_prod = 0;
 	info->page->out_cons = info->page->out_prod = 0;
 
-	input_dev = input_allocate_device();
-	if (!input_dev)
+	kbd = input_allocate_device();
+	if (!kbd)
 		goto error_nomem;
+	ptr = input_allocate_device();
+	if (!ptr)
+		goto error_nomem_2;
 
-	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
-	input_dev->keybit[LONG(BTN_MOUSE)]
+	ptr->evbit[0] = BIT(EV_REL) | BIT(EV_ABS);
+	ptr->keybit[LONG(BTN_MOUSE)]
 		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
 	/* TODO additional buttons */
-	input_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
+	ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
 
+	kbd->evbit[0] = BIT(EV_KEY);
 	/* FIXME not sure this is quite right */
 	for (i = 0; i < 256; i++)
-		set_bit(i, input_dev->keybit);
+		set_bit(i, kbd->keybit);
 
-	input_dev->name = "Xen Virtual Keyboard/Mouse";
+	kbd->name = "Xen Virtual Keyboard";
+	ptr->name = "Xen Virtual Touchscreen";
 
-	input_set_abs_params(input_dev, ABS_X, 0, XENFB_WIDTH, 0, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
+	input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0);
+	input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
 
-	ret = input_register_device(input_dev);
+	ret = input_register_device(kbd);
 	if (ret) {
-		input_free_device(input_dev);
-		xenbus_dev_fatal(dev, ret, "input_register_device");
+		input_free_device(kbd);
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
 		goto error;
 	}
-	info->dev = input_dev;
+	info->kbd = kbd;
+
+	ret = input_register_device(ptr);
+	if (ret) {
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
+		goto error;
+	}
+	info->ptr = ptr;
 
 	ret = xenkbd_connect_backend(dev, info);
 	if (ret < 0)
@@ -134,6 +151,8 @@ int __devinit xenkbd_probe(struct xenbus
 
 	return 0;
 
+ error_nomem_2:
+	input_free_device(kbd);
  error_nomem:
 	ret = -ENOMEM;
 	xenbus_dev_fatal(dev, ret, "allocating device memory");
@@ -155,7 +174,8 @@ static int xenkbd_remove(struct xenbus_d
 	struct xenkbd_info *info = dev->dev.driver_data;
 
 	xenkbd_disconnect_backend(info);
-	input_unregister_device(info->dev);
+	input_unregister_device(info->kbd);
+	input_unregister_device(info->ptr);
 	free_page((unsigned long)info->page);
 	kfree(info);
 	return 0;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Markus Armbruster]

Gerd Hoffmann <kraxel@suse.de> writes:

>   Hi,
>
> This patch creates two separate input devices for keyboard and mouse
> events.  The reason for this is to separate them in the linux input
> layer and allow them being routed different ways.
>
> Use case:  Configure the X-Server like this to get the mouse
> events directly from the linux input layer, which has the major
> advantage that absolute coordinates work correctly:
>
> Section "InputDevice"
>   Driver       "evdev"
>   Identifier   "Mouse[1]"
>   Option       "Device" "/dev/input/event<nr>"
> EndSection
>
> This makes the keyboard stop working though in case mouse and
> keyboard events are coming through the same input device.
>
> please apply,

I tried it out.  With the configuration sketched above, I get a
working keyboard and mouse, and the mouse behaves much better than
before.  However, with the default configuration, the mouse doesn't
work at all.

The drawbacks of just applying the patch are obvious, I think.

Gerd, any ideas how to make X recognize the split devices
automatically?

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Daniel P. Berrange]

On Thu, Feb 01, 2007 at 06:37:12PM +0100, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@suse.de> writes:
> 
> >   Hi,
> >
> > This patch creates two separate input devices for keyboard and mouse
> > events.  The reason for this is to separate them in the linux input
> > layer and allow them being routed different ways.
> >
> > Use case:  Configure the X-Server like this to get the mouse
> > events directly from the linux input layer, which has the major
> > advantage that absolute coordinates work correctly:
> >
> > Section "InputDevice"
> >   Driver       "evdev"
> >   Identifier   "Mouse[1]"
> >   Option       "Device" "/dev/input/event<nr>"
> > EndSection
> >
> > This makes the keyboard stop working though in case mouse and
> > keyboard events are coming through the same input device.
> >
> > please apply,
> 
> I tried it out.  With the configuration sketched above, I get a
> working keyboard and mouse, and the mouse behaves much better than
> before.  However, with the default configuration, the mouse doesn't
> work at all.

Yes, if the current Xorg server automatic configuration doesn't 'just work' 
for both keyboard and mouse then applying this patch is a non-starter. 

Is there some way we can keep the original device supplying both keyboard
and mouse events as before, and just have this second device as an opt-in
'absolute pointer' event device. That way existing Xorg setups will still
work correctly without needing any config changes, while providing the 
ability to opt-in to getting absolute events by adding the extra config.

Ideally Xorg hardware probing could then be modified, so that future Xorg
releases would automatically utilize the extra device (if present) for 
absolute co-ords without needing the extra config at all.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

Daniel P. Berrange wrote:
> Is there some way we can keep the original device supplying both keyboard
> and mouse events as before, and just have this second device as an opt-in
> 'absolute pointer' event device. That way existing Xorg setups will still
> work correctly without needing any config changes, while providing the 
> ability to opt-in to getting absolute events by adding the extra config.

Should be possible.  Some linux input layer background:

You'll get one /dev/input/event<nr> file per input device.

Applications can open it, they can ask for exclusive access to it (what
usually is a good idea), and if they do so nobody else gets events from
that device.

Input events which are not grabbed that way go the usual route through
the linux input layer and end up as normal key presses in the keyboard
driver or as mouse events in the ps/2 mouse emulation (aka /dev/input/mice).

Splitting keyboard and mouse into two devices makes it possible to
handle the mouse events via /dev/input/event<nr> and let the keyboard
events still go the usual route.

> Ideally Xorg hardware probing could then be modified, so that future Xorg
> releases would automatically utilize the extra device (if present) for 
> absolute co-ords without needing the extra config at all.

Xorg 7.2 comes with a manpage for evdev ;)

Have played only with sles10 guests so far, which unfortunaly has an
older Xorg version, where the evdev driver has much less features, thus
the stuff below is untested ...

Xorg 7.2 evdev can match input devices by name and by id, so you can add
a sections like this:

  Section "InputDevice"
    Driver       "evdev"
    Identifier   "Mouse[1]"
    Option       "Name" "Xen Virtual Pointer"
  EndSection

and be done with it.  The /dev/input/mice section can stay.  It doesn't
hurt, there are no mouse events coming due to evdev asking for exclusive
access.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Keir Fraser]

On 2/2/07 08:39, "Gerd Hoffmann" <kraxel@suse.de> wrote:

> Daniel P. Berrange wrote:
>> Is there some way we can keep the original device supplying both keyboard
>> and mouse events as before, and just have this second device as an opt-in
>> 'absolute pointer' event device. That way existing Xorg setups will still
>> work correctly without needing any config changes, while providing the
>> ability to opt-in to getting absolute events by adding the extra config.
> 
> Should be possible.

I guess I'll wait for this before applying the patch then.

 -- Keir

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

Keir Fraser wrote:
> On 2/2/07 08:39, "Gerd Hoffmann" <kraxel@suse.de> wrote:
> 
>> Daniel P. Berrange wrote:
>>> Is there some way we can keep the original device supplying both keyboard
>>> and mouse events as before, and just have this second device as an opt-in
>>> 'absolute pointer' event device. That way existing Xorg setups will still
>>> work correctly without needing any config changes, while providing the
>>> ability to opt-in to getting absolute events by adding the extra config.
>> Should be possible.
> 
> I guess I'll wait for this before applying the patch then.

With Xorg 7.2, this patch, and the device id patch on top you can add
this ...

Section "InputDevice"
  Driver        "evdev"
  Identifier    "xenptr"
  Option        "SendCoreEvents" "true"
  Option        "vendor"        "0x5853"
  Option        "product"       "0x0003"
EndSection

... as additional (to the default /dev/input/mice) input device.  The
new input device must also be added to the serverlayout section.
WorksForMe[tm].

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Keir Fraser]

On 2/2/07 16:29, "Gerd Hoffmann" <kraxel@suse.de> wrote:

>> I guess I'll wait for this before applying the patch then.
> 
> With Xorg 7.2, this patch, and the device id patch on top you can add
> this ...
> 
> Section "InputDevice"
>   Driver        "evdev"
>   Identifier    "xenptr"
>   Option        "SendCoreEvents" "true"
>   Option        "vendor"        "0x5853"
>   Option        "product"       "0x0003"
> EndSection
> 
> ... as additional (to the default /dev/input/mice) input device.  The
> new input device must also be added to the serverlayout section.
> WorksForMe[tm].

The complaint is that it doesn't work out of the box with current Xorg. If
that is a showstopper (it certainly is at the very least very undesirable!)
then it doesn't matter how simple the required config change is -- if any
change is required at all then the patch is untenable.

How hard would it be to contineu to provide the combined device as well as
the new split-out pointer device?

 -- Keir

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Daniel P. Berrange]

[-- Attachment #1: Type: text/plain, Size: 3481 bytes --]

On Fri, Feb 02, 2007 at 06:11:54PM +0000, Keir Fraser wrote:
> On 2/2/07 16:29, "Gerd Hoffmann" <kraxel@suse.de> wrote:
> 
> >> I guess I'll wait for this before applying the patch then.
> > 
> > With Xorg 7.2, this patch, and the device id patch on top you can add
> > this ...
> > 
> > Section "InputDevice"
> >   Driver        "evdev"
> >   Identifier    "xenptr"
> >   Option        "SendCoreEvents" "true"
> >   Option        "vendor"        "0x5853"
> >   Option        "product"       "0x0003"
> > EndSection
> > 
> > ... as additional (to the default /dev/input/mice) input device.  The
> > new input device must also be added to the serverlayout section.
> > WorksForMe[tm].
> 
> The complaint is that it doesn't work out of the box with current Xorg. If
> that is a showstopper (it certainly is at the very least very undesirable!)
> then it doesn't matter how simple the required config change is -- if any
> change is required at all then the patch is untenable.
> 
> How hard would it be to contineu to provide the combined device as well as
> the new split-out pointer device?

I've just hacked up such a version and it appears to work fine - although
you need a slightly more complicated Xorg config to get the absolute
pointer working.

So what the patch does is this:

  - One input device supplies both mouse & keyboard events - this is 
    basically same as current PVFB setup (appears /dev/input/event0)
  - A second device supplies only mouse events (/dev/input/event1)

So with a default Xorg config, X works just as before - the server sees
the relative co-ords via its default 'mouse' driver.

If we want to explicitly configure absolute co-ords, we first define an
input device for /dev/input/event1 - this deals with the pure mouse
only stream of absolute coords. The evdev driver ensures that the events
from event1 no longer get reported via the unified mouse channel. Of 
course we still have the relative coords coming in on event0 though 
and thus into X via the 'mouse' driver which mess things up. So we have
a second 'void' input device which explicitly kills off the default
mouse handling. The result, mouse events only get processed from event1

Section "ServerLayout"
   ...snip...
       InputDevice    "Mouse0" "CorePointer"
       InputDevice    "KillDefaultMouse"
EndSection

Section "InputDevice"
       Identifier  "Mouse0"
       Driver      "evdev"
       Option      "Device" "/dev/input/event1"
EndSection

Section "InputDevice"
       Identifier  "KillDefaultMouse"
       Driver      "void"
EndSection


The attached patch was against Gerd's first version of the patch, so don't
have his other fixes in yet. But it shows we can setup the input devices
such that existing relative coords work with no config changes, while allowing 
people to opt in to using absolute coords only via a xorg.conf change. 
We probably want to adjust the naming again such that event0 is called 
'Xen Virtual Keyboard/Mouse' while event1 is just called 'Xen Virtual Mouse".
This gives compatability with original xenkbd driver for programs like kudzu 
which might be doing  device lookups based on the current naming.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

[-- Attachment #2: xen-abs-mouse.patch --]
[-- Type: text/plain, Size: 5475 bytes --]

diff -r 2e80cd715047 linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
--- a/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c	Thu Feb 01 11:42:50 2007 +0000
+++ b/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c	Fri Feb 02 19:17:00 2007 -0500
@@ -29,8 +29,10 @@
 
 struct xenkbd_info
 {
-	struct input_dev *dev;
+	struct input_dev *kbd;
+	struct input_dev *ptr;
 	struct xenkbd_page *page;
+	unsigned evtchn;
 	int irq;
 	struct xenbus_device *xbdev;
 };
@@ -60,22 +62,30 @@ static irqreturn_t input_handler(int rq,
 
 		switch (event->type) {
 		case XENKBD_TYPE_MOTION:
-			input_report_rel(info->dev, REL_X, event->motion.rel_x);
-			input_report_rel(info->dev, REL_Y, event->motion.rel_y);
+			input_report_rel(info->ptr, REL_X, event->motion.rel_x);
+			input_report_rel(info->ptr, REL_Y, event->motion.rel_y);
+			input_sync(info->ptr);
+			input_report_rel(info->kbd, REL_X, event->motion.rel_x);
+			input_report_rel(info->kbd, REL_Y, event->motion.rel_y);
+			input_sync(info->kbd);
 			break;
 		case XENKBD_TYPE_KEY:
-			input_report_key(info->dev, event->key.keycode, event->key.pressed);
+			input_report_key(info->kbd, event->key.keycode, event->key.pressed);
+			input_sync(info->kbd);
 			break;
 		case XENKBD_TYPE_POS:
-			input_report_abs(info->dev, ABS_X, event->pos.abs_x);
-			input_report_abs(info->dev, ABS_Y, event->pos.abs_y);
+			input_report_abs(info->ptr, ABS_X, event->pos.abs_x);
+			input_report_abs(info->ptr, ABS_Y, event->pos.abs_y);
+			input_sync(info->ptr);
+			input_report_abs(info->kbd, ABS_X, event->pos.abs_x);
+			input_report_abs(info->kbd, ABS_Y, event->pos.abs_y);
+			input_sync(info->kbd);
 			break;
 		}
 	}
-	input_sync(info->dev);
 	mb();			/* ensure we got ring contents */
 	page->in_cons = cons;
-	notify_remote_via_irq(info->irq);
+	notify_remote_via_evtchn(info->evtchn);
 
 	return IRQ_HANDLED;
 }
@@ -85,7 +95,7 @@ int __devinit xenkbd_probe(struct xenbus
 {
 	int ret, i;
 	struct xenkbd_info *info;
-	struct input_dev *input_dev;
+	struct input_dev *kbd, *ptr;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {
@@ -101,32 +111,50 @@ int __devinit xenkbd_probe(struct xenbus
 	info->page->in_cons = info->page->in_prod = 0;
 	info->page->out_cons = info->page->out_prod = 0;
 
-	input_dev = input_allocate_device();
-	if (!input_dev)
+	kbd = input_allocate_device();
+	ptr = input_allocate_device();
+	if (!kbd || !ptr)
 		goto error_nomem;
 
-	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
-	input_dev->keybit[LONG(BTN_MOUSE)]
+	ptr->evbit[0] = BIT(EV_REL) | BIT(EV_ABS);
+	ptr->keybit[LONG(BTN_MOUSE)]
 		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
 	/* TODO additional buttons */
-	input_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
-
+	ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
+
+	kbd->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
+	kbd->keybit[LONG(BTN_MOUSE)]
+		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
+	/* TODO additional buttons */
+	kbd->relbit[0] = BIT(REL_X) | BIT(REL_Y);
 	/* FIXME not sure this is quite right */
 	for (i = 0; i < 256; i++)
-		set_bit(i, input_dev->keybit);
-
-	input_dev->name = "Xen Virtual Keyboard/Mouse";
-
-	input_set_abs_params(input_dev, ABS_X, 0, XENFB_WIDTH, 0, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
-
-	ret = input_register_device(input_dev);
-	if (ret) {
-		input_free_device(input_dev);
-		xenbus_dev_fatal(dev, ret, "input_register_device");
+		set_bit(i, kbd->keybit);
+
+	kbd->name = "Xen Virtual Keyboard";
+	ptr->name = "Xen Virtual Touchscreen";
+
+	input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0);
+	input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
+	input_set_abs_params(kbd, ABS_X, 0, XENFB_WIDTH, 0, 0);
+	input_set_abs_params(kbd, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
+
+	ret = input_register_device(kbd);
+	if (ret) {
+		input_free_device(kbd);
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
 		goto error;
 	}
-	info->dev = input_dev;
+	info->kbd = kbd;
+
+	ret = input_register_device(ptr);
+	if (ret) {
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
+		goto error;
+	}
+	info->ptr = ptr;
 
 	ret = xenkbd_connect_backend(dev, info);
 	if (ret < 0)
@@ -155,7 +183,8 @@ static int xenkbd_remove(struct xenbus_d
 	struct xenkbd_info *info = dev->dev.driver_data;
 
 	xenkbd_disconnect_backend(info);
-	input_unregister_device(info->dev);
+	input_unregister_device(info->kbd);
+	input_unregister_device(info->ptr);
 	free_page((unsigned long)info->page);
 	kfree(info);
 	return 0;
@@ -167,11 +196,14 @@ static int xenkbd_connect_backend(struct
 	int ret;
 	struct xenbus_transaction xbt;
 
-	ret = bind_listening_port_to_irqhandler(
-		dev->otherend_id, input_handler, 0, "xenkbd", info);
+	ret = xenbus_alloc_evtchn(dev, &info->evtchn);
+	if (ret)
+		return ret;
+	ret = bind_evtchn_to_irqhandler(info->evtchn, input_handler, 0,
+					"xenkbd", info);
 	if (ret < 0) {
-		xenbus_dev_fatal(dev, ret,
-				 "bind_listening_port_to_irqhandler");
+		xenbus_free_evtchn(dev, info->evtchn);
+		xenbus_dev_fatal(dev, ret, "bind_evtchn_to_irqhandler");
 		return ret;
 	}
 	info->irq = ret;
@@ -187,7 +219,7 @@ static int xenkbd_connect_backend(struct
 	if (ret)
 		goto error_xenbus;
 	ret = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
-			    irq_to_evtchn_port(info->irq));
+			    info->evtchn);
 	if (ret)
 		goto error_xenbus;
 	ret = xenbus_transaction_end(xbt, 0);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Daniel P. Berrange]

On Sat, Feb 03, 2007 at 12:28:25AM +0000, Daniel P. Berrange wrote:
> On Fri, Feb 02, 2007 at 06:11:54PM +0000, Keir Fraser wrote:
> > On 2/2/07 16:29, "Gerd Hoffmann" <kraxel@suse.de> wrote:
> > 
> > >> I guess I'll wait for this before applying the patch then.
> > > 
> > > With Xorg 7.2, this patch, and the device id patch on top you can add
> > > this ...
> > > 
> > > Section "InputDevice"
> > >   Driver        "evdev"
> > >   Identifier    "xenptr"
> > >   Option        "SendCoreEvents" "true"
> > >   Option        "vendor"        "0x5853"
> > >   Option        "product"       "0x0003"
> > > EndSection
> > > 
> > > ... as additional (to the default /dev/input/mice) input device.  The
> > > new input device must also be added to the serverlayout section.
> > > WorksForMe[tm].
> > 
> > The complaint is that it doesn't work out of the box with current Xorg. If
> > that is a showstopper (it certainly is at the very least very undesirable!)
> > then it doesn't matter how simple the required config change is -- if any
> > change is required at all then the patch is untenable.
> > 
> > How hard would it be to contineu to provide the combined device as well as
> > the new split-out pointer device?
> 
> I've just hacked up such a version and it appears to work fine - although
> you need a slightly more complicated Xorg config to get the absolute
> pointer working.
> 
> So what the patch does is this:
> 
>   - One input device supplies both mouse & keyboard events - this is 
>     basically same as current PVFB setup (appears /dev/input/event0)
>   - A second device supplies only mouse events (/dev/input/event1)
> 
> So with a default Xorg config, X works just as before - the server sees
> the relative co-ords via its default 'mouse' driver.

Well, this all really got me wondering why are we trying to jump through
all these hoops to let have keyboard events routed differently from
mouse events. Since a single /dev/input/eventX device can provide both
keyboard and mouse events, and the evdev driver is perfectly capable of 
processing both keyboard and mouse events, why can't a single InputDevice
config do the whole job.

So I tried out:

  Section "ServerLayout"
        Identifier     "Default Layout"
        Screen      0  "Screen0" 0 0
        InputDevice    "XenInput0" "CoreKeyboard"
        InputDevice    "XenInput0" "CorePointer"
  EndSection

  Section "InputDevice"
        Identifier  "XenInput0"
        Driver      "evdev"
        Option      "Device" "/dev/input/event0"
  EndSection

This made X rather unhappy - it fails to start, segv'ing badly. So, I put 
back in the generic keyboard device and set is as the CoreKeyboard:

  Section "InputDevice"
        Identifier  "Keyboard0"
        Driver      "kbd"
        Option      "XkbModel" "pc105"
        Option      "XkbLayout" "us"
  EndSection

X now starts, but the Keyboard0 driver never sees any key events. Then it
occurred to me that perhaps if I set 'SendCoreEvents' in the XenInput0
device, the keyboard events would get merged into the feed from Keyboard0.
Which they did.  So unless I'm missing something, it would seem that all
the patches in this thread are unneccessary. We can simply have a single
input device, and have the flexibility of being able to let X autoconfigure
itself in relative mouse mode, or explicitly add the config to switch it
in absolute mouse mode. 

For the record, my complete config file which makes absolute mouse events
work is:

  Section "ServerLayout"
        Identifier     "Default Layout"
        Screen      0  "Screen0" 0 0
        InputDevice    "Keyboard0" "CoreKeyboard"
        InputDevice    "XenInput0" "CorePointer"
  EndSection

  Section "ServerFlags"
        Option      "AllowMouseOpenFail" "yes"
  EndSection

  Section "InputDevice"
        Identifier  "XenInput0"
        Driver      "evdev"
        Option      "CorePointer"
        Option      "SendCoreEvents"
        Option      "Device" "/dev/input/event0"
        Option      "XkbModel" "pc105"
        Option      "XkbLayout" "us"
  EndSection

  Section "InputDevice"
        Identifier  "Keyboard0"
        Driver      "kbd"
        Option      "XkbModel" "pc105"
        Option      "XkbLayout" "us"
  EndSection

  Section "Device"
        Identifier  "Videocard0"
        Driver      "fbdev"
  EndSection

  Section "Screen"
        Identifier "Screen0"
        Device     "Videocard0"
        DefaultDepth     24
        SubSection "Display"
                Viewport   0 0
                Depth     24
        EndSubSection
  EndSection

NB, the Fedora Core 6  evdev has a bug in it causing slightly jittery
mouse even in absolute mode, but there's trivial fix for it, and I think
the upstream version is already fixed.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

Daniel P. Berrange wrote:
>   - One input device supplies both mouse & keyboard events - this is 
>     basically same as current PVFB setup (appears /dev/input/event0)
>   - A second device supplies only mouse events (/dev/input/event1)

Bad idea IMHO ...

> from event1 no longer get reported via the unified mouse channel. Of 
> course we still have the relative coords coming in on event0 though 
> and thus into X via the 'mouse' driver which mess things up.

... exactly thats why.

As long as /dev/input/event* isn't used there is absolutely no
difference in having a single or two separate devices for keyboard and
mouse events.  In both cases all events go through the kernel's keyboard
driver and /dev/input/mice.  The mouse "just works" with the default
X-Server configuration, using relative coordinates though.

Having two separate devices allows to handle mouse events only via
/dev/input/event to (optionally) have a better configuration with
absolute coordinates, without messing up the keyboard.

You can even create a configuration file which works fine in both cases:
xen virtual mouse being present and being not present.

Why do you want to keep the device with both keyboard and mouse events?
 It makes things much more complicated IMHO.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

Daniel P. Berrange wrote:
> Well, this all really got me wondering why are we trying to jump through
> all these hoops to let have keyboard events routed differently from
> mouse events.

Simply because it causes much less trouble.  With real, physical devices
it rarely happens that you have *one* device for *both* keyboard and
mouse.  Applications are simply not prepared to handle that ...

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

Daniel P. Berrange wrote:
>   Section "InputDevice"
>         Identifier  "XenInput0"
>         Driver      "evdev"
>         Option      "CorePointer"
>         Option      "SendCoreEvents"
>         Option      "Device" "/dev/input/event0"
>         Option      "XkbModel" "pc105"
>         Option      "XkbLayout" "us"
>   EndSection

That doesn't work with older Xorg (aka evdev driver) versions ...

I've attached a newer version of the patch, also my xorg.conf config
file (for Xorg 7.2+).  It is possible to match capability bits in the
config file.  Using this it is easy to make Xorg use evdev for pointing
devices which deliver absolute coordinates.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[-- Attachment #2: fix-xenkbd.diff --]
[-- Type: text/x-patch, Size: 5731 bytes --]

pvfb: Split mouse and keyboard into separate devices.

This patch creates two separate input devices for keyboard and mouse
events.  The reason for this is to separate them in the linux input
layer and allow them being routed different ways.

Use case:  Configure the X-Server like this to get the mouse
events directly from the linux input layer, which has the major
advantage that absolute coordinates work correctly:

Section "InputDevice"
  Driver       "evdev"
  Identifier   "Mouse[1]"
  Option       "Device" "/dev/input/event<nr>"
EndSection

This makes the keyboard stop working though in case mouse and
keyboard events are coming through the same input device.

Signed-off-by: Gerd Hoffmann <kraxel@suse.de>
Cc: Markus Armbruster <armbru@redhat.com>
---
 linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c |   80 ++++++++++++++--------
 1 file changed, 54 insertions(+), 26 deletions(-)

Index: build-32-unstable-13800/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
===================================================================
--- build-32-unstable-13800.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
+++ build-32-unstable-13800/linux-2.6-xen-sparse/drivers/xen/fbfront/xenkbd.c
@@ -29,10 +29,12 @@
 
 struct xenkbd_info
 {
-	struct input_dev *dev;
+	struct input_dev *kbd;
+	struct input_dev *ptr;
 	struct xenkbd_page *page;
 	int irq;
 	struct xenbus_device *xbdev;
+	char phys[32];
 };
 
 static int xenkbd_remove(struct xenbus_device *);
@@ -56,23 +58,27 @@ static irqreturn_t input_handler(int rq,
 	rmb();			/* ensure we see ring contents up to prod */
 	for (cons = page->in_cons; cons != prod; cons++) {
 		union xenkbd_in_event *event;
+		struct input_dev *dev;
 		event = &XENKBD_IN_RING_REF(page, cons);
 
+		dev = info->ptr;
 		switch (event->type) {
 		case XENKBD_TYPE_MOTION:
-			input_report_rel(info->dev, REL_X, event->motion.rel_x);
-			input_report_rel(info->dev, REL_Y, event->motion.rel_y);
+			input_report_rel(dev, REL_X, event->motion.rel_x);
+			input_report_rel(dev, REL_Y, event->motion.rel_y);
 			break;
 		case XENKBD_TYPE_KEY:
-			input_report_key(info->dev, event->key.keycode, event->key.pressed);
+			if (test_bit(event->key.keycode, info->kbd->keybit))
+				dev = info->kbd;
+			input_report_key(dev, event->key.keycode, event->key.pressed);
 			break;
 		case XENKBD_TYPE_POS:
-			input_report_abs(info->dev, ABS_X, event->pos.abs_x);
-			input_report_abs(info->dev, ABS_Y, event->pos.abs_y);
+			input_report_abs(dev, ABS_X, event->pos.abs_x);
+			input_report_abs(dev, ABS_Y, event->pos.abs_y);
 			break;
 		}
+		input_sync(dev);
 	}
-	input_sync(info->dev);
 	mb();			/* ensure we got ring contents */
 	page->in_cons = cons;
 	notify_remote_via_irq(info->irq);
@@ -85,7 +91,7 @@ int __devinit xenkbd_probe(struct xenbus
 {
 	int ret, i;
 	struct xenkbd_info *info;
-	struct input_dev *input_dev;
+	struct input_dev *kbd, *ptr;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {
@@ -94,6 +100,7 @@ int __devinit xenkbd_probe(struct xenbus
 	}
 	dev->dev.driver_data = info;
 	info->xbdev = dev;
+	snprintf(info->phys, sizeof(info->phys), "xenbus/%s", dev->nodename);
 
 	info->page = (void *)__get_free_page(GFP_KERNEL);
 	if (!info->page)
@@ -101,32 +108,52 @@ int __devinit xenkbd_probe(struct xenbus
 	info->page->in_cons = info->page->in_prod = 0;
 	info->page->out_cons = info->page->out_prod = 0;
 
-	input_dev = input_allocate_device();
-	if (!input_dev)
+	/* keyboard */
+	kbd = input_allocate_device();
+	if (!kbd)
 		goto error_nomem;
-
-	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
-	input_dev->keybit[LONG(BTN_MOUSE)]
-		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
-	/* TODO additional buttons */
-	input_dev->relbit[0] = BIT(REL_X) | BIT(REL_Y);
-
+	kbd->name = "Xen Virtual Keyboard";
+	kbd->phys = info->phys;
+	kbd->id.bustype = BUS_PCI;
+	kbd->id.vendor = 0x5853;
+	kbd->id.product = 0x2;
+	kbd->evbit[0] = BIT(EV_KEY);
 	/* FIXME not sure this is quite right */
 	for (i = 0; i < 256; i++)
-		set_bit(i, input_dev->keybit);
+		set_bit(i, kbd->keybit);
 
-	input_dev->name = "Xen Virtual Keyboard/Mouse";
+	ret = input_register_device(kbd);
+	if (ret) {
+		input_free_device(kbd);
+		xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
+		goto error;
+	}
+	info->kbd = kbd;
 
-	input_set_abs_params(input_dev, ABS_X, 0, XENFB_WIDTH, 0, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
+	/* pointing device */
+	ptr = input_allocate_device();
+	if (!ptr)
+		goto error_nomem;
+	ptr->name = "Xen Virtual Pointer";
+	ptr->phys = info->phys;
+	ptr->id.bustype = BUS_PCI;
+	ptr->id.vendor = 0x5853;
+	ptr->id.product = 0x3;
+	ptr->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
+	ptr->keybit[LONG(BTN_MOUSE)]
+		= BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
+	/* TODO additional buttons */
+	ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
+	input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0);
+	input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0);
 
-	ret = input_register_device(input_dev);
+	ret = input_register_device(ptr);
 	if (ret) {
-		input_free_device(input_dev);
-		xenbus_dev_fatal(dev, ret, "input_register_device");
+		input_free_device(ptr);
+		xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
 		goto error;
 	}
-	info->dev = input_dev;
+	info->ptr = ptr;
 
 	ret = xenkbd_connect_backend(dev, info);
 	if (ret < 0)
@@ -155,7 +182,8 @@ static int xenkbd_remove(struct xenbus_d
 	struct xenkbd_info *info = dev->dev.driver_data;
 
 	xenkbd_disconnect_backend(info);
-	input_unregister_device(info->dev);
+	input_unregister_device(info->kbd);
+	input_unregister_device(info->ptr);
 	free_page((unsigned long)info->page);
 	kfree(info);
 	return 0;

[-- Attachment #3: xorg.conf --]
[-- Type: text/plain, Size: 947 bytes --]

# xorg 7.2+ config file, fbdev

# default keyboard
Section "InputDevice"
  Driver	"kbd"
  Identifier	"Keyboard"
  Option	"XkbModel"	"pc105"
  Option	"XkbLayout"	"us"
EndSection

# default mouse
Section "InputDevice"
  Driver	"mouse"
  Identifier	"Mouse"
  Option	"Device"	"/dev/input/mice"
EndSection

# pointing device sending absolute x/y coordinates,
# works much better with evdev driver.
Section "InputDevice"
  Driver	"evdev"
  Identifier	"AbsPtr"
  Option	"SendCoreEvents" "true"
  Option        "evBits"	"+1 +3"		# ev_key, ev_abs
  Option        "keyBits"	"~256-287"	# btn_*
  Option	"absBits"	"+0 +1"		# abs_x, abs_y
EndSection

Section "Screen"
  Device	"fbdev"
  Identifier	"fbdev"
EndSection

Section "Device"
  Driver	"fbdev"
  Identifier	"fbdev"
EndSection

Section "ServerLayout"
  Identifier	"default"
  Screen	"fbdev"
  InputDevice	"Keyboard"	"CoreKeyboard"
  InputDevice	"Mouse"		"CorePointer"
  InputDevice	"AbsPtr"
EndSection


[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Daniel P. Berrange]

On Mon, Feb 05, 2007 at 10:10:45AM +0100, Gerd Hoffmann wrote:
> Daniel P. Berrange wrote:
> >   - One input device supplies both mouse & keyboard events - this is 
> >     basically same as current PVFB setup (appears /dev/input/event0)
> >   - A second device supplies only mouse events (/dev/input/event1)
> 
> Bad idea IMHO ...

Yep, that's unneccessary since I realized you can have a single
device doing both mouse&keyboard, and get absolute co-ords from it 
with no issues.

> > from event1 no longer get reported via the unified mouse channel. Of 
> > course we still have the relative coords coming in on event0 though 
> > and thus into X via the 'mouse' driver which mess things up.
> 
> ... exactly thats why.
> 
> As long as /dev/input/event* isn't used there is absolutely no
> difference in having a single or two separate devices for keyboard and
> mouse events.  In both cases all events go through the kernel's keyboard
> driver and /dev/input/mice.  The mouse "just works" with the default
> X-Server configuration, using relative coordinates though.

Yep, I was mistaken in my previous mail about the dual device not working
with relative mouse out of the box.

> Having two separate devices allows to handle mouse events only via
> /dev/input/event to (optionally) have a better configuration with
> absolute coordinates, without messing up the keyboard.
> 
> You can even create a configuration file which works fine in both cases:
> xen virtual mouse being present and being not present.
> 
> Why do you want to keep the device with both keyboard and mouse events?
>
>  It makes things much more complicated IMHO.

AFAICT, there is zero difference in complexity of configuration. In both
cases, if you have a default Xorg config, the relative mouse will work
out of the box.

With the existing single keyboard+mouse device there is a single
config section to add:

  Section "InputDevice"
        Identifier  "XenInput0"
        Driver      "evdev"
        Option      "CorePointer"
        Option      "SendCoreEvents"
        Option      "Device" "/dev/input/event0"
        Option      "XkbModel" "pc105"
        Option      "XkbLayout" "us"
  EndSection

(Possibily with the extra 'Option' tags you noted in the previous
 mail for legacy xorg releases of evdev.)

While with the two separate devices, there is a single section to add:

  Section "InputDevice"
        Identifier  "XenInput0"
        Driver      "evdev"
        Option      "CorePointer"
        Option      "Device" "/dev/input/event1"
        Option      "XkbModel" "pc105"
        Option      "XkbLayout" "us"
  EndSection

So there is only 1 line difference in the Xorg config required to get the
absolute mouse working in both cases. If we were implementing PVFB from
scratch today, I think I'd agree that having separate devices for mouse
and keyboard would be the approach to take. At this time, though we already
have done releases with the current single device. Both approaches have the
same end result - relative mouse just works, and absolute mouse requires 1
section added to the xorg config. 

In addition, Xorg is moving towards auto-configuring all devices so I hope
that we'll be able to get X to auto-configure absolute mouse correctly
without need for any config at all regardless of which impl we have.

So I don't really see any compelling reason to change the way the input
devices are exposed.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

  Hi,

> Yep, that's unneccessary since I realized you can have a single
> device doing both mouse&keyboard, and get absolute co-ords from it 
> with no issues.

That happens to work with Xorg 7.2, and even for that you'll have to
play tricks like providing a dummy keyboard config section because Xorg
expects one device for the keyboard and one for the mouse.

It doesn't work with older Xorg releases, 6.9 for example.

I havn't even checked other applications.  gpm?  framebuffer-based stuff?

Nobody expects mouse and keyboard events coming from a single device.
I still think it is much better to split it.

> With the existing single keyboard+mouse device there is a single
> config section to add:
> 
>   Section "InputDevice"
>         Identifier  "XenInput0"
>         Driver      "evdev"
>         Option      "CorePointer"
>         Option      "SendCoreEvents"
>         Option      "Device" "/dev/input/event0"
>         Option      "XkbModel" "pc105"
>         Option      "XkbLayout" "us"
>   EndSection
> 
> (Possibily with the extra 'Option' tags you noted in the previous
>  mail for legacy xorg releases of evdev.)

This is xorg 7.2+ only.  With older xorg releases it just doesn't work
as they don't support keyboards via evdev yet.

> While with the two separate devices, there is a single section to add:
> 
>   Section "InputDevice"
>         Identifier  "XenInput0"
>         Driver      "evdev"
>         Option      "CorePointer"
>         Option      "Device" "/dev/input/event1"
>         Option      "XkbModel" "pc105"
>         Option      "XkbLayout" "us"
>   EndSection

With two separate devices you can also simply change the mouse device
section to use the "evdev" driver instead of the "mouse" driver, config
file attached.

The third section is needed only if you want to be conservative and
continue to use the "mouse" driver unless you know the "evdev" driver
works better with the device in question.  And you better match by name
or id then instead of specifying the device (that requires xorg 7.2+
though).

> If we were implementing PVFB from
> scratch today, I think I'd agree that having separate devices for mouse
> and keyboard would be the approach to take. At this time, though we already
> have done releases with the current single device.

So what?  Nobody forces you to change that in RHEL5.  That is no reason
to not pick the better solution for the long run.  Changes like this
happen all day in linux land.

> Both approaches have the
> same end result

No, see above.

> In addition, Xorg is moving towards auto-configuring all devices so I hope
> that we'll be able to get X to auto-configure absolute mouse correctly
> without need for any config at all regardless of which impl we have.

Using evdev by default for the mouse maybe?

> So I don't really see any compelling reason to change the way the input
> devices are exposed.

I do.

cheers,

  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[-- Attachment #2: xorg.conf --]
[-- Type: text/plain, Size: 715 bytes --]

# xorg 7.2+ config file, using ...
#  - fbdev for the screen
#  - evdev for the mouse

# keyboard
Section "InputDevice"
  Driver	"kbd"
  Identifier	"Keyboard"
  Option	"XkbModel"	"pc105"
  Option	"XkbLayout"	"us"
EndSection

# mouse, using evdev
Section "InputDevice"
  Driver	"evdev"
  Identifier	"Mouse"
  Option        "evBits"	"+1 ~2-3"	# ev_key && (ev_rel || ev_abs)
  Option        "keyBits"	"~256-287"	# btn_*
EndSection

Section "Screen"
  Device	"fbdev"
  Identifier	"fbdev"
EndSection

Section "Device"
  Driver	"fbdev"
  Identifier	"fbdev"
EndSection

Section "ServerLayout"
  Identifier	"default"
  Screen	"fbdev"
  InputDevice	"Keyboard"	"CoreKeyboard"
  InputDevice	"Mouse"		"CorePointer"
EndSection


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Daniel P. Berrange]

On Tue, Feb 06, 2007 at 09:48:51AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Yep, that's unneccessary since I realized you can have a single
> > device doing both mouse&keyboard, and get absolute co-ords from it 
> > with no issues.
> 
> That happens to work with Xorg 7.2, and even for that you'll have to
> play tricks like providing a dummy keyboard config section because Xorg
> expects one device for the keyboard and one for the mouse.
> 
> It doesn't work with older Xorg releases, 6.9 for example.

Ahh, I did not know that.  Not a problem for current Fedora, but it'll be
important for guest distros based off old Xorg.

> > So I don't really see any compelling reason to change the way the input
> > devices are exposed.
> 
> I do.

The compatability with older Xorg servers does actually make it compelling

> Section "InputDevice"
>   Driver	"evdev"
>   Identifier	"Mouse"
>   Option        "evBits"	"+1 ~2-3"	# ev_key && (ev_rel || ev_abs)
>   Option        "keyBits"	"~256-287"	# btn_*
> EndSection

I notice you've no explicit device listed here. Am I understanding
right that, the evBits / keyBits options here will make it automatically
'find' the correct input device based on its declared capabilities ?

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

  Hi,

>> Section "InputDevice"
>>   Driver	"evdev"
>>   Identifier	"Mouse"
>>   Option        "evBits"	"+1 ~2-3"	# ev_key && (ev_rel || ev_abs)
>>   Option        "keyBits"	"~256-287"	# btn_*
>> EndSection
> 
> I notice you've no explicit device listed here. Am I understanding
> right that, the evBits / keyBits options here will make it automatically
> 'find' the correct input device based on its declared capabilities ?

Exactly.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Markus Armbruster]

Gerd Hoffmann <kraxel@suse.de> writes:

>   Hi,
>
>> Yep, that's unneccessary since I realized you can have a single
>> device doing both mouse&keyboard, and get absolute co-ords from it 
>> with no issues.
>
> That happens to work with Xorg 7.2, and even for that you'll have to
> play tricks like providing a dummy keyboard config section because Xorg
> expects one device for the keyboard and one for the mouse.
>
> It doesn't work with older Xorg releases, 6.9 for example.
>
> I havn't even checked other applications.  gpm?  framebuffer-based stuff?
>
> Nobody expects mouse and keyboard events coming from a single device.
> I still think it is much better to split it.

Nobody?  Aren't you overstating your case here?  The input layer is
clearly designed to transmit any kind of input event on the same
device, and recent Xorg works fine with it.

>> With the existing single keyboard+mouse device there is a single
>> config section to add:
>> 
>>   Section "InputDevice"
>>         Identifier  "XenInput0"
>>         Driver      "evdev"
>>         Option      "CorePointer"
>>         Option      "SendCoreEvents"
>>         Option      "Device" "/dev/input/event0"
>>         Option      "XkbModel" "pc105"
>>         Option      "XkbLayout" "us"
>>   EndSection
>> 
>> (Possibily with the extra 'Option' tags you noted in the previous
>>  mail for legacy xorg releases of evdev.)
>
> This is xorg 7.2+ only.  With older xorg releases it just doesn't work
> as they don't support keyboards via evdev yet.
>
>> While with the two separate devices, there is a single section to add:
>> 
>>   Section "InputDevice"
>>         Identifier  "XenInput0"
>>         Driver      "evdev"
>>         Option      "CorePointer"
>>         Option      "Device" "/dev/input/event1"
>>         Option      "XkbModel" "pc105"
>>         Option      "XkbLayout" "us"
>>   EndSection
>
> With two separate devices you can also simply change the mouse device
> section to use the "evdev" driver instead of the "mouse" driver, config
> file attached.
>
> The third section is needed only if you want to be conservative and
> continue to use the "mouse" driver unless you know the "evdev" driver
> works better with the device in question.  And you better match by name
> or id then instead of specifying the device (that requires xorg 7.2+
> though).
>
>> If we were implementing PVFB from
>> scratch today, I think I'd agree that having separate devices for mouse
>> and keyboard would be the approach to take. At this time, though we already
>> have done releases with the current single device.
>
> So what?  Nobody forces you to change that in RHEL5.  That is no reason
> to not pick the better solution for the long run.  Changes like this
> happen all day in linux land.

Improvement is good.  Change for change's sake is not.

If we think the proposed change will earn its keep, I'm all for it.

>> Both approaches have the
>> same end result
>
> No, see above.
>
>> In addition, Xorg is moving towards auto-configuring all devices so I hope
>> that we'll be able to get X to auto-configure absolute mouse correctly
>> without need for any config at all regardless of which impl we have.
>
> Using evdev by default for the mouse maybe?
>
>> So I don't really see any compelling reason to change the way the input
>> devices are exposed.
>
> I do.

If there are evdev clients we care about that can't cope with the
current unified device, I'm fine with splitting it.

Re: Re: [patch] pvfb: Split mouse and keyboard into separate devices.

[Gerd Hoffmann]

Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@suse.de> writes:
> 
> Nobody?  Aren't you overstating your case here?  The input layer is
> clearly designed to transmit any kind of input event on the same
> device, and recent Xorg works fine with it.

Older Xorg versions are not, evdev supports mice only in early versions.

And even with the latest Xorg you need a dummy keyboard input section to
make the X-Server happy.  That may change in the future with a decent
evdev driver being available in Xorg now, but we are clearly not yet at
the point yet where the unified device works without trouble.  And given
the fact that for physical hardware keyboard and mouse most likely stay
separate devices I wouldn't be surprised if various issues keep poping
up now and then ...

> If there are evdev clients we care about that can't cope with the
> current unified device, I'm fine with splitting it.

As mentioned:  Older Xorg versions.

I'll go prepare and re-submit a patch ...

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>