Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 11, 2011 at 2:05 PM, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote:
>> >> The above statement is to avoid going through the input_mt_slot and
>> >> input_mt_report_slot_state routines without posting any meaningful
>> >> events. I guess it could be considered as a performance enhancement?
>> >
>> > It won't be posting events unless something changed.
>>
>> But it still go through the  input_mt_slot and
>> input_mt_report_slot_state routines, which is unnecessary. If we know
>> up front there is no events posting, what benefit do we get to go
>> through the routines?
>
> Clarity. :-)

Oh, that. We could add some comments to make it clear.

In fact, Chris has suggested to add more comments in general. But I
have seen review comments like "the code tells the others what you are
saying". So, we deferred.

How about this:

+
+       /* no touch events should be posted when pen is in proximity
and touch was up */
+       if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down)
+               return 0;

>> >> No,  wacom_tpc_mt_touch can set touch_down to false too. When touch is
>> >> false, touch_down will be false. This happens when pen in prox or when
>> >> both fingers are up.
>> >
>> > Now, that is what cannot happen, because of the max() function.
>>
>> This time is real. How about this one?
>>
>>
>> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
>> index 39c289d..3cdafb9 100644
>> --- a/drivers/input/tablet/wacom_wac.c
>> +++ b/drivers/input/tablet/wacom_wac.c
>> @@ -699,7 +699,10 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
>>                 }
>>
>>                 /* keep touch bit to send proper touch up event */
>> -               wacom->shared->touch_down = max(touch,
>> wacom->shared->touch_down);
>> +               if (i == 1)
>> +                       wacom->shared->touch_down = max(touch,
>> wacom->shared->touch_down);
>> +               else
>> +                       wacom->shared->touch_down = touch;
>>         }
>>
>>         input_mt_report_pointer_emulation(input, true);
>
> Well... it might help to return to what the main goal is here. If the
> purpose is to fix userspace, maybe it should actually be done in
> userspace instead.

The goal is to support legacy ST client. A clean support would be to
send pen data and pen data only whenever pen is in prox, no matter
touch is down or not; sending ST touch data only when pen is out-prox.
The concept of ST is that in any single moment, there would be only
one type of data in action.

In that sense, avoiding both pen and touch in action at the same time
is our (kernel's) responsibility. ST clients do not have the ability
to deal with dual types.

Can I put your reviewed-by if I make a v2 of this patch?

Thank you for your help.

Ping
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux