From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support Date: Wed, 17 Aug 2011 10:34:44 -0700 Message-ID: <20110817173444.GE4877@core.coreip.homeip.net> References: <1313169407-4358-1-git-send-email-djkurtz@chromium.org> <1313169407-4358-5-git-send-email-djkurtz@chromium.org> <20110812210911.GA9124@polaris.bitmath.org> <20110817163226.GB4877@core.coreip.homeip.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-iy0-f170.google.com ([209.85.210.170]:41496 "EHLO mail-iy0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750840Ab1HQReu (ORCPT ); Wed, 17 Aug 2011 13:34:50 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Daniel Kurtz Cc: Henrik Rydberg , chase.douglas@canonical.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, olofj@chromium.org, chris@cnpbagwell.com On Thu, Aug 18, 2011 at 12:43:39AM +0800, Daniel Kurtz wrote: > On Thu, Aug 18, 2011 at 12:32 AM, Dmitry Torokhov > wrote: > > > On Wed, Aug 17, 2011 at 10:05:48PM +0800, Daniel Kurtz wrote: > > > On Mon, Aug 15, 2011 at 3:17 PM, Daniel Kurtz > > wrote: > > > > > > > > Hi Henrik, > > > > > > > > Thanks for the great feedback. > > > > > > > > On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg > > wrote: > > > > > Hi Daniel, > > > > > > > > > > looks good, some details below. > > > > > > > > > > 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... > > > > .... > > > > > > > > > > Looks good otherwise. > > > > > > > > > > Thanks, > > > > > Henrik > > > > > > > > > > > > > Ahh.... ok, this makes sense. I missed this detail previously. > > > > Basically: > > > > * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards > > compatibility) > > > > * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really > > > > 'width of finger in surface units'), and ABS_PRESSURE (for backwards > > > > compatibility). > > > > * Don't use ABS_MT_PRESSURE. > > > > * This will simplify agm/sgm reporting by making them both reoprt > > > > the same axes. > > > > > > Hi Henrik, > > > > > > I think I was wrong in my previous comment. > > > > > > I agree we should drop reporting synaptics 'w' (4-15, arbitrary units) > > > in ABS_MT_TOUCH_MAJOR. > > > Instead, to support legacy, report 'w' in ABS_TOOL_WIDTH, and do not > > > report ABS_MT_TOUCH_MAJOR at all. > > > > > > However, I believe we should still report synaptics 'z' (0-255, in > > > arbitrary units) as ABS_MT_PRESSURE. > > > This will, in turn, be used by input_mt_report_pointer_emulation() to > > > determine the correct ABS_PRESSURE to report for legacy userspace > > > uses. > > > > Alternatively, you can use input_mt_set_value() to save Z value to be > > used by input_mt_report_pointer_emulation() later, but not report it as > > part of MT packet. > > > > But I want userspace to get per-contact pressure, since it is used to > determine whether a given contact is a palm or finger, etc. The trackpad > does not report a useful per-contact "width" that can be used to generate > "TOUCH_MAJOR". If the data reported is indeed pressure and it is indeed per-contact then yes, it makes sense to report it as such. If the data reported is more like contact size then report it as ABS_MT_TOUCH_MAJOR and use input_mt_set_value() to "stash" it for legacy emulation. -- Dmitry