WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: PVFB wheel events (z-axis)

To: Pat Campbell <plc@xxxxxxxxxx>
Subject: [Xen-devel] Re: PVFB wheel events (z-axis)
From: Markus Armbruster <armbru@xxxxxxxxxx>
Date: Mon, 25 Feb 2008 14:33:44 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 25 Feb 2008 05:34:09 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <47C0287F.5070901@xxxxxxxxxx> (Pat Campbell's message of "Sat\, 23 Feb 2008 07\:06\:55 -0700")
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <87tzk19xqf.fsf@xxxxxxxxxxxxxxxxx> <47C0287F.5070901@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux)
Pat Campbell <plc@xxxxxxxxxx> 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@xxxxxxxxxx>
>>
>>     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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel