diff mbox

[RFC/PATCH] Input: release MT fingers on device reset/disconnect

Message ID 1348255105-24558-1-git-send-email-miletus@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Yufeng Shen Sept. 21, 2012, 7:18 p.m. UTC
We are already releasing all pressed keys on device reset and
disconnect. This patch adds the finger release for multi-touch
device.

Different than the key release logic that only the pressed keys
are released, here all the possible fingers are released even
it is not recorded as touched down in the input_dev's internal
state. The motivation is that while releasing an alreay released
finger won't do harm to downstream, it is helpful in the case
when the downstream is out of sync with input_dev's internal
state, we are bringing them back to in sync by cleaning up both
input_dev and downstream's finger states.

Signed-off-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/input.c |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

Comments

Henrik Rydberg Sept. 24, 2012, 4:59 p.m. UTC | #1
Hi Yufeng,

> We are already releasing all pressed keys on device reset and
> disconnect. This patch adds the finger release for multi-touch
> device.
> 
> Different than the key release logic that only the pressed keys
> are released, here all the possible fingers are released even
> it is not recorded as touched down in the input_dev's internal
> state. The motivation is that while releasing an alreay released
> finger won't do harm to downstream, it is helpful in the case
> when the downstream is out of sync with input_dev's internal
> state, we are bringing them back to in sync by cleaning up both
> input_dev and downstream's finger states.
> 
> Signed-off-by: Yufeng Shen <miletus@chromium.org>
> ---

To reset the input state centrally upon disconnect makes some sense;
upon suspend/resume, less so. It is really up to the driver to
determine what part of the input state survives a suspend/resume
cycle

>  drivers/input/input.c |   34 ++++++++++++++++++++++++++++++++--
>  1 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 768e46b..ebdc450 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -584,6 +584,34 @@ void input_close_device(struct input_handle *handle)
>  EXPORT_SYMBOL(input_close_device);
>  
>  /*
> + * Simulate finger release events for all possible fingers. It has no harm
> + * to release an alreay released finger, but has the benefit that if the
> + * downstream finger states are out of sync with input_dev's finger states,
> + * we are helping to clean them up.
> + * The function must be called with dev->event_lock held.
> + */
> +static void input_dev_release_mt(struct input_dev *dev)

This should be moved to input-mt.c instead.

> +{
> +	int i;
> +
> +	if (!dev->mt)
> +		return;
> +
> +	if (!is_event_supported(EV_ABS, dev->evbit, EV_MAX) ||
> +	    !is_event_supported(ABS_MT_SLOT, dev->absbit, ABS_MAX) ||
> +	    !is_event_supported(ABS_MT_TRACKING_ID, dev->absbit, ABS_MAX))
> +		return;

No need to check those; implied by mtsize != 0.

> +
> +	for (i = 0; i < dev->mtsize; i++) {
> +		input_mt_set_value(&dev->mt[i], ABS_MT_TRACKING_ID, -1);
> +		input_pass_event(dev, EV_ABS, ABS_MT_SLOT, i);
> +		input_pass_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> +	}

Please use the proper api here (see any mt driver for hints).

> +
> +	input_pass_event(dev, EV_SYN, SYN_REPORT, 1);

input_sync()

> +}
> +
> +/*
>   * Simulate keyup events for all keys that are marked as pressed.
>   * The function must be called with dev->event_lock held.
>   */
> @@ -627,6 +655,7 @@ static void input_disconnect_device(struct input_dev *dev)
>  	 * reach any handlers.
>  	 */
>  	input_dev_release_keys(dev);
> +	input_dev_release_mt(dev);
>  
>  	list_for_each_entry(handle, &dev->h_list, d_node)
>  		handle->open = 0;
> @@ -1569,7 +1598,7 @@ static void input_dev_toggle(struct input_dev *dev, bool activate)
>   * @dev: input device whose state needs to be reset
>   *
>   * This function tries to reset the state of an opened input device and
> - * bring internal state and state if the hardware in sync with each other.
> + * bring internal state and state of the hardware in sync with each other.
>   * We mark all keys as released, restore LED state, repeat rate, etc.
>   */
>  void input_reset_device(struct input_dev *dev)
> @@ -1581,10 +1610,11 @@ void input_reset_device(struct input_dev *dev)
>  
>  		/*
>  		 * Keys that have been pressed at suspend time are unlikely
> -		 * to be still pressed when we resume.
> +		 * to be still pressed when we resume. Same logic for MT.
>  		 */
>  		spin_lock_irq(&dev->event_lock);
>  		input_dev_release_keys(dev);
> +		input_dev_release_mt(dev);

Since this is called from input_dev_resume(), I am not so sure this is
a good idea. Rather, we should be able to rely on the driver's
suspend/resume implementation here.

>  		spin_unlock_irq(&dev->event_lock);
>  	}
>  
> -- 
> 1.7.7.3
> 

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

Patch

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 768e46b..ebdc450 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -584,6 +584,34 @@  void input_close_device(struct input_handle *handle)
 EXPORT_SYMBOL(input_close_device);
 
 /*
+ * Simulate finger release events for all possible fingers. It has no harm
+ * to release an alreay released finger, but has the benefit that if the
+ * downstream finger states are out of sync with input_dev's finger states,
+ * we are helping to clean them up.
+ * The function must be called with dev->event_lock held.
+ */
+static void input_dev_release_mt(struct input_dev *dev)
+{
+	int i;
+
+	if (!dev->mt)
+		return;
+
+	if (!is_event_supported(EV_ABS, dev->evbit, EV_MAX) ||
+	    !is_event_supported(ABS_MT_SLOT, dev->absbit, ABS_MAX) ||
+	    !is_event_supported(ABS_MT_TRACKING_ID, dev->absbit, ABS_MAX))
+		return;
+
+	for (i = 0; i < dev->mtsize; i++) {
+		input_mt_set_value(&dev->mt[i], ABS_MT_TRACKING_ID, -1);
+		input_pass_event(dev, EV_ABS, ABS_MT_SLOT, i);
+		input_pass_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+	}
+
+	input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+}
+
+/*
  * Simulate keyup events for all keys that are marked as pressed.
  * The function must be called with dev->event_lock held.
  */
@@ -627,6 +655,7 @@  static void input_disconnect_device(struct input_dev *dev)
 	 * reach any handlers.
 	 */
 	input_dev_release_keys(dev);
+	input_dev_release_mt(dev);
 
 	list_for_each_entry(handle, &dev->h_list, d_node)
 		handle->open = 0;
@@ -1569,7 +1598,7 @@  static void input_dev_toggle(struct input_dev *dev, bool activate)
  * @dev: input device whose state needs to be reset
  *
  * This function tries to reset the state of an opened input device and
- * bring internal state and state if the hardware in sync with each other.
+ * bring internal state and state of the hardware in sync with each other.
  * We mark all keys as released, restore LED state, repeat rate, etc.
  */
 void input_reset_device(struct input_dev *dev)
@@ -1581,10 +1610,11 @@  void input_reset_device(struct input_dev *dev)
 
 		/*
 		 * Keys that have been pressed at suspend time are unlikely
-		 * to be still pressed when we resume.
+		 * to be still pressed when we resume. Same logic for MT.
 		 */
 		spin_lock_irq(&dev->event_lock);
 		input_dev_release_keys(dev);
+		input_dev_release_mt(dev);
 		spin_unlock_irq(&dev->event_lock);
 	}