Re: kern/56476 Pointer jumps with synaptics driver

[Michael van Elst <mlelstv%serpens.de@localhost>]

The following reply was made to PR kern/56476; it has been noted by GNATS.

From: Michael van Elst <mlelstv%serpens.de@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/56476 Pointer jumps with synaptics driver
Date: Tue, 19 Jul 2022 10:23:32 +0200

 Here is a patch that fixes the behaviour for me. It mostly corrects the
 broken movement filtering, but I may have dropped some magic that is
 required for other hardware. Comments?
 
 ? sys/dev/pckbport/ChangeLog
 Index: sys/dev/pckbport/synaptics.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pckbport/synaptics.c,v
 retrieving revision 1.79
 diff -p -u -r1.79 synaptics.c
 --- sys/dev/pckbport/synaptics.c	31 May 2022 08:43:16 -0000	1.79
 +++ sys/dev/pckbport/synaptics.c	19 Jul 2022 08:19:43 -0000
 @@ -1636,76 +1636,13 @@ skip_position:
  	    nsp.sp_primary, nsp.sp_secondary, v, primary_finger,
  	    secondary_finger);
  
 -
 -	/* If no fingers and we at least saw the primary finger
 -	 * or the buttons changed then process the last packet.
 -	 */
 -	if (pms_synaptics_get_fingers(psc, nsp.sp_w, nsp.sp_z) == 0 ||
 -	    nsp.sp_left != packet.sp_left ||
 -	    nsp.sp_right != packet.sp_right ||
 -	    nsp.sp_middle != packet.sp_middle ||
 -	    nsp.sp_up != packet.sp_up ||
 -	    nsp.sp_down != packet.sp_down) {
 -		if (nsp.sp_primary == 1) {
 -			pms_synaptics_process_packet(psc, &nsp);
 -			sc->packet_count[SYN_PRIMARY_FINGER] = 0;
 -			sc->packet_count[SYN_SECONDARY_FINGER] = 0;
 -		}
 -
 -		/* clear the fingers seen since we have processed */
 -		nsp.sp_primary = 0;
 -		nsp.sp_secondary = 0;
 -		nsp.sp_finger_status = 0;
 -	} else if (nsp.sp_finger_count != packet.sp_finger_count) {
 -		/*
 -		 * If the number of fingers changes then send the current packet
 -		 * for processing and restart the process.
 -		 */
 -		if (packet.sp_primary == 1) {
 -			pms_synaptics_process_packet(psc, &packet);
 -			sc->packet_count[SYN_PRIMARY_FINGER]++;
 -		}
 -
 +	/* Clear history when number of fingers changes */
 +	if (nsp.sp_finger_count != packet.sp_finger_count) {
  		sc->packet_count[SYN_PRIMARY_FINGER] = 0;
  		sc->packet_count[SYN_SECONDARY_FINGER] = 0;
  	}
  
 -	/* Only one finger, process the new packet */
 -	if (nsp.sp_finger_count == 1) {
 -		if (nsp.sp_finger_count != packet.sp_finger_count) {
 -			sc->packet_count[SYN_PRIMARY_FINGER] = 0;
 -			sc->packet_count[SYN_SECONDARY_FINGER] = 0;
 -		}
 -		pms_synaptics_process_packet(psc, &nsp);
 -
 -		/* clear the fingers seen since we have processed */
 -		nsp.sp_primary = 0;
 -		nsp.sp_secondary = 0;
 -		nsp.sp_finger_status = 0;
 -
 -		sc->packet_count[SYN_PRIMARY_FINGER]++;
 -	}
 -
 -	/*
 -	 *  More than one finger and we have seen the primary and secondary
 -	 * fingers then process the packet.
 -	 */
 -	if ((nsp.sp_finger_count > 1) && (nsp.sp_primary == 1) 
 -	    && (nsp.sp_secondary == 1)) {
 -		if (nsp.sp_finger_count != packet.sp_finger_count) {
 -			sc->packet_count[SYN_PRIMARY_FINGER] = 0;
 -			sc->packet_count[SYN_SECONDARY_FINGER] = 0;
 -		}
 -		pms_synaptics_process_packet(psc, &nsp);
 -
 -		/* clear the fingers seen since we have processed */
 -		nsp.sp_primary = 0;
 -		nsp.sp_secondary = 0;
 -		nsp.sp_finger_status = 0;
 -
 -		sc->packet_count[SYN_PRIMARY_FINGER]++;
 -		sc->packet_count[SYN_SECONDARY_FINGER]++;
 -	}
 +	pms_synaptics_process_packet(psc, &nsp);
  
  	memcpy(&packet, &nsp, sizeof(packet));
  }
 @@ -2095,10 +2032,10 @@ synaptics_filter_policy(struct synaptics
  	 * tiny finger movements.
  	 */
  	if (count >= SYN_HIST_SIZE) {
 -		a = (history[(count + 0) % SYN_HIST_SIZE] +
 -		    history[(count + 1) % SYN_HIST_SIZE]) / 2;
 +		a = (history[(count - 2) % SYN_HIST_SIZE] +
 +		     history[(count - 1) % SYN_HIST_SIZE]) / 2;
  
 -		b = (value + history[(count + 0) % SYN_HIST_SIZE]) / 2;
 +		b = (value + history[(count - 1) % SYN_HIST_SIZE]) / 2;
  
  		rv = b - a;
  
 @@ -2114,7 +2051,7 @@ synaptics_filter_policy(struct synaptics
  	/*
  	 * Add the new value to the history buffer.
  	 */
 -	history[(count + 1) % SYN_HIST_SIZE] = value;
 +	history[(count + 0) % SYN_HIST_SIZE] = value;
  
  	return (rv);
  }
 @@ -2201,6 +2138,7 @@ synaptics_movement(struct synaptics_soft
  	dy = synaptics_filter_policy(sc, 0,
  	    sc->history_y[SYN_PRIMARY_FINGER], sp->sp_y,
  	    sc->packet_count[SYN_PRIMARY_FINGER]);
 +	sc->packet_count[SYN_PRIMARY_FINGER]++;
  
  	if (sp->sp_finger_count > 1) {
  		sdx = synaptics_filter_policy(sc, 1,
 @@ -2209,6 +2147,7 @@ synaptics_movement(struct synaptics_soft
  		sdy = synaptics_filter_policy(sc, 1,
  		    sc->history_y[SYN_SECONDARY_FINGER], sp->sp_sy,
  		    sc->packet_count[SYN_SECONDARY_FINGER]);
 +		sc->packet_count[SYN_SECONDARY_FINGER]++;
  		DPRINTF(10, sc, "synaptics_movement: dx %d dy %d sdx %d sdy %d\n",
  		    dx, dy, sdx, sdy);
  	}
 @@ -2419,6 +2358,9 @@ pms_synaptics_process_packet(struct pms_
  			sc->rem_x[SYN_SECONDARY_FINGER] = 0;
  			sc->rem_y[SYN_SECONDARY_FINGER] = 0;
  			dx = dy = 0;
 +
 +			sc->packet_count[SYN_PRIMARY_FINGER] = 0;
 +			sc->packet_count[SYN_SECONDARY_FINGER] = 0;
  		}
  	} else {
  		/*
 
 
 -- 
                                 Michael van Elst
 Internet: mlelstv%serpens.de@localhost
                                 "A potential Snark may lurk in every tree."