| Message ID | 20180531233707.80702-2-rkir@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Roman, On Thu, May 31, 2018 at 04:37:07PM -0700, rkir@google.com wrote: > From: Lingfeng Yang <lfy@google.com> > > If we send SYN_REPORT on every single > multitouch event, it breaks the multitouch. > > The multitouch becomes janky and > having to click 2-3 times to > do stuff (plus randomly activating notification > bars when not clicking) > > If we suppress these SYN_REPORTS, > multitouch will work fine, plus the events > will have a protocol that looks nice. I believe I mentioned this before: please fix goldfish backend. If it decided to send input events it should follow the input protocol. > > In addition, we need to register Goldfish Events > as a multitouch device by issuing > input_mt_init_slots, otherwise > input_handle_abs_event in drivers/input/input.c > will silently drop all ABS_MT_SLOT events, > making it so that touches with more than 1 finger > do not work properly. Can we extend the protocol to supply the expected number of contacts? > > Signed-off-by: "Lingfeng Yang" <lfy@google.com> > Signed-off-by: Roman Kiryanov <rkir@google.com> > --- > drivers/input/keyboard/goldfish_events.c | 28 +++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/keyboard/goldfish_events.c b/drivers/input/keyboard/goldfish_events.c > index e8dae6195b30..055accfeb0dd 100644 > --- a/drivers/input/keyboard/goldfish_events.c > +++ b/drivers/input/keyboard/goldfish_events.c > @@ -17,6 +17,7 @@ > #include <linux/interrupt.h> > #include <linux/types.h> > #include <linux/input.h> > +#include <linux/input/mt.h> > #include <linux/kernel.h> > #include <linux/platform_device.h> > #include <linux/slab.h> > @@ -24,6 +25,8 @@ > #include <linux/io.h> > #include <linux/acpi.h> > > +#define GOLDFISH_MAX_FINGERS 5 > + > enum { > REG_READ = 0x00, > REG_SET_PAGE = 0x00, > @@ -52,7 +55,21 @@ static irqreturn_t events_interrupt(int irq, void *dev_id) > value = __raw_readl(edev->addr + REG_READ); > > input_event(edev->input, type, code, value); > - input_sync(edev->input); > + // Send an extra (EV_SYN, SYN_REPORT, 0x0) event > + // if a key was pressed. Some keyboard device > + // drivers may only send the EV_KEY event and > + // not EV_SYN. > + // Note that sending an extra SYN_REPORT is not > + // necessary nor correct protocol with other > + // devices such as touchscreens, which will send > + // their own SYN_REPORT's when sufficient event > + // information has been collected (e.g., for > + // touchscreens, when pressure and X/Y coordinates > + // have been received). Hence, we will only send > + // this extra SYN_REPORT if type == EV_KEY. > + if (type == EV_KEY) > + input_sync(edev->input); > + > return IRQ_HANDLED; > } > > @@ -155,6 +172,15 @@ static int events_probe(struct platform_device *pdev) > > input_dev->name = edev->name; > input_dev->id.bustype = BUS_HOST; > + // Set the Goldfish Device to be multi-touch. > + // In the Ranchu kernel, there is multi-touch-specific > + // code for handling ABS_MT_SLOT events. > + // See drivers/input/input.c:input_handle_abs_event. > + // If we do not issue input_mt_init_slots, > + // the kernel will filter out needed ABS_MT_SLOT > + // events when we touch the screen in more than one place, > + // preventing multi-touch with more than one finger from working. > + input_mt_init_slots(input_dev, GOLDFISH_MAX_FINGERS, 0); > > events_import_bits(edev, input_dev->evbit, EV_SYN, EV_MAX); > events_import_bits(edev, input_dev->keybit, EV_KEY, KEY_MAX); > -- > 2.17.0.921.gf22659ad46-goog > Thanks.
diff --git a/drivers/input/keyboard/goldfish_events.c b/drivers/input/keyboard/goldfish_events.c index e8dae6195b30..055accfeb0dd 100644 --- a/drivers/input/keyboard/goldfish_events.c +++ b/drivers/input/keyboard/goldfish_events.c @@ -17,6 +17,7 @@ #include <linux/interrupt.h> #include <linux/types.h> #include <linux/input.h> +#include <linux/input/mt.h> #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/slab.h> @@ -24,6 +25,8 @@ #include <linux/io.h> #include <linux/acpi.h> +#define GOLDFISH_MAX_FINGERS 5 + enum { REG_READ = 0x00, REG_SET_PAGE = 0x00, @@ -52,7 +55,21 @@ static irqreturn_t events_interrupt(int irq, void *dev_id) value = __raw_readl(edev->addr + REG_READ); input_event(edev->input, type, code, value); - input_sync(edev->input); + // Send an extra (EV_SYN, SYN_REPORT, 0x0) event + // if a key was pressed. Some keyboard device + // drivers may only send the EV_KEY event and + // not EV_SYN. + // Note that sending an extra SYN_REPORT is not + // necessary nor correct protocol with other + // devices such as touchscreens, which will send + // their own SYN_REPORT's when sufficient event + // information has been collected (e.g., for + // touchscreens, when pressure and X/Y coordinates + // have been received). Hence, we will only send + // this extra SYN_REPORT if type == EV_KEY. + if (type == EV_KEY) + input_sync(edev->input); + return IRQ_HANDLED; } @@ -155,6 +172,15 @@ static int events_probe(struct platform_device *pdev) input_dev->name = edev->name; input_dev->id.bustype = BUS_HOST; + // Set the Goldfish Device to be multi-touch. + // In the Ranchu kernel, there is multi-touch-specific + // code for handling ABS_MT_SLOT events. + // See drivers/input/input.c:input_handle_abs_event. + // If we do not issue input_mt_init_slots, + // the kernel will filter out needed ABS_MT_SLOT + // events when we touch the screen in more than one place, + // preventing multi-touch with more than one finger from working. + input_mt_init_slots(input_dev, GOLDFISH_MAX_FINGERS, 0); events_import_bits(edev, input_dev->evbit, EV_SYN, EV_MAX); events_import_bits(edev, input_dev->keybit, EV_KEY, KEY_MAX);