From 42c5c30bc750d796f548c2802604aabe3e2163c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20Rodr=C3=ADguez=20P=C3=A9rez?= Date: Tue, 18 Nov 2025 10:08:41 +0000 Subject: [PATCH] ps/2 elantech: v4 - implement v4 (funtional) v4 - HEAD packet: Handling of finger id 0 only Process finger id 0 keeping code separated handling preparing for multifinger support. Add extra checks with hardware limits. Keep separated event and cookie position handling. Improve tracing. v4 - Improved pressure handling Do not provide pressure when there are not fingers touching and set a default pressure valud pressuming it will be taken into account for tapping on the user space on packets where pressure information is not provided by hardware. v4 - Hardcode fingerWitdh to defaut value Always provide default finger width value which is not a Synaptics w flag to enable tapping getting rid of previous 0 value which is the Synaptics flag for 2-fingers on that variable. This gives the chance to decide from the user space when to issue tapping or scrolling instead of being hardcoded on the kernel driver. At the same time it makes MOTION packets that contain information of 2 fingers on the same packet to be taken into account for cursor movement if applicable for instance when two-finger scroll is disabled. v4 - Fix tracing text and comment The value provided is not the number of fingers or fingers map but a finger id number. Move comment to the next line as that applies to next if condition and not to the trace instruction. v4 - MOTION packet: Fix getting finger 0 info Sometimes the information on MOTION packet which contains 2 finger positions could come on descending order by id. For insteance finger 1 on the first location of the packet and finger 0 coming on the second one. Get finger 0 values from first or second finger information location in the packet if available instead of checking for the first position only. Report to ignore packet otherwise. Provide extra checks and improve tracing. The set of changes of this commit fixes suttle cursor jumps caused by missing finger 0 information when that is provided on the second location of the MOTION packet instead of coming from the first location. On that scenario, if a following HEAD packet containing absulute positioning for finger 0 is comming the issue was reproduced. v4 - MOTION limit positions to boundaries This fixes cursor jumps and cursor disapearing among other misbehaviours when relative positions for the movement added up to the expected previous absolute position of the finger id on touchpad coordinates result on underflow or overflow. v4 - Manipulate finger bitmap computation based on previous packets with TODO There is no consensus on BSDs and Linux drivers, nor even clear documented behaviour for this so this is implemented based on experimentation. FreeBSD considers STATUS packet comes first prividing the list of fingers detected following a sequence of HEAD packets prividing absolute positioning for fingers that hadn't prevously been tracked and MOTION packets for relative positions for subsequent movement packing 2 finger information per packet until interation with the touch area stops so a final STATUS packet with no fingers detected and filled out with zeroes for other fields signals the end of movement. Motion packets with fingers ids not mapped form the HEAD are discarted as invalid data. On Linux there is not such restriction so HEAD or STATUS packets are taking into account regardless fingers on STATUS information matches with next packet. So that, the implementation here uses the STATUS information as informative for the following events. For instance if a status packet signals that two-fingers are pressed and the next packet is a HEAD packet, the latter only can provide information for one finger only, then the number of fingers reported is 2 on the head packet if the id of the finger overlaps with the ones from the HEAD packet or 3 fingers in case the id is a new one. Same logic applies to MOTION packets. This makes the asumption that a started event can only retain or increment the number of fingers until a movement action is finished providing continuity for dragging actions and scrolling actions. A TODO is added to investigate which should be the right behaviour for this and check if differnt v4 hardware and firmware match. v4 - Implement hardware palm detection as compile option disabled by default Fixed hardware palm detection to be taken into account on a separate field instead of being a hack messing with the number of fingers reported on STATUS packet and complete the conditions propagating the palm detected value on HEAD and MOTION packets fixing some previous non-accurated conditions. This fixes incorrect reports for taps among others on both cases whether the compile option is enabled or not. Apart from that a TODO is added from completeness to encourage looking for software palm detection instead that will benefit other devices as well. Palm detection will be more accurate to do by software so is is disabled by default. Some posible options are: - Improvements on padblocker input filter. - Improvements in user mode input_server device. - General input_server modifications. - Other inprovements on this driver. v4 - Initialise event structure so that new or unasigend fields will be zeroed This prevents potential misbehavious by dealing with incorrect data in case the event structure is modified in the future with new fields after recompiling this driver with unchanged code. At least those field will be zeroed with usually mean that they are at least initialised. Change-Id: Iccfdd356ff2834e5dea64481259fd890dd95406b Reviewed-on: https://review.haiku-os.org/c/haiku/+/10013 Reviewed-by: Adrien Destugues --- .../kernel/bus_managers/ps2/ps2_elantech.cpp | 177 +++++++++++++++--- .../kernel/bus_managers/ps2/ps2_elantech.h | 11 ++ 2 files changed, 157 insertions(+), 31 deletions(-) diff --git a/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.cpp b/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.cpp index ddb10cf647..af6c04319a 100644 --- a/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.cpp +++ b/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.cpp @@ -61,12 +61,17 @@ const char* kElantechPath[4] = { #define REAL_MAX_PRESSURE 50 #define MAX_PRESSURE 255 +#define DEFAULT_PRESSURE 30 +#define DEFAULT_FINGER_WIDTH 4 + #define ELANTECH_HISTORY_SIZE 256 #define STATUS_PACKET 0x0 #define HEAD_PACKET 0x1 #define MOTION_PACKET 0x2 +#define ELANTECH_MAX_FINGERS 5 + // Error code used by MouseDevice::_ControlThread() in MouseInputDevice.cpp to reuse previous // event, basically ignoring the packet. #define IGNORE_EVENT B_BAD_DATA @@ -117,7 +122,7 @@ static status_t elantech_process_packet_v4(elantech_cookie *cookie, touchpad_movement *_event, uint8 packet[PS2_PACKET_ELANTECH]) { - touchpad_movement event; + touchpad_movement event = {}; int invalidAt = 0; @@ -160,6 +165,14 @@ elantech_process_packet_v4(elantech_cookie *cookie, touchpad_movement *_event, */ event.buttons = (packet[0] & 0x3); +#ifdef ELANTECH_ENABLE_HARDWARE_PALM_DETECTION + cookie->palm = (packet[4] & 0x80) != 0; + if (cookie->palm) { + TRACE("ELANTECH: Hardware palm detected (HEAD)\n"); + return IGNORE_EVENT; + } +#endif + // Event fingers contains a bitmap of fingers. event.fingers = packet[1] & 0x1f; TRACE("ELANTECH: Fingers bitmap %" B_PRId32 ", raw %x (STATUS)\n", @@ -169,11 +182,11 @@ elantech_process_packet_v4(elantech_cookie *cookie, touchpad_movement *_event, event.xPosition = 0; event.yPosition = 0; - // Pressure is not provided on this packet, so use a backup value. - event.zPressure = cookie->previousZ; - - //fingers, no palm - cookie->fingers = (packet[4] & 0x80) == 0 ? event.fingers : 0; + // Pressure is not provided on this packet, so make it up with sensible values + if (event.fingers == 0) + event.zPressure = 0; + else + event.zPressure = DEFAULT_PRESSURE; TRACE("ELANTECH: Pos: %" B_PRId32 ":%" B_PRId32 " (STATUS)\n", cookie->x, cookie->y); @@ -196,23 +209,53 @@ elantech_process_packet_v4(elantech_cookie *cookie, touchpad_movement *_event, */ event.buttons = (packet[0] & 0x3); - TRACE("ELANTECH: Fingers %d, raw %x (HEAD)\n", (packet[3] & 0xe0) >>5, packet[3]); - // only process first finger - if ((packet[3] & 0xe0) != 0x20) + TRACE("ELANTECH: Finger id %d, raw %x (HEAD)\n", (packet[3] & 0xe0) >>5, packet[3]); + int id; + id = ((packet[3] & 0xe0) >> 5) - 1; + if (id < 0 || id >= ELANTECH_MAX_FINGERS) { + TRACE("ELANTECH: Not right fingers (HEAD)"); return IGNORE_EVENT; + } - event.fingers = cookie->fingers; +#ifdef ELANTECH_ENABLE_HARDWARE_PALM_DETECTION + if (cookie->palm) { + TRACE("ELANTECH: Hardware palm detected (HEAD)\n"); + return IGNORE_EVENT; + } +#endif + + // Head packet processes 1 finger only. Question is if the id is different than + // the one provided by STATUS packet, is this a new finger or a replacement of + // the previous ones? + // As per testing let's assume a new finger is added. + // That's in sync with the logic on BSDs and Linux drivers providing MT events. + //event.fingers = cookie->fingers; + event.fingers = cookie->fingers | (1 << id); + //event.fingers = (1 << id); + + // only process first finger + if (id != 0) { + TRACE("ELANTECH: RET Only process first finger. (HEAD)\n"); + return IGNORE_EVENT; + } event.zPressure = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4); cookie->previousZ = event.zPressure; - cookie->x = event.xPosition = ((packet[1] & 0xf) << 8) | packet[2]; - cookie->y = event.yPosition = ((packet[4] & 0xf) << 8) | packet[5]; - TRACE("ELANTECH: Pos: %" B_PRId32 ":%" B_PRId32 "\n (HEAD)", + event.xPosition = ((packet[1] & 0xf) << 8) | packet[2]; + event.yPosition = ((packet[4] & 0xf) << 8) | packet[5]; + + TRACE("ELANTECH: dx: %d dy: %d (HEAD)\n", + (int)event.xPosition - (int)cookie->x, + (int)event.yPosition - (int)cookie->y); + + cookie->x = event.xPosition; + cookie->y = event.yPosition; + TRACE("ELANTECH: Pos: %" B_PRId32 ":%" B_PRId32 " (HEAD)\n", cookie->x, cookie->y); TRACE("ELANTECH: buttons 0x%x x %" B_PRIu32 " y %" B_PRIu32 - " z %d\n", event.buttons, event.xPosition, event.yPosition, + " z %d (HEAD)\n", event.buttons, event.xPosition, event.yPosition, event.zPressure); break; case MOTION_PACKET: @@ -236,26 +279,97 @@ elantech_process_packet_v4(elantech_cookie *cookie, touchpad_movement *_event, event.buttons = (packet[0] & 0x3); - // Pressure is not provided on this packet, so use a backup value. - event.zPressure = cookie->previousZ; + // Pressure is not provided on this packet, so make it up with sensible values + event.zPressure = DEFAULT_PRESSURE; - TRACE("ELANTECH: Fingers %d, raw %x (MOTION)\n", (packet[3] & 0xe0) >>5, packet[3]); + TRACE("ELANTECH: Finger %d, raw %x (MOTION id)\n", (packet[0] & 0xe0) >>5, packet[0]); + TRACE("ELANTECH: Finger %d, raw %x (MOTION sid)\n", (packet[3] & 0xe0) >>5, packet[3]); - // Most likely palm - if (cookie->fingers == 0) + id = ((packet[0] & 0xe0) >> 5) - 1; + int sid; + sid = ((packet[3] & 0xe0) >> 5) - 1; + + // Motion packet processes 2 fingers only. Question is if the id is different than + // the one provided by STATUS packet, are the fingers of these packet new fingers + // or a replacement of the previous ones? + // As per testing let's assume a new fingers are added. + // That's in sync with the logic BSDs and Linux drivers providing MT events. + //event.fingers = cookie->fingers; + event.fingers = cookie->fingers | (1 << id) | (1 << sid); + //event.fingers = (1 << id) | (1 << sid); + + if ((id < 0 || id >= ELANTECH_MAX_FINGERS) + && (sid < 0 || sid >= ELANTECH_MAX_FINGERS)) { + TRACE("ELANTECH: Not right fingers (MOTION)"); return IGNORE_EVENT; - - event.fingers = cookie->fingers; - - // handle overflow and delta values - if ((packet[0] & 0x10) != 0) { - event.xPosition = cookie->x += 5 * (int8)packet[1]; - event.yPosition = cookie->y += 5 * (int8)packet[2]; - } else { - event.xPosition = cookie->x += (int8)packet[1]; - event.yPosition = cookie->y += (int8)packet[2]; } - TRACE("ELANTECH: Pos: %" B_PRId32 ":%" B_PRId32 " (Motion)\n", + +#ifdef ELANTECH_ENABLE_HARDWARE_PALM_DETECTION + if (cookie->palm) { + TRACE("ELANTECH: Hardware palm detected (MOTION)\n"); + return IGNORE_EVENT; + } +#endif + + int deltaX; + int deltaY; + + deltaX = 0; + deltaY = 0; + + int weight; + weight = (packet[0] & 0x10) ? 5 : 1; + + // Only take care for finger id 0 for now. + // TODO: Change this when support for Multi-finger is available on Haiku + if (id == 0 || sid ==0) { + if (id < 0 || id >= ELANTECH_MAX_FINGERS) + event.fingers |= 1 << id; + if (sid < 0 || sid >= ELANTECH_MAX_FINGERS) + event.fingers |= 1 << sid; + + if (id == 0) { + deltaX += weight * (int8)packet[1]; + deltaY += weight * (int8)packet[2]; + } else if (sid == 0) { + deltaX += weight * (int8)packet[4]; + deltaY += weight * (int8)packet[5]; + } + + TRACE("ELANTECH: dx: %d dy: %d (MOTION)\n", deltaX, deltaY); + } else { + TRACE("ELANTECH: Ignore invalid or non 0 finger ids (MOTION)\n"); + return IGNORE_EVENT; + } + + // TODO: Avoid this conversion from rel to abs coordinates when Haiku supports both + // type of coordinate systems for touchpads. Do the conversion as part of the driver + // and pretend the device always provide absolute coordinates for the time being. + // BEGIN: Conversion and tracking of abs coordinates: + // Avoid underoverflow + if (deltaX < 0 && (int)cookie->x < abs(deltaX)) + deltaX = -cookie->x; + if (deltaY < 0 && (int)cookie->y < abs(deltaY)) + deltaY = -cookie->y; + + event.xPosition = cookie->x + deltaX; + event.yPosition = cookie->y + deltaY; + + // Adjust to area boundaries + if (event.xPosition < gHardwareSpecs.areaStartX) + event.xPosition = gHardwareSpecs.areaStartX; + if (event.xPosition > gHardwareSpecs.areaEndX) + event.xPosition = gHardwareSpecs.areaEndX; + if (event.yPosition < gHardwareSpecs.areaStartY) + event.yPosition = gHardwareSpecs.areaStartY; + if (event.yPosition > gHardwareSpecs.areaEndY) + event.yPosition = gHardwareSpecs.areaEndY; + + cookie->x = event.xPosition; + cookie->y = event.yPosition; + // END: Conversion and tracking of abs coordinates + + TRACE("ELANTECH: Pos: %" B_PRId32 ":%" B_PRId32 " (MOTION)\n", cookie->x, cookie->y); break; @@ -264,10 +378,11 @@ elantech_process_packet_v4(elantech_cookie *cookie, touchpad_movement *_event, return IGNORE_EVENT; } + cookie->fingers = event.fingers; TRACE("ELANTECH: buttons %d\n", event.buttons); TRACE("ELANTECH: zPressure %d\n", event.zPressure); - event.fingerWidth = cookie->fingers == 1 ? 4 :0; + event.fingerWidth = DEFAULT_FINGER_WIDTH; *_event = event; return B_OK; diff --git a/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.h b/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.h index 6c64f9c48a..14df54b59c 100644 --- a/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.h +++ b/src/add-ons/kernel/bus_managers/ps2/ps2_elantech.h @@ -24,6 +24,13 @@ #include "packet_buffer.h" #include "ps2_dev.h" +// Palm detection will be more accurate to do by software. +// Options are: +// - Improvements on padblocker input filter. +// - Improvements in user mode input_server device. +// - General input_server modifications. +// - Other inprovements on this driver. +//#define ELANTECH_ENABLE_HARDWARE_PALM_DETECTION typedef struct { ps2_dev* dev; @@ -51,6 +58,10 @@ typedef struct { uint8 mode; bool crcEnabled; +#ifdef ELANTECH_ENABLE_HARDWARE_PALM_DETECTION + bool palm; +#endif + status_t (*send_command)(ps2_dev* dev, uint8 cmd, uint8 *in, int in_count); } elantech_cookie;