[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110812210911.GA9124@polaris.bitmath.org>
Date: Fri, 12 Aug 2011 23:09:12 +0200
From: "Henrik Rydberg" <rydberg@...omail.se>
To: Daniel Kurtz <djkurtz@...omium.org>
Cc: chase.douglas@...onical.com, dmitry.torokhov@...il.com,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
olofj@...omium.org, chris@...bagwell.com
Subject: Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support
Hi Daniel,
looks good, some details below.
> +static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
> +{
> + input_mt_slot(dev, slot);
> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
> +}
> +
> +static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
> + const struct synaptics_hw_state *sgm)
> +{
> + input_mt_slot(dev, slot);
> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> + input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
> + input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
> + input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
> + /* SGM can sometimes contain valid width */
> + if (sgm->w >= 4)
> + input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
> +}
The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
compatible with user space handling of type A devices. Also, the scale
should match the screen/pad size, such that the actual size of the
touch area can be deduced. In addition, based on the current sensor
technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
mutually exclusive.
All in all, I would prefer to only report width via (the single-finger
axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
> +
> +static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
> + const struct synaptics_hw_state *agm)
> +{
> + input_mt_slot(dev, slot);
> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> + input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> + input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> + input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> +}
With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
> +
> +static void synaptics_report_mt(struct psmouse *psmouse,
> + int count,
> + const struct synaptics_hw_state *sgm)
> +{
> + struct input_dev *dev = psmouse->dev;
> + struct synaptics_data *priv = psmouse->private;
> + struct synaptics_hw_state *agm = &priv->agm;
> +
> + switch (count) {
> + case 0:
> + synaptics_report_slot_empty(dev, 0);
> + synaptics_report_slot_empty(dev, 1);
> + break;
> + case 1:
> + synaptics_report_slot_sgm(dev, 0, sgm);
> + synaptics_report_slot_empty(dev, 1);
> + break;
> + case 2:
> + case 3: /* Fall-through case */
> + synaptics_report_slot_sgm(dev, 0, sgm);
> + synaptics_report_slot_agm(dev, 1, agm);
> + break;
> + }
> +
> + /* Don't use active slot count to generate BTN_TOOL events. */
> + input_mt_report_pointer_emulation(dev, false);
> +
> + /* Send the number of fingers reported by touchpad itself. */
> + input_mt_report_finger_count(dev, count);
> +
> + input_report_key(dev, BTN_LEFT, sgm->left);
> + input_sync(dev);
> +}
> +
> +static void synaptics_image_sensor_process(struct psmouse *psmouse,
> + struct synaptics_hw_state *sgm)
> +{
> + int count;
> +
> + if (sgm->z == 0)
> + count = 0;
> + else if (sgm->w >= 4)
> + count = 1;
> + else if (sgm->w == 0)
> + count = 2;
> + else
> + count = 3;
> +
> + /* Send resulting input events to user space */
> + synaptics_report_mt(psmouse, count, sgm);
> +}
> +
> /*
> * called for each full received packet from the touchpad
> */
> @@ -558,6 +642,11 @@ static void synaptics_process_packet(struct psmouse *psmouse)
> if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
> return;
>
> + if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> + synaptics_image_sensor_process(psmouse, &hw);
> + return;
> + }
> +
> if (hw.scroll) {
> priv->scroll += hw.scroll;
>
> @@ -739,7 +828,16 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
> set_abs_position_params(dev, priv, ABS_X, ABS_Y);
> input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
>
> - if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
> + if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> + input_mt_init_slots(dev, 2);
> + set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> + ABS_MT_POSITION_Y);
> + /* Image sensors can report per-contact pressure */
> + input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> + /* Image sensors can sometimes report per-contact width */
> + input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> + } else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
> + /* Non-image sensors with AGM use semi-mt */
> __set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
> input_mt_init_slots(dev, 2);
> set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index a9efbf3..0ea7616 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -74,6 +74,8 @@
> * 2 0x04 reduced filtering firmware does less filtering on
> * position data, driver should watch
> * for noise.
> + * 2 0x08 image sensor image sensor tracks 5 fingers, but only
> + * reports 2.
> * 2 0x20 report min query 0x0f gives min coord reported
> */
> #define SYN_CAP_CLICKPAD(ex0c) ((ex0c) & 0x100000) /* 1-button ClickPad */
> @@ -82,6 +84,7 @@
> #define SYN_CAP_MIN_DIMENSIONS(ex0c) ((ex0c) & 0x002000)
> #define SYN_CAP_ADV_GESTURE(ex0c) ((ex0c) & 0x080000)
> #define SYN_CAP_REDUCED_FILTERING(ex0c) ((ex0c) & 0x000400)
> +#define SYN_CAP_IMAGE_SENSOR(ex0c) ((ex0c) & 0x000800)
>
> /* synaptics modes query bits */
> #define SYN_MODE_ABSOLUTE(m) ((m) & (1 << 7))
> --
> 1.7.3.1
>
Looks good otherwise.
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists