Hi, On 03/24/2014 12:45 AM, Peter Hutterer wrote: > On Fri, Mar 21, 2014 at 11:03:36AM +0100, Hans de Goede wrote: >> Hi, >> >> On 03/21/2014 10:31 AM, Hans de Goede wrote: >>> Hi, >>> >>> On 03/20/2014 11:18 PM, Peter Hutterer wrote: >>>> Signed-off-by: Peter Hutterer <[email protected]> >>>> --- >>>> conf/11-x11-synaptics.fdi | 9 +++++++++ >>>> conf/50-synaptics.conf | 12 ++++++++++++ >>>> 2 files changed, 21 insertions(+) >>>> >>>> diff --git a/conf/11-x11-synaptics.fdi b/conf/11-x11-synaptics.fdi >>>> index a898875..44dfce3 100644 >>>> --- a/conf/11-x11-synaptics.fdi >>>> +++ b/conf/11-x11-synaptics.fdi >>>> @@ -34,6 +34,15 @@ >>>> <match key="info.product" contains="Apple|bcm5974"> >>>> <merge key="input.x11_options.SoftButtonAreas" >>>> type="string">0 0 0 0 0 0 0 0</merge> >>>> </match> >>>> + >>>> + <!--The Cypress touchpads provide BTN_RIGHT in firmware, together >>>> with >>>> + clickfinger, and two-finger scrolling. Disable Clickpads, >>>> otherwise we >>>> + get flaky button behaviour. >>>> + https://bugs.freedesktop.org/show_bug.cgi?id=70819 >>>> + https://bugs.freedesktop.org/show_bug.cgi?id=76341 --> >>> >>> I'm not so sure that using a xorg.conf snippet for this is the best way to >>> go about this, I would >>> prefer to see a fix to the kernel to not report the clickpad flag for non >>> clickpads. Esp. as some >>> newer or future Cypress touchpads may very well get rid of the firmware >>> mode and do everything in >>> the drivers as other vendors do. >>> >>> I really think that "CyPS/2 Cypress Trackpad" is a way to generic match >>> string and is >>> going to bite us in the future. At a minimum we should use the still to be >>> merged DMI matching >>> here to limit this to certain laptop models, but IHMO it would be much >>> better to simply fix >>> this in the kernel. For starters we could add a dmi quirk in the kernel for >>> the XPS 13, until we >>> figure out a better way to decide which Cypress Trackpads use this firmware >>> emulation and which >>> models don't. >>> >>> And maybe the one in the Dell XPS 13 even has a command we can send to it >>> to turn of the >>> emulation too, which would allow us to really fix 70819 too. >> >> So I've spend some time reading the kernel cypress_ps2 driver, conclusions: >> >> 1) It can only do multi-touch for upto 2 fingers, it can detect tripple / >> quad >> fingers being down but will only report coordinates for 2. >> >> 2) It seems to do a lot of processing in firmware, so in general at least for >> the currently supported generation not reporting INPUT_PROP_BUTTONPAD seems >> to be >> the right thing to do for all currently supported Cypress Trackpads. >> >> I still believe the kernel is the right place to fix this. Adam, can you >> build a kernel >> with the attached patch, remove your xorg.conf changes and then test that >> setup please ? >> >> Regards, >> >> Hans > >> From 4b0668abbea0068cbdf7f9b0d149ccc245d79fab Mon Sep 17 00:00:00 2001 >> From: Hans de Goede <[email protected]> >> Date: Fri, 21 Mar 2014 10:55:11 +0100 >> Subject: [PATCH] input: cypress_ps2: Don't report the cypress PS/2 trackpads >> as a button pad >> >> The cypress PS/2 trackpad models supported by the cypress_ps2 driver do >> right button emulation in firmware, filtering out a bunch of events userspace >> needs to do clickpad handling on its own. So reporting them as clickpads >> actually causes a bunch of issues: >> >> https://bugs.freedesktop.org/show_bug.cgi?id=70819 >> https://bugs.freedesktop.org/show_bug.cgi?id=76341 >> >> Signed-off-by: Hans de Goede <[email protected]> >> --- >> drivers/input/mouse/cypress_ps2.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/input/mouse/cypress_ps2.c >> b/drivers/input/mouse/cypress_ps2.c >> index 87095e2..8af34ff 100644 >> --- a/drivers/input/mouse/cypress_ps2.c >> +++ b/drivers/input/mouse/cypress_ps2.c >> @@ -409,7 +409,6 @@ static int cypress_set_input_params(struct input_dev >> *input, >> __clear_bit(REL_X, input->relbit); >> __clear_bit(REL_Y, input->relbit); >> >> - __set_bit(INPUT_PROP_BUTTONPAD, input->propbit); >> __set_bit(EV_KEY, input->evbit); >> __set_bit(BTN_LEFT, input->keybit); >> __set_bit(BTN_RIGHT, input->keybit); >> -- >> 1.9.0 > > I recommend that you expand the commit message with a blurb on why it causes > issues before you send it to the lkml, it'll make it easier to review. > Specifically that BTN_RIGHT is emulated in firmware based on the finger > position, and that no motion events are sent when the finger is in the > button area. > > other than that this seems to be the best solution, thanks. > Reviewed-by: Peter Hutterer <[email protected]>
Thanks for the review, I've send this upstream with an amended commit msg. > though I'll still ship the xorg.conf snippet in fedora until this filters > down. Ok. Regards, Hans _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
