Mailing List Archive: PVFB wheel events (z-axis)

PVFB wheel events (z-axis)

Feb 22, 2008, 1:55 AM

Post #1 of 8 (93 views)

I got questions on this changeset:

changeset: 354:c3ff0b26f664
date: Mon Dec 10 13:52:47 2007 +0000

Decode mouse event packet dz value and passes it as a wheel event into
the input stream.

Signed-off-by: Pat Campbell <plc@novell.com>

diff -r 7232a025140f -r c3ff0b26f664 drivers/xen/fbfront/xenkbd.c
--- a/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:51:57 2007 +0000
+++ b/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:52:47 2007 +0000
@@ -64,8 +64,13 @@ static irqreturn_t input_handler(int rq,
dev = info->ptr;
switch (event->type) {
case XENKBD_TYPE_MOTION:
- input_report_rel(dev, REL_X, event->motion.rel_x);
- input_report_rel(dev, REL_Y, event->motion.rel_y);
+ if ( event->motion.rel_z == 1 || event->motion.rel_z == -1 ) {
+ input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
+ }
+ else {
+ input_report_rel(dev, REL_X, event->motion.rel_x);
+ input_report_rel(dev, REL_Y, event->motion.rel_y);
+ }

I don't understand the conditional. Why is rel_z to be used *only*
when it's 1 or -1, and why are rel_x and rel_y to be used *only* when
rel_z isn't? That sure is a weird protocol, and it isn't documented
anywhere...

break;
case XENKBD_TYPE_KEY:
dev = NULL;
@@ -81,8 +86,13 @@ static irqreturn_t input_handler(int rq,
event->key.keycode);
break;
case XENKBD_TYPE_POS:
- input_report_abs(dev, ABS_X, event->pos.abs_x);
- input_report_abs(dev, ABS_Y, event->pos.abs_y);
+ if ( event->pos.abs_z == 1 || event->pos.abs_z == -1 ) {
+ input_report_rel(dev, REL_WHEEL, 0 - event->pos.abs_z);
+ }
+ else {
+ input_report_abs(dev, ABS_X, event->pos.abs_x);
+ input_report_abs(dev, ABS_Y, event->pos.abs_y);
+ }

And why isn't this using REL_ABS?

break;
}
if (dev)
@@ -152,7 +162,7 @@ int __devinit xenkbd_probe(struct xenbus
ptr->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
for (i = BTN_LEFT; i <= BTN_TASK; i++)
set_bit(i, ptr->keybit);
- ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
+ 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);

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

Re: PVFB wheel events (z-axis) [ In reply to ]

plc at novell

Feb 23, 2008, 7:06 AM

Post #2 of 8 (95 views)

Permalink

Markus Armbruster wrote:
> I got questions on this changeset:
>
> changeset: 354:c3ff0b26f664
> date: Mon Dec 10 13:52:47 2007 +0000
>
> Decode mouse event packet dz value and passes it as a wheel event into
> the input stream.
>
> Signed-off-by: Pat Campbell <plc@novell.com>
>
> diff -r 7232a025140f -r c3ff0b26f664 drivers/xen/fbfront/xenkbd.c
> --- a/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:51:57 2007 +0000
> +++ b/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:52:47 2007 +0000
> @@ -64,8 +64,13 @@ static irqreturn_t input_handler(int rq,
> dev = info->ptr;
> switch (event->type) {
> case XENKBD_TYPE_MOTION:
> - input_report_rel(dev, REL_X, event->motion.rel_x);
> - input_report_rel(dev, REL_Y, event->motion.rel_y);
> + if ( event->motion.rel_z == 1 || event->motion.rel_z == -1 ) {
> + input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
> + }
> + else {
> + input_report_rel(dev, REL_X, event->motion.rel_x);
> + input_report_rel(dev, REL_Y, event->motion.rel_y);
> + }
>
> I don't understand the conditional. Why is rel_z to be used *only*
> when it's 1 or -1, and why are rel_x and rel_y to be used *only* when
> rel_z isn't? That sure is a weird protocol, and it isn't documented
> anywhere...
>
In my testing I never saw a case where the rel_x and rel_y were not zero
or the abs_x and abs_y changed when a z event came thru. A small
attempt to not flood the input stream with redundant data.
Probably for clarity should have been: (same for the abs case)
if (event->motion.rel_z != 0)
input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
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:
> dev = NULL;
> @@ -81,8 +86,13 @@ static irqreturn_t input_handler(int rq,
> event->key.keycode);
> break;
> case XENKBD_TYPE_POS:
> - input_report_abs(dev, ABS_X, event->pos.abs_x);
> - input_report_abs(dev, ABS_Y, event->pos.abs_y);
> + if ( event->pos.abs_z == 1 || event->pos.abs_z == -1 ) {
> + input_report_rel(dev, REL_WHEEL, 0 - event->pos.abs_z);
> + }
> + else {
> + input_report_abs(dev, ABS_X, event->pos.abs_x);
> + input_report_abs(dev, ABS_Y, event->pos.abs_y);
> + }
>
> And why isn't this using REL_ABS?
>
I assume you meant to ask why not ABS_WHEEL. Xorg 6.9 evdev driver does
not decode ABS_WHEEL. Xorg 7.3 decodes both REL and ABS WHEEL but
ABS_WHEEL requires extra xorg.conf input options. We get greater
coverage by using REL_WHEEL and reduce the need to edit xorg.conf.
> break;
> }
> if (dev)
> @@ -152,7 +162,7 @@ int __devinit xenkbd_probe(struct xenbus
> ptr->evbit[0] = BIT(EV_KEY) | BIT(EV_REL) | BIT(EV_ABS);
> for (i = BTN_LEFT; i <= BTN_TASK; i++)
> set_bit(i, ptr->keybit);
> - ptr->relbit[0] = BIT(REL_X) | BIT(REL_Y);
> + 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);
>
>

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

Re: PVFB wheel events (z-axis) [ In reply to ]

armbru at redhat

Feb 25, 2008, 6:33 AM

Post #3 of 8 (96 views)

Permalink

Pat Campbell <plc@novell.com> writes:

> Markus Armbruster wrote:
>> I got questions on this changeset:
>>
>> changeset: 354:c3ff0b26f664
>> date: Mon Dec 10 13:52:47 2007 +0000
>>
>> Decode mouse event packet dz value and passes it as a wheel event into
>> the input stream.
>>
>> Signed-off-by: Pat Campbell <plc@novell.com>
>>
>> diff -r 7232a025140f -r c3ff0b26f664 drivers/xen/fbfront/xenkbd.c
>> --- a/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:51:57 2007 +0000
>> +++ b/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:52:47 2007 +0000
>> @@ -64,8 +64,13 @@ static irqreturn_t input_handler(int rq,
>> dev = info->ptr;
>> switch (event->type) {
>> case XENKBD_TYPE_MOTION:
>> - input_report_rel(dev, REL_X, event->motion.rel_x);
>> - input_report_rel(dev, REL_Y, event->motion.rel_y);
>> + if ( event->motion.rel_z == 1 || event->motion.rel_z == -1 ) {
>> + input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
>> + }
>> + else {
>> + input_report_rel(dev, REL_X, event->motion.rel_x);
>> + input_report_rel(dev, REL_Y, event->motion.rel_y);
>> + }
>>
>> I don't understand the conditional. Why is rel_z to be used *only*
>> when it's 1 or -1, and why are rel_x and rel_y to be used *only* when
>> rel_z isn't? That sure is a weird protocol, and it isn't documented
>> anywhere...
>>
> In my testing I never saw a case where the rel_x and rel_y were not zero
> or the abs_x and abs_y changed when a z event came thru. A small
> attempt to not flood the input stream with redundant data.

Assuming conditions you observed in your tests will hold elsehwere in
space or time is calling for trouble :)

> Probably for clarity should have been: (same for the abs case)
> if (event->motion.rel_z != 0)
> input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
> input_report_rel(dev, REL_X, event->motion.rel_x);
> input_report_rel(dev, REL_Y, event->motion.rel_y);

Why use REL_WHEEL and not REL_Z?

Why suppress zero Z-axis motion, but not X/Y-axis?

>> break;
>> case XENKBD_TYPE_KEY:
>> dev = NULL;
>> @@ -81,8 +86,13 @@ static irqreturn_t input_handler(int rq,
>> event->key.keycode);
>> break;
>> case XENKBD_TYPE_POS:
>> - input_report_abs(dev, ABS_X, event->pos.abs_x);
>> - input_report_abs(dev, ABS_Y, event->pos.abs_y);
>> + if ( event->pos.abs_z == 1 || event->pos.abs_z == -1 ) {
>> + input_report_rel(dev, REL_WHEEL, 0 - event->pos.abs_z);
>> + }
>> + else {
>> + input_report_abs(dev, ABS_X, event->pos.abs_x);
>> + input_report_abs(dev, ABS_Y, event->pos.abs_y);
>> + }
>>
>> And why isn't this using REL_ABS?
>>
> I assume you meant to ask why not ABS_WHEEL. Xorg 6.9 evdev driver does

Yes.

> not decode ABS_WHEEL. Xorg 7.3 decodes both REL and ABS WHEEL but
> ABS_WHEEL requires extra xorg.conf input options. We get greater
> coverage by using REL_WHEEL and reduce the need to edit xorg.conf.

Okay, that's fair.

[...]

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

Re: Re: PVFB wheel events (z-axis) [ In reply to ]

plc at novell

Feb 25, 2008, 10:43 AM

Post #4 of 8 (94 views)

Permalink

Markus Armbruster wrote:
> Pat Campbell <plc@novell.com> writes:
>
>
>> Markus Armbruster wrote:
>>
>>> I got questions on this changeset:
>>>
>>> changeset: 354:c3ff0b26f664
>>> date: Mon Dec 10 13:52:47 2007 +0000
>>>
>>> Decode mouse event packet dz value and passes it as a wheel event into
>>> the input stream.
>>>
>>> Signed-off-by: Pat Campbell <plc@novell.com>
>>>
>>> diff -r 7232a025140f -r c3ff0b26f664 drivers/xen/fbfront/xenkbd.c
>>> --- a/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:51:57 2007 +0000
>>> +++ b/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:52:47 2007 +0000
>>> @@ -64,8 +64,13 @@ static irqreturn_t input_handler(int rq,
>>> dev = info->ptr;
>>> switch (event->type) {
>>> case XENKBD_TYPE_MOTION:
>>> - input_report_rel(dev, REL_X, event->motion.rel_x);
>>> - input_report_rel(dev, REL_Y, event->motion.rel_y);
>>> + if ( event->motion.rel_z == 1 || event->motion.rel_z == -1 ) {
>>> + input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
>>> + }
>>> + else {
>>> + input_report_rel(dev, REL_X, event->motion.rel_x);
>>> + input_report_rel(dev, REL_Y, event->motion.rel_y);
>>> + }
>>>
>>> I don't understand the conditional. Why is rel_z to be used *only*
>>> when it's 1 or -1, and why are rel_x and rel_y to be used *only* when
>>> rel_z isn't? That sure is a weird protocol, and it isn't documented
>>> anywhere...
>>>
>>>
>> In my testing I never saw a case where the rel_x and rel_y were not zero
>> or the abs_x and abs_y changed when a z event came thru. A small
>> attempt to not flood the input stream with redundant data.
>>
>
> Assuming conditions you observed in your tests will hold elsehwere in
> space or time is calling for trouble :)
>
>
>> Probably for clarity should have been: (same for the abs case)
>> if (event->motion.rel_z != 0)
>> input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
>> input_report_rel(dev, REL_X, event->motion.rel_x);
>> input_report_rel(dev, REL_Y, event->motion.rel_y);
>>
>
> Why use REL_WHEEL and not REL_Z?
>
Same answer as below, Xorg 6.9 does not decode REL_Z. Xorg.7.3 handles
REL_Z and REL_WHEEL as the same.
> Why suppress zero Z-axis motion, but not X/Y-axis?
>
>
Hmm. Delving back into X
Xorg 6.9
case REL_WHEEL:
if (value > 0)
PostButtonClicks(pInfo, wheel_up_button, value);
if (value < 0)
PostButtonClicks(pInfo, wheel_down_button, -value);
break;
Xorg 7.3
Just sends the value up, gets ignored later

I guess I saw that sending a '0' for REL_WHEEL was a waste of time and
decided to filtered it out. Also not really necessary if it makes the
code easier to understand.

>>> break;
>>> case XENKBD_TYPE_KEY:
>>> dev = NULL;
>>> @@ -81,8 +86,13 @@ static irqreturn_t input_handler(int rq,
>>> event->key.keycode);
>>> break;
>>> case XENKBD_TYPE_POS:
>>> - input_report_abs(dev, ABS_X, event->pos.abs_x);
>>> - input_report_abs(dev, ABS_Y, event->pos.abs_y);
>>> + if ( event->pos.abs_z == 1 || event->pos.abs_z == -1 ) {
>>> + input_report_rel(dev, REL_WHEEL, 0 - event->pos.abs_z);
>>> + }
>>> + else {
>>> + input_report_abs(dev, ABS_X, event->pos.abs_x);
>>> + input_report_abs(dev, ABS_Y, event->pos.abs_y);
>>> + }
>>>
>>> And why isn't this using REL_ABS?
>>>
>>>
>> I assume you meant to ask why not ABS_WHEEL. Xorg 6.9 evdev driver does
>>
>
> Yes.
>
>
>> not decode ABS_WHEEL. Xorg 7.3 decodes both REL and ABS WHEEL but
>> ABS_WHEEL requires extra xorg.conf input options. We get greater
>> coverage by using REL_WHEEL and reduce the need to edit xorg.conf.
>>
>
> Okay, that's fair.
>
> [...]
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

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

Re: Re: PVFB wheel events (z-axis) [ In reply to ]

armbru at redhat

Feb 27, 2008, 9:01 AM

Post #5 of 8 (95 views)

Permalink

All right, I think I understand now how this works.

1. X reports wheel motion as button 4 & 5.

2. VNC viewer duly transmits that into QEMU.

3. QEMU converts it to -1/+1 on z-axis (pointer_event() in vnc.c).

4. Your patch transmits that to the vkbd frontend.

Bug: struct xenkbd_position claims abs_z is absolute, which is not
true.

Question: is that the protocol we want? More below.

5. The vkbd frontend stuffs the z-axis motion into the input layer as
REL_WHEEL, with the sign reversed.

Bug: it ignores movement other than -1/+1. A case can be made for
ignoring 0.

Bug: when it acts on z-axis movement, it ignores x/y movement /
position.

6. X converts the wheel movement back to button 4 & 5.

Weird, isn't it?

I'm not sure we want to encode wheel events as z-axis motion in the
vkbd frontend/backend protocol. Wouldn't it make more sense to encode
it as buttons?

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

Re: Re: PVFB wheel events (z-axis) [ In reply to ]

armbru at redhat

Feb 28, 2008, 2:02 AM

Post #6 of 8 (94 views)

Permalink

Markus Armbruster <armbru@redhat.com> writes:

> All right, I think I understand now how this works.
>
> 1. X reports wheel motion as button 4 & 5.
>
> 2. VNC viewer duly transmits that into QEMU.
>
> 3. QEMU converts it to -1/+1 on z-axis (pointer_event() in vnc.c).
>
> 4. Your patch transmits that to the vkbd frontend.
>
> Bug: struct xenkbd_position claims abs_z is absolute, which is not
> true.

Patch appended.

> Question: is that the protocol we want? More below.
>
> 5. The vkbd frontend stuffs the z-axis motion into the input layer as
> REL_WHEEL, with the sign reversed.
>
> Bug: it ignores movement other than -1/+1. A case can be made for
> ignoring 0.
>
> Bug: when it acts on z-axis movement, it ignores x/y movement /
> position.

Fixed in changeset 439:1edfea26a2a9

> 6. X converts the wheel movement back to button 4 & 5.
[...]

diff -r 43de9d7c3c63 drivers/xen/fbfront/xenkbd.c
--- a/drivers/xen/fbfront/xenkbd.c Tue Feb 26 17:59:18 2008 +0000
+++ b/drivers/xen/fbfront/xenkbd.c Thu Feb 28 09:44:59 2008 +0100
@@ -66,7 +66,7 @@ static irqreturn_t input_handler(int rq,
case XENKBD_TYPE_MOTION:
if (event->motion.rel_z)
input_report_rel(dev, REL_WHEEL,
- 0 - event->motion.rel_z);
+ -event->motion.rel_z);
input_report_rel(dev, REL_X, event->motion.rel_x);
input_report_rel(dev, REL_Y, event->motion.rel_y);
break;
@@ -84,9 +84,9 @@ static irqreturn_t input_handler(int rq,
event->key.keycode);
break;
case XENKBD_TYPE_POS:
- if (event->pos.abs_z)
+ if (event->pos.rel_z)
input_report_rel(dev, REL_WHEEL,
- 0 - event->pos.abs_z);
+ -event->pos.rel_z);
input_report_abs(dev, ABS_X, event->pos.abs_x);
input_report_abs(dev, ABS_Y, event->pos.abs_y);
break;
diff -r 43de9d7c3c63 include/xen/interface/io/kbdif.h
--- a/include/xen/interface/io/kbdif.h Tue Feb 26 17:59:18 2008 +0000
+++ b/include/xen/interface/io/kbdif.h Thu Feb 28 09:44:59 2008 +0100
@@ -65,7 +65,7 @@ struct xenkbd_position
uint8_t type; /* XENKBD_TYPE_POS */
int32_t abs_x; /* absolute X position (in FB pixels) */
int32_t abs_y; /* absolute Y position (in FB pixels) */
- int32_t abs_z; /* absolute Z position (wheel) */
+ int32_t rel_z; /* relative Z motion (wheel) */
};

#define XENKBD_IN_EVENT_SIZE 40

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

Re: Re: PVFB wheel events (z-axis) [ In reply to ]

Keir.Fraser at cl

Feb 28, 2008, 3:35 AM

Post #7 of 8 (93 views)

Permalink

On 28/2/08 09:02, "Markus Armbruster" <armbru@redhat.com> wrote:

>> 4. Your patch transmits that to the vkbd frontend.
>>
>> Bug: struct xenkbd_position claims abs_z is absolute, which is not
>> true.
>
> Patch appended.

Also needs changes to xen/include/public/io/kbdif.h and to
tools/ioemu/hw/xenfb.c. I won't check in the Linux side without having a
patch for the other half too.

-- Keir

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

Re: Re: PVFB wheel events (z-axis) [ In reply to ]

armbru at redhat

Feb 28, 2008, 5:59 AM

Post #8 of 8 (93 views)

Permalink

Buddy patch for ioemu:

Rename struct xenkbd_position member abs_z to rel_z

Z-axis motion is relative, not absolute.

diff -r 2a8eaba24bf0 tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c Tue Feb 26 15:11:51 2008 +0000
+++ b/tools/ioemu/hw/xenfb.c Thu Feb 28 13:57:13 2008 +0100
@@ -592,7 +592,8 @@ static int xenfb_send_key(struct xenfb *
}

/* Send a relative mouse movement event */
-static int xenfb_send_motion(struct xenfb *xenfb, int rel_x, int rel_y, int rel_z)
+static int xenfb_send_motion(struct xenfb *xenfb,
+ int rel_x, int rel_y, int rel_z)
{
union xenkbd_in_event event;

@@ -606,7 +607,8 @@ static int xenfb_send_motion(struct xenf
}

/* Send an absolute mouse movement event */
-static int xenfb_send_position(struct xenfb *xenfb, int abs_x, int abs_y, int abs_z)
+static int xenfb_send_position(struct xenfb *xenfb,
+ int abs_x, int abs_y, int rel_z)
{
union xenkbd_in_event event;

@@ -614,7 +616,7 @@ static int xenfb_send_position(struct xe
event.type = XENKBD_TYPE_POS;
event.pos.abs_x = abs_x;
event.pos.abs_y = abs_y;
- event.pos.abs_z = abs_z;
+ event.pos.rel_z = rel_z;

return xenfb_kbd_event(xenfb, &event);
}
diff -r 2a8eaba24bf0 xen/include/public/io/kbdif.h
--- a/xen/include/public/io/kbdif.h Tue Feb 26 15:11:51 2008 +0000
+++ b/xen/include/public/io/kbdif.h Thu Feb 28 13:57:13 2008 +0100
@@ -65,7 +65,7 @@ struct xenkbd_position
uint8_t type; /* XENKBD_TYPE_POS */
int32_t abs_x; /* absolute X position (in FB pixels) */
int32_t abs_y; /* absolute Y position (in FB pixels) */
- int32_t abs_z; /* absolute Z position (wheel) */
+ int32_t rel_z; /* relative Z motion (wheel) */
};

#define XENKBD_IN_EVENT_SIZE 40

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