Re: PVFB wheel events (z-axis)

[Pat Campbell <plc@novell.com>]

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);
>
>