On Mon, Jan 05, 2015 at 05:04:55PM -0500, Benjamin Tissoires wrote: > On Mon, Jan 5, 2015 at 5:00 PM, Gabriele Mazzotta > <gabriele.mzt@xxxxxxxxx> wrote: > > On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote: > >> Hi Gabriele, > >> > >> [Adding Peter and Hans as this change will impact both > >> xf86-input-synaptics and libinput] > >> > >> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta > >> <gabriele.mzt@xxxxxxxxx> wrote: > >> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors > >> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR > >> > instead ABS_TOOL_WIDTH. > >> > >> It looks like the current xorg-synaptics code already handles > >> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in > >> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter > >> confirm this because xorg-synaptics still relies a lot on the single > >> touch emulation. > >> > >> > > >> > Since the 'w' slot is used to report the finger count when two or more > >> > fingers are on the touchpad, make sure that only meaningful values are > >> > emitted, i.e. values greater than or equal to 4, and assign the correct > >> > range to ABS_MT_TOUCH_MAJOR. > >> > > >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161 > >> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx> > >> > --- > >> > drivers/input/mouse/synaptics.c | 11 +++++++++-- > >> > 1 file changed, 9 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > >> > index f947292..ea0563e 100644 > >> > --- a/drivers/input/mouse/synaptics.c > >> > +++ b/drivers/input/mouse/synaptics.c > >> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot, > >> > >> Just FYI, this does not apply anymore on top of Dmitry's tree. > >> synaptics_report_slot() has been removed, and you should now report > >> the slot state in synaptics_report_mt_data(). > >> > >> > input_report_abs(dev, ABS_MT_POSITION_X, hw->x); > >> > input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y)); > >> > input_report_abs(dev, ABS_MT_PRESSURE, hw->z); > >> > + if (hw->w >= 4) > >> > >> That I don't like. IMO, at this point, .w should only contain the > >> finger width, unconditionally. > >> Also, with 2/2, .w is computed accurately for the second finger, but > >> not for the first. > >> > >> I tried to figure out a way to properly extract the actual width > >> information from the sgm packet when the w is 0 or 1, and the only way > >> I found was to do the fix in synaptics_image_sensor_process(). I would > >> have preferred dealing with that in synaptics_parse_hw_state() > >> directly, but I think the final code would be more and more ugly. > >> Dealing with the true finger width in synaptics_image_sensor_process() > >> is not a problem for cr48 sensors, because they will not have the > >> ABS_MT_TOUCH_MAJOR event exported. > > > > Regarding the last part on cr48 sensors. > > Currently these sensors are not reporting widths through ABS_TOOL_WIDTH > > and I don't see what could go wrong if they start reporting > > ABS_MT_TOUCH_MAJOR. If I understood correctly, they can report widths > > only when one finger is on the touchpad. This means that they will > > report widths through slot 0, but they won't through slot 1. Nothing > > bad should happen. > > I am not entirely sure. The entire purpose of having widht for palm > detection is to filter palm from true finger events. So if we only > have the width info on the first slot, it would be useless IMO. > Still I agree with "nothing bad should happen" :) >From conceptual perspective if device is not capable of reporting contact size for each contact then it should not be sending ABS_MT_TOUCH_MAJOR, same as we do not send ABS_MT_PRESSURE if we can't provide per-contact pressure. For such devices we revert to ST-events ABS_PRESSURE and I guess we'll have to continue with ABS_TOOL_WIDTH for contact sizes in Synaptics. Thanks. -- Dmitry -- 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
